[libvirt] [PATCH 00/18] Allow domains to start with a dot
Also introduce virDirOpen* and VIR_DIR_CLOSE helpers. https://bugzilla.redhat.com/show_bug.cgi?id=1333248 Ján Tomko (18): Do not save errno in virUSBDeviceSearch Fix error detection in virStorageBackendISCSIGetHostNumber Do not check the return value of closedir Introduce VIR_DIR_CLOSE Introduce virDirOpen Use virDirOpen Add virDirOpenIfExists Use virDirOpenIfExists Introduce virDirOpenQuiet Use virDirOpenQuiet Prohibit opendir in syntax-check Skip '.' and '..' in virDirRead Do not check for '.' and '..' after virDirRead Fix comment in virStorageBackendFileSystemRefresh Do not ignore hidden files in /sys and /proc Do not skip hidden entries when looking for a stable path Allow configs to start with a dot Do not skip files starting with a dot in leases directory cfg.mk | 7 +- src/conf/network_conf.c | 31 +++------ src/conf/nwfilter_conf.c | 15 ++--- src/conf/storage_conf.c | 30 +++------ src/conf/virdomainobjlist.c | 16 ++--- src/conf/virsecretobj.c | 14 ++-- src/libvirt_private.syms | 4 ++ src/network/bridge_driver.c | 17 ++--- src/openvz/openvz_conf.c | 5 +- src/qemu/qemu_driver.c | 14 +--- src/qemu/qemu_hostdev.c | 10 +-- src/storage/storage_backend.c | 11 ++-- src/storage/storage_backend_fs.c | 14 ++-- src/storage/storage_backend_iscsi.c | 47 +++++++------- src/storage/storage_backend_scsi.c | 30 ++------- src/util/vircgroup.c | 43 ++++--------- src/util/virfile.c | 124 ++++++++++++++++++++++++++---------- src/util/virfile.h | 9 +++ src/util/virhostcpu.c | 19 ++---- src/util/virnetdev.c | 10 +-- src/util/virnetdevtap.c | 9 +-- src/util/virnuma.c | 17 ++--- src/util/virpci.c | 36 +++-------- src/util/virprocess.c | 9 +-- src/util/virscsi.c | 22 ++----- src/util/virusb.c | 16 +---- src/util/virutil.c | 32 ++-------- src/xen/xen_inotify.c | 13 ++-- src/xen/xm_internal.c | 10 +-- tests/virschematest.c | 8 +-- tools/nss/libvirt_nss.c | 13 +--- 31 files changed, 245 insertions(+), 410 deletions(-) -- 2.7.3
The virUSBDeviceFind* callers do not check errno after calling this function. --- src/util/virusb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virusb.c b/src/util/virusb.c index 520610b..5c39667 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -202,11 +202,8 @@ virUSBDeviceSearch(unsigned int vendor, ret = list; cleanup: - if (dir) { - int saved_errno = errno; + if (dir) closedir(dir); - errno = saved_errno; - } if (!ret) virObjectUnref(list); -- 2.7.3
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value. Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..876c14c 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -90,7 +90,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, if (sysdir == NULL) { virReportSystemError(errno, _("Failed to opendir path '%s'"), sysfs_path); - retval = -1; - goto out; + goto cleanup; } while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) { - if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1; + if (direrr == 0) { + virReportSystemError(errno, + _("Failed to get host number for iSCSI session " + "with path '%s'"), + sysfs_path); + goto cleanup; + } + + cleanup: closedir(sysdir); - out: - return retval; + return ret; } static int @@ -138,13 +145,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, "/sys/class/iscsi_session/session%s/device", session) < 0) goto cleanup; - if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) { - virReportSystemError(errno, - _("Failed to get host number for iSCSI session " - "with path '%s'"), - sysfs_path); + if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) goto cleanup; - } if (virStorageBackendSCSIFindLUs(pool, host) < 0) goto cleanup; -- 2.7.3
On 06/21/2016 12:05 PM, Ján Tomko wrote:
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value.
Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..876c14c 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -90,7 +90,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, if (sysdir == NULL) { virReportSystemError(errno, _("Failed to opendir path '%s'"), sysfs_path); - retval = -1; - goto out; + goto cleanup; }
while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
While we're here and changing, could use STRPREFIX
- if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1;
+ if (direrr == 0) {
Should this be <= ? If virDirRead() returns -1, then we don't return with an error... If virDirRead() returns 0, then we've gone through the entire directory without finding an entry that starts with target - seems that would be a system configuration error and I assume errno would be ENOENT ACK with obvious adjustments - John
+ virReportSystemError(errno, + _("Failed to get host number for iSCSI session " + "with path '%s'"), + sysfs_path); + goto cleanup; + } + + cleanup: closedir(sysdir); - out: - return retval; + return ret; }
static int @@ -138,13 +145,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, "/sys/class/iscsi_session/session%s/device", session) < 0) goto cleanup;
- if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) { - virReportSystemError(errno, - _("Failed to get host number for iSCSI session " - "with path '%s'"), - sysfs_path); + if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) goto cleanup; - }
if (virStorageBackendSCSIFindLUs(pool, host) < 0) goto cleanup;
On Thu, Jun 23, 2016 at 08:26:06AM -0400, John Ferlan wrote:
On 06/21/2016 12:05 PM, Ján Tomko wrote:
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value.
Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..876c14c 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -90,7 +90,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, if (sysdir == NULL) { virReportSystemError(errno, _("Failed to opendir path '%s'"), sysfs_path); - retval = -1; - goto out; + goto cleanup; }
while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
While we're here and changing, could use STRPREFIX
I'll send a separate patch for that.
- if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1;
+ if (direrr == 0) {
Should this be <= ?
If virDirRead() returns -1, then we don't return with an error...
virDirRead does report errors.
If virDirRead() returns 0, then we've gone through the entire directory without finding an entry that starts with target - seems that would be a system configuration error and I assume errno would be ENOENT
Actually, errno would probably be 0. I'll fix the error in v2. Jan
ACK with obvious adjustments -
John
The only possible error is EBADFD. Since we only use the directory stream returned by opendir, this should never happen. --- src/util/virhostcpu.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 4ed7b21..0cdba0a 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -477,11 +477,8 @@ virHostCPUParseNode(const char *node, ret = processors; cleanup: - /* don't shadow a more serious error */ - if (cpudir && closedir(cpudir) < 0 && ret >= 0) { - virReportSystemError(errno, _("problem closing %s"), node); - ret = -1; - } + if (cpudir) + closedir(cpudir); if (cores_maps) for (i = 0; i < sock_max; i++) virBitmapFree(cores_maps[i]); @@ -777,12 +774,8 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, ret = 0; cleanup: - /* don't shadow a more serious error */ - if (nodedir && closedir(nodedir) < 0 && ret >= 0) { - virReportSystemError(errno, _("problem closing %s"), sysfs_nodedir); - ret = -1; - } - + if (nodedir) + closedir(nodedir); virBitmapFree(present_cpus_map); virBitmapFree(online_cpus_map); VIR_FREE(sysfs_nodedir); -- 2.7.3
Introduce a helper that only calls closedir if DIR* is non-NULL and sets it to NULL afterwards. --- cfg.mk | 4 ++-- src/conf/network_conf.c | 4 ++-- src/conf/nwfilter_conf.c | 2 +- src/conf/storage_conf.c | 4 ++-- src/conf/virdomainobjlist.c | 2 +- src/conf/virsecretobj.c | 2 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_hostdev.c | 4 +--- src/storage/storage_backend.c | 6 +++--- src/storage/storage_backend_fs.c | 6 ++---- src/storage/storage_backend_iscsi.c | 2 +- src/storage/storage_backend_scsi.c | 8 +++----- src/util/vircgroup.c | 12 ++++-------- src/util/virfile.c | 16 ++++++++++++---- src/util/virfile.h | 3 +++ src/util/virhostcpu.c | 6 ++---- src/util/virnetdev.c | 2 +- src/util/virnetdevtap.c | 2 +- src/util/virnuma.c | 3 +-- src/util/virpci.c | 10 ++++------ src/util/virprocess.c | 3 +-- src/util/virscsi.c | 6 ++---- src/util/virusb.c | 4 +--- src/util/virutil.c | 6 +++--- src/xen/xen_inotify.c | 6 +++--- src/xen/xm_internal.c | 4 ++-- tests/virschematest.c | 2 +- tools/nss/libvirt_nss.c | 6 ++---- 31 files changed, 66 insertions(+), 77 deletions(-) diff --git a/cfg.mk b/cfg.mk index d82070d..a2576d1 100644 --- a/cfg.mk +++ b/cfg.mk @@ -421,9 +421,9 @@ sc_prohibit_gethostname: $(_sc_search_regexp) sc_prohibit_readdir: - @prohibit='\breaddir *\(' \ + @prohibit='\b(read|close)dir *\(' \ exclude='exempt from syntax-check' \ - halt='use virDirRead, not readdir' \ + halt='use virDirRead and VIR_DIR_CLOSE' \ $(_sc_search_regexp) sc_prohibit_gettext_noop: diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 02b8cd7..1e4b719 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3258,7 +3258,7 @@ virNetworkLoadAllState(virNetworkObjListPtr nets, virNetworkObjEndAPI(&net); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } @@ -3298,7 +3298,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets, virNetworkObjEndAPI(&net); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 3f90f65..56f8b86 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -3227,7 +3227,7 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters, virNWFilterObjUnlock(nwfilter); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6932195..5c044d2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1964,7 +1964,7 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, virStoragePoolObjUnlock(pool); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } @@ -2015,7 +2015,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, VIR_FREE(autostartLink); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 485671e..4f7756d 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -616,7 +616,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, } } - closedir(dir); + VIR_DIR_CLOSE(dir); virObjectUnlock(doms); return ret; } diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index c46d22c..46042eb 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -1000,6 +1000,6 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets, virSecretObjEndAPI(&secret); } - closedir(dir); + VIR_DIR_CLOSE(dir); return 0; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 501c23e..f4dc88d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1493,6 +1493,7 @@ saferead; safewrite; safezero; virBuildPathInternal; +virDirClose; virDirCreate; virDirRead; virFileAbsPath; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7c8d2cc..7b021d8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -577,7 +577,7 @@ networkMigrateStateFiles(virNetworkDriverStatePtr driver) ret = 0; cleanup: - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(oldPath); VIR_FREE(newPath); VIR_FREE(contents); diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 820dc22..ff5e5b8 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -1072,7 +1072,7 @@ static int openvzAssignUUIDs(void) openvzSetUUID(vpsid); } - closedir(dp); + VIR_DIR_CLOSE(dp); VIR_FREE(conf_dir); return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d065e45..76ee3c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -585,8 +585,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, ret = 0; cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(snapDir); virObjectUnref(caps); virObjectUnlock(vm); diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index e16d5fd..84f3615 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -134,9 +134,7 @@ qemuHostdevHostSupportsPassthroughVFIO(void) ret = true; cleanup: - if (iommuDir) - closedir(iommuDir); - + VIR_DIR_CLOSE(iommuDir); return ret; } diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8f03a6e..955d983 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1957,12 +1957,12 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, if (virAsprintf(&stablepath, "%s/%s", pool->def->target.path, dent->d_name) == -1) { - closedir(dh); + VIR_DIR_CLOSE(dh); return NULL; } if (virFileLinkPointsTo(stablepath, devpath)) { - closedir(dh); + VIR_DIR_CLOSE(dh); return stablepath; } @@ -1974,7 +1974,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, goto retry; } - closedir(dh); + VIR_DIR_CLOSE(dh); ret_strdup: /* Couldn't find any matching stable link so give back diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index a11df36..152f9f3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -972,8 +972,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, } if (direrr < 0) goto cleanup; - closedir(dir); - dir = NULL; + VIR_DIR_CLOSE(dir); vol = NULL; if (VIR_ALLOC(target)) @@ -1019,8 +1018,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, ret = 0; cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FORCE_CLOSE(fd); virStorageVolDefFree(vol); virStorageSourceFree(target); diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 876c14c..e50158f 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -129,7 +129,7 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, } cleanup: - closedir(sysdir); + VIR_DIR_CLOSE(sysdir); return ret; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index be993f1..b08d960 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -290,8 +290,7 @@ getNewStyleBlockDevice(const char *lun_path, retval = 0; cleanup: - if (block_dir) - closedir(block_dir); + VIR_DIR_CLOSE(block_dir); VIR_FREE(block_path); return retval; } @@ -387,8 +386,7 @@ getBlockDevice(uint32_t host, retval = 0; cleanup: - if (lun_dir) - closedir(lun_dir); + VIR_DIR_CLOSE(lun_dir); VIR_FREE(lun_path); return retval; } @@ -501,7 +499,7 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, found++; } - closedir(devicedir); + VIR_DIR_CLOSE(devicedir); if (retval < 0) return -1; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 7bab086..c76c94f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3409,7 +3409,7 @@ virCgroupRemoveRecursively(char *grppath) VIR_ERROR(_("Failed to readdir for %s (%d)"), grppath, errno); } - closedir(grpdir); + VIR_DIR_CLOSE(grpdir); VIR_DEBUG("Removing cgroup %s", grppath); if (rmdir(grppath) != 0 && errno != ENOENT) { @@ -3669,9 +3669,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, cleanup: virCgroupFree(&subgroup); VIR_FREE(keypath); - if (dp) - closedir(dp); - + VIR_DIR_CLOSE(dp); return ret; } @@ -3993,15 +3991,13 @@ int virCgroupSetOwner(virCgroupPtr cgroup, } VIR_FREE(base); - closedir(dh); - dh = NULL; + VIR_DIR_CLOSE(dh); } ret = 0; cleanup: - if (dh) - closedir(dh); + VIR_DIR_CLOSE(dh); VIR_FREE(entry); VIR_FREE(base); return ret; diff --git a/src/util/virfile.c b/src/util/virfile.c index f47bf39..ce8f7fd 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -667,8 +667,7 @@ static int virFileLoopDeviceOpenSearch(char **dev_name) VIR_DEBUG("No free loop devices available"); VIR_FREE(looppath); } - if (dh) - closedir(dh); + VIR_DIR_CLOSE(dh); return fd; } @@ -807,7 +806,7 @@ virFileNBDDeviceFindUnused(void) _("No free NBD devices")); cleanup: - closedir(dh); + VIR_DIR_CLOSE(dh); return ret; } @@ -994,7 +993,7 @@ int virFileDeleteTree(const char *dir) cleanup: VIR_FREE(filepath); - closedir(dh); + VIR_DIR_CLOSE(dh); return ret; } @@ -2770,6 +2769,15 @@ int virDirRead(DIR *dirp, struct dirent **ent, const char *name) return !!*ent; } +void virDirClose(DIR **dirp) +{ + if (!*dirp) + return; + + closedir(*dirp); /* exempt from syntax-check */ + *dirp = NULL; +} + static int virFileMakePathHelper(char *path, mode_t mode) { diff --git a/src/util/virfile.h b/src/util/virfile.h index dae234e..ab9eeba 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -232,6 +232,9 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +void virDirClose(DIR **dirp) + ATTRIBUTE_NONNULL(1); +# define VIR_DIR_CLOSE(dir) virDirClose(&(dir)) int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK; int virFileMakePathWithMode(const char *path, diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 0cdba0a..6883466 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -477,8 +477,7 @@ virHostCPUParseNode(const char *node, ret = processors; cleanup: - if (cpudir) - closedir(cpudir); + VIR_DIR_CLOSE(cpudir); if (cores_maps) for (i = 0; i < sock_max; i++) virBitmapFree(cores_maps[i]); @@ -774,8 +773,7 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, ret = 0; cleanup: - if (nodedir) - closedir(nodedir); + VIR_DIR_CLOSE(nodedir); virBitmapFree(present_cpus_map); virBitmapFree(online_cpus_map); VIR_FREE(sysfs_nodedir); diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5a4ccc6..75ec484 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3188,7 +3188,7 @@ virNetDevRDMAFeature(const char *ifname, ret = 0; cleanup: - closedir(dirp); + VIR_DIR_CLOSE(dirp); VIR_FREE(eth_devpath); VIR_FREE(ib_devpath); VIR_FREE(eth_res_buf); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index b34cbb7..eec7614 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -144,7 +144,7 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED) cleanup: VIR_FREE(devpath); VIR_FORCE_CLOSE(fd); - closedir(dirp); + VIR_DIR_CLOSE(dirp); return ret; #else return NULL; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 23064ff..b756f7f 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -833,8 +833,7 @@ virNumaGetPages(int node, VIR_FREE(tmp_free); VIR_FREE(tmp_avail); VIR_FREE(tmp_size); - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(path); return ret; } diff --git a/src/util/virpci.c b/src/util/virpci.c index 095d706..5cb5d3a 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -509,7 +509,7 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, virPCIDeviceFree(check); } - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } @@ -1993,8 +1993,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, ret = 0; cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(file); VIR_FREE(pcidir); return ret; @@ -2051,8 +2050,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, cleanup: VIR_FREE(groupPath); - if (groupDir) - closedir(groupDir); + VIR_DIR_CLOSE(groupDir); return ret; } @@ -2713,7 +2711,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) break; } - closedir(dir); + VIR_DIR_CLOSE(dir); out: VIR_FREE(pcidev_sysfs_net_path); diff --git a/src/util/virprocess.c b/src/util/virprocess.c index bf6a6df..b0ca1ce 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -637,8 +637,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) ret = 0; cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(taskPath); if (ret < 0) VIR_FREE(*pids); diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 66b9017..72a5661 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -143,8 +143,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, } cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(path); return sg; } @@ -189,8 +188,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, } cleanup: - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(path); return name; } diff --git a/src/util/virusb.c b/src/util/virusb.c index 5c39667..33b188e 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -202,9 +202,7 @@ virUSBDeviceSearch(unsigned int vendor, ret = list; cleanup: - if (dir) - closedir(dir); - + VIR_DIR_CLOSE(dir); if (!ret) virObjectUnref(list); return ret; diff --git a/src/util/virutil.c b/src/util/virutil.c index 60da17b..a6c1273 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1915,7 +1915,7 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, } cleanup: - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(unique_path); VIR_FREE(host_link); VIR_FREE(host_path); @@ -2265,7 +2265,7 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, cleanup: # undef READ_WWN - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(wwnn_path); VIR_FREE(wwpn_path); VIR_FREE(wwnn_buf); @@ -2354,7 +2354,7 @@ virFindFCHostCapableVport(const char *sysfs_prefix) } cleanup: - closedir(dir); + VIR_DIR_CLOSE(dir); VIR_FREE(max_vports); VIR_FREE(vports); return ret; diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index d81a35d..cd169e0 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -372,21 +372,21 @@ xenInotifyOpen(virConnectPtr conn, /* Build the full file path */ if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) { - closedir(dh); + VIR_DIR_CLOSE(dh); return -1; } if (xenInotifyAddDomainConfigInfo(conn, path, now) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error adding file to config list")); - closedir(dh); + VIR_DIR_CLOSE(dh); VIR_FREE(path); return -1; } VIR_FREE(path); } - closedir(dh); + VIR_DIR_CLOSE(dh); if (direrr < 0) return -1; } diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index f7486b5..e7ac57e 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -358,7 +358,7 @@ xenXMConfigCacheRefresh(virConnectPtr conn) /* Build the full file path */ if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) { - closedir(dh); + VIR_DIR_CLOSE(dh); return -1; } @@ -386,7 +386,7 @@ xenXMConfigCacheRefresh(virConnectPtr conn) args.priv = priv; virHashRemoveSet(priv->configCache, xenXMConfigReaper, &args); - closedir(dh); + VIR_DIR_CLOSE(dh); return ret; } diff --git a/tests/virschematest.c b/tests/virschematest.c index 638fd0f..14a9e20 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -112,7 +112,7 @@ testSchemaDir(const char *schema, cleanup: VIR_FREE(test_name); VIR_FREE(xml_path); - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index de34baf..d179514 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -159,8 +159,7 @@ findLease(const char *name, VIR_FREE(path); } - closedir(dir); - dir = NULL; + VIR_DIR_CLOSE(dir); nleases = virJSONValueArraySize(leases_array); DEBUG("Read %zd leases", nleases); @@ -231,8 +230,7 @@ findLease(const char *name, *errnop = errno; VIR_FREE(tmpAddress); virJSONValueFree(leases_array); - if (dir) - closedir(dir); + VIR_DIR_CLOSE(dir); return ret; } -- 2.7.3
A helper that calls opendir and reports an error if it fails. --- src/libvirt_private.syms | 1 + src/util/virfile.c | 33 +++++++++++++++++++++++++++++---- src/util/virfile.h | 2 ++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f4dc88d..457fe19 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1495,6 +1495,7 @@ safezero; virBuildPathInternal; virDirClose; virDirCreate; +virDirOpen; virDirRead; virFileAbsPath; virFileAccessibleAs; diff --git a/src/util/virfile.c b/src/util/virfile.c index ce8f7fd..aac0324 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2736,6 +2736,31 @@ virFileRemove(const char *path, } #endif /* WIN32 */ +static int +virDirOpenInternal(DIR **dirp, const char *name) +{ + *dirp = opendir(name); + if (!*dirp) { + virReportSystemError(errno, _("cannot open directory '%s'"), name); + return -1; + } + return 1; +} + +/** + * virDirOpen + * @dirp: directory stream + * @name: path of the directory + * + * Returns 1 on success. + * On failure, -1 is returned and an error is reported. + */ +int +virDirOpen(DIR **dirp, const char *name) +{ + return virDirOpenInternal(dirp, name); +} + /** * virDirRead: * @dirp: directory to read @@ -2744,13 +2769,13 @@ virFileRemove(const char *path, * * Wrapper around readdir. Typical usage: * struct dirent ent; - * int value; + * int rc; * DIR *dir; - * if (!(dir = opendir(name))) + * if (virDirOpen(&dir, name) < 0) * goto error; - * while ((value = virDirRead(dir, &ent, name)) > 0) + * while ((rc = virDirRead(dir, &ent, name)) > 0) * process ent; - * if (value < 0) + * if (rc < 0) * goto error; * * Returns -1 on error, with error already reported if @name was diff --git a/src/util/virfile.h b/src/util/virfile.h index ab9eeba..c618842 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -230,6 +230,8 @@ enum { }; int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; +int virDirOpen(DIR **dirp, const char *dirname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; void virDirClose(DIR **dirp) -- 2.7.3
Switch from opendir to virDirOpen everywhere we need to report an error. --- src/storage/storage_backend_fs.c | 6 +----- src/storage/storage_backend_iscsi.c | 7 +------ src/storage/storage_backend_scsi.c | 19 +++---------------- src/util/vircgroup.c | 5 +---- src/util/virfile.c | 16 +++------------- src/util/virhostcpu.c | 4 +--- src/util/virnetdev.c | 6 +----- src/util/virnetdevtap.c | 7 +------ src/util/virpci.c | 13 +++---------- src/util/virprocess.c | 2 +- src/util/virscsi.c | 10 ++-------- src/util/virusb.c | 7 +------ src/util/virutil.c | 18 +++--------------- src/xen/xen_inotify.c | 7 ++----- src/xen/xm_internal.c | 6 +----- tests/virschematest.c | 6 +----- tools/nss/libvirt_nss.c | 4 +--- 17 files changed, 27 insertions(+), 116 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 152f9f3..2054309 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -900,12 +900,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, int direrr; int fd = -1, ret = -1; - if (!(dir = opendir(pool->def->target.path))) { - virReportSystemError(errno, - _("cannot open path '%s'"), - pool->def->target.path); + if (virDirOpen(&dir, pool->def->target.path) < 0) goto cleanup; - } while ((direrr = virDirRead(dir, &ent, pool->def->target.path)) > 0) { int err; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index e50158f..6283837 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -99,13 +99,8 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, virFileWaitForDevices(); - sysdir = opendir(sysfs_path); - - if (sysdir == NULL) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), sysfs_path); + if (virDirOpen(&sysdir, sysfs_path) < 0) goto cleanup; - } while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) { diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index b08d960..df683e4 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -265,12 +265,8 @@ getNewStyleBlockDevice(const char *lun_path, VIR_DEBUG("Looking for block device in '%s'", block_path); - if (!(block_dir = opendir(block_path))) { - virReportSystemError(errno, - _("Failed to opendir sysfs path '%s'"), - block_path); + if (virDirOpen(&block_dir, block_path) < 0) goto cleanup; - } while ((direrr = virDirRead(block_dir, &block_dirent, block_path)) > 0) { if (STREQLEN(block_dirent->d_name, ".", 1)) @@ -353,12 +349,8 @@ getBlockDevice(uint32_t host, host, bus, target, lun) < 0) goto cleanup; - if (!(lun_dir = opendir(lun_path))) { - virReportSystemError(errno, - _("Failed to opendir sysfs path '%s'"), - lun_path); + if (virDirOpen(&lun_dir, lun_path) < 0) goto cleanup; - } while ((direrr = virDirRead(lun_dir, &lun_dirent, lun_path)) > 0) { if (STREQLEN(lun_dirent->d_name, "block", 5)) { @@ -470,13 +462,8 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, virFileWaitForDevices(); - devicedir = opendir(device_path); - - if (devicedir == NULL) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), device_path); + if (virDirOpen(&devicedir, device_path) < 0) return -1; - } snprintf(devicepattern, sizeof(devicepattern), "%u:%%u:%%u:%%u\n", scanhost); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c76c94f..ce9b942 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3957,11 +3957,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup, cgroup->controllers[i].placement) < 0) goto cleanup; - if (!(dh = opendir(base))) { - virReportSystemError(errno, - _("Unable to open dir '%s'"), base); + if (virDirOpen(&dh, base) < 0) goto cleanup; - } while ((direrr = virDirRead(dh, &de, base)) > 0) { if (STREQ(de->d_name, ".") || diff --git a/src/util/virfile.c b/src/util/virfile.c index aac0324..7dee3d9 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -614,11 +614,8 @@ static int virFileLoopDeviceOpenSearch(char **dev_name) VIR_DEBUG("Looking for loop devices in /dev"); - if (!(dh = opendir("/dev"))) { - virReportSystemError(errno, "%s", - _("Unable to read /dev")); + if (virDirOpen(&dh, "/dev") < 0) goto cleanup; - } while ((direrr = virDirRead(dh, &de, "/dev")) > 0) { /* Checking 'loop' prefix is insufficient, since @@ -782,12 +779,8 @@ virFileNBDDeviceFindUnused(void) struct dirent *de; int direrr; - if (!(dh = opendir(SYSFS_BLOCK_DIR))) { - virReportSystemError(errno, - _("Cannot read directory %s"), - SYSFS_BLOCK_DIR); + if (virDirOpen(&dh, SYSFS_BLOCK_DIR) < 0) return NULL; - } while ((direrr = virDirRead(dh, &de, SYSFS_BLOCK_DIR)) > 0) { if (STRPREFIX(de->d_name, "nbd")) { @@ -942,11 +935,8 @@ int virFileDeleteTree(const char *dir) if (!dir || !virFileExists(dir)) return 0; - if (!(dh = opendir(dir))) { - virReportSystemError(errno, _("Cannot open dir '%s'"), - dir); + if (virDirOpen(&dh, dir) < 0) return -1; - } while ((direrr = virDirRead(dh, &de, dir)) > 0) { struct stat sb; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 6883466..087ce22 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -339,10 +339,8 @@ virHostCPUParseNode(const char *node, *cores = 0; *sockets = 0; - if (!(cpudir = opendir(node))) { - virReportSystemError(errno, _("cannot opendir %s"), node); + if (virDirOpen(&cpudir, node) < 0) goto cleanup; - } /* Keep track of the CPUs that belong to the current node */ if (!(node_cpus_map = virBitmapNew(npresent_cpus))) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 75ec484..84406de 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3157,12 +3157,8 @@ virNetDevRDMAFeature(const char *ifname, if (!virFileExists(SYSFS_INFINIBAND_DIR)) return 0; - if (!(dirp = opendir(SYSFS_INFINIBAND_DIR))) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - SYSFS_INFINIBAND_DIR); + if (virDirOpen(&dirp, SYSFS_INFINIBAND_DIR) < 0) return -1; - } if (virAsprintf(ð_devpath, SYSFS_NET_DIR "%s/device/resource", ifname) < 0) goto cleanup; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index eec7614..98e27bb 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -98,13 +98,8 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED) char *devpath = NULL; int fd; - DIR *dirp = opendir("/dev"); - if (dirp == NULL) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - "/dev"); + if (virDirOpen(&dirp, "/dev") < 0) return NULL; - } while (virDirRead(dirp, &dp, "/dev") > 0) { if (STRPREFIX(dp->d_name, "tap")) { diff --git a/src/util/virpci.c b/src/util/virpci.c index 5cb5d3a..77ae9b4 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -460,11 +460,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, VIR_DEBUG("%s %s: iterating over " PCI_SYSFS "devices", dev->id, dev->name); - dir = opendir(PCI_SYSFS "devices"); - if (!dir) { - VIR_WARN("Failed to open " PCI_SYSFS "devices"); + if (virDirOpen(&dir, PCI_SYSFS "devices") < 0) return -1; - } while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { unsigned int domain, bus, slot, function; @@ -1962,11 +1959,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, dev->address.slot, dev->address.function) < 0) goto cleanup; - if (!(dir = opendir(pcidir))) { - virReportSystemError(errno, - _("cannot open %s"), pcidir); + if (virDirOpen(&dir, pcidir) < 0) goto cleanup; - } while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) { /* Device assignment requires: @@ -2696,8 +2690,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) return -1; } - dir = opendir(pcidev_sysfs_net_path); - if (dir == NULL) + if (virDirOpen(&dir, pcidev_sysfs_net_path) < 0) goto out; while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { diff --git a/src/util/virprocess.c b/src/util/virprocess.c index b0ca1ce..a3e7f4e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -612,7 +612,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) (unsigned long long)pid) < 0) goto cleanup; - if (!(dir = opendir(taskPath))) + if (virDirOpen(&dir, taskPath) < 0) goto cleanup; while ((value = virDirRead(dir, &ent, taskPath)) > 0) { diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 72a5661..0f57494 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -127,11 +127,8 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, prefix, adapter_id, bus, target, unit) < 0) return NULL; - if (!(dir = opendir(path))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to open %s"), path); + if (virDirOpen(&dir, path) < 0) goto cleanup; - } while (virDirRead(dir, &entry, path) > 0) { if (entry->d_name[0] == '.') @@ -173,11 +170,8 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, prefix, adapter_id, bus, target, unit) < 0) return NULL; - if (!(dir = opendir(path))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to open %s"), path); + if (virDirOpen(&dir, path) < 0) goto cleanup; - } while (virDirRead(dir, &entry, path) > 0) { if (entry->d_name[0] == '.') diff --git a/src/util/virusb.c b/src/util/virusb.c index 33b188e..1db8173 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -138,13 +138,8 @@ virUSBDeviceSearch(unsigned int vendor, if (!(list = virUSBDeviceListNew())) goto cleanup; - dir = opendir(USB_SYSFS "/devices"); - if (!dir) { - virReportSystemError(errno, - _("Could not open directory %s"), - USB_SYSFS "/devices"); + if (virDirOpen(&dir, USB_SYSFS "/devices") < 0) goto cleanup; - } while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) { unsigned int found_prod, found_vend, found_bus, found_devno; diff --git a/src/util/virutil.c b/src/util/virutil.c index a6c1273..46313c2 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1860,12 +1860,8 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, char *unique_path = NULL; unsigned int read_unique_id; - if (!(dir = opendir(prefix))) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - prefix); + if (virDirOpen(&dir, prefix) < 0) return NULL; - } while (virDirRead(dir, &entry, prefix) > 0) { if (entry->d_name[0] == '.' || !virFileIsLink(entry->d_name)) @@ -2198,12 +2194,8 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, char *p; char *ret = NULL; - if (!(dir = opendir(prefix))) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - prefix); + if (virDirOpen(&dir, prefix) < 0) return NULL; - } # define READ_WWN(wwn_path, buf) \ do { \ @@ -2292,12 +2284,8 @@ virFindFCHostCapableVport(const char *sysfs_prefix) char *state = NULL; char *ret = NULL; - if (!(dir = opendir(prefix))) { - virReportSystemError(errno, - _("Failed to opendir path '%s'"), - prefix); + if (virDirOpen(&dir, prefix) < 0) return NULL; - } while (virDirRead(dir, &entry, prefix) > 0) { unsigned int host; diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index cd169e0..ac4070c 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -360,12 +360,9 @@ xenInotifyOpen(virConnectPtr conn, return -1; /* populate initial list */ - if (!(dh = opendir(priv->configDir))) { - virReportSystemError(errno, - _("cannot open directory: %s"), - priv->configDir); + if (virDirOpen(&dh, priv->configDir) < 0) return -1; - } + while ((direrr = virDirRead(dh, &ent, priv->configDir)) > 0) { if (STRPREFIX(ent->d_name, ".")) continue; diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index e7ac57e..3c34652 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -319,12 +319,8 @@ xenXMConfigCacheRefresh(virConnectPtr conn) priv->lastRefresh = now; /* Process the files in the config dir */ - if (!(dh = opendir(priv->configDir))) { - virReportSystemError(errno, - _("cannot read directory %s"), - priv->configDir); + if (virDirOpen(&dh, priv->configDir) < 0) return -1; - } while ((ret = virDirRead(dh, &ent, priv->configDir)) > 0) { struct stat st; diff --git a/tests/virschematest.c b/tests/virschematest.c index 14a9e20..c372c43 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -80,12 +80,8 @@ testSchemaDir(const char *schema, .validator = validator, }; - if (!(dir = opendir(dir_path))) { - virReportSystemError(errno, - "Failed to opendir path '%s'", - dir_path); + if (virDirOpen(&dir, dir_path) < 0) return -1; - } while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { if (!virFileHasSuffix(ent->d_name, ".xml")) diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index d179514..724cb06 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -125,10 +125,8 @@ findLease(const char *name, } - if (!(dir = opendir(leaseDir))) { - ERROR("Failed to open dir '%s'", leaseDir); + if (virDirOpen(&dir, leaseDir) < 0) goto cleanup; - } if (!(leases_array = virJSONValueNewArray())) { ERROR("Failed to create json array"); -- 2.7.3
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Switch from opendir to virDirOpen everywhere we need to report an error. --- src/storage/storage_backend_fs.c | 6 +----- src/storage/storage_backend_iscsi.c | 7 +------ src/storage/storage_backend_scsi.c | 19 +++---------------- src/util/vircgroup.c | 5 +---- src/util/virfile.c | 16 +++------------- src/util/virhostcpu.c | 4 +--- src/util/virnetdev.c | 6 +----- src/util/virnetdevtap.c | 7 +------ src/util/virpci.c | 13 +++---------- src/util/virprocess.c | 2 +- src/util/virscsi.c | 10 ++-------- src/util/virusb.c | 7 +------ src/util/virutil.c | 18 +++--------------- src/xen/xen_inotify.c | 7 ++----- src/xen/xm_internal.c | 6 +----- tests/virschematest.c | 6 +----- tools/nss/libvirt_nss.c | 4 +--- 17 files changed, 27 insertions(+), 116 deletions(-)
There were 4 in this pile which it's not 100% clear if they should actually be using the event virDirOpenQuiet... [...]
diff --git a/src/util/virpci.c b/src/util/virpci.c index 5cb5d3a..77ae9b4 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -460,11 +460,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
VIR_DEBUG("%s %s: iterating over " PCI_SYSFS "devices", dev->id, dev->name);
- dir = opendir(PCI_SYSFS "devices"); - if (!dir) { - VIR_WARN("Failed to open " PCI_SYSFS "devices"); + if (virDirOpen(&dir, PCI_SYSFS "devices") < 0) return -1; - }
This one it seems eventually would cause an error (although on opendir failure, the failure to open message could be issued twice...). It seems virPCIDeviceBusContainsActiveDevices may not want the error since it's caller virPCIDeviceTrySecondaryBusReset on failure will call virPCIDeviceGetParent which would seem to want the error. In this situation, I believe we'll see two failure to open errors.
while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { unsigned int domain, bus, slot, function; @@ -1962,11 +1959,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, dev->address.slot, dev->address.function) < 0) goto cleanup;
- if (!(dir = opendir(pcidir))) { - virReportSystemError(errno, - _("cannot open %s"), pcidir); + if (virDirOpen(&dir, pcidir) < 0) goto cleanup; - }
while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) { /* Device assignment requires: @@ -2696,8 +2690,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) return -1; }
- dir = opendir(pcidev_sysfs_net_path); - if (dir == NULL) + if (virDirOpen(&dir, pcidev_sysfs_net_path) < 0) goto out;
Quite a few callers - I didn't chase them all, but it's whether they all expect to have an error is a matter of interpretation. The callers (PF and VF searches) seem to have varying opinions on whether a < 0 return is really an error.
while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { diff --git a/src/util/virprocess.c b/src/util/virprocess.c index b0ca1ce..a3e7f4e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -612,7 +612,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) (unsigned long long)pid) < 0) goto cleanup;
- if (!(dir = opendir(taskPath))) + if (virDirOpen(&dir, taskPath) < 0) goto cleanup;
Another, although perhaps it's good in this case.
while ((value = virDirRead(dir, &ent, taskPath)) > 0) {
[...]
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index d179514..724cb06 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -125,10 +125,8 @@ findLease(const char *name, }
- if (!(dir = opendir(leaseDir))) { - ERROR("Failed to open dir '%s'", leaseDir); + if (virDirOpen(&dir, leaseDir) < 0) goto cleanup; - }
For this one ERROR is eaten by the # define; however, if ERROR was defined, then I don't believe ERROR would process things properly. Conditional ACK - I would think virOpenDirQuiet could be the better choice for these though John
if (!(leases_array = virJSONValueNewArray())) { ERROR("Failed to create json array");
On Thu, Jun 23, 2016 at 08:26:30AM -0400, John Ferlan wrote:
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Switch from opendir to virDirOpen everywhere we need to report an error. --- src/storage/storage_backend_fs.c | 6 +----- src/storage/storage_backend_iscsi.c | 7 +------ src/storage/storage_backend_scsi.c | 19 +++---------------- src/util/vircgroup.c | 5 +---- src/util/virfile.c | 16 +++------------- src/util/virhostcpu.c | 4 +--- src/util/virnetdev.c | 6 +----- src/util/virnetdevtap.c | 7 +------ src/util/virpci.c | 13 +++---------- src/util/virprocess.c | 2 +- src/util/virscsi.c | 10 ++-------- src/util/virusb.c | 7 +------ src/util/virutil.c | 18 +++--------------- src/xen/xen_inotify.c | 7 ++----- src/xen/xm_internal.c | 6 +----- tests/virschematest.c | 6 +----- tools/nss/libvirt_nss.c | 4 +--- 17 files changed, 27 insertions(+), 116 deletions(-)
There were 4 in this pile which it's not 100% clear if they should actually be using the event virDirOpenQuiet...
[...]
diff --git a/src/util/virpci.c b/src/util/virpci.c index 5cb5d3a..77ae9b4 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -460,11 +460,8 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
VIR_DEBUG("%s %s: iterating over " PCI_SYSFS "devices", dev->id, dev->name);
- dir = opendir(PCI_SYSFS "devices"); - if (!dir) { - VIR_WARN("Failed to open " PCI_SYSFS "devices"); + if (virDirOpen(&dir, PCI_SYSFS "devices") < 0) return -1; - }
This one it seems eventually would cause an error (although on opendir failure, the failure to open message could be issued twice...).
Both of the other error paths (virDirRead and virPCIDeviceNew) report an error, so opendir should be noisy too. Also, failing to open /sys/bus/pci/devices is unlikely.
It seems virPCIDeviceBusContainsActiveDevices may not want the error since it's caller virPCIDeviceTrySecondaryBusReset on failure will call virPCIDeviceGetParent which would seem to want the error.
If virPCIDeviceBusContainsActiveDevices fails and returns -1 (which is unlikely - either we failed to opendir /sys/bus/pci/devices, or we got EBADF from readdir on that same DIR*, or OOM), then if ((conflict = virPCIDeviceBusContainsActiveDevices(dev, inactiveDevs))) { is true and it exits early. I do not see the virPCIDeviceGetParent call.
In this situation, I believe we'll see two failure to open errors.
while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) { unsigned int domain, bus, slot, function; @@ -1962,11 +1959,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, dev->address.slot, dev->address.function) < 0) goto cleanup;
- if (!(dir = opendir(pcidir))) { - virReportSystemError(errno, - _("cannot open %s"), pcidir); + if (virDirOpen(&dir, pcidir) < 0) goto cleanup; - }
while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) { /* Device assignment requires: @@ -2696,8 +2690,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) return -1; }
- dir = opendir(pcidev_sysfs_net_path); - if (dir == NULL) + if (virDirOpen(&dir, pcidev_sysfs_net_path) < 0) goto out;
Quite a few callers - I didn't chase them all, but it's whether they all expect to have an error is a matter of interpretation. The callers (PF and VF searches) seem to have varying opinions on whether a < 0 return is really an error.
I'll change this one to use the Quiet version.
while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { diff --git a/src/util/virprocess.c b/src/util/virprocess.c index b0ca1ce..a3e7f4e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -612,7 +612,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) (unsigned long long)pid) < 0) goto cleanup;
- if (!(dir = opendir(taskPath))) + if (virDirOpen(&dir, taskPath) < 0) goto cleanup;
Another, although perhaps it's good in this case.
while ((value = virDirRead(dir, &ent, taskPath)) > 0) {
[...]
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index d179514..724cb06 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -125,10 +125,8 @@ findLease(const char *name, }
- if (!(dir = opendir(leaseDir))) { - ERROR("Failed to open dir '%s'", leaseDir); + if (virDirOpen(&dir, leaseDir) < 0) goto cleanup; - }
For this one ERROR is eaten by the # define; however, if ERROR was defined, then I don't believe ERROR would process things properly.
The error handling is a bit strange in that file, but it also uses VIR_REALLOC_QUIET, so I'll change this one too. Jan
Conditional ACK - I would think virOpenDirQuiet could be the better choice for these though
John
if (!(leases_array = virJSONValueNewArray())) { ERROR("Failed to create json array");
Just like virDirOpen, but it returns 0 without reporting an error on ENOENT. --- src/libvirt_private.syms | 1 + src/util/virfile.c | 21 +++++++++++++++++++-- src/util/virfile.h | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 457fe19..2bb1d95 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1496,6 +1496,7 @@ virBuildPathInternal; virDirClose; virDirCreate; virDirOpen; +virDirOpenIfExists; virDirRead; virFileAbsPath; virFileAccessibleAs; diff --git a/src/util/virfile.c b/src/util/virfile.c index 7dee3d9..efdb98b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2727,10 +2727,12 @@ virFileRemove(const char *path, #endif /* WIN32 */ static int -virDirOpenInternal(DIR **dirp, const char *name) +virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT) { *dirp = opendir(name); if (!*dirp) { + if (ignoreENOENT && errno == ENOENT) + return 0; virReportSystemError(errno, _("cannot open directory '%s'"), name); return -1; } @@ -2748,7 +2750,22 @@ virDirOpenInternal(DIR **dirp, const char *name) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name); + return virDirOpenInternal(dirp, name, false); +} + +/** + * virDirOpenIfExists + * @dirp: directory stream + * @name: path of the directory + * + * Returns 1 on success. + * If opendir returns ENOENT, 0 is returned without reporting an error. + * On other errors, -1 is returned and an error is reported. + */ +int +virDirOpenIfExists(DIR **dirp, const char *name) +{ + return virDirOpenInternal(dirp, name, true); } /** diff --git a/src/util/virfile.h b/src/util/virfile.h index c618842..42c65f2 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -232,6 +232,8 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virDirOpen(DIR **dirp, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virDirOpenIfExists(DIR **dirp, const char *dirname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; void virDirClose(DIR **dirp) -- 2.7.3
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Just like virDirOpen, but it returns 0 without reporting an error on ENOENT. --- src/libvirt_private.syms | 1 + src/util/virfile.c | 21 +++++++++++++++++++-- src/util/virfile.h | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 457fe19..2bb1d95 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1496,6 +1496,7 @@ virBuildPathInternal; virDirClose; virDirCreate; virDirOpen; +virDirOpenIfExists; virDirRead; virFileAbsPath; virFileAccessibleAs; diff --git a/src/util/virfile.c b/src/util/virfile.c index 7dee3d9..efdb98b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2727,10 +2727,12 @@ virFileRemove(const char *path, #endif /* WIN32 */
static int -virDirOpenInternal(DIR **dirp, const char *name) +virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT) { *dirp = opendir(name); if (!*dirp) { + if (ignoreENOENT && errno == ENOENT) + return 0;
Why not pass "ignore_errno" as an int and compare errno against it... I would think passing 0 otherwise would work unless open What you have works and is fine and ENOENT is the errno du jour to check, but just trying to future proof adding yet another argument. ACK to what's here - this is only a suggestion... John
virReportSystemError(errno, _("cannot open directory '%s'"), name); return -1; } @@ -2748,7 +2750,22 @@ virDirOpenInternal(DIR **dirp, const char *name) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name); + return virDirOpenInternal(dirp, name, false); +} + +/** + * virDirOpenIfExists + * @dirp: directory stream + * @name: path of the directory + * + * Returns 1 on success. + * If opendir returns ENOENT, 0 is returned without reporting an error. + * On other errors, -1 is returned and an error is reported. + */ +int +virDirOpenIfExists(DIR **dirp, const char *name) +{ + return virDirOpenInternal(dirp, name, true); }
/** diff --git a/src/util/virfile.h b/src/util/virfile.h index c618842..42c65f2 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -232,6 +232,8 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virDirOpen(DIR **dirp, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virDirOpenIfExists(DIR **dirp, const char *dirname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; void virDirClose(DIR **dirp)
Use it instead of opendir everywhere we need to check for ENOENT. --- src/conf/network_conf.c | 21 ++++++--------------- src/conf/nwfilter_conf.c | 10 +++------- src/conf/storage_conf.c | 20 ++++++-------------- src/conf/virdomainobjlist.c | 11 +++-------- src/conf/virsecretobj.c | 9 +++------ src/network/bridge_driver.c | 11 +++-------- src/qemu/qemu_driver.c | 8 +------- src/util/vircgroup.c | 14 ++++++-------- src/util/virnuma.c | 14 ++++---------- 9 files changed, 35 insertions(+), 83 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 1e4b719..2aa7242 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3236,14 +3236,10 @@ virNetworkLoadAllState(virNetworkObjListPtr nets, DIR *dir; struct dirent *entry; int ret = -1; + int rc; - if (!(dir = opendir(stateDir))) { - if (errno == ENOENT) - return 0; - - virReportSystemError(errno, _("Failed to open dir '%s'"), stateDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, stateDir)) <= 0) + return rc; while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virNetworkObjPtr net; @@ -3270,15 +3266,10 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets, DIR *dir; struct dirent *entry; int ret = -1; + int rc; - if (!(dir = opendir(configDir))) { - if (errno == ENOENT) - return 0; - virReportSystemError(errno, - _("Failed to open dir '%s'"), - configDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNetworkObjPtr net; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 56f8b86..74f2a14 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -3204,14 +3204,10 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters, DIR *dir; struct dirent *entry; int ret = -1; + int rc; - if (!(dir = opendir(configDir))) { - if (errno == ENOENT) - return 0; - virReportSystemError(errno, _("Failed to open dir '%s'"), - configDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNWFilterObjPtr nwfilter; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5c044d2..d1ca08b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1941,14 +1941,10 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, DIR *dir; struct dirent *entry; int ret = -1; + int rc; - if (!(dir = opendir(stateDir))) { - if (errno == ENOENT) - return 0; - - virReportSystemError(errno, _("Failed to open dir '%s'"), stateDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, stateDir)) <= 0) + return rc; while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virStoragePoolObjPtr pool; @@ -1977,14 +1973,10 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, DIR *dir; struct dirent *entry; int ret; + int rc; - if (!(dir = opendir(configDir))) { - if (errno == ENOENT) - return 0; - virReportSystemError(errno, _("Failed to open dir '%s'"), - configDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; while ((ret = virDirRead(dir, &entry, configDir)) > 0) { char *path; diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 4f7756d..51753fe 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -566,17 +566,12 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, DIR *dir; struct dirent *entry; int ret = -1; + int rc; VIR_INFO("Scanning for configs in %s", configDir); - if (!(dir = opendir(configDir))) { - if (errno == ENOENT) - return 0; - virReportSystemError(errno, - _("Failed to open dir '%s'"), - configDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; virObjectLock(doms); diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 46042eb..a093258 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -966,13 +966,10 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets, { DIR *dir = NULL; struct dirent *de; + int rc; - if (!(dir = opendir(configDir))) { - if (errno == ENOENT) - return 0; - virReportSystemError(errno, _("cannot open '%s'"), configDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) + return rc; /* Ignore errors reported by readdir or other calls within the * loop (if any). It's better to keep the secrets we managed to find. */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7b021d8..b108152 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -509,15 +509,10 @@ networkMigrateStateFiles(virNetworkDriverStatePtr driver) struct dirent *entry; char *oldPath = NULL, *newPath = NULL; char *contents = NULL; + int rc; - if (!(dir = opendir(oldStateDir))) { - if (errno == ENOENT) - return 0; - - virReportSystemError(errno, _("failed to open directory '%s'"), - oldStateDir); - return -1; - } + if ((rc = virDirOpenIfExists(&dir, oldStateDir)) <= 0) + return rc; if (virFileMakePath(driver->stateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 76ee3c1..7769272 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -498,14 +498,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name, snapDir); - if (!(dir = opendir(snapDir))) { - if (errno != ENOENT) - virReportSystemError(errno, - _("Failed to open snapshot directory %s " - "for domain %s"), - snapDir, vm->def->name); + if (virDirOpenIfExists(&dir, snapDir) <= 0) goto cleanup; - } while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { if (entry->d_name[0] == '.') diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index ce9b942..634f659 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3625,15 +3625,13 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, killedAny = true; VIR_DEBUG("Iterate over children of %s (killedAny=%d)", keypath, killedAny); - if (!(dp = opendir(keypath))) { - if (errno == ENOENT) { - VIR_DEBUG("Path %s does not exist, assuming done", keypath); - killedAny = false; - goto done; - } - virReportSystemError(errno, - _("Cannot open %s"), keypath); + if ((rc = virDirOpenIfExists(&dp, keypath)) < 0) goto cleanup; + + if (rc == 0) { + VIR_DEBUG("Path %s does not exist, assuming done", keypath); + killedAny = false; + goto done; } while ((direrr = virDirRead(dp, &ent, keypath)) > 0) { diff --git a/src/util/virnuma.c b/src/util/virnuma.c index b756f7f..fc25051 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -737,16 +737,10 @@ virNumaGetPages(int node, if (virNumaGetHugePageInfoDir(&path, node) < 0) goto cleanup; - if (!(dir = opendir(path))) { - /* It's okay if the @path doesn't exist. Maybe we are running on - * system without huge pages support where the path may not exist. */ - if (errno != ENOENT) { - virReportSystemError(errno, - _("unable to open path: %s"), - path); - goto cleanup; - } - } + /* It's okay if the @path doesn't exist. Maybe we are running on + * system without huge pages support where the path may not exist. */ + if (virDirOpenIfExists(&dir, path) < 0) + goto cleanup; while (dir && (direrr = virDirRead(dir, &entry, path)) > 0) { const char *page_name = entry->d_name; -- 2.7.3
A helper function that does not report any errors. --- src/libvirt_private.syms | 1 + src/util/virfile.c | 25 ++++++++++++++++++++++--- src/util/virfile.h | 2 ++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2bb1d95..8ebe6f3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1497,6 +1497,7 @@ virDirClose; virDirCreate; virDirOpen; virDirOpenIfExists; +virDirOpenQuiet; virDirRead; virFileAbsPath; virFileAccessibleAs; diff --git a/src/util/virfile.c b/src/util/virfile.c index efdb98b..f6c43d4 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2727,10 +2727,13 @@ virFileRemove(const char *path, #endif /* WIN32 */ static int -virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT) +virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) { *dirp = opendir(name); if (!*dirp) { + if (quiet) + return -1; + if (ignoreENOENT && errno == ENOENT) return 0; virReportSystemError(errno, _("cannot open directory '%s'"), name); @@ -2750,7 +2753,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT) int virDirOpen(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, false); + return virDirOpenInternal(dirp, name, false, false); } /** @@ -2765,7 +2768,23 @@ virDirOpen(DIR **dirp, const char *name) int virDirOpenIfExists(DIR **dirp, const char *name) { - return virDirOpenInternal(dirp, name, true); + return virDirOpenInternal(dirp, name, true, false); +} + +/** + * virDirOpenQuiet + * @dirp: directory stream + * @name: path of the directory + * + * Returns 1 on success. + * -1 on failure. + * + * Does not report any errors and errno is preserved. + */ +int +virDirOpenQuiet(DIR **dirp, const char *name) +{ + return virDirOpenInternal(dirp, name, false, true); } /** diff --git a/src/util/virfile.h b/src/util/virfile.h index 42c65f2..b4ae6ea 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -234,6 +234,8 @@ int virDirOpen(DIR **dirp, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virDirOpenIfExists(DIR **dirp, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virDirOpenQuiet(DIR **dirp, const char *dirname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; void virDirClose(DIR **dirp) -- 2.7.3
Remove all the remaining usage of opendir. --- src/openvz/openvz_conf.c | 3 +-- src/qemu/qemu_hostdev.c | 2 +- src/storage/storage_backend.c | 2 +- src/util/vircgroup.c | 3 +-- src/util/virhostcpu.c | 2 +- src/util/virpci.c | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index ff5e5b8..b678456 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -1057,8 +1057,7 @@ static int openvzAssignUUIDs(void) if (conf_dir == NULL) return -1; - dp = opendir(conf_dir); - if (dp == NULL) { + if (virDirOpenQuiet(&dp, conf_dir) < 0) { VIR_FREE(conf_dir); return 0; } diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 84f3615..2fdfae9 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -111,7 +111,7 @@ qemuHostdevHostSupportsPassthroughVFIO(void) int direrr; /* condition 1 - /sys/kernel/iommu_groups/ contains entries */ - if (!(iommuDir = opendir("/sys/kernel/iommu_groups/"))) + if (virDirOpenQuiet(&iommuDir, "/sys/kernel/iommu_groups/") < 0) goto cleanup; while ((direrr = virDirRead(iommuDir, &iommuGroup, NULL)) > 0) { diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 955d983..31c2974 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1928,7 +1928,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, * get created. */ reopen: - if ((dh = opendir(pool->def->target.path)) == NULL) { + if (virDirOpenQuiet(&dh, pool->def->target.path) < 0) { opentries++; if (loop && errno == ENOENT && opentries < 50) { usleep(100 * 1000); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 634f659..da20ba5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3378,8 +3378,7 @@ virCgroupRemoveRecursively(char *grppath) int rc = 0; int direrr; - grpdir = opendir(grppath); - if (grpdir == NULL) { + if (virDirOpenQuiet(&grpdir, grppath) < 0) { if (errno == ENOENT) return 0; rc = -errno; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 087ce22..f38fbec 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -641,7 +641,7 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, if (virAsprintf(&sysfs_nodedir, "%s/node", sysfs_system_path) < 0) goto cleanup; - if (!(nodedir = opendir(sysfs_nodedir))) { + if (virDirOpenQuiet(&nodedir, sysfs_nodedir) < 0) { /* the host isn't probably running a NUMA architecture */ goto fallback; } diff --git a/src/util/virpci.c b/src/util/virpci.c index 77ae9b4..c497f02 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2015,7 +2015,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, orig->domain, orig->bus, orig->slot, orig->function) < 0) goto cleanup; - if (!(groupDir = opendir(groupPath))) { + if (virDirOpenQuiet(&groupDir, groupPath) < 0) { /* just process the original device, nothing more */ ret = (actor)(orig, opaque); goto cleanup; -- 2.7.3
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Remove all the remaining usage of opendir. --- src/openvz/openvz_conf.c | 3 +-- src/qemu/qemu_hostdev.c | 2 +- src/storage/storage_backend.c | 2 +- src/util/vircgroup.c | 3 +-- src/util/virhostcpu.c | 2 +- src/util/virpci.c | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-)
[...]
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 634f659..da20ba5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3378,8 +3378,7 @@ virCgroupRemoveRecursively(char *grppath) int rc = 0; int direrr;
- grpdir = opendir(grppath); - if (grpdir == NULL) { + if (virDirOpenQuiet(&grpdir, grppath) < 0) { if (errno == ENOENT) return 0;
This one seems to be an ENOENT case...
rc = -errno;
[...] ACK with the obvious adjustment John
On Thu, Jun 23, 2016 at 08:36:21AM -0400, John Ferlan wrote:
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Remove all the remaining usage of opendir. --- src/openvz/openvz_conf.c | 3 +-- src/qemu/qemu_hostdev.c | 2 +- src/storage/storage_backend.c | 2 +- src/util/vircgroup.c | 3 +-- src/util/virhostcpu.c | 2 +- src/util/virpci.c | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-)
[...]
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 634f659..da20ba5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3378,8 +3378,7 @@ virCgroupRemoveRecursively(char *grppath) int rc = 0; int direrr;
- grpdir = opendir(grppath); - if (grpdir == NULL) { + if (virDirOpenQuiet(&grpdir, grppath) < 0) { if (errno == ENOENT) return 0;
This one seems to be an ENOENT case...
The IfExists version is not quiet. Instead of introducing another helper, I've left this one open-coded until someone rewrites the virCgroup* APIs to report errors. Jan
Prefer virDirOpen. --- cfg.mk | 7 +++++-- src/util/virfile.c | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index a2576d1..de1c1da 100644 --- a/cfg.mk +++ b/cfg.mk @@ -421,9 +421,9 @@ sc_prohibit_gethostname: $(_sc_search_regexp) sc_prohibit_readdir: - @prohibit='\b(read|close)dir *\(' \ + @prohibit='\b(read|close|open)dir *\(' \ exclude='exempt from syntax-check' \ - halt='use virDirRead and VIR_DIR_CLOSE' \ + halt='use virDirOpen, virDirRead and VIR_DIR_CLOSE' \ $(_sc_search_regexp) sc_prohibit_gettext_noop: @@ -1293,3 +1293,6 @@ exclude_file_name_regexp--sc_prohibit_dt_without_code = \ exclude_file_name_regexp--sc_prohibit_always-defined_macros = \ ^tests/virtestmock.c$$ + +exclude_file_name_regexp--sc_prohibit_readdir = \ + ^tests/.*mock\.c$$ diff --git a/src/util/virfile.c b/src/util/virfile.c index f6c43d4..1820e80 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2729,7 +2729,7 @@ virFileRemove(const char *path, static int virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet) { - *dirp = opendir(name); + *dirp = opendir(name); /* exempt from syntax-check */ if (!*dirp) { if (quiet) return -1; -- 2.7.3
All of the callers either skip these explicitly, skip all entries starting with a dot or match the entry name against stricter patterns. --- src/util/virfile.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 1820e80..2772089 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2809,14 +2809,17 @@ virDirOpenQuiet(DIR **dirp, const char *name) */ int virDirRead(DIR *dirp, struct dirent **ent, const char *name) { - errno = 0; - *ent = readdir(dirp); /* exempt from syntax-check */ - if (!*ent && errno) { - if (name) - virReportSystemError(errno, _("Unable to read directory '%s'"), - name); - return -1; - } + do { + errno = 0; + *ent = readdir(dirp); /* exempt from syntax-check */ + if (!*ent && errno) { + if (name) + virReportSystemError(errno, _("Unable to read directory '%s'"), + name); + return -1; + } + } while (*ent && (STREQ((*ent)->d_name, ".") || + STREQ((*ent)->d_name, ".."))); return !!*ent; } -- 2.7.3
It skips those directory entries. --- src/conf/virsecretobj.c | 3 --- src/network/bridge_driver.c | 4 ---- src/util/vircgroup.c | 8 -------- src/util/virfile.c | 4 ---- src/util/virpci.c | 4 ---- 5 files changed, 23 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index a093258..30a5e80 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -977,9 +977,6 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets, char *path; virSecretObjPtr secret; - if (STREQ(de->d_name, ".") || STREQ(de->d_name, "..")) - continue; - if (!virFileHasSuffix(de->d_name, ".xml")) continue; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b108152..58ceaf2 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -525,10 +525,6 @@ networkMigrateStateFiles(virNetworkDriverStatePtr driver) entry->d_type != DT_REG) continue; - if (STREQ(entry->d_name, ".") || - STREQ(entry->d_name, "..")) - continue; - if (virAsprintf(&oldPath, "%s/%s", oldStateDir, entry->d_name) < 0) goto cleanup; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index da20ba5..971894a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3634,10 +3634,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, } while ((direrr = virDirRead(dp, &ent, keypath)) > 0) { - if (STREQ(ent->d_name, ".")) - continue; - if (STREQ(ent->d_name, "..")) - continue; if (ent->d_type != DT_DIR) continue; @@ -3958,10 +3954,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup, goto cleanup; while ((direrr = virDirRead(dh, &de, base)) > 0) { - if (STREQ(de->d_name, ".") || - STREQ(de->d_name, "..")) - continue; - if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) goto cleanup; diff --git a/src/util/virfile.c b/src/util/virfile.c index 2772089..a45279a 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -941,10 +941,6 @@ int virFileDeleteTree(const char *dir) while ((direrr = virDirRead(dh, &de, dir)) > 0) { struct stat sb; - if (STREQ(de->d_name, ".") || - STREQ(de->d_name, "..")) - continue; - if (virAsprintf(&filepath, "%s/%s", dir, de->d_name) < 0) goto cleanup; diff --git a/src/util/virpci.c b/src/util/virpci.c index c497f02..948fdbf 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2694,10 +2694,6 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname) goto out; while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { - if (STREQ(entry->d_name, ".") || - STREQ(entry->d_name, "..")) - continue; - /* Assume a single directory entry */ if (VIR_STRDUP(*netname, entry->d_name) > 0) ret = 0; -- 2.7.3
'.' and '..' are now skipped by virDirRead, there's no need to mention them in the comment. --- src/storage/storage_backend_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 2054309..44dabf4 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -932,7 +932,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, &vol->target.encryption)) < 0) { if (err == -2) { /* Silently ignore non-regular files, - * eg '.' '..', 'lost+found', dangling symbolic link */ + * eg 'lost+found', dangling symbolic link */ virStorageVolDefFree(vol); vol = NULL; continue; -- 2.7.3
The directories we iterate over are unlikely to contain any entries starting with a dot, other than '.' and '..' which is already skipped by virDirRead. --- src/qemu/qemu_hostdev.c | 4 ---- src/storage/storage_backend_scsi.c | 3 --- src/util/vircgroup.c | 1 - src/util/virnetdev.c | 2 -- src/util/virpci.c | 7 ------- src/util/virprocess.c | 4 ---- src/util/virscsi.c | 6 ------ src/util/virusb.c | 2 +- src/util/virutil.c | 8 +------- 9 files changed, 2 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 2fdfae9..dd3a3cf 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -115,10 +115,6 @@ qemuHostdevHostSupportsPassthroughVFIO(void) goto cleanup; while ((direrr = virDirRead(iommuDir, &iommuGroup, NULL)) > 0) { - /* skip ./ ../ */ - if (STRPREFIX(iommuGroup->d_name, ".")) - continue; - /* assume we found a group */ break; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index df683e4..558b3cf 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -269,9 +269,6 @@ getNewStyleBlockDevice(const char *lun_path, goto cleanup; while ((direrr = virDirRead(block_dir, &block_dirent, block_path)) > 0) { - if (STREQLEN(block_dirent->d_name, ".", 1)) - continue; - if (VIR_STRDUP(*block_device, block_dirent->d_name) < 0) goto cleanup; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 971894a..04f3818 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3391,7 +3391,6 @@ virCgroupRemoveRecursively(char *grppath) while ((direrr = virDirRead(grpdir, &ent, NULL)) > 0) { char *path; - if (ent->d_name[0] == '.') continue; if (ent->d_type != DT_DIR) continue; if (virAsprintf(&path, "%s/%s", grppath, ent->d_name) == -1) { diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 84406de..cdbc41b 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3168,8 +3168,6 @@ virNetDevRDMAFeature(const char *ifname, goto cleanup; while (virDirRead(dirp, &dp, SYSFS_INFINIBAND_DIR) > 0) { - if (dp->d_name[0] == '.') - continue; if (virAsprintf(&ib_devpath, SYSFS_INFINIBAND_DIR "%s/device/resource", dp->d_name) < 0) continue; diff --git a/src/util/virpci.c b/src/util/virpci.c index 948fdbf..04ff68b 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -468,10 +468,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, virPCIDevicePtr check; char *tmp; - /* Ignore '.' and '..' */ - if (entry->d_name[0] == '.') - continue; - /* expected format: <domain>:<bus>:<slot>.<function> */ if (/* domain */ virStrToLong_ui(entry->d_name, &tmp, 16, &domain) < 0 || *tmp != ':' || @@ -2024,9 +2020,6 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, while ((direrr = virDirRead(groupDir, &ent, groupPath)) > 0) { virPCIDeviceAddress newDev; - if (ent->d_name[0] == '.') - continue; - if (virPCIDeviceAddressParse(ent->d_name, &newDev) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Found invalid device link '%s' in '%s'"), diff --git a/src/util/virprocess.c b/src/util/virprocess.c index a3e7f4e..09dd3c9 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -619,10 +619,6 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) long long tmp; pid_t tmp_pid; - /* Skip . and .. */ - if (STRPREFIX(ent->d_name, ".")) - continue; - if (virStrToLong_ll(ent->d_name, NULL, 10, &tmp) < 0) goto cleanup; tmp_pid = tmp; diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 0f57494..4843367 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -131,9 +131,6 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, goto cleanup; while (virDirRead(dir, &entry, path) > 0) { - if (entry->d_name[0] == '.') - continue; - /* Assume a single directory entry */ ignore_value(VIR_STRDUP(sg, entry->d_name)); break; @@ -174,9 +171,6 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, goto cleanup; while (virDirRead(dir, &entry, path) > 0) { - if (entry->d_name[0] == '.') - continue; - ignore_value(VIR_STRDUP(name, entry->d_name)); break; } diff --git a/src/util/virusb.c b/src/util/virusb.c index 1db8173..6a001a7 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -145,7 +145,7 @@ virUSBDeviceSearch(unsigned int vendor, unsigned int found_prod, found_vend, found_bus, found_devno; char *tmpstr = de->d_name; - if (de->d_name[0] == '.' || strchr(de->d_name, ':')) + if (strchr(de->d_name, ':')) continue; if (virUSBSysReadFile("idVendor", de->d_name, diff --git a/src/util/virutil.c b/src/util/virutil.c index 46313c2..170dd59 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1864,7 +1864,7 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, return NULL; while (virDirRead(dir, &entry, prefix) > 0) { - if (entry->d_name[0] == '.' || !virFileIsLink(entry->d_name)) + if (!virFileIsLink(entry->d_name)) continue; if (virAsprintf(&host_link, "%s/%s", prefix, entry->d_name) < 0) @@ -2210,9 +2210,6 @@ virGetFCHostNameByWWN(const char *sysfs_prefix, } while (0) while (virDirRead(dir, &entry, prefix) > 0) { - if (entry->d_name[0] == '.') - continue; - if (virAsprintf(&wwnn_path, "%s/%s/node_name", prefix, entry->d_name) < 0) goto cleanup; @@ -2291,9 +2288,6 @@ virFindFCHostCapableVport(const char *sysfs_prefix) unsigned int host; char *p = NULL; - if (entry->d_name[0] == '.') - continue; - p = entry->d_name + strlen("host"); if (virStrToLong_ui(p, NULL, 10, &host) == -1) { VIR_DEBUG("Failed to parse host number from '%s'", -- 2.7.3
The device names are unlikely to start with a dot. '.' and '..' are already skipped by virDirRead. --- src/storage/storage_backend.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 31c2974..32f0517 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1951,9 +1951,6 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, */ retry: while ((direrr = virDirRead(dh, &dent, NULL)) > 0) { - if (dent->d_name[0] == '.') - continue; - if (virAsprintf(&stablepath, "%s/%s", pool->def->target.path, dent->d_name) == -1) { -- 2.7.3
This fixes the disappearance of domains and networks starting with a dot. https://bugzilla.redhat.com/show_bug.cgi?id=1333248 --- src/conf/network_conf.c | 6 ------ src/conf/nwfilter_conf.c | 3 --- src/conf/storage_conf.c | 6 ------ src/conf/virdomainobjlist.c | 3 --- src/qemu/qemu_driver.c | 3 --- 5 files changed, 21 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 2aa7242..dfa851b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3244,9 +3244,6 @@ virNetworkLoadAllState(virNetworkObjListPtr nets, while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virNetworkObjPtr net; - if (entry->d_name[0] == '.') - continue; - if (!virFileStripSuffix(entry->d_name, ".xml")) continue; @@ -3274,9 +3271,6 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNetworkObjPtr net; - if (entry->d_name[0] == '.') - continue; - if (!virFileStripSuffix(entry->d_name, ".xml")) continue; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 74f2a14..2cdcfa7 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -3212,9 +3212,6 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNWFilterObjPtr nwfilter; - if (entry->d_name[0] == '.') - continue; - if (!virFileStripSuffix(entry->d_name, ".xml")) continue; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index d1ca08b..05a1a49 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1949,9 +1949,6 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr pools, while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virStoragePoolObjPtr pool; - if (entry->d_name[0] == '.') - continue; - if (!virFileStripSuffix(entry->d_name, ".xml")) continue; @@ -1983,9 +1980,6 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, char *autostartLink; virStoragePoolObjPtr pool; - if (entry->d_name[0] == '.') - continue; - if (!virFileHasSuffix(entry->d_name, ".xml")) continue; diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 51753fe..41c9910 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -578,9 +578,6 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virDomainObjPtr dom; - if (entry->d_name[0] == '.') - continue; - if (!virFileStripSuffix(entry->d_name, ".xml")) continue; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7769272..1174317 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -502,9 +502,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, goto cleanup; while ((direrr = virDirRead(dir, &entry, NULL)) > 0) { - if (entry->d_name[0] == '.') - continue; - /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ VIR_INFO("Loading snapshot file '%s'", entry->d_name); -- 2.7.3
'.' and '..' are skipped by virDirRead already. --- tools/nss/libvirt_nss.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 724cb06..66e338a 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -137,9 +137,6 @@ findLease(const char *name, while ((ret = virDirRead(dir, &entry, leaseDir)) > 0) { char *path; - if (entry->d_name[0] == '.') - continue; - if (!virFileHasSuffix(entry->d_name, ".status")) continue; -- 2.7.3
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Also introduce virDirOpen* and VIR_DIR_CLOSE helpers.
https://bugzilla.redhat.com/show_bug.cgi?id=1333248
Ján Tomko (18): Do not save errno in virUSBDeviceSearch Fix error detection in virStorageBackendISCSIGetHostNumber Do not check the return value of closedir Introduce VIR_DIR_CLOSE Introduce virDirOpen Use virDirOpen Add virDirOpenIfExists Use virDirOpenIfExists Introduce virDirOpenQuiet Use virDirOpenQuiet Prohibit opendir in syntax-check Skip '.' and '..' in virDirRead Do not check for '.' and '..' after virDirRead Fix comment in virStorageBackendFileSystemRefresh Do not ignore hidden files in /sys and /proc Do not skip hidden entries when looking for a stable path Allow configs to start with a dot Do not skip files starting with a dot in leases directory
cfg.mk | 7 +- src/conf/network_conf.c | 31 +++------ src/conf/nwfilter_conf.c | 15 ++--- src/conf/storage_conf.c | 30 +++------ src/conf/virdomainobjlist.c | 16 ++--- src/conf/virsecretobj.c | 14 ++-- src/libvirt_private.syms | 4 ++ src/network/bridge_driver.c | 17 ++--- src/openvz/openvz_conf.c | 5 +- src/qemu/qemu_driver.c | 14 +--- src/qemu/qemu_hostdev.c | 10 +-- src/storage/storage_backend.c | 11 ++-- src/storage/storage_backend_fs.c | 14 ++-- src/storage/storage_backend_iscsi.c | 47 +++++++------- src/storage/storage_backend_scsi.c | 30 ++------- src/util/vircgroup.c | 43 ++++--------- src/util/virfile.c | 124 ++++++++++++++++++++++++++---------- src/util/virfile.h | 9 +++ src/util/virhostcpu.c | 19 ++---- src/util/virnetdev.c | 10 +-- src/util/virnetdevtap.c | 9 +-- src/util/virnuma.c | 17 ++--- src/util/virpci.c | 36 +++-------- src/util/virprocess.c | 9 +-- src/util/virscsi.c | 22 ++----- src/util/virusb.c | 16 +---- src/util/virutil.c | 32 ++-------- src/xen/xen_inotify.c | 13 ++-- src/xen/xm_internal.c | 10 +-- tests/virschematest.c | 8 +-- tools/nss/libvirt_nss.c | 13 +--- 31 files changed, 245 insertions(+), 410 deletions(-)
ACK series modulo the couple of notes I made along the way. John
On Thu, Jun 23, 2016 at 08:37:30AM -0400, John Ferlan wrote:
On 06/21/2016 12:05 PM, Ján Tomko wrote:
Also introduce virDirOpen* and VIR_DIR_CLOSE helpers.
https://bugzilla.redhat.com/show_bug.cgi?id=1333248
Ján Tomko (18):
...
31 files changed, 245 insertions(+), 410 deletions(-)
ACK series modulo the couple of notes I made along the way.
Thanks, I have left patches 2 and 5-11 for later and pushed the rest.
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
John Ferlan -
Ján Tomko