On 09/13/2013 08:19 AM, Peter Krempa wrote: s/wether/whether/ or even: s/wether/true if the/ ACK once you avoid the typo. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The function existed in two identical instances in lxc and qemu. Move it to vircgroup.c and simplify it. Refactor the callers too. --- src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 52 +++++------------------------------------------- src/qemu/qemu_driver.c | 51 +++++------------------------------------------ src/util/vircgroup.c | 29 +++++++++++++++++++++++++++ src/util/vircgroup.h | 2 ++ 5 files changed, 42 insertions(+), 93 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..82e3d6f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1217,6 +1217,7 @@ virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; +virCgroupSupportsCpuBW; # util/virclosecallbacks.h diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b587c22..87ced95 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1574,45 +1574,10 @@ static char *lxcConnectGetHostname(virConnectPtr conn) } - -/* - * check whether the host supports CFS bandwidth - * - * Return 1 when CFS bandwidth is supported, 0 when CFS bandwidth is not - * supported, -1 on error. - */ -static int lxcGetCpuBWStatus(virCgroupPtr cgroup) -{ - char *cfs_period_path = NULL; - int ret = -1; - - if (!cgroup) - return 0; - - if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", &cfs_period_path) < 0) { - VIR_INFO("cannot get the path of cgroup CPU controller"); - ret = 0; - goto cleanup; - } - - if (access(cfs_period_path, F_OK) < 0) { - ret = 0; - } else { - ret = 1; - } - -cleanup: - VIR_FREE(cfs_period_path); - return ret; -} - - static char *lxcDomainGetSchedulerType(virDomainPtr dom, int *nparams) { char *ret = NULL; - int rc; virDomainObjPtr vm; virLXCDomainObjPrivatePtr priv; @@ -1639,13 +1604,10 @@ static char *lxcDomainGetSchedulerType(virDomainPtr dom, } if (nparams) { - rc = lxcGetCpuBWStatus(priv->cgroup); - if (rc < 0) - goto cleanup; - else if (rc == 0) - *nparams = 1; - else + if (virCgroupSupportsCpuBW(priv->cgroup)) *nparams = 3; + else + *nparams = 1; } ignore_value(VIR_STRDUP(ret, "posix")); @@ -1872,12 +1834,8 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, if (virDomainGetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (*nparams > 1) { - rc = lxcGetCpuBWStatus(priv->cgroup); - if (rc < 0) - goto cleanup; - cpu_bw_status = !!rc; - } + if (*nparams > 1) + cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup); if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ae1948f..0caeb08 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7325,44 +7325,10 @@ cleanup: } -/* - * check whether the host supports CFS bandwidth - * - * Return 1 when CFS bandwidth is supported, 0 when CFS bandwidth is not - * supported, -1 on error. - */ -static int qemuGetCpuBWStatus(virCgroupPtr cgroup) -{ - char *cfs_period_path = NULL; - int ret = -1; - - if (!cgroup) - return 0; - - if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", &cfs_period_path) < 0) { - VIR_INFO("cannot get the path of cgroup CPU controller"); - ret = 0; - goto cleanup; - } - - if (access(cfs_period_path, F_OK) < 0) { - ret = 0; - } else { - ret = 1; - } - -cleanup: - VIR_FREE(cfs_period_path); - return ret; -} - - static char *qemuDomainGetSchedulerType(virDomainPtr dom, int *nparams) { char *ret = NULL; - int rc; virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv; @@ -7389,13 +7355,10 @@ static char *qemuDomainGetSchedulerType(virDomainPtr dom, } if (nparams) { - rc = qemuGetCpuBWStatus(priv->cgroup); - if (rc < 0) - goto cleanup; - else if (rc == 0) - *nparams = 1; - else + if (virCgroupSupportsCpuBW(priv->cgroup)) *nparams = 5; + else + *nparams = 1; } ignore_value(VIR_STRDUP(ret, "posix")); @@ -8728,12 +8691,8 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, if (virDomainGetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (*nparams > 1) { - rc = qemuGetCpuBWStatus(priv->cgroup); - if (rc < 0) - goto cleanup; - cpu_bw_status = !!rc; - } + if (*nparams > 1) + cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 626bbc6..0b4e901 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3647,3 +3647,32 @@ virCgroupIsolateMount(virCgroupPtr group ATTRIBUTE_UNUSED, } #endif /* !VIR_CGROUP_SUPPORTED */ + +/** + * virCgroupSupportsCpuBW(): + * Check whether the host supports CFS bandwidth. + * + * Return true when CFS bandwidth is supported, + * false when CFS bandwidth is not supported. + */ +bool +virCgroupSupportsCpuBW(virCgroupPtr cgroup) +{ + char *path = NULL; + int ret = false; + + if (!cgroup) + return false; + + if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, + "cpu.cfs_period_us", &path) < 0) { + virResetLastError(); + goto cleanup; + } + + ret = virFileExists(path); + +cleanup: + VIR_FREE(path); + return ret; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 7bb4b2a..835eb30 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -207,4 +207,6 @@ int virCgroupIsolateMount(virCgroupPtr group, const char *oldroot, const char *mountopts); +bool virCgroupSupportsCpuBW(virCgroupPtr cgroup); + #endif /* __VIR_CGROUP_H__ */ -- 1.8.3.2

Semantics of the libvirt helper are more clear. This change also allows to clean up some pieces of code. --- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_storage.c | 2 +- src/qemu/qemu_capabilities.c | 4 ++-- src/qemu/qemu_cgroup.c | 6 +++--- src/qemu/qemu_process.c | 2 +- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_mpath.c | 8 +------- src/storage/storage_backend_scsi.c | 3 +-- src/util/vircgroup.c | 2 +- src/util/virfile.c | 2 +- src/util/virpci.c | 2 +- src/util/virscsi.c | 2 +- src/util/virutil.c | 6 +++--- 14 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 93f2377..0dbaa4a 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -940,7 +940,7 @@ openvzLocateConfDir(void) char *ret = NULL; while (conf_dir_list[i]) { - if (!access(conf_dir_list[i], F_OK)) { + if (virFileExists(conf_dir_list[i])) { ignore_value(VIR_STRDUP(ret, conf_dir_list[i])); goto cleanup; } diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 44246a7..bb5066f 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -362,7 +362,7 @@ static int parallelsFindVmVolumes(virStoragePoolObjPtr pool, "DiskDescriptor", ".xml"))) goto cleanup; - if (access(diskDescPath, F_OK)) + if (!virFileExists(diskDescPath)) continue; /* here we know, that ent->d_name is a disk image directory */ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d94188a..d830e2a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -731,13 +731,13 @@ virQEMUCapsInitGuest(virCapsPtr caps, if (!binary) return 0; - if (access("/dev/kvm", F_OK) == 0 && + if (virFileExists("/dev/kvm") && (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_KVM) || virQEMUCapsGet(qemubinCaps, QEMU_CAPS_ENABLE_KVM) || kvmbin)) haskvm = true; - if (access("/dev/kqemu", F_OK) == 0 && + if (virFileExists("/dev/kqemu") && virQEMUCapsGet(qemubinCaps, QEMU_CAPS_KQEMU)) haskqemu = true; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index cf41c33..f95c7f2 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -33,6 +33,7 @@ #include "domain_audit.h" #include "virscsi.h" #include "virstring.h" +#include "virfile.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -501,9 +502,8 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, } for (i = 0; deviceACL[i] != NULL; i++) { - if (access(deviceACL[i], F_OK) < 0) { - VIR_DEBUG("Ignoring non-existant device %s", - deviceACL[i]); + if (!virFileExists(deviceACL[i])) { + VIR_DEBUG("Ignoring non-existant device %s", deviceACL[i]); continue; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c7cec5a..dd16f6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3632,7 +3632,7 @@ int qemuProcessStart(virConnectPtr conn, if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) { VIR_DEBUG("Checking for KVM availability"); - if (access("/dev/kvm", F_OK) != 0) { + if (!virFileExists("/dev/kvm")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Domain requires KVM, but it is not available. " "Check that virtualization is enabled in the host BIOS, " diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d305b06..956cfce 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -503,7 +503,7 @@ virStorageBackendFileSystemCheck(virConnectPtr conn ATTRIBUTE_UNUSED, { *isActive = false; if (pool->def->type == VIR_STORAGE_POOL_DIR) { - if (access(pool->def->target.path, F_OK) == 0) + if (virFileExists(pool->def->target.path)) *isActive = true; #if WITH_STORAGE_FS } else { diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 0b14679..a1a37a1 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -453,7 +453,7 @@ virStorageBackendLogicalCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, bool *isActive) { - *isActive = (access(pool->def->target.path, F_OK) == 0); + *isActive = virFileExists(pool->def->target.path); return 0; } diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 8333f18..9a19484 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -287,13 +287,7 @@ virStorageBackendMpathCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, bool *isActive) { - const char *path = "/dev/mpath"; - - *isActive = false; - - if (access(path, F_OK) == 0) - *isActive = true; - + *isActive = virFileExists("/dev/mpath"); return 0; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 8cb762a..5769799 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -700,8 +700,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (virAsprintf(&path, "/sys/class/scsi_host/host%d", host) < 0) goto cleanup; - if (access(path, F_OK) == 0) - *isActive = true; + *isActive = virFileExists(path); ret = 0; cleanup: diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0b4e901..2f73e47 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -905,7 +905,7 @@ virCgroupMakeGroup(virCgroupPtr parent, sa_assert(group->controllers[i].mountPoint); VIR_DEBUG("Make controller %s", path); - if (access(path, F_OK) != 0) { + if (!virFileExists(path)) { if (!create || mkdir(path, 0755) < 0) { /* With a kernel that doesn't support multi-level directory diff --git a/src/util/virfile.c b/src/util/virfile.c index 67adc55..fe769dd 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -735,7 +735,7 @@ virFileNBDDeviceIsBusy(const char *devname) devname) < 0) return -1; - if (access(path, F_OK) < 0) { + if (!virFileExists(path)) { if (errno == ENOENT) ret = 0; else diff --git a/src/util/virpci.c b/src/util/virpci.c index 92927aa..65d7168 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1501,7 +1501,7 @@ virPCIDeviceNew(unsigned int domain, dev->name) < 0) goto error; - if (access(dev->path, F_OK) != 0) { + if (!virFileExists(dev->path)) { virReportSystemError(errno, _("Device %s not found: could not access %s"), dev->name, dev->path); diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 32e438b..43658c0 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -216,7 +216,7 @@ virSCSIDeviceNew(const char *adapter, virAsprintf(&dev->sg_path, "/dev/%s", sg) < 0) goto cleanup; - if (access(dev->sg_path, F_OK) != 0) { + if (!virFileExists(dev->sg_path)) { virReportSystemError(errno, _("SCSI device '%s': could not access %s"), dev->name, dev->sg_path); diff --git a/src/util/virutil.c b/src/util/virutil.c index 39d4717..d9e0bc4 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1711,7 +1711,7 @@ virIsCapableFCHost(const char *sysfs_prefix, host) < 0) return false; - if (access(sysfs_path, F_OK) == 0) + if (virFileExists(sysfs_path)) ret = true; VIR_FREE(sysfs_path); @@ -1740,8 +1740,8 @@ virIsCapableVport(const char *sysfs_prefix, "vport_create") < 0) goto cleanup; - if ((access(fc_host_path, F_OK) == 0) || - (access(scsi_host_path, F_OK) == 0)) + if (virFileExists(fc_host_path) || + virFileExists(scsi_host_path)) ret = true; cleanup: -- 1.8.3.2

On 09/13/13 17:31, Martin Kletzander wrote: [...] I cleaned up the part you've pointed out and pushed this patch. Thanks Peter