[libvirt] [PATCH 0/5] Refactoring of cgroups usage
Currently libvirtd creates its basic cgroup hierarchy when it starts up. This is bad because if people mount cgroups after libvirtd it is running it doesn't detect this. The second issue is that the driver code re-creates the virCgroupPtr instance for a domain over & over again, which is inefficient and bloats the code. Finally, if the driver is configured to only use a couple of the cgroups controllers, we are still creating the directories under all of them
From: "Daniel P. Berrange" <berrange@redhat.com> The virCgroupGetAppRoot is not clear in its meaning. Change to virCgroupForSelf to highlight that this returns the cgroup config for the caller's process Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/util/vircgroup.c | 9 ++++++--- src/util/vircgroup.h | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 21bc615..f75d681 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1090,9 +1090,9 @@ virCgroupDenyDevicePath; virCgroupForDomain; virCgroupForDriver; virCgroupForEmulator; +virCgroupForSelf; virCgroupForVcpu; virCgroupFree; -virCgroupGetAppRoot; virCgroupGetBlkioWeight; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index df468da..33c305a 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -293,7 +293,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) int ret; virCgroupPtr cgroup; - ret = virCgroupGetAppRoot(&cgroup); + ret = virCgroupForSelf(&cgroup); if (ret < 0) { virReportSystemError(-ret, "%s", _("Unable to get cgroup for container")); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 6998f13..266cecb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -967,19 +967,22 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, #endif /** -* virCgroupGetAppRoot: +* virCgroupForSelf: * * @group: Pointer to returned virCgroupPtr * +* Obtain a cgroup representing the config of the +* current process +* * Returns 0 on success */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupGetAppRoot(virCgroupPtr *group) +int virCgroupForSelf(virCgroupPtr *group) { return virCgroupNew("/", group); } #else -int virCgroupGetAppRoot(virCgroupPtr *group ATTRIBUTE_UNUSED) +int virCgroupForSelf(virCgroupPtr *group ATTRIBUTE_UNUSED) { return -ENXIO; } diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index ea42fa2..45a2006 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -49,7 +49,7 @@ int virCgroupForDriver(const char *name, bool privileged, bool create); -int virCgroupGetAppRoot(virCgroupPtr *group); +int virCgroupForSelf(virCgroupPtr *group); int virCgroupForDomain(virCgroupPtr driver, const char *name, -- 1.8.1.4
On 03/22/2013 10:28 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virCgroupGetAppRoot is not clear in its meaning. Change to virCgroupForSelf to highlight that this returns the cgroup config for the caller's process
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/util/vircgroup.c | 9 ++++++--- src/util/vircgroup.h | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 21bc615..f75d681 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1090,9 +1090,9 @@ virCgroupDenyDevicePath; virCgroupForDomain; virCgroupForDriver; virCgroupForEmulator; +virCgroupForSelf; virCgroupForVcpu; virCgroupFree; -virCgroupGetAppRoot; virCgroupGetBlkioWeight; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index df468da..33c305a 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -293,7 +293,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) int ret; virCgroupPtr cgroup;
- ret = virCgroupGetAppRoot(&cgroup); + ret = virCgroupForSelf(&cgroup); if (ret < 0) { virReportSystemError(-ret, "%s", _("Unable to get cgroup for container")); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 6998f13..266cecb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -967,19 +967,22 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, #endif
/** -* virCgroupGetAppRoot: +* virCgroupForSelf: * * @group: Pointer to returned virCgroupPtr * +* Obtain a cgroup representing the config of the +* current process +* * Returns 0 on success */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupGetAppRoot(virCgroupPtr *group) +int virCgroupForSelf(virCgroupPtr *group) { return virCgroupNew("/", group); } #else -int virCgroupGetAppRoot(virCgroupPtr *group ATTRIBUTE_UNUSED) +int virCgroupForSelf(virCgroupPtr *group ATTRIBUTE_UNUSED) { return -ENXIO; } diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index ea42fa2..45a2006 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -49,7 +49,7 @@ int virCgroupForDriver(const char *name, bool privileged, bool create);
-int virCgroupGetAppRoot(virCgroupPtr *group); +int virCgroupForSelf(virCgroupPtr *group);
int virCgroupForDomain(virCgroupPtr driver, const char *name,
ACK
From: "Daniel P. Berrange" <berrange@redhat.com> Currently when getting an instance of virCgroupPtr we will create the path in all cgroup controllers. Only at the virt driver layer are we attempting to filter controllers. This is bad because the mere act of creating the dirs in the controllers can have a functional impact on the kernel, particularly for performance. Update the virCgroupForDriver() method to accept a bitmask of controllers to use. Only create dirs in the controllers that are requested. When creating cgroups for domains, respect the active controller list from the parent cgroup Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_cgroup.c | 27 +++----- src/qemu/qemu_conf.c | 16 +---- src/qemu/qemu_driver.c | 3 +- src/util/vircgroup.c | 181 +++++++++++++++++++++++++++++++------------------ src/util/vircgroup.h | 6 +- 7 files changed, 132 insertions(+), 105 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 33c305a..545acaa 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -530,7 +530,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) int ret = -1; int rc; - rc = virCgroupForDriver("lxc", &driver, 1, 0); + rc = virCgroupForDriver("lxc", &driver, true, false, 0); if (rc != 0) { virReportSystemError(-rc, "%s", _("Unable to get cgroup for driver")); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8603078..989c2f3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1460,7 +1460,7 @@ static int lxcStartup(bool privileged, lxc_driver->log_libvirtd = 0; /* by default log to container logfile */ lxc_driver->have_netns = lxcCheckNetNsSupport(); - rc = virCgroupForDriver("lxc", &lxc_driver->cgroup, privileged, 1); + rc = virCgroupForDriver("lxc", &lxc_driver->cgroup, privileged, true, -1); if (rc < 0) { char buf[1024] ATTRIBUTE_UNUSED; VIR_DEBUG("Unable to create cgroup for LXC driver: %s", diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index c9b4ca2..2cdc2b7 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -57,8 +57,6 @@ bool qemuCgroupControllerActive(virQEMUDriverPtr driver, goto cleanup; if (!virCgroupMounted(driver->cgroup, controller)) goto cleanup; - if (cfg->cgroupControllers & (1 << controller)) - ret = true; cleanup: virObjectUnref(cfg); @@ -668,7 +666,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, virDomainDefPtr def = vm->def; unsigned long long period = vm->def->cputune.emulator_period; long long quota = vm->def->cputune.emulator_quota; - int rc, i; + int rc; if ((period || quota) && (!driver->cgroup || @@ -697,22 +695,13 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, goto cleanup; } - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (i != VIR_CGROUP_CONTROLLER_CPU && - i != VIR_CGROUP_CONTROLLER_CPUACCT && - i != VIR_CGROUP_CONTROLLER_CPUSET) - continue; - - if (!qemuCgroupControllerActive(driver, i)) - continue; - rc = virCgroupMoveTask(cgroup, cgroup_emulator, i); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to move tasks from domain cgroup to " - "emulator cgroup in controller %d for %s"), - i, vm->def->name); - goto cleanup; - } + rc = virCgroupMoveTask(cgroup, cgroup_emulator); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to move tasks from domain cgroup to " + "emulator cgroup for %s"), + vm->def->name); + goto cleanup; } if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c2e2e10..5a3dde0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -134,14 +134,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } cfg->dynamicOwnership = privileged; - cfg->cgroupControllers = - (1 << VIR_CGROUP_CONTROLLER_CPU) | - (1 << VIR_CGROUP_CONTROLLER_DEVICES) | - (1 << VIR_CGROUP_CONTROLLER_MEMORY) | - (1 << VIR_CGROUP_CONTROLLER_BLKIO) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET) | - (1 << VIR_CGROUP_CONTROLLER_CPUACCT); - + cfg->cgroupControllers = -1; /* -1 == auto-detect */ if (privileged) { if (virAsprintf(&cfg->logDir, @@ -454,6 +447,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, p = virConfGetValue(conf, "cgroup_controllers"); CHECK_TYPE("cgroup_controllers", VIR_CONF_LIST); if (p) { + cfg->cgroupControllers = 0; virConfValuePtr pp; for (i = 0, pp = p->list; pp; ++i, pp = pp->next) { int ctl; @@ -472,12 +466,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, cfg->cgroupControllers |= (1 << ctl); } } - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - if (cfg->cgroupControllers & (1 << i)) { - VIR_INFO("Configured cgroup controller '%s'", - virCgroupControllerTypeToString(i)); - } - } p = virConfGetValue(conf, "cgroup_device_acl"); CHECK_TYPE("cgroup_device_acl", VIR_CONF_LIST); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1f9b8b2..aa9eb0f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -628,7 +628,8 @@ qemuStartup(bool privileged, goto error; } - rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, 1); + rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, true, + cfg->cgroupControllers); if (rc < 0) { VIR_INFO("Unable to create cgroup for driver: %s", virStrerror(-rc, ebuf, sizeof(ebuf))); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 266cecb..0d6db58 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -70,8 +70,6 @@ typedef enum { * before creating subcgroups and * attaching tasks */ - VIR_CGROUP_VCPU = 1 << 1, /* create subdir only under the cgroup cpu, - * cpuacct and cpuset if possible. */ } virCgroupFlags; /** @@ -108,6 +106,16 @@ bool virCgroupMounted(virCgroupPtr cgroup, int controller) return cgroup->controllers[controller].mountPoint != NULL; } +static int virCgroupMountMask(virCgroupPtr cgroup) +{ + int controllers = 0; + int i; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) + if (cgroup->controllers[i].mountPoint != NULL) + controllers |= (1 << i); + return controllers; +} + #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R /* * Process /proc/mounts figuring out what controllers are @@ -230,11 +238,12 @@ no_memory: } -static int virCgroupDetect(virCgroupPtr group) +static int virCgroupDetect(virCgroupPtr group, + int controllers) { - int any = 0; int rc; int i; + int j; rc = virCgroupDetectMounts(group); if (rc < 0) { @@ -242,14 +251,55 @@ static int virCgroupDetect(virCgroupPtr group) return rc; } - /* Check that at least 1 controller is available */ - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - if (group->controllers[i].mountPoint != NULL) - any = 1; + if (controllers >= 0) { + VIR_DEBUG("Validating controllers %d", controllers); + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + VIR_DEBUG("Controller '%s' wanted=%s", + virCgroupControllerTypeToString(i), + (1 << i) & controllers ? "yes" : "no"); + if (((1 << i) & controllers)) { + /* Ensure requested controller is present */ + if (!group->controllers[i].mountPoint) { + VIR_DEBUG("Requested controlled '%s' not mounted", + virCgroupControllerTypeToString(i)); + return -ENOENT; + } + } else { + /* Check whether a request to disable a controller + * clashes with co-mounting of controllers */ + for (j = 0 ; j < VIR_CGROUP_CONTROLLER_LAST ; j++) { + if (j == i) + continue; + if (!((1 << j) & controllers)) + continue; + + if (STREQ_NULLABLE(group->controllers[i].mountPoint, + group->controllers[j].mountPoint)) { + VIR_ERROR("Controller '%s' is not wanted, but '%s' is co-mounted", + virCgroupControllerTypeToString(i), + virCgroupControllerTypeToString(j)); + return -EINVAL; + } + } + VIR_FREE(group->controllers[i].mountPoint); + } + } + } else { + VIR_DEBUG("Auto-detecting controllers"); + controllers = 0; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + VIR_DEBUG("Controller '%s' present=%s", + virCgroupControllerTypeToString(i), + group->controllers[i].mountPoint ? "yes" : "no"); + if (group->controllers[i].mountPoint == NULL) + continue; + controllers |= (1 << i); + } } - if (!any) - return -ENXIO; + /* Check that at least 1 controller is available */ + if (!controllers) + return -ENXIO; rc = virCgroupDetectPlacement(group); @@ -542,16 +592,6 @@ static int virCgroupMakeGroup(virCgroupPtr parent, if (!group->controllers[i].mountPoint) continue; - /* We need to control cpu bandwidth for each vcpu now */ - if ((flags & VIR_CGROUP_VCPU) && - (i != VIR_CGROUP_CONTROLLER_CPU && - i != VIR_CGROUP_CONTROLLER_CPUACCT && - i != VIR_CGROUP_CONTROLLER_CPUSET)) { - /* treat it as unmounted and we can use virCgroupAddTask */ - VIR_FREE(group->controllers[i].mountPoint); - continue; - } - rc = virCgroupPathOfController(group, i, "", &path); if (rc < 0) return rc; @@ -611,12 +651,13 @@ static int virCgroupMakeGroup(virCgroupPtr parent, static int virCgroupNew(const char *path, + int controllers, virCgroupPtr *group) { int rc = 0; char *typpath = NULL; - VIR_DEBUG("New group %s", path); + VIR_DEBUG("path=%s controllers=%d", path, controllers); *group = NULL; if (VIR_ALLOC((*group)) != 0) { @@ -629,7 +670,7 @@ static int virCgroupNew(const char *path, goto err; } - rc = virCgroupDetect(*group); + rc = virCgroupDetect(*group, controllers); if (rc < 0) goto err; @@ -645,17 +686,18 @@ err: static int virCgroupAppRoot(bool privileged, virCgroupPtr *group, - bool create) + bool create, + int controllers) { virCgroupPtr rootgrp = NULL; int rc; - rc = virCgroupNew("/", &rootgrp); + rc = virCgroupNew("/", controllers, &rootgrp); if (rc != 0) return rc; if (privileged) { - rc = virCgroupNew("/libvirt", group); + rc = virCgroupNew("/libvirt", controllers, group); } else { char *rootname; char *username; @@ -671,7 +713,7 @@ static int virCgroupAppRoot(bool privileged, goto cleanup; } - rc = virCgroupNew(rootname, group); + rc = virCgroupNew(rootname, controllers, group); VIR_FREE(rootname); } if (rc != 0) @@ -779,6 +821,7 @@ int virCgroupRemove(virCgroupPtr group) return rc; } + /** * virCgroupAddTask: * @@ -872,45 +915,30 @@ cleanup: * * Returns: 0 on success or -errno on failure */ -int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group, - int controller) +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) { - int rc = 0, err = 0; + int rc = 0; char *content = NULL; + int i; - if (controller < VIR_CGROUP_CONTROLLER_CPU || - controller > VIR_CGROUP_CONTROLLER_BLKIO) - return -EINVAL; - - if (!src_group->controllers[controller].mountPoint || - !dest_group->controllers[controller].mountPoint) { - return -EINVAL; - } - - rc = virCgroupGetValueStr(src_group, controller, "tasks", &content); - if (rc != 0) - return rc; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + if (!src_group->controllers[i].mountPoint || + !dest_group->controllers[i].mountPoint) + continue; - rc = virCgroupAddTaskStrController(dest_group, content, controller); - if (rc != 0) - goto cleanup; + rc = virCgroupGetValueStr(src_group, i, "tasks", &content); + if (rc != 0) + return rc; - VIR_FREE(content); + rc = virCgroupAddTaskStrController(dest_group, content, i); + if (rc != 0) + goto cleanup; - return 0; + VIR_FREE(content); + } cleanup: - /* - * We don't need to recover dest_cgroup because cgroup will make sure - * that one task only resides in one cgroup of the same controller. - */ - err = virCgroupAddTaskStrController(src_group, content, controller); - if (err != 0) - VIR_ERROR(_("Cannot recover cgroup %s from %s"), - src_group->controllers[controller].mountPoint, - dest_group->controllers[controller].mountPoint); VIR_FREE(content); - return rc; } @@ -926,13 +954,15 @@ cleanup: int virCgroupForDriver(const char *name, virCgroupPtr *group, bool privileged, - bool create) + bool create, + int controllers) { int rc; char *path = NULL; virCgroupPtr rootgrp = NULL; - rc = virCgroupAppRoot(privileged, &rootgrp, create); + rc = virCgroupAppRoot(privileged, &rootgrp, + create, controllers); if (rc != 0) goto out; @@ -941,7 +971,7 @@ int virCgroupForDriver(const char *name, goto out; } - rc = virCgroupNew(path, group); + rc = virCgroupNew(path, controllers, group); VIR_FREE(path); if (rc == 0) { @@ -979,7 +1009,7 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupForSelf(virCgroupPtr *group) { - return virCgroupNew("/", group); + return virCgroupNew("/", -1, group); } #else int virCgroupForSelf(virCgroupPtr *group ATTRIBUTE_UNUSED) @@ -1005,6 +1035,7 @@ int virCgroupForDomain(virCgroupPtr driver, { int rc; char *path; + int controllers; if (driver == NULL) return -EINVAL; @@ -1012,7 +1043,9 @@ int virCgroupForDomain(virCgroupPtr driver, if (virAsprintf(&path, "%s/%s", driver->path, name) < 0) return -ENOMEM; - rc = virCgroupNew(path, group); + controllers = virCgroupMountMask(driver); + + rc = virCgroupNew(path, controllers, group); VIR_FREE(path); if (rc == 0) { @@ -1060,6 +1093,7 @@ int virCgroupForVcpu(virCgroupPtr driver, { int rc; char *path; + int controllers; if (driver == NULL) return -EINVAL; @@ -1067,11 +1101,18 @@ int virCgroupForVcpu(virCgroupPtr driver, if (virAsprintf(&path, "%s/vcpu%d", driver->path, vcpuid) < 0) return -ENOMEM; - rc = virCgroupNew(path, group); + controllers = virCgroupMountMask(driver); + + /* Filter parent controllers to just the CPU ones */ + controllers &= ((1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET)); + + rc = virCgroupNew(path, controllers, group); VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); + rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_NONE); if (rc != 0) virCgroupFree(group); } @@ -1103,6 +1144,7 @@ int virCgroupForEmulator(virCgroupPtr driver, { int rc; char *path; + int controllers; if (driver == NULL) return -EINVAL; @@ -1110,11 +1152,18 @@ int virCgroupForEmulator(virCgroupPtr driver, if (virAsprintf(&path, "%s/emulator", driver->path) < 0) return -ENOMEM; - rc = virCgroupNew(path, group); + controllers = virCgroupMountMask(driver); + + /* Filter parent controllers to just the CPU ones */ + controllers &= ((1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET)); + + rc = virCgroupNew(path, controllers, group); VIR_FREE(path); if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); + rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_NONE); if (rc != 0) virCgroupFree(group); } @@ -2014,7 +2063,7 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas goto cleanup; } - if ((rc = virCgroupNew(subpath, &subgroup)) != 0) + if ((rc = virCgroupNew(subpath, -1, &subgroup)) != 0) goto cleanup; if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, true)) < 0) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 45a2006..725d2d0 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -47,7 +47,8 @@ VIR_ENUM_DECL(virCgroupController); int virCgroupForDriver(const char *name, virCgroupPtr *group, bool privileged, - bool create); + bool create, + int controllers); int virCgroupForSelf(virCgroupPtr *group); @@ -77,8 +78,7 @@ int virCgroupAddTaskController(virCgroupPtr group, int controller); int virCgroupMoveTask(virCgroupPtr src_group, - virCgroupPtr dest_group, - int controller); + virCgroupPtr dest_group); int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); -- 1.8.1.4
From: "Daniel P. Berrange" <berrange@redhat.com> The virCgroupMounted method is badly named, since a controller can be mounted, but disabled in the current object. Rename the method to be virCgroupHasController. Also make it tolerant to a NULL virCgroupPtr and out-of-range controller index, to avoid duplication of these checks in all callers Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 2 +- src/lxc/lxc_driver.c | 12 +----------- src/lxc/lxc_process.c | 10 +++++----- src/qemu/qemu_cgroup.c | 14 +------------- src/util/vircgroup.c | 13 +++++++++---- src/util/vircgroup.h | 2 +- 6 files changed, 18 insertions(+), 35 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f75d681..f23f80d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1111,7 +1111,7 @@ virCgroupGetMemSwapUsage; virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; -virCgroupMounted; +virCgroupHasController; virCgroupMoveTask; virCgroupPathOfController; virCgroupRemove; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 989c2f3..6737f13 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1641,17 +1641,7 @@ cleanup: static bool lxcCgroupControllerActive(virLXCDriverPtr driver, int controller) { - if (driver->cgroup == NULL) - return false; - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) - return false; - if (!virCgroupMounted(driver->cgroup, controller)) - return false; -#if 0 - if (driver->cgroupControllers & (1 << controller)) - return true; -#endif - return true; + return virCgroupHasController(driver->cgroup, controller); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 670a032..8c8778a 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1006,20 +1006,20 @@ int virLXCProcessStart(virConnectPtr conn, return -1; } - if (!virCgroupMounted(lxc_driver->cgroup, + if (!virCgroupHasController(lxc_driver->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'cpuacct' cgroups controller mount")); return -1; } - if (!virCgroupMounted(lxc_driver->cgroup, - VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(lxc_driver->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'devices' cgroups controller mount")); return -1; } - if (!virCgroupMounted(lxc_driver->cgroup, - VIR_CGROUP_CONTROLLER_MEMORY)) { + if (!virCgroupHasController(lxc_driver->cgroup, + VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'memory' cgroups controller mount")); return -1; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2cdc2b7..5aa9416 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -48,19 +48,7 @@ static const char *const defaultDeviceACL[] = { bool qemuCgroupControllerActive(virQEMUDriverPtr driver, int controller) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - bool ret = false; - - if (driver->cgroup == NULL) - goto cleanup; - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) - goto cleanup; - if (!virCgroupMounted(driver->cgroup, controller)) - goto cleanup; - -cleanup: - virObjectUnref(cfg); - return ret; + return virCgroupHasController(driver->cgroup, controller); } static int diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0d6db58..ccdb1dd 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -94,15 +94,20 @@ void virCgroupFree(virCgroupPtr *group) } /** - * virCgroupMounted: query whether a cgroup subsystem is mounted or not + * virCgroupHasController: query whether a cgroup controller is present * - * @cgroup: The group structure to be queried + * @cgroup: The group structure to be queried, or NULL * @controller: cgroup subsystem id * - * Returns true if a cgroup is subsystem is mounted. + * Returns true if a cgroup controller is mounted and is associated + * with this cgroup object. */ -bool virCgroupMounted(virCgroupPtr cgroup, int controller) +bool virCgroupHasController(virCgroupPtr cgroup, int controller) { + if (!cgroup) + return false; + if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) + return false; return cgroup->controllers[controller].mountPoint != NULL; } diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 725d2d0..4c1134d 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -161,7 +161,7 @@ int virCgroupRemoveRecursively(char *grppath); int virCgroupRemove(virCgroupPtr group); void virCgroupFree(virCgroupPtr *group); -bool virCgroupMounted(virCgroupPtr cgroup, int controller); +bool virCgroupHasController(virCgroupPtr cgroup, int controller); int virCgroupKill(virCgroupPtr group, int signum); int virCgroupKillRecursive(virCgroupPtr group, int signum); -- 1.8.1.4
From: "Daniel P. Berrange" <berrange@redhat.com> Instead of calling virCgroupForDomain every time we need the virCgrouPtr instance, just do it once at Vm startup and cache a reference to the object in qemuDomainObjPrivatePtr until shutdown of the VM. Removing the virCgroupPtr from the QEMU driver state also means we don't have stale mount info, if someone mounts the cgroups filesystem after libvirtd has been started Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_cgroup.c | 283 +++++++++++++++------------------ src/qemu/qemu_cgroup.h | 22 +-- src/qemu/qemu_conf.h | 4 - src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 397 +++++++++++++++------------------------------- src/qemu/qemu_hotplug.c | 53 +------ src/qemu/qemu_migration.c | 25 +-- src/qemu/qemu_process.c | 13 +- 9 files changed, 291 insertions(+), 510 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 5aa9416..019aa2e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -45,26 +45,21 @@ static const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 -bool qemuCgroupControllerActive(virQEMUDriverPtr driver, - int controller) -{ - return virCgroupHasController(driver->cgroup, controller); -} - static int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, const char *path, size_t depth ATTRIBUTE_UNUSED, void *opaque) { - qemuCgroupData *data = opaque; + virDomainObjPtr vm = opaque; + qemuDomainObjPrivatePtr priv = vm->privateData; int rc; VIR_DEBUG("Process path %s for disk", path); - rc = virCgroupAllowDevicePath(data->cgroup, path, + rc = virCgroupAllowDevicePath(priv->cgroup, path, (disk->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW)); - virDomainAuditCgroupPath(data->vm, data->cgroup, "allow", path, + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, disk->readonly ? "r" : "rw", rc); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ @@ -81,14 +76,18 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, int qemuSetupDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk) { - qemuCgroupData data = { vm, cgroup }; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + return virDomainDiskDefForeachPath(disk, true, qemuSetupDiskPathAllow, - &data); + vm); } @@ -98,13 +97,14 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, size_t depth ATTRIBUTE_UNUSED, void *opaque) { - qemuCgroupData *data = opaque; + virDomainObjPtr vm = opaque; + qemuDomainObjPrivatePtr priv = vm->privateData; int rc; VIR_DEBUG("Process path %s for disk", path); - rc = virCgroupDenyDevicePath(data->cgroup, path, + rc = virCgroupDenyDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(data->vm, data->cgroup, "deny", path, "rwm", rc); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rc); if (rc < 0) { if (rc == -EACCES) { /* Get this for root squash NFS */ VIR_DEBUG("Ignoring EACCES for %s", path); @@ -120,14 +120,18 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, int qemuTeardownDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk) { - qemuCgroupData data = { vm, cgroup }; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; + return virDomainDiskDefForeachPath(disk, true, qemuTeardownDiskPathDeny, - &data); + vm); } @@ -136,7 +140,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def, virDomainChrDefPtr dev, void *opaque) { - qemuCgroupData *data = opaque; + virDomainObjPtr vm = opaque; + qemuDomainObjPrivatePtr priv = vm->privateData; int rc; if (dev->source.type != VIR_DOMAIN_CHR_TYPE_DEV) @@ -144,9 +149,9 @@ qemuSetupChardevCgroup(virDomainDefPtr def, VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); - rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path, + rc = virCgroupAllowDevicePath(priv->cgroup, dev->source.data.file.path, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(data->vm, data->cgroup, "allow", + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", dev->source.data.file.path, "rw", rc); if (rc < 0) { virReportSystemError(-rc, @@ -163,13 +168,14 @@ int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, const char *path, void *opaque) { - qemuCgroupData *data = opaque; + virDomainObjPtr vm = opaque; + qemuDomainObjPrivatePtr priv = vm->privateData; int rc; VIR_DEBUG("Process path '%s' for USB device", path); - rc = virCgroupAllowDevicePath(data->cgroup, path, + rc = virCgroupAllowDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(data->vm, data->cgroup, "allow", path, "rw", rc); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rc); if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s"), @@ -180,34 +186,73 @@ int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, return 0; } + +int qemuInitCgroup(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + int rc; + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr driverGroup = NULL; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + virCgroupFree(&priv->cgroup); + + rc = virCgroupForDriver("qemu", &driverGroup, + cfg->privileged, true, + cfg->cgroupControllers); + if (rc != 0) { + if (rc == -ENXIO || + rc == -EPERM || + rc == -EACCES) { /* No cgroups mounts == success */ + VIR_DEBUG("No cgroups present/configured/accessible, ignoring error"); + goto done; + } + + virReportSystemError(-rc, + _("Unable to create cgroup for %s"), + vm->def->name); + goto cleanup; + } + + rc = virCgroupForDomain(driverGroup, vm->def->name, &priv->cgroup, 1); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to create cgroup for %s"), + vm->def->name); + goto cleanup; + } + +done: + rc = 0; +cleanup: + virCgroupFree(&driverGroup); + virObjectUnref(cfg); + return rc; +} + + int qemuSetupCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask) { - virCgroupPtr cgroup = NULL; - int rc; + int rc = -1; unsigned int i; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; const char *const *deviceACL = cfg->cgroupDeviceACL ? (const char *const *)cfg->cgroupDeviceACL : defaultDeviceACL; - if (driver->cgroup == NULL) - goto done; /* Not supported, so claim success */ + if (qemuInitCgroup(driver, vm) < 0) + return -1; - rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 1); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - vm->def->name); - goto cleanup; - } + if (!priv->cgroup) + goto done; - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - qemuCgroupData data = { vm, cgroup }; - rc = virCgroupDenyAllDevices(cgroup); - virDomainAuditCgroup(vm, cgroup, "deny", "all", rc == 0); + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { + rc = virCgroupDenyAllDevices(priv->cgroup); + virDomainAuditCgroup(vm, priv->cgroup, "deny", "all", rc == 0); if (rc != 0) { if (rc == -EPERM) { VIR_WARN("Group devices ACL is not accessible, disabling whitelisting"); @@ -220,13 +265,13 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } for (i = 0; i < vm->def->ndisks ; i++) { - if (qemuSetupDiskCgroup(vm, cgroup, vm->def->disks[i]) < 0) + if (qemuSetupDiskCgroup(vm,vm->def->disks[i]) < 0) goto cleanup; } - rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR, + rc = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_PTY_MAJOR, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR, + virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_PTY_MAJOR, "pty", "rw", rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", @@ -239,9 +284,9 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, ((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && cfg->vncAllowHostAudio) || (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) { - rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR, + rc = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_SND_MAJOR, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR, + virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_SND_MAJOR, "sound", "rw", rc == 0); if (rc != 0) { virReportSystemError(-rc, "%s", @@ -257,9 +302,9 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, continue; } - rc = virCgroupAllowDevicePath(cgroup, deviceACL[i], + rc = virCgroupAllowDevicePath(priv->cgroup, deviceACL[i], VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], "rw", rc); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", deviceACL[i], "rw", rc); if (rc < 0 && rc != -ENOENT) { virReportSystemError(-rc, @@ -272,7 +317,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, if (virDomainChrDefForeach(vm->def, true, qemuSetupChardevCgroup, - &data) < 0) + vm) < 0) goto cleanup; for (i = 0; i < vm->def->nhostdevs; i++) { @@ -292,7 +337,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, goto cleanup; if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, - &data) < 0) { + vm) < 0) { virUSBDeviceFree(usb); goto cleanup; } @@ -301,8 +346,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } if (vm->def->blkio.weight != 0) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { - rc = virCgroupSetBlkioWeight(cgroup, vm->def->blkio.weight); + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { + rc = virCgroupSetBlkioWeight(priv->cgroup, vm->def->blkio.weight); if (rc != 0) { virReportSystemError(-rc, _("Unable to set io weight for domain %s"), @@ -317,12 +362,12 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } if (vm->def->blkio.ndevices) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { for (i = 0; i < vm->def->blkio.ndevices; i++) { virBlkioDeviceWeightPtr dw = &vm->def->blkio.devices[i]; if (!dw->weight) continue; - rc = virCgroupSetBlkioDeviceWeight(cgroup, dw->path, + rc = virCgroupSetBlkioDeviceWeight(priv->cgroup, dw->path, dw->weight); if (rc != 0) { virReportSystemError(-rc, @@ -339,7 +384,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { unsigned long long hard_limit = vm->def->mem.hard_limit; if (!hard_limit) { @@ -357,7 +402,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, hard_limit += vm->def->ndisks * 32768; } - rc = virCgroupSetMemoryHardLimit(cgroup, hard_limit); + rc = virCgroupSetMemoryHardLimit(priv->cgroup, hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set memory hard limit for domain %s"), @@ -365,7 +410,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, goto cleanup; } if (vm->def->mem.soft_limit != 0) { - rc = virCgroupSetMemorySoftLimit(cgroup, vm->def->mem.soft_limit); + rc = virCgroupSetMemorySoftLimit(priv->cgroup, vm->def->mem.soft_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set memory soft limit for domain %s"), @@ -375,7 +420,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetMemSwapHardLimit(cgroup, vm->def->mem.swap_hard_limit); + rc = virCgroupSetMemSwapHardLimit(priv->cgroup, vm->def->mem.swap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), @@ -393,8 +438,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } if (vm->def->cputune.shares != 0) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - rc = virCgroupSetCpuShares(cgroup, vm->def->cputune.shares); + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { + rc = virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares); if (rc != 0) { virReportSystemError(-rc, _("Unable to set io cpu shares for domain %s"), @@ -411,7 +456,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, (vm->def->numatune.memory.placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO)) && vm->def->numatune.memory.mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { char *mask = NULL; if (vm->def->numatune.memory.placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) @@ -424,7 +469,7 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, goto cleanup; } - rc = virCgroupSetCpusetMems(cgroup, mask); + rc = virCgroupSetCpusetMems(priv->cgroup, mask); VIR_FREE(mask); if (rc != 0) { virReportSystemError(-rc, @@ -433,18 +478,12 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, goto cleanup; } } -done: - virObjectUnref(cfg); - virCgroupFree(&cgroup); - return 0; +done: + rc = 0; cleanup: virObjectUnref(cfg); - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } - return -1; + return rc == 0 ? 0 : -1; } int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, @@ -538,9 +577,8 @@ cleanup: return rc; } -int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) +int qemuSetupCgroupForVcpu(virDomainObjPtr vm) { - virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; @@ -550,8 +588,7 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) long long quota = vm->def->cputune.quota; if ((period || quota) && - (!driver->cgroup || - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cgroup cpu is required for scheduler tuning")); return -1; @@ -561,28 +598,19 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal * here. */ - if (driver->cgroup == NULL) + if (priv->cgroup == NULL) return 0; - rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { /* If we don't know VCPU<->PID mapping or all vcpu runs in the same * thread, we cannot control each vcpu. */ VIR_WARN("Unable to get vcpus' pids."); - virCgroupFree(&cgroup); return 0; } for (i = 0; i < priv->nvcpupids; i++) { - rc = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 1); + rc = virCgroupForVcpu(priv->cgroup, i, &cgroup_vcpu, 1); if (rc < 0) { virReportSystemError(-rc, _("Unable to create vcpu cgroup for %s(vcpu:" @@ -606,7 +634,7 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) } /* Set vcpupin in cgroup if vcpupin xml is provided */ - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { /* find the right CPU to pin, otherwise * qemuSetupCgroupVcpuPin will fail. */ for (j = 0; j < def->cputune.nvcpupin; j++) { @@ -626,7 +654,6 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) virCgroupFree(&cgroup_vcpu); } - virCgroupFree(&cgroup); return 0; cleanup: @@ -635,11 +662,6 @@ cleanup: virCgroupFree(&cgroup_vcpu); } - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } - return -1; } @@ -649,33 +671,24 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, { virBitmapPtr cpumask = NULL; virBitmapPtr cpumap = NULL; - virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; virDomainDefPtr def = vm->def; + qemuDomainObjPrivatePtr priv = vm->privateData; unsigned long long period = vm->def->cputune.emulator_period; long long quota = vm->def->cputune.emulator_quota; int rc; if ((period || quota) && - (!driver->cgroup || - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cgroup cpu is required for scheduler tuning")); return -1; } - if (driver->cgroup == NULL) + if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - - rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 1); + rc = virCgroupForEmulator(priv->cgroup, &cgroup_emulator, 1); if (rc < 0) { virReportSystemError(-rc, _("Unable to create emulator cgroup for %s"), @@ -683,7 +696,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, goto cleanup; } - rc = virCgroupMoveTask(cgroup, cgroup_emulator); + rc = virCgroupMoveTask(priv->cgroup, cgroup_emulator); if (rc < 0) { virReportSystemError(-rc, _("Unable to move tasks from domain cgroup to " @@ -703,7 +716,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, } if (cpumask) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask); if (rc < 0) goto cleanup; @@ -712,7 +725,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, } if (period || quota) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { if ((rc = qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota)) < 0) goto cleanup; @@ -720,7 +733,6 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, } virCgroupFree(&cgroup_emulator); - virCgroupFree(&cgroup); virBitmapFree(cpumap); return 0; @@ -732,67 +744,34 @@ cleanup: virCgroupFree(&cgroup_emulator); } - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } - return rc; } -int qemuRemoveCgroup(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int quiet) +int qemuRemoveCgroup(virDomainObjPtr vm) { - virCgroupPtr cgroup; - int rc; + qemuDomainObjPrivatePtr priv = vm->privateData; - if (driver->cgroup == NULL) + if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); - if (rc != 0) { - if (!quiet) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - return rc; - } - - rc = virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - return rc; + return virCgroupRemove(priv->cgroup); } -int qemuAddToCgroup(virQEMUDriverPtr driver, - virDomainDefPtr def) +int qemuAddToCgroup(virDomainObjPtr vm) { - virCgroupPtr cgroup = NULL; - int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; int rc; - if (driver->cgroup == NULL) + if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupForDomain(driver->cgroup, def->name, &cgroup, 0); - if (rc != 0) { - virReportSystemError(-rc, - _("unable to find cgroup for domain %s"), - def->name); - goto cleanup; - } - - rc = virCgroupAddTask(cgroup, getpid()); + rc = virCgroupAddTask(priv->cgroup, getpid()); if (rc != 0) { virReportSystemError(-rc, _("unable to add domain %s task %d to cgroup"), - def->name, getpid()); - goto cleanup; + vm->def->name, getpid()); + return -1; } - ret = 0; - -cleanup: - virCgroupFree(&cgroup); - return ret; + return 0; } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index a677d07..6cbfebc 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -25,26 +25,19 @@ # define __QEMU_CGROUP_H__ # include "virusb.h" +# include "vircgroup.h" # include "domain_conf.h" # include "qemu_conf.h" -struct _qemuCgroupData { - virDomainObjPtr vm; - virCgroupPtr cgroup; -}; -typedef struct _qemuCgroupData qemuCgroupData; - -bool qemuCgroupControllerActive(virQEMUDriverPtr driver, - int controller); int qemuSetupDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk); int qemuTeardownDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk); int qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev, const char *path, void *opaque); +int qemuInitCgroup(virQEMUDriverPtr driver, + virDomainObjPtr vm); int qemuSetupCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask); @@ -56,14 +49,11 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, int nvcpupin, int vcpuid); int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr cpumask); -int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm); +int qemuSetupCgroupForVcpu(virDomainObjPtr vm); int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask); -int qemuRemoveCgroup(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int quiet); -int qemuAddToCgroup(virQEMUDriverPtr driver, - virDomainDefPtr def); +int qemuRemoveCgroup(virDomainObjPtr vm); +int qemuAddToCgroup(virDomainObjPtr vm); #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c5ddaad..21ddd38 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -34,7 +34,6 @@ # include "domain_event.h" # include "virthread.h" # include "security/security_manager.h" -# include "vircgroup.h" # include "virpci.h" # include "virusb.h" # include "cpu_conf.h" @@ -164,9 +163,6 @@ struct _virQEMUDriver { /* Atomic increment only */ int nextvmid; - /* Immutable pointer. Immutable object */ - virCgroupPtr cgroup; - /* Atomic inc/dec only */ unsigned int nactive; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c79b05d..6e2966f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -235,6 +235,7 @@ qemuDomainObjPrivateFree(void *data) virObjectUnref(priv->qemuCaps); + virCgroupFree(&priv->cgroup); qemuDomainPCIAddressSetFree(priv->pciaddrs); qemuDomainCCWAddressSetFree(priv->ccwaddrs); virDomainChrSourceDefFree(priv->monConfig); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 26d5859..e68f2e0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -25,6 +25,7 @@ # define __QEMU_DOMAIN_H__ # include "virthread.h" +# include "vircgroup.h" # include "domain_conf.h" # include "snapshot_conf.h" # include "qemu_monitor.h" @@ -165,6 +166,8 @@ struct _qemuDomainObjPrivate { qemuDomainCleanupCallback *cleanupCallbacks; size_t ncleanupCallbacks; size_t ncleanupCallbacks_max; + + virCgroupPtr cgroup; }; struct qemuDomainWatchdogEvent diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa9eb0f..00dee75 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -551,7 +551,6 @@ qemuStartup(bool privileged, void *opaque) { char *driverConf = NULL; - int rc; virConnectPtr conn = NULL; char ebuf[1024]; char *membase = NULL; @@ -628,13 +627,6 @@ qemuStartup(bool privileged, goto error; } - rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, true, - cfg->cgroupControllers); - if (rc < 0) { - VIR_INFO("Unable to create cgroup for driver: %s", - virStrerror(-rc, ebuf, sizeof(ebuf))); - } - qemu_driver->qemuImgBinary = virFindFileInPath("kvm-img"); if (!qemu_driver->qemuImgBinary) qemu_driver->qemuImgBinary = virFindFileInPath("qemu-img"); @@ -976,8 +968,6 @@ qemuShutdown(void) { /* Free domain callback list */ virDomainEventStateFree(qemu_driver->domainEventState); - virCgroupFree(&qemu_driver->cgroup); - virLockManagerPluginUnref(qemu_driver->lockManager); virMutexDestroy(&qemu_driver->lock); @@ -3541,9 +3531,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, int vcpus = oldvcpus; pid_t *cpupids = NULL; int ncpupids; - virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_vcpu = NULL; - bool cgroup_available = false; qemuDomainObjEnterMonitor(driver, vm); @@ -3606,15 +3594,12 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, goto cleanup; } - cgroup_available = (virCgroupForDomain(driver->cgroup, vm->def->name, - &cgroup, 0) == 0); - if (nvcpus > oldvcpus) { for (i = oldvcpus; i < nvcpus; i++) { - if (cgroup_available) { + if (priv->cgroup) { int rv = -1; /* Create cgroup for the onlined vcpu */ - rv = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 1); + rv = virCgroupForVcpu(priv->cgroup, i, &cgroup_vcpu, 1); if (rv < 0) { virReportSystemError(-rv, _("Unable to create vcpu cgroup for %s(vcpu:" @@ -3657,7 +3642,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, vcpupin->vcpuid = i; vm->def->cputune.vcpupin[vm->def->cputune.nvcpupin++] = vcpupin; - if (cgroup_available) { + if (cgroup_vcpu) { if (qemuSetupCgroupVcpuPin(cgroup_vcpu, vm->def->cputune.vcpupin, vm->def->cputune.nvcpupin, i) < 0) { @@ -3685,10 +3670,10 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, for (i = oldvcpus - 1; i >= nvcpus; i--) { virDomainVcpuPinDefPtr vcpupin = NULL; - if (cgroup_available) { + if (priv->cgroup) { int rv = -1; - rv = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 0); + rv = virCgroupForVcpu(priv->cgroup, i, &cgroup_vcpu, 0); if (rv < 0) { virReportSystemError(-rv, _("Unable to access vcpu cgroup for %s(vcpu:" @@ -3719,8 +3704,6 @@ cleanup: vm->def->vcpus = vcpus; VIR_FREE(cpupids); virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); - if (cgroup) - virCgroupFree(&cgroup); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; @@ -3853,7 +3836,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; virDomainDefPtr persistentDef = NULL; - virCgroupPtr cgroup_dom = NULL; virCgroupPtr cgroup_vcpu = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -3929,9 +3911,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } /* Configure the corresponding cpuset cgroup before set affinity. */ - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup_dom, 0) == 0 && - virCgroupForVcpu(cgroup_dom, vcpu, &cgroup_vcpu, 0) == 0 && + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupForVcpu(priv->cgroup, vcpu, &cgroup_vcpu, 0) == 0 && qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, _("failed to set cpuset.cpus in cgroup" @@ -4008,8 +3989,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cleanup: if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); - if (cgroup_dom) - virCgroupFree(&cgroup_dom); if (vm) virObjectUnlock(vm); virBitmapFree(pcpumap); @@ -4120,7 +4099,6 @@ qemuDomainPinEmulator(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup_dom = NULL; virCgroupPtr cgroup_emulator = NULL; pid_t pid; virDomainDefPtr persistentDef = NULL; @@ -4184,22 +4162,19 @@ qemuDomainPinEmulator(virDomainPtr dom, goto cleanup; } - if (qemuCgroupControllerActive(driver, - VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_CPUSET)) { /* * Configure the corresponding cpuset cgroup. * If no cgroup for domain or hypervisor exists, do nothing. */ - if (virCgroupForDomain(driver->cgroup, vm->def->name, - &cgroup_dom, 0) == 0) { - if (virCgroupForEmulator(cgroup_dom, &cgroup_emulator, 0) == 0) { - if (qemuSetupCgroupEmulatorPin(cgroup_emulator, - newVcpuPin[0]->cpumask) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("failed to set cpuset.cpus in cgroup" - " for emulator threads")); - goto cleanup; - } + if (virCgroupForEmulator(priv->cgroup, &cgroup_emulator, 0) == 0) { + if (qemuSetupCgroupEmulatorPin(cgroup_emulator, + newVcpuPin[0]->cpumask) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); + goto cleanup; } } } else { @@ -4263,8 +4238,6 @@ qemuDomainPinEmulator(virDomainPtr dom, cleanup: if (cgroup_emulator) virCgroupFree(&cgroup_emulator); - if (cgroup_dom) - virCgroupFree(&cgroup_dom); virBitmapFree(pcpumap); virObjectUnref(caps); if (vm) @@ -5757,16 +5730,8 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (qemuDomainDetermineDiskChain(driver, disk, false) < 0) goto end; - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto end; - } - if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) - goto end; - } + if (qemuSetupDiskCgroup(vm, disk) < 0) + goto end; switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: @@ -5832,7 +5797,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, } if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) + if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(disk->src)); } @@ -5840,8 +5805,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, end: if (ret != 0) ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); - if (cgroup) - virCgroupFree(&cgroup); virObjectUnref(caps); virDomainDeviceDefFree(dev_copy); return ret; @@ -6024,7 +5987,6 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; virDomainDiskDefPtr tmp = NULL; - virCgroupPtr cgroup = NULL; virDomainDeviceDefPtr dev_copy = NULL; virCapsPtr caps = NULL; int ret = -1; @@ -6032,17 +5994,8 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, if (qemuDomainDetermineDiskChain(driver, disk, false) < 0) goto end; - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, - vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto end; - } - if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) - goto end; - } + if (qemuSetupDiskCgroup(vm, disk) < 0) + goto end; switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: @@ -6090,14 +6043,12 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, break; } - if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(disk->src)); - } + if (ret != 0 && + qemuTeardownDiskCgroup(vm, disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(disk->src)); + end: - if (cgroup) - virCgroupFree(&cgroup); virObjectUnref(caps); virDomainDeviceDefFree(dev_copy); return ret; @@ -6731,15 +6682,25 @@ static char *qemuGetSchedulerType(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; char *ret = NULL; int rc; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + + vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + if (vm == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + priv = vm->privateData; - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } if (nparams) { - rc = qemuGetCpuBWStatus(driver->cgroup); + rc = qemuGetCpuBWStatus(priv->cgroup); if (rc < 0) goto cleanup; else if (rc == 0) @@ -6753,6 +6714,8 @@ static char *qemuGetSchedulerType(virDomainPtr dom, virReportOOMError(); cleanup: + if (vm) + virObjectUnlock(vm); return ret; } @@ -6892,12 +6855,12 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -6915,6 +6878,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -6924,18 +6888,11 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), - vm->def->name); - goto cleanup; - } } ret = 0; @@ -6952,7 +6909,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } - rc = virCgroupSetBlkioWeight(group, params[i].value.ui); + rc = virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set blkio weight tunable")); @@ -6970,7 +6927,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } for (j = 0; j < ndevices; j++) { - rc = virCgroupSetBlkioDeviceWeight(group, + rc = virCgroupSetBlkioDeviceWeight(priv->cgroup, devices[j].path, devices[j].weight); if (rc < 0) { @@ -7033,7 +6990,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, } cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -7049,13 +7005,13 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i, j; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; int rc; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7073,6 +7029,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7089,17 +7046,11 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } } if (flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -7109,7 +7060,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, switch (i) { case 0: /* fill blkio weight here */ - rc = virCgroupGetBlkioWeight(group, &val); + rc = virCgroupGetBlkioWeight(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get blkio weight")); @@ -7222,8 +7173,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -7238,7 +7187,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainDefPtr persistentDef = NULL; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; unsigned long long swap_hard_limit; unsigned long long memory_hard_limit; @@ -7250,6 +7198,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, int ret = -1; int rc; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7268,6 +7217,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -7278,17 +7228,11 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } } #define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ @@ -7316,7 +7260,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, if (set_swap_hard_limit) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((rc = virCgroupSetMemSwapHardLimit(group, swap_hard_limit)) < 0) { + if ((rc = virCgroupSetMemSwapHardLimit(priv->cgroup, swap_hard_limit)) < 0) { virReportSystemError(-rc, "%s", _("unable to set memory swap_hard_limit tunable")); goto cleanup; @@ -7330,7 +7274,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, if (set_memory_hard_limit) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((rc = virCgroupSetMemoryHardLimit(group, memory_hard_limit)) < 0) { + if ((rc = virCgroupSetMemoryHardLimit(priv->cgroup, memory_hard_limit)) < 0) { virReportSystemError(-rc, "%s", _("unable to set memory hard_limit tunable")); goto cleanup; @@ -7344,7 +7288,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, if (set_memory_soft_limit) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((rc = virCgroupSetMemorySoftLimit(group, memory_soft_limit)) < 0) { + if ((rc = virCgroupSetMemorySoftLimit(priv->cgroup, memory_soft_limit)) < 0) { virReportSystemError(-rc, "%s", _("unable to set memory soft_limit tunable")); goto cleanup; @@ -7363,7 +7307,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, ret = 0; cleanup: - virCgroupFree(&group); virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7378,12 +7321,12 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; int rc; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7400,6 +7343,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7408,17 +7352,11 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } } if ((*nparams) == 0) { @@ -7469,12 +7407,9 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, virTypedParameterPtr param = ¶ms[i]; unsigned long long val = 0; - /* Coverity does not realize that if we get here, group is set. */ - sa_assert(group); - switch (i) { case 0: /* fill memory hard limit here */ - rc = virCgroupGetMemoryHardLimit(group, &val); + rc = virCgroupGetMemoryHardLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get memory hard limit")); @@ -7487,7 +7422,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 1: /* fill memory soft limit here */ - rc = virCgroupGetMemorySoftLimit(group, &val); + rc = virCgroupGetMemorySoftLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get memory soft limit")); @@ -7500,7 +7435,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetMemSwapHardLimit(group, &val); + rc = virCgroupGetMemSwapHardLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); @@ -7524,8 +7459,6 @@ out: ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -7541,11 +7474,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; int i; virDomainDefPtr persistentDef = NULL; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7564,6 +7497,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -7574,18 +7508,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup cpuset controller is not mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), - vm->def->name); - goto cleanup; - } } ret = 0; @@ -7638,7 +7565,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, continue; } - if ((rc = virCgroupSetCpusetMems(group, nodeset_str) != 0)) { + if ((rc = virCgroupSetCpusetMems(priv->cgroup, nodeset_str) != 0)) { virReportSystemError(-rc, "%s", _("unable to set numa tunable")); virBitmapFree(nodeset); @@ -7678,7 +7605,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -7694,13 +7620,13 @@ qemuDomainGetNumaParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; char *nodeset = NULL; int ret = -1; int rc; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7718,6 +7644,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7733,18 +7660,11 @@ qemuDomainGetNumaParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), - vm->def->name); - goto cleanup; - } } for (i = 0; i < QEMU_NB_NUMA_PARAM && i < *nparams; i++) { @@ -7767,7 +7687,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, if (!nodeset) nodeset = strdup(""); } else { - rc = virCgroupGetCpusetMems(group, &nodeset); + rc = virCgroupGetCpusetMems(priv->cgroup, &nodeset); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get numa nodeset")); @@ -7794,7 +7714,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, cleanup: VIR_FREE(nodeset); - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -7902,6 +7821,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, int rc; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7927,6 +7847,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -7944,17 +7865,11 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), - vm->def->name); - goto cleanup; - } } for (i = 0; i < nparams; i++) { @@ -7964,7 +7879,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((rc = virCgroupSetCpuShares(group, value_ul))) { + if ((rc = virCgroupSetCpuShares(priv->cgroup, value_ul))) { virReportSystemError(-rc, "%s", _("unable to set cpu shares tunable")); goto cleanup; @@ -8050,7 +7965,6 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, cleanup: virDomainDefFree(vmdef); - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -8094,7 +8008,7 @@ qemuGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, } static int -qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, +qemuGetVcpusBWLive(virDomainObjPtr vm, unsigned long long *period, long long *quota) { virCgroupPtr cgroup_vcpu = NULL; @@ -8105,7 +8019,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, priv = vm->privateData; if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { /* We do not create sub dir for each vcpu */ - rc = qemuGetVcpuBWLive(cgroup, period, quota); + rc = qemuGetVcpuBWLive(priv->cgroup, period, quota); if (rc < 0) goto cleanup; @@ -8115,7 +8029,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, } /* get period and quota for vcpu0 */ - rc = virCgroupForVcpu(cgroup, 0, &cgroup_vcpu, 0); + rc = virCgroupForVcpu(priv->cgroup, 0, &cgroup_vcpu, 0); if (!cgroup_vcpu) { virReportSystemError(-rc, _("Unable to find vcpu cgroup for %s(vcpu: 0)"), @@ -8179,7 +8093,6 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; unsigned long long shares; unsigned long long period; @@ -8192,6 +8105,7 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, int saved_nparams = 0; virDomainDefPtr persistentDef; virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -8200,13 +8114,6 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, /* We don't return strings, and thus trivially support this flag. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - if (*nparams > 1) { - rc = qemuGetCpuBWStatus(driver->cgroup); - if (rc < 0) - goto cleanup; - cpu_bw_status = !!rc; - } - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); if (vm == NULL) { @@ -8215,6 +8122,15 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; + + if (*nparams > 1) { + rc = qemuGetCpuBWStatus(priv->cgroup); + if (rc < 0) + goto cleanup; + cpu_bw_status = !!rc; + } + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -8233,19 +8149,13 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, goto out; } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - - rc = virCgroupGetCpuShares(group, &shares); + rc = virCgroupGetCpuShares(priv->cgroup, &shares); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get cpu shares tunable")); @@ -8253,13 +8163,13 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, } if (*nparams > 1 && cpu_bw_status) { - rc = qemuGetVcpusBWLive(vm, group, &period, "a); + rc = qemuGetVcpusBWLive(vm, &period, "a); if (rc != 0) goto cleanup; } if (*nparams > 3 && cpu_bw_status) { - rc = qemuGetEmulatorBandwidthLive(vm, group, &emulator_period, + rc = qemuGetEmulatorBandwidthLive(vm, priv->cgroup, &emulator_period, &emulator_quota); if (rc != 0) goto cleanup; @@ -8312,7 +8222,6 @@ out: ret = 0; cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -8708,7 +8617,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; @@ -8872,7 +8780,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, cleanup: virNetDevBandwidthFree(bandwidth); virNetDevBandwidthFree(newBandwidth); - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -8889,7 +8796,6 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; @@ -8996,8 +8902,6 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virObjectUnlock(vm); virObjectUnref(caps); @@ -10603,7 +10507,6 @@ typedef enum { static int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk, const char *file, qemuDomainDiskChainMode mode) @@ -10627,13 +10530,13 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src); - if (cgroup && qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) + if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) VIR_WARN("Unable to release lock on %s", disk->src); } else if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || - (cgroup && qemuSetupDiskCgroup(vm, cgroup, disk) < 0) || + qemuSetupDiskCgroup(vm, disk) < 0 || virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { goto cleanup; @@ -11069,7 +10972,6 @@ cleanup: static int qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainSnapshotDiskDefPtr snap, virDomainDiskDefPtr disk, virDomainDiskDefPtr persistDisk, @@ -11119,9 +11021,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; - if (qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, source, + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, source, + qemuDomainPrepareDiskChainElement(driver, vm, disk, source, VIR_DISK_CHAIN_NO_ACCESS); goto cleanup; } @@ -11163,7 +11065,6 @@ cleanup: static void qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr origdisk, virDomainDiskDefPtr disk, virDomainDiskDefPtr persistDisk, @@ -11180,7 +11081,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, goto cleanup; } - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, origdisk->src, + qemuDomainPrepareDiskChainElement(driver, vm, disk, origdisk->src, VIR_DISK_CHAIN_NO_ACCESS); if (need_unlink && stat(disk->src, &st) == 0 && S_ISREG(st.st_mode) && unlink(disk->src) < 0) @@ -11217,7 +11118,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, int i; bool persist = false; bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - virCgroupPtr cgroup = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (!virDomainObjIsActive(vm)) { @@ -11226,15 +11126,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, goto cleanup; } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - /* 'cgroup' is still NULL if cgroups are disabled. */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { if (!(actions = virJSONValueNewArray())) { virReportOOMError(); @@ -11270,7 +11161,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } } - ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, cgroup, + ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, @@ -11299,7 +11190,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, persistDisk = vm->newDef->disks[indx]; } - qemuDomainSnapshotUndoSingleDiskActive(driver, vm, cgroup, + qemuDomainSnapshotUndoSingleDiskActive(driver, vm, snap->def->dom->disks[i], vm->def->disks[i], persistDisk, @@ -11310,7 +11201,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); cleanup: - virCgroupFree(&cgroup); if (ret == 0 || !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { if (virDomainSaveStatus(driver->xmlconf, cfg->stateDir, vm) < 0 || @@ -13061,7 +12951,6 @@ qemuDomainBlockPivot(virConnectPtr conn, virDomainBlockJobInfo info; const char *format = virStorageFileFormatTypeToString(disk->mirrorFormat); bool resume = false; - virCgroupPtr cgroup = NULL; char *oldsrc = NULL; int oldformat; virStorageFileMetadataPtr oldchain = NULL; @@ -13121,14 +13010,6 @@ qemuDomainBlockPivot(virConnectPtr conn, * label the entire chain. This action is safe even if the * backing chain has already been labeled; but only necessary when * we know for sure that there is a backing chain. */ - if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW && - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } oldsrc = disk->src; oldformat = disk->format; oldchain = disk->backingChain; @@ -13144,7 +13025,7 @@ qemuDomainBlockPivot(virConnectPtr conn, if (disk->mirrorFormat && disk->mirrorFormat != VIR_STORAGE_FILE_RAW && (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || - (cgroup && qemuSetupDiskCgroup(vm, cgroup, disk) < 0) || + qemuSetupDiskCgroup(vm, disk) < 0 || virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0)) { disk->src = oldsrc; @@ -13188,8 +13069,6 @@ qemuDomainBlockPivot(virConnectPtr conn, disk->mirroring = false; cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (resume && virDomainObjIsActive(vm) && qemuProcessStartCPUs(driver, vm, conn, VIR_DOMAIN_RUNNING_UNPAUSED, @@ -13417,7 +13296,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, struct stat st; bool need_unlink = false; char *mirror = NULL; - virCgroupPtr cgroup = NULL; virQEMUDriverConfigPtr cfg = NULL; /* Preliminaries: find the disk we are editing, sanity checks */ @@ -13433,13 +13311,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, _("domain is not running")); goto cleanup; } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } device = qemuDiskPathToAlias(vm, path, &idx); if (!device) { @@ -13541,9 +13412,9 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, goto endjob; } - if (qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, dest, + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, dest, + qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, VIR_DISK_CHAIN_NO_ACCESS); goto endjob; } @@ -13555,7 +13426,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, dest, + qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, VIR_DISK_CHAIN_NO_ACCESS); goto endjob; } @@ -13577,8 +13448,6 @@ endjob: } cleanup: - if (cgroup) - virCgroupFree(&cgroup); VIR_FREE(device); if (vm) virObjectUnlock(vm); @@ -13634,7 +13503,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, virStorageFileMetadataPtr top_meta = NULL; const char *top_parent = NULL; const char *base_canon = NULL; - virCgroupPtr cgroup = NULL; bool clean_access = false; virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); @@ -13718,18 +13586,11 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, * revoke access to files removed from the chain, when the commit * operation succeeds, but doing that requires tracking the * operation in XML across libvirtd restarts. */ - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto endjob; - } clean_access = true; - if (qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, base_canon, + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, base_canon, VIR_DISK_CHAIN_READ_WRITE) < 0 || (top_parent && top_parent != disk->src && - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, + qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; @@ -13743,15 +13604,13 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, endjob: if (ret < 0 && clean_access) { /* Revert access to read-only, if possible. */ - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, base_canon, + qemuDomainPrepareDiskChainElement(driver, vm, disk, base_canon, VIR_DISK_CHAIN_READ_ONLY); if (top_parent && top_parent != disk->src) - qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, + qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, VIR_DISK_CHAIN_READ_ONLY); } - if (cgroup) - virCgroupFree(&cgroup); if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; goto cleanup; @@ -14395,17 +14254,18 @@ cleanup: /* qemuDomainGetCPUStats() with start_cpu == -1 */ static int -qemuDomainGetTotalcpuStats(virCgroupPtr group, +qemuDomainGetTotalcpuStats(virDomainObjPtr vm, virTypedParameterPtr params, int nparams) { unsigned long long cpu_time; int ret; + qemuDomainObjPrivatePtr priv = vm->privateData; if (nparams == 0) /* return supported number of params */ return QEMU_NB_TOTAL_CPU_STAT_PARAM; /* entry 0 is cputime */ - ret = virCgroupGetCpuacctUsage(group, &cpu_time); + ret = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); if (ret < 0) { virReportSystemError(-ret, "%s", _("unable to get cpu account")); return -1; @@ -14419,7 +14279,7 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group, unsigned long long user; unsigned long long sys; - ret = virCgroupGetCpuacctStat(group, &user, &sys); + ret = virCgroupGetCpuacctStat(priv->cgroup, &user, &sys); if (ret < 0) { virReportSystemError(-ret, "%s", _("unable to get cpu account")); return -1; @@ -14457,22 +14317,22 @@ qemuDomainGetTotalcpuStats(virCgroupPtr group, * s3 = t03 + t13 */ static int -getSumVcpuPercpuStats(virCgroupPtr group, - unsigned int nvcpu, +getSumVcpuPercpuStats(virDomainObjPtr vm, unsigned long long *sum_cpu_time, unsigned int num) { int ret = -1; int i; char *buf = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr group_vcpu = NULL; - for (i = 0; i < nvcpu; i++) { + for (i = 0; i < priv->nvcpupids; i++) { char *pos; unsigned long long tmp; int j; - if (virCgroupForVcpu(group, i, &group_vcpu, 0) < 0) { + if (virCgroupForVcpu(priv->cgroup, i, &group_vcpu, 0) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("error accessing cgroup cpuacct for vcpu")); goto cleanup; @@ -14504,7 +14364,6 @@ cleanup: static int qemuDomainGetPercpuStats(virDomainObjPtr vm, - virCgroupPtr group, virTypedParameterPtr params, unsigned int nparams, int start_cpu, @@ -14544,7 +14403,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, } /* we get percpu cputime accounting info. */ - if (virCgroupGetCpuacctPercpuUsage(group, &buf)) + if (virCgroupGetCpuacctPercpuUsage(priv->cgroup, &buf)) goto cleanup; pos = buf; memset(params, 0, nparams * ncpus); @@ -14584,7 +14443,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, virReportOOMError(); goto cleanup; } - if (getSumVcpuPercpuStats(group, priv->nvcpupids, sum_cpu_time, n) < 0) + if (getSumVcpuPercpuStats(vm, sum_cpu_time, n) < 0) goto cleanup; sum_cpu_pos = sum_cpu_time; @@ -14610,17 +14469,17 @@ cleanup: static int qemuDomainGetCPUStats(virDomainPtr domain, - virTypedParameterPtr params, - unsigned int nparams, - int start_cpu, - unsigned int ncpus, - unsigned int flags) + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus, + unsigned int flags) { virQEMUDriverPtr driver = domain->conn->privateData; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; bool isActive; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -14630,6 +14489,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, _("No such domain %s"), domain->uuid); goto cleanup; } + priv = vm->privateData; isActive = virDomainObjIsActive(vm); if (!isActive) { @@ -14638,25 +14498,18 @@ qemuDomainGetCPUStats(virDomainPtr domain, goto cleanup; } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUACCT)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPUACCT controller is not mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - if (start_cpu == -1) - ret = qemuDomainGetTotalcpuStats(group, params, nparams); + ret = qemuDomainGetTotalcpuStats(vm, params, nparams); else - ret = qemuDomainGetPercpuStats(vm, group, params, nparams, + ret = qemuDomainGetPercpuStats(vm, params, nparams, start_cpu, ncpus); cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); return ret; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index de9edd4..353c665 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1133,27 +1133,16 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, goto error; } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - virCgroupPtr cgroup = NULL; + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virUSBDevicePtr usb; - qemuCgroupData data; - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto error; - } if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device, NULL)) == NULL) goto error; - data.vm = vm; - data.cgroup = cgroup; if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, - &data) < 0) { + vm) < 0) { virUSBDeviceFree(usb); goto error; } @@ -2029,7 +2018,6 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, int i, ret = -1; virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup = NULL; char *drivestr = NULL; i = qemuFindDisk(vm->def, dev->data.disk->dst); @@ -2049,15 +2037,6 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - } - if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, @@ -2127,11 +2106,9 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, vm->def, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); - if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(vm, cgroup, dev->data.disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(dev->data.disk->src)); - } + if (qemuTeardownDiskCgroup(vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); if (virDomainLockDiskDetach(driver->lockManager, vm, dev->data.disk) < 0) VIR_WARN("Unable to release lock on %s", dev->data.disk->src); @@ -2139,7 +2116,6 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, ret = 0; cleanup: - virCgroupFree(&cgroup); VIR_FREE(drivestr); return ret; } @@ -2151,7 +2127,6 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, int i, ret = -1; virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup = NULL; char *drivestr = NULL; i = qemuFindDisk(vm->def, dev->data.disk->dst); @@ -2178,15 +2153,6 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - } - /* build the actual drive id string as the disk->info.alias doesn't * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ if (virAsprintf(&drivestr, "%s%s", @@ -2219,11 +2185,9 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, vm->def, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); - if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(vm, cgroup, dev->data.disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(dev->data.disk->src)); - } + if (qemuTeardownDiskCgroup(vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); if (virDomainLockDiskDetach(driver->lockManager, vm, dev->data.disk) < 0) VIR_WARN("Unable to release lock on disk %s", dev->data.disk->src); @@ -2232,7 +2196,6 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, cleanup: VIR_FREE(drivestr); - virCgroupFree(&cgroup); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 537b834..fcc37ed 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4114,7 +4114,6 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, enum qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup = NULL; int ret = -1; int rc; bool restoreLabel = false; @@ -4148,21 +4147,13 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, * given cgroup ACL permission. We might also stumble on * a race present in some qemu versions where it does a wait() * that botches pclose. */ - if (qemuCgroupControllerActive(driver, - VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, - &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto cleanup; - } - rc = virCgroupAllowDevicePath(cgroup, path, + if (virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) { + rc = virCgroupAllowDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, cgroup, "allow", path, "rw", rc); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rc); if (rc == 1) { /* path was not a device, no further need for cgroup */ - virCgroupFree(&cgroup); } else if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), @@ -4263,14 +4254,14 @@ cleanup: vm->def, path) < 0) VIR_WARN("failed to restore save state label on %s", path); - if (cgroup != NULL) { - rc = virCgroupDenyDevicePath(cgroup, path, + if (virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_DEVICES)) { + rc = virCgroupDenyDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(vm, cgroup, "deny", path, "rwm", rc); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rc); if (rc < 0) VIR_WARN("Unable to deny device %s for %s %d", path, vm->def->name, rc); - virCgroupFree(&cgroup); } return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8c4bfb7..a86e62c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1395,6 +1395,7 @@ qemuProcessReadLogOutput(virDomainObjPtr vm, /* Filter out debug messages from intermediate libvirt process */ while ((eol = strchr(filter_next, '\n'))) { *eol = '\0'; + VIR_ERROR("<<<<<<<<<<<<%s>>>>>>>>>>", filter_next); if (virLogProbablyLogMessage(filter_next)) { memmove(filter_next, eol + 1, got - (eol - buf)); got -= eol + 1 - filter_next; @@ -2529,7 +2530,7 @@ static int qemuProcessHook(void *data) * memory allocation is on the correct NUMA node */ VIR_DEBUG("Moving process to cgroup"); - if (qemuAddToCgroup(h->driver, h->vm->def) < 0) + if (qemuAddToCgroup(h->vm) < 0) goto cleanup; /* This must be done after cgroup placement to avoid resetting CPU @@ -3004,6 +3005,9 @@ qemuProcessReconnect(void *opaque) if (qemuUpdateActiveUsbHostdevs(driver, obj->def) < 0) goto error; + if (qemuInitCgroup(driver, obj) < 0) + goto error; + /* XXX: Need to change as long as lock is introduced for * qemu_driver->sharedDisks. */ @@ -3379,7 +3383,7 @@ int qemuProcessStart(virConnectPtr conn, /* Ensure no historical cgroup for this VM is lying around bogus * settings */ VIR_DEBUG("Ensuring no historical cgroup is lying around"); - qemuRemoveCgroup(driver, vm, 1); + qemuRemoveCgroup(vm); for (i = 0 ; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; @@ -3740,7 +3744,7 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; VIR_DEBUG("Setting cgroup for each VCPU (if required)"); - if (qemuSetupCgroupForVcpu(driver, vm) < 0) + if (qemuSetupCgroupForVcpu(vm) < 0) goto cleanup; VIR_DEBUG("Setting cgroup for emulator (if required)"); @@ -4075,7 +4079,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, } retry: - if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) { + if ((ret = qemuRemoveCgroup(vm)) < 0) { if (ret == -EBUSY && (retries++ < 5)) { usleep(200*1000); goto retry; @@ -4083,6 +4087,7 @@ retry: VIR_WARN("Failed to remove cgroup for %s", vm->def->name); } + virCgroupFree(&priv->cgroup); qemuProcessRemoveDomainStatus(driver, vm); -- 1.8.1.4
From: "Daniel P. Berrange" <berrange@redhat.com> Instead of calling virCgroupForDomain every time we need the virCgrouPtr instance, just do it once at Vm startup and cache a reference to the object in virLXCDomainObjPrivatePtr until shutdown of the VM. Removing the virCgroupPtr from the LXC driver state also means we don't have stale mount info, if someone mounts the cgroups filesystem after libvirtd has been started Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_cgroup.h | 1 + src/lxc/lxc_conf.h | 3 - src/lxc/lxc_domain.c | 2 + src/lxc/lxc_domain.h | 3 + src/lxc/lxc_driver.c | 354 ++++++++++++++------------------------------------ src/lxc/lxc_process.c | 46 ++++--- 7 files changed, 135 insertions(+), 276 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 545acaa..0a2c795 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -530,7 +530,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) int ret = -1; int rc; - rc = virCgroupForDriver("lxc", &driver, true, false, 0); + rc = virCgroupForDriver("lxc", &driver, true, false, -1); if (rc != 0) { virReportSystemError(-rc, "%s", _("Unable to get cgroup for driver")); diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h index 942e0fc..46dc3bb 100644 --- a/src/lxc/lxc_cgroup.h +++ b/src/lxc/lxc_cgroup.h @@ -22,6 +22,7 @@ #ifndef __VIR_LXC_CGROUP_H__ # define __VIR_LXC_CGROUP_H__ +# include "vircgroup.h" # include "domain_conf.h" # include "lxc_fuse.h" # include "virusb.h" diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index b46dc32..dbe13a5 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -32,7 +32,6 @@ # include "domain_event.h" # include "capabilities.h" # include "virthread.h" -# include "vircgroup.h" # include "security/security_manager.h" # include "configmake.h" # include "virusb.h" @@ -53,8 +52,6 @@ struct _virLXCDriver { virCapsPtr caps; virDomainXMLConfPtr xmlconf; - virCgroupPtr cgroup; - size_t nactive; virStateInhibitCallback inhibitCallback; void *inhibitOpaque; diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 08cf8f6..1364e8e 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -43,6 +43,8 @@ static void virLXCDomainObjPrivateFree(void *data) { virLXCDomainObjPrivatePtr priv = data; + virCgroupFree(&priv->cgroup); + VIR_FREE(priv); } diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h index 007ea84..1bc8ce5 100644 --- a/src/lxc/lxc_domain.h +++ b/src/lxc/lxc_domain.h @@ -23,6 +23,7 @@ #ifndef __LXC_DOMAIN_H__ # define __LXC_DOMAIN_H__ +# include "vircgroup.h" # include "lxc_conf.h" # include "lxc_monitor.h" @@ -36,6 +37,8 @@ struct _virLXCDomainObjPrivate { bool wantReboot; pid_t initpid; + + virCgroupPtr cgroup; }; extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 6737f13..91f14cd 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -527,8 +527,8 @@ static int lxcDomainGetInfo(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup = NULL; int ret = -1, rc; + virLXCDomainObjPrivatePtr priv; lxcDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); @@ -541,24 +541,20 @@ static int lxcDomainGetInfo(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; + info->state = virDomainObjGetState(vm, NULL); - if (!virDomainObjIsActive(vm) || driver->cgroup == NULL) { + if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; info->memory = vm->def->mem.cur_balloon; } else { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to get cgroup for %s"), vm->def->name); - goto cleanup; - } - - if (virCgroupGetCpuacctUsage(cgroup, &(info->cpuTime)) < 0) { + if (virCgroupGetCpuacctUsage(priv->cgroup, &(info->cpuTime)) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Cannot read cputime for domain")); goto cleanup; } - if ((rc = virCgroupGetMemoryUsage(cgroup, &(info->memory))) < 0) { + if ((rc = virCgroupGetMemoryUsage(priv->cgroup, &(info->memory))) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Cannot read memory usage for domain")); if (rc == -ENOENT) { @@ -576,8 +572,6 @@ static int lxcDomainGetInfo(virDomainPtr dom, cleanup: lxcDriverUnlock(driver); - if (cgroup) - virCgroupFree(&cgroup); if (vm) virObjectUnlock(vm); return ret; @@ -708,8 +702,8 @@ cleanup: static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup = NULL; int ret = -1; + virLXCDomainObjPrivatePtr priv; lxcDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); @@ -721,6 +715,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { _("No domain with matching uuid '%s'"), uuidstr); goto cleanup; } + priv = vm->privateData; if (newmem > vm->def->mem.max_balloon) { virReportError(VIR_ERR_INVALID_ARG, @@ -734,19 +729,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto cleanup; } - if (driver->cgroup == NULL) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroups must be configured on the host")); - goto cleanup; - } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to get cgroup for %s"), vm->def->name); - goto cleanup; - } - - if (virCgroupSetMemory(cgroup, newmem) < 0) { + if (virCgroupSetMemory(priv->cgroup, newmem) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Failed to set memory for domain")); goto cleanup; @@ -757,8 +740,6 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { cleanup: if (vm) virObjectUnlock(vm); - if (cgroup) - virCgroupFree(&cgroup); return ret; } @@ -770,10 +751,10 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr cgroup = NULL; virDomainObjPtr vm = NULL; int ret = -1; int rc; + virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); if (virTypedParameterArrayValidate(params, nparams, @@ -796,33 +777,28 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, _("No domain with matching uuid '%s'"), uuidstr); goto cleanup; } - - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } + priv = vm->privateData; ret = 0; for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { - rc = virCgroupSetMemoryHardLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemoryHardLimit(priv->cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set memory hard_limit tunable")); ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { - rc = virCgroupSetMemorySoftLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemorySoftLimit(priv->cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set memory soft_limit tunable")); ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { - rc = virCgroupSetMemSwapHardLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(priv->cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); @@ -832,8 +808,6 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, } cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -848,11 +822,11 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr cgroup = NULL; virDomainObjPtr vm = NULL; unsigned long long val; int ret = -1; int rc; + virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -866,6 +840,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, _("No domain with matching uuid '%s'"), uuidstr); goto cleanup; } + priv = vm->privateData; if ((*nparams) == 0) { /* Current number of memory parameters supported by cgroups */ @@ -874,19 +849,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to get cgroup for %s"), vm->def->name); - goto cleanup; - } - for (i = 0; i < LXC_NB_MEM_PARAM && i < *nparams; i++) { virTypedParameterPtr param = ¶ms[i]; val = 0; switch (i) { case 0: /* fill memory hard limit here */ - rc = virCgroupGetMemoryHardLimit(cgroup, &val); + rc = virCgroupGetMemoryHardLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get memory hard limit")); @@ -897,7 +866,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; break; case 1: /* fill memory soft limit here */ - rc = virCgroupGetMemorySoftLimit(cgroup, &val); + rc = virCgroupGetMemorySoftLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get memory soft limit")); @@ -908,7 +877,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetMemSwapHardLimit(cgroup, &val); + rc = virCgroupGetMemSwapHardLimit(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); @@ -932,8 +901,6 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, ret = 0; cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -1417,7 +1384,6 @@ static int lxcStartup(bool privileged, void *opaque ATTRIBUTE_UNUSED) { char *ld; - int rc; /* Valgrind gets very annoyed when we clone containers, so * disable LXC when under valgrind @@ -1460,16 +1426,6 @@ static int lxcStartup(bool privileged, lxc_driver->log_libvirtd = 0; /* by default log to container logfile */ lxc_driver->have_netns = lxcCheckNetNsSupport(); - rc = virCgroupForDriver("lxc", &lxc_driver->cgroup, privileged, true, -1); - if (rc < 0) { - char buf[1024] ATTRIBUTE_UNUSED; - VIR_DEBUG("Unable to create cgroup for LXC driver: %s", - virStrerror(-rc, buf, sizeof(buf))); - /* Don't abort startup. We will explicitly report to - * the user when they try to start a VM - */ - } - /* Call function to load lxc driver configuration information */ if (lxcLoadDriverConfig(lxc_driver) < 0) goto cleanup; @@ -1638,30 +1594,32 @@ cleanup: } -static bool lxcCgroupControllerActive(virLXCDriverPtr driver, - int controller) -{ - return virCgroupHasController(driver->cgroup, controller); -} - - - -static char *lxcGetSchedulerType(virDomainPtr domain, +static char *lxcGetSchedulerType(virDomainPtr dom, int *nparams) { - virLXCDriverPtr driver = domain->conn->privateData; + virLXCDriverPtr driver = dom->conn->privateData; char *ret = NULL; int rc; + virDomainObjPtr vm; + virLXCDomainObjPrivatePtr priv; lxcDriverLock(driver); - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + if (vm == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + priv = vm->privateData; + + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } if (nparams) { - rc = lxcGetCpuBWStatus(driver->cgroup); + rc = lxcGetCpuBWStatus(priv->cgroup); if (rc < 0) goto cleanup; else if (rc == 0) @@ -1675,6 +1633,8 @@ static char *lxcGetSchedulerType(virDomainPtr domain, virReportOOMError(); cleanup: + if (vm) + virObjectUnlock(vm); lxcDriverUnlock(driver); return ret; } @@ -1761,11 +1721,11 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; int ret = -1; int rc; + virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -1788,6 +1748,7 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; if (virDomainLiveConfigHelperMethod(driver->caps, driver->xmlconf, vm, &flags, &vmdef) < 0) @@ -1801,17 +1762,11 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), - vm->def->name); - goto cleanup; - } } for (i = 0; i < nparams; i++) { @@ -1819,7 +1774,7 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - rc = virCgroupSetCpuShares(group, params[i].value.ul); + rc = virCgroupSetCpuShares(priv->cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set cpu shares tunable")); @@ -1834,7 +1789,7 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - rc = lxcSetVcpuBWLive(group, params[i].value.ul, 0); + rc = lxcSetVcpuBWLive(priv->cgroup, params[i].value.ul, 0); if (rc != 0) goto cleanup; @@ -1847,7 +1802,7 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - rc = lxcSetVcpuBWLive(group, 0, params[i].value.l); + rc = lxcSetVcpuBWLive(priv->cgroup, 0, params[i].value.l); if (rc != 0) goto cleanup; @@ -1878,7 +1833,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, cleanup: virDomainDefFree(vmdef); - virCgroupFree(&group); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -1900,7 +1854,6 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef; unsigned long long shares = 0; @@ -1910,19 +1863,13 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, int rc; bool cpu_bw_status = false; int saved_nparams = 0; + virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); lxcDriverLock(driver); - if (*nparams > 1) { - rc = lxcGetCpuBWStatus(driver->cgroup); - if (rc < 0) - goto cleanup; - cpu_bw_status = !!rc; - } - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); if (vm == NULL) { @@ -1930,6 +1877,14 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; + + if (*nparams > 1) { + rc = lxcGetCpuBWStatus(priv->cgroup); + if (rc < 0) + goto cleanup; + cpu_bw_status = !!rc; + } if (virDomainLiveConfigHelperMethod(driver->caps, driver->xmlconf, vm, &flags, &persistentDef) < 0) @@ -1944,19 +1899,13 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, goto out; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - - rc = virCgroupGetCpuShares(group, &shares); + rc = virCgroupGetCpuShares(priv->cgroup, &shares); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get cpu shares tunable")); @@ -1964,7 +1913,7 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, } if (*nparams > 1 && cpu_bw_status) { - rc = lxcGetVcpuBWLive(group, &period, "a); + rc = lxcGetVcpuBWLive(priv->cgroup, &period, "a); if (rc != 0) goto cleanup; } @@ -1997,7 +1946,6 @@ out: ret = 0; cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -2021,10 +1969,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; + virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -2043,24 +1991,19 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; if (virDomainLiveConfigHelperMethod(driver->caps, driver->xmlconf, vm, &flags, &persistentDef) < 0) goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; @@ -2073,7 +2016,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, goto cleanup; } - rc = virCgroupSetBlkioWeight(group, params[i].value.ui); + rc = virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set blkio weight tunable")); @@ -2106,7 +2049,6 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - virCgroupFree(&group); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -2123,12 +2065,12 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; int rc; + virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -2141,6 +2083,7 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, _("No such domain %s"), dom->uuid); goto cleanup; } + priv = vm->privateData; if ((*nparams) == 0) { /* Current number of blkio parameters supported by cgroups */ @@ -2154,25 +2097,19 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = ¶ms[i]; val = 0; switch (i) { case 0: /* fill blkio weight here */ - rc = virCgroupGetBlkioWeight(group, &val); + rc = virCgroupGetBlkioWeight(priv->cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get blkio weight")); @@ -2214,8 +2151,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virObjectUnlock(vm); lxcDriverUnlock(driver); @@ -2385,7 +2320,7 @@ cleanup: return ret; } -static int lxcFreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) +static int lxcFreezeContainer(virDomainObjPtr vm) { int timeout = 1000; /* In milliseconds */ int check_interval = 1; /* In milliseconds */ @@ -2393,13 +2328,7 @@ static int lxcFreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) int waited_time = 0; int ret = -1; char *state = NULL; - virCgroupPtr cgroup = NULL; - - if (!(driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) - return -1; - - /* From here on, we know that cgroup != NULL. */ + virLXCDomainObjPrivatePtr priv = vm->privateData; while (waited_time < timeout) { int r; @@ -2410,7 +2339,7 @@ static int lxcFreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) * to "FROZEN". * (see linux-2.6/Documentation/cgroups/freezer-subsystem.txt) */ - r = virCgroupSetFreezerState(cgroup, "FROZEN"); + r = virCgroupSetFreezerState(priv->cgroup, "FROZEN"); /* * Returning EBUSY explicitly indicates that the group is @@ -2437,7 +2366,7 @@ static int lxcFreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) */ usleep(check_interval * 1000); - r = virCgroupGetFreezerState(cgroup, &state); + r = virCgroupGetFreezerState(priv->cgroup, &state); if (r < 0) { VIR_DEBUG("Reading freezer.state failed with errno: %d", r); @@ -2469,11 +2398,10 @@ error: * activate the group again and return an error. * This is likely to fall the group back again gracefully. */ - virCgroupSetFreezerState(cgroup, "THAWED"); + virCgroupSetFreezerState(priv->cgroup, "THAWED"); ret = -1; cleanup: - virCgroupFree(&cgroup); VIR_FREE(state); return ret; } @@ -2503,7 +2431,7 @@ static int lxcDomainSuspend(virDomainPtr dom) } if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - if (lxcFreezeContainer(driver, vm) < 0) { + if (lxcFreezeContainer(vm) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Suspend operation failed")); goto cleanup; @@ -2528,27 +2456,13 @@ cleanup: return ret; } -static int lxcUnfreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) -{ - int ret; - virCgroupPtr cgroup = NULL; - - if (!(driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) - return -1; - - ret = virCgroupSetFreezerState(cgroup, "THAWED"); - - virCgroupFree(&cgroup); - return ret; -} - static int lxcDomainResume(virDomainPtr dom) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; int ret = -1; + virLXCDomainObjPrivatePtr priv; lxcDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); @@ -2561,6 +2475,8 @@ static int lxcDomainResume(virDomainPtr dom) goto cleanup; } + priv = vm->privateData; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); @@ -2568,7 +2484,7 @@ static int lxcDomainResume(virDomainPtr dom) } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (lxcUnfreezeContainer(driver, vm) < 0) { + if (virCgroupSetFreezerState(priv->cgroup, "THAWED") < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Resume operation failed")); goto cleanup; @@ -3111,7 +3027,6 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainDiskDefPtr def = dev->data.disk; - virCgroupPtr group = NULL; int ret = -1; char *dst = NULL; struct stat sb; @@ -3196,19 +3111,13 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, vm->def, def) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - - if (virCgroupAllowDevicePath(group, def->src, + if (virCgroupAllowDevicePath(priv->cgroup, def->src, (def->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | @@ -3226,8 +3135,6 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, cleanup: def->src = tmpsrc; virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); - if (group) - virCgroupFree(&group); if (dst && created && ret < 0) unlink(dst); return ret; @@ -3381,7 +3288,6 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, mode_t mode; bool created = false; virUSBDevicePtr usb = NULL; - virCgroupPtr group = NULL; if (virDomainHostdevFind(vm->def, def, NULL) >= 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -3416,18 +3322,12 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - if (!(usb = virUSBDeviceNew(def->source.subsys.u.usb.bus, def->source.subsys.u.usb.device, vroot))) goto cleanup; @@ -3468,8 +3368,8 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, goto cleanup; if (virUSBDeviceFileIterate(usb, - virLXCSetupHostUsbDeviceCgroup, - &group) < 0) + virLXCSetupHostUsbDeviceCgroup, + &priv->cgroup) < 0) goto cleanup; ret = 0; @@ -3480,7 +3380,6 @@ cleanup: unlink(dstfile); virUSBDeviceFree(usb); - virCgroupFree(&group); VIR_FREE(src); VIR_FREE(dstfile); VIR_FREE(dstdir); @@ -3496,7 +3395,6 @@ lxcDomainAttachDeviceHostdevStorageLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = dev->data.hostdev; - virCgroupPtr group = NULL; int ret = -1; char *dst = NULL; char *vroot = NULL; @@ -3565,19 +3463,13 @@ lxcDomainAttachDeviceHostdevStorageLive(virLXCDriverPtr driver, vm->def, def, vroot) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - - if (virCgroupAllowDevicePath(group, def->source.caps.u.storage.block, + if (virCgroupAllowDevicePath(priv->cgroup, def->source.caps.u.storage.block, VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3592,8 +3484,6 @@ lxcDomainAttachDeviceHostdevStorageLive(virLXCDriverPtr driver, cleanup: virDomainAuditHostdev(vm, def, "attach", ret == 0); - if (group) - virCgroupFree(&group); if (dst && created && ret < 0) unlink(dst); VIR_FREE(dst); @@ -3609,7 +3499,6 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = dev->data.hostdev; - virCgroupPtr group = NULL; int ret = -1; char *dst = NULL; char *vroot = NULL; @@ -3678,19 +3567,13 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver, vm->def, def, vroot) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - - if (virCgroupAllowDevicePath(group, def->source.caps.u.misc.chardev, + if (virCgroupAllowDevicePath(priv->cgroup, def->source.caps.u.misc.chardev, VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3705,8 +3588,6 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver, cleanup: virDomainAuditHostdev(vm, def, "attach", ret == 0); - if (group) - virCgroupFree(&group); if (dst && created && ret < 0) unlink(dst); VIR_FREE(dst); @@ -3823,13 +3704,11 @@ lxcDomainAttachDeviceLive(virConnectPtr conn, static int -lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, - virDomainObjPtr vm, +lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainDiskDefPtr def = NULL; - virCgroupPtr group = NULL; int i, ret = -1; char *dst = NULL; @@ -3855,18 +3734,12 @@ lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - VIR_DEBUG("Unlinking %s (backed by %s)", dst, def->src); if (unlink(dst) < 0 && errno != ENOENT) { virDomainAuditDisk(vm, def->src, NULL, "detach", false); @@ -3876,7 +3749,7 @@ lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, } virDomainAuditDisk(vm, def->src, NULL, "detach", true); - if (virCgroupDenyDevicePath(group, def->src, VIR_CGROUP_DEVICE_RWM) != 0) + if (virCgroupDenyDevicePath(priv->cgroup, def->src, VIR_CGROUP_DEVICE_RWM) != 0) VIR_WARN("cannot deny device %s for domain %s", def->src, vm->def->name); @@ -3887,8 +3760,6 @@ lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, cleanup: VIR_FREE(dst); - if (group) - virCgroupFree(&group); return ret; } @@ -3966,7 +3837,6 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = NULL; - virCgroupPtr group = NULL; int idx, ret = -1; char *dst = NULL; char *vroot; @@ -3994,18 +3864,12 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - if (!(usb = virUSBDeviceNew(def->source.subsys.u.usb.bus, def->source.subsys.u.usb.device, vroot))) goto cleanup; @@ -4020,8 +3884,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, virDomainAuditHostdev(vm, def, "detach", true); if (virUSBDeviceFileIterate(usb, - virLXCTeardownHostUsbDeviceCgroup, - &group) < 0) + virLXCTeardownHostUsbDeviceCgroup, + &priv->cgroup) < 0) VIR_WARN("cannot deny device %s for domain %s", dst, vm->def->name); @@ -4035,19 +3899,16 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, cleanup: virUSBDeviceFree(usb); VIR_FREE(dst); - virCgroupFree(&group); return ret; } static int -lxcDomainDetachDeviceHostdevStorageLive(virLXCDriverPtr driver, - virDomainObjPtr vm, +lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = NULL; - virCgroupPtr group = NULL; int i, ret = -1; char *dst = NULL; @@ -4073,18 +3934,12 @@ lxcDomainDetachDeviceHostdevStorageLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - VIR_DEBUG("Unlinking %s", dst); if (unlink(dst) < 0 && errno != ENOENT) { virDomainAuditHostdev(vm, def, "detach", false); @@ -4094,7 +3949,7 @@ lxcDomainDetachDeviceHostdevStorageLive(virLXCDriverPtr driver, } virDomainAuditHostdev(vm, def, "detach", true); - if (virCgroupDenyDevicePath(group, def->source.caps.u.storage.block, VIR_CGROUP_DEVICE_RWM) != 0) + if (virCgroupDenyDevicePath(priv->cgroup, def->source.caps.u.storage.block, VIR_CGROUP_DEVICE_RWM) != 0) VIR_WARN("cannot deny device %s for domain %s", def->source.caps.u.storage.block, vm->def->name); @@ -4105,20 +3960,16 @@ lxcDomainDetachDeviceHostdevStorageLive(virLXCDriverPtr driver, cleanup: VIR_FREE(dst); - if (group) - virCgroupFree(&group); return ret; } static int -lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, - virDomainObjPtr vm, +lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = NULL; - virCgroupPtr group = NULL; int i, ret = -1; char *dst = NULL; @@ -4144,18 +3995,12 @@ lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("devices cgroup isn't mounted")); goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; - } - VIR_DEBUG("Unlinking %s", dst); if (unlink(dst) < 0 && errno != ENOENT) { virDomainAuditHostdev(vm, def, "detach", false); @@ -4165,7 +4010,7 @@ lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, } virDomainAuditHostdev(vm, def, "detach", true); - if (virCgroupDenyDevicePath(group, def->source.caps.u.misc.chardev, VIR_CGROUP_DEVICE_RWM) != 0) + if (virCgroupDenyDevicePath(priv->cgroup, def->source.caps.u.misc.chardev, VIR_CGROUP_DEVICE_RWM) != 0) VIR_WARN("cannot deny device %s for domain %s", def->source.caps.u.misc.chardev, vm->def->name); @@ -4176,8 +4021,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, cleanup: VIR_FREE(dst); - if (group) - virCgroupFree(&group); return ret; } @@ -4201,16 +4044,15 @@ lxcDomainDetachDeviceHostdevSubsysLive(virLXCDriverPtr driver, static int -lxcDomainDetachDeviceHostdevCapsLive(virLXCDriverPtr driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) +lxcDomainDetachDeviceHostdevCapsLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev) { switch (dev->data.hostdev->source.caps.type) { case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE: - return lxcDomainDetachDeviceHostdevStorageLive(driver, vm, dev); + return lxcDomainDetachDeviceHostdevStorageLive(vm, dev); case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC: - return lxcDomainDetachDeviceHostdevMiscLive(driver, vm, dev); + return lxcDomainDetachDeviceHostdevMiscLive(vm, dev); default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4239,7 +4081,7 @@ lxcDomainDetachDeviceHostdevLive(virLXCDriverPtr driver, return lxcDomainDetachDeviceHostdevSubsysLive(driver, vm, dev); case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: - return lxcDomainDetachDeviceHostdevCapsLive(driver, vm, dev); + return lxcDomainDetachDeviceHostdevCapsLive(vm, dev); default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4259,7 +4101,7 @@ lxcDomainDetachDeviceLive(virLXCDriverPtr driver, switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = lxcDomainDetachDeviceDiskLive(driver, vm, dev); + ret = lxcDomainDetachDeviceDiskLive(vm, dev); break; case VIR_DOMAIN_DEVICE_NET: diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 8c8778a..e9f221f 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -219,7 +219,6 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason) { - virCgroupPtr cgroup; int i; virLXCDomainObjPrivatePtr priv = vm->privateData; virNetDevVPortProfilePtr vport = NULL; @@ -277,10 +276,9 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virDomainConfVMNWFilterTeardown(vm); - if (driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); + if (priv->cgroup) { + virCgroupRemove(priv->cgroup); + virCgroupFree(&priv->cgroup); } /* now that we know it's stopped call the hook if present */ @@ -727,8 +725,8 @@ int virLXCProcessStop(virLXCDriverPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason) { - virCgroupPtr group = NULL; int rc; + virLXCDomainObjPrivatePtr priv; VIR_DEBUG("Stopping VM name=%s pid=%d reason=%d", vm->def->name, (int)vm->pid, (int)reason); @@ -737,6 +735,8 @@ int virLXCProcessStop(virLXCDriverPtr driver, return 0; } + priv = vm->privateData; + if (vm->pid <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PID %d for container"), vm->pid); @@ -754,8 +754,8 @@ int virLXCProcessStop(virLXCDriverPtr driver, VIR_FREE(vm->def->seclabels[0]->imagelabel); } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) == 0) { - rc = virCgroupKillPainfully(group); + if (priv->cgroup) { + rc = virCgroupKillPainfully(priv->cgroup); if (rc < 0) { virReportSystemError(-rc, "%s", _("Failed to kill container PIDs")); @@ -779,7 +779,6 @@ int virLXCProcessStop(virLXCDriverPtr driver, rc = 0; cleanup: - virCgroupFree(&group); return rc; } @@ -999,27 +998,42 @@ int virLXCProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; virLXCDomainObjPrivatePtr priv = vm->privateData; virErrorPtr err = NULL; + virCgroupPtr driverGroup; - if (!lxc_driver->cgroup) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("The 'cpuacct', 'devices' & 'memory' cgroups controllers must be mounted")); + virCgroupFree(&priv->cgroup); + + rc = virCgroupForDriver("lxc", &driverGroup, true, true, -1); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("Unable to get cgroup for driverxxxx")); + return -1; + } + + rc = virCgroupForDomain(driverGroup, vm->def->name, &priv->cgroup, 1); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to create cgroup for domain %s"), + vm->def->name); return -1; } - if (!virCgroupHasController(lxc_driver->cgroup, - VIR_CGROUP_CONTROLLER_CPUACCT)) { + if (!virCgroupHasController(priv->cgroup, + VIR_CGROUP_CONTROLLER_CPUACCT)) { + virCgroupFree(&priv->cgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'cpuacct' cgroups controller mount")); return -1; } - if (!virCgroupHasController(lxc_driver->cgroup, + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { + virCgroupFree(&priv->cgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'devices' cgroups controller mount")); return -1; } - if (!virCgroupHasController(lxc_driver->cgroup, + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { + virCgroupFree(&priv->cgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'memory' cgroups controller mount")); return -1; -- 1.8.1.4
participants (2)
-
Daniel P. Berrange -
Stefan Berger