From: Eric Blake Date: Fri, 25 Apr 2014 20:45:49 +0000 (-0600) Subject: util: use virDirRead API X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=ac1d42ac7226008ea0903569fe04c131fc2a829a;p=people%2Fliuw%2Flibxenctrl-split%2Flibvirt.git util: use virDirRead API In making the conversion to the new API, I fixed a couple bugs: virSCSIDeviceGetSgName would leak memory if a directory unexpectedly contained multiple entries; virNetDevTapGetRealDeviceName could report a spurious error from a stale errno inherited before starting the readdir search. The decision on whether to store the result of virDirRead into a variable is based on whether the end of the loop falls through to cleanup code automatically. In some cases, we have loops that are documented to return NULL on failure, and which raise an error on most failure paths but not in the case where the directory was unexpectedly empty; it may be worth a followup patch to explicitly report an error if readdir was successful but the directory was empty, so that a NULL return always has an error set. * src/util/vircgroup.c (virCgroupRemoveRecursively): Use new interface. (virCgroupKillRecursiveInternal, virCgroupSetOwner): Report readdir failures. * src/util/virfile.c (virFileLoopDeviceOpenSearch) (virFileNBDDeviceFindUnused, virFileDeleteTree): Use new interface. * src/util/virnetdevtap.c (virNetDevTapGetRealDeviceName): Properly check readdir errors. * src/util/virpci.c (virPCIDeviceIterDevices) (virPCIDeviceFileIterate, virPCIGetNetName): Report readdir failures. (virPCIDeviceAddressIOMMUGroupIterate): Use new interface. * src/util/virscsi.c (virSCSIDeviceGetSgName): Report readdir failures, and avoid memory leak. (virSCSIDeviceGetDevName): Report readdir failures. * src/util/virusb.c (virUSBDeviceSearch): Report readdir failures. * src/util/virutil.c (virGetFCHostNameByWWN) (virFindFCHostCapableVport): Report readdir failures. Signed-off-by: Eric Blake --- diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c7ebf8d27..fce380ab5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3137,6 +3137,7 @@ virCgroupRemoveRecursively(char *grppath) DIR *grpdir; struct dirent *ent; int rc = 0; + int direrr; grpdir = opendir(grppath); if (grpdir == NULL) { @@ -3147,17 +3148,11 @@ virCgroupRemoveRecursively(char *grppath) return rc; } - for (;;) { + /* This is best-effort cleanup: we want to log failures with just + * VIR_ERROR instead of normal virReportError */ + while ((direrr = virDirRead(grpdir, &ent, NULL)) > 0) { char *path; - errno = 0; - ent = readdir(grpdir); - if (ent == NULL) { - if ((rc = -errno)) - VIR_ERROR(_("Failed to readdir for %s (%d)"), grppath, errno); - break; - } - if (ent->d_name[0] == '.') continue; if (ent->d_type != DT_DIR) continue; @@ -3170,6 +3165,11 @@ virCgroupRemoveRecursively(char *grppath) if (rc != 0) break; } + if (direrr < 0) { + rc = -errno; + VIR_ERROR(_("Failed to readdir for %s (%d)"), grppath, errno); + } + closedir(grpdir); VIR_DEBUG("Removing cgroup %s", grppath); @@ -3373,6 +3373,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, DIR *dp; virCgroupPtr subgroup = NULL; struct dirent *ent; + int direrr; VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); @@ -3396,7 +3397,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, return -1; } - while ((ent = readdir(dp))) { + while ((direrr = virDirRead(dp, &ent, keypath)) > 0) { if (STREQ(ent->d_name, ".")) continue; if (STREQ(ent->d_name, "..")) @@ -3420,6 +3421,8 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, virCgroupFree(&subgroup); } + if (direrr < 0) + goto cleanup; done: ret = killedAny ? 1 : 0; @@ -3701,6 +3704,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, size_t i; char *base = NULL, *entry = NULL; DIR *dh = NULL; + int direrr; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { struct dirent *de; @@ -3721,7 +3725,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, goto cleanup; } - while ((de = readdir(dh)) != NULL) { + while ((direrr = virDirRead(dh, &de, base)) > 0) { if (STREQ(de->d_name, ".") || STREQ(de->d_name, "..")) continue; @@ -3738,6 +3742,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup, VIR_FREE(entry); } + if (direrr < 0) + goto cleanup; if (chown(base, uid, gid) < 0) { virReportSystemError(errno, diff --git a/src/util/virfile.c b/src/util/virfile.c index 34811243c..bb1e7ba7b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -607,6 +607,7 @@ static int virFileLoopDeviceOpenSearch(char **dev_name) struct dirent *de; char *looppath = NULL; struct loop_info64 lo; + int direrr; VIR_DEBUG("Looking for loop devices in /dev"); @@ -616,8 +617,7 @@ static int virFileLoopDeviceOpenSearch(char **dev_name) goto cleanup; } - errno = 0; - while ((de = readdir(dh)) != NULL) { + while ((direrr = virDirRead(dh, &de, "/dev")) > 0) { /* Checking 'loop' prefix is insufficient, since * new kernels have a dev named 'loop-control' */ @@ -650,15 +650,11 @@ static int virFileLoopDeviceOpenSearch(char **dev_name) /* Oh well, try the next device */ VIR_FORCE_CLOSE(fd); VIR_FREE(looppath); - errno = 0; } - - if (errno != 0) - virReportSystemError(errno, "%s", - _("Unable to iterate over loop devices")); - else - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find a free loop device in /dev")); + if (direrr < 0) + goto cleanup; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to find a free loop device in /dev")); cleanup: if (fd != -1) { @@ -781,6 +777,7 @@ virFileNBDDeviceFindUnused(void) DIR *dh; char *ret = NULL; struct dirent *de; + int direrr; if (!(dh = opendir(SYSFS_BLOCK_DIR))) { virReportSystemError(errno, @@ -789,8 +786,7 @@ virFileNBDDeviceFindUnused(void) return NULL; } - errno = 0; - while ((de = readdir(dh)) != NULL) { + while ((direrr = virDirRead(dh, &de, SYSFS_BLOCK_DIR)) > 0) { if (STRPREFIX(de->d_name, "nbd")) { int rv = virFileNBDDeviceIsBusy(de->d_name); if (rv < 0) @@ -801,15 +797,11 @@ virFileNBDDeviceFindUnused(void) goto cleanup; } } - errno = 0; } - - if (errno != 0) - virReportSystemError(errno, "%s", - _("Unable to iterate over NBD devices")); - else - virReportSystemError(EBUSY, "%s", - _("No free NBD devices")); + if (direrr < 0) + goto cleanup; + virReportSystemError(EBUSY, "%s", + _("No free NBD devices")); cleanup: closedir(dh); @@ -918,6 +910,7 @@ int virFileDeleteTree(const char *dir) struct dirent *de; char *filepath = NULL; int ret = -1; + int direrr; if (!dh) { virReportSystemError(errno, _("Cannot open dir '%s'"), @@ -925,8 +918,7 @@ int virFileDeleteTree(const char *dir) return -1; } - errno = 0; - while ((de = readdir(dh)) != NULL) { + while ((direrr = virDirRead(dh, &de, dir)) > 0) { struct stat sb; if (STREQ(de->d_name, ".") || @@ -956,14 +948,9 @@ int virFileDeleteTree(const char *dir) } VIR_FREE(filepath); - errno = 0; } - - if (errno) { - virReportSystemError(errno, _("Cannot read dir '%s'"), - dir); + if (direrr < 0) goto cleanup; - } if (rmdir(dir) < 0 && errno != ENOENT) { virReportSystemError(errno, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 03bb41856..0b444fad3 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2013 Red Hat, Inc. + * Copyright (C) 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -105,7 +105,7 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED) return NULL; } - while ((dp = readdir(dirp)) != NULL) { + while (virDirRead(dirp, &dp, "/dev") > 0) { if (STRPREFIX(dp->d_name, "tap")) { struct ifreq ifr; if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) { @@ -139,14 +139,8 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED) VIR_FREE(devpath); VIR_FORCE_CLOSE(fd); } - - errno = 0; } - if (errno != 0) - virReportSystemError(errno, "%s", - _("Unable to iterate over TAP devices")); - cleanup: VIR_FREE(devpath); VIR_FORCE_CLOSE(fd); diff --git a/src/util/virpci.c b/src/util/virpci.c index af0bfa8b3..e0f234428 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1,7 +1,7 @@ /* * virpci.c: helper APIs for managing host PCI devices * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -445,7 +445,7 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, return -1; } - while ((entry = readdir(dir))) { + while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { unsigned int domain, bus, slot, function; virPCIDevicePtr check; char *tmp; @@ -1907,6 +1907,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, DIR *dir = NULL; int ret = -1; struct dirent *ent; + int direrr; if (virAsprintf(&pcidir, "/sys/bus/pci/devices/%04x:%02x:%02x.%x", dev->domain, dev->bus, dev->slot, dev->function) < 0) @@ -1918,7 +1919,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, goto cleanup; } - while ((ent = readdir(dir)) != NULL) { + while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) { /* Device assignment requires: * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, * $PCIDIR/rom, $PCIDIR/reset @@ -1935,6 +1936,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, VIR_FREE(file); } } + if (direrr < 0) + goto cleanup; ret = 0; @@ -1961,6 +1964,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, DIR *groupDir = NULL; int ret = -1; struct dirent *ent; + int direrr; if (virAsprintf(&groupPath, PCI_SYSFS "devices/%04x:%02x:%02x.%x/iommu_group/devices", @@ -1973,7 +1977,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, goto cleanup; } - while ((errno = 0, ent = readdir(groupDir)) != NULL) { + while ((direrr = virDirRead(groupDir, &ent, groupPath)) > 0) { virPCIDeviceAddress newDev; if (ent->d_name[0] == '.') @@ -1989,12 +1993,8 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, if ((actor)(&newDev, opaque) < 0) goto cleanup; } - if (errno != 0) { - virReportSystemError(errno, - _("Failed to read directory entry for %s"), - groupPath); + if (direrr < 0) goto cleanup; - } ret = 0; @@ -2638,7 +2638,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) if (dir == NULL) goto out; - while ((entry = readdir(dir))) { + while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { if (STREQ(entry->d_name, ".") || STREQ(entry->d_name, "..")) continue; diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 572b818e0..9a0205ff8 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -133,12 +133,13 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, goto cleanup; } - while ((entry = readdir(dir))) { + while (virDirRead(dir, &entry, path) > 0) { if (entry->d_name[0] == '.') continue; - if (VIR_STRDUP(sg, entry->d_name) < 0) - goto cleanup; + /* Assume a single directory entry */ + ignore_value(VIR_STRDUP(sg, entry->d_name)); + break; } cleanup: @@ -178,7 +179,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, goto cleanup; } - while ((entry = readdir(dir))) { + while (virDirRead(dir, &entry, path) > 0) { if (entry->d_name[0] == '.') continue; diff --git a/src/util/virusb.c b/src/util/virusb.c index d959f5fd5..8244771a9 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -1,7 +1,7 @@ /* * virusb.c: helper APIs for managing host USB devices * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -134,6 +134,7 @@ virUSBDeviceSearch(unsigned int vendor, struct dirent *de; virUSBDeviceListPtr list = NULL, ret = NULL; virUSBDevicePtr usb; + int direrr; if (!(list = virUSBDeviceListNew())) goto cleanup; @@ -146,7 +147,7 @@ virUSBDeviceSearch(unsigned int vendor, goto cleanup; } - while ((de = readdir(dir))) { + while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) { unsigned int found_prod, found_vend, found_bus, found_devno; char *tmpstr = de->d_name; @@ -197,6 +198,8 @@ virUSBDeviceSearch(unsigned int vendor, if (found) break; } + if (direrr < 0) + goto cleanup; ret = list; cleanup: diff --git a/src/util/virutil.c b/src/util/virutil.c index 9be15907b..875e2b0f1 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1867,7 +1867,7 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, p = buf; \ } while (0) - while ((entry = readdir(dir))) { + while (virDirRead(dir, &entry, prefix) > 0) { if (entry->d_name[0] == '.') continue; @@ -1949,7 +1949,7 @@ virFindFCHostCapableVport(const char *sysfs_prefix) return NULL; } - while ((entry = readdir(dir))) { + while (virDirRead(dir, &entry, prefix) > 0) { unsigned int host; char *p = NULL;