[libvirt] [RFC PATCH v1 0/7] virCgroup refactor

Hi, This series is posted for early review. This series refactors virCgroup. The changes are: - virCgroupItem is associated with a cgroup directory. The directory is created only when needed, and removed if no one is using it. - Anyone using cgroups creates instances of virCgroupItem and maintains their lifetime. Please focus on patch #5, which brings the main change(virCgroupItem), and qemu part in patch #6, which shows the usage of virCgroupItem(I've not tested lxc part yet). Hu Tao (7): call virstateCleanup to do the cleanup before libvirtd exits include util.h in cgroup.h include virterror_internal.h in threads.h refactor virCgroupDetectMounts and virCgroupDetectPlacement cgroup: refactor virCgroup deploy the newly introduced virCgroupItem. tests for virCgroup. daemon/libvirtd.c | 6 + src/conf/domain_audit.c | 16 +- src/conf/domain_audit.h | 6 +- src/conf/domain_conf.h | 5 + src/libvirt_private.syms | 17 +- src/lxc/lxc_cgroup.c | 91 ++- src/lxc/lxc_conf.h | 2 +- src/lxc/lxc_driver.c | 268 +++----- src/lxc/lxc_process.c | 56 +- src/qemu/qemu_cgroup.c | 287 +++------ src/qemu/qemu_cgroup.h | 17 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 478 +++++--------- src/qemu/qemu_hotplug.c | 44 +- src/qemu/qemu_migration.c | 36 +- src/qemu/qemu_process.c | 29 +- src/util/vircgroup.c | 1570 ++++++++++++++++++++------------------------- src/util/vircgroup.h | 108 ++-- src/util/virthread.h | 1 + tests/Makefile.am | 5 + tests/vircgrouptest.c | 103 +++ 21 files changed, 1338 insertions(+), 1809 deletions(-) create mode 100644 tests/vircgrouptest.c -- 1.8.0.1.240.ge8a1f5a

--- daemon/libvirtd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9cdf4d9..7cb99b1 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1500,5 +1500,7 @@ cleanup: daemonConfigFree(config); + virStateCleanup(); + return ret; } -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:03AM +0800, Hu Tao wrote:
--- daemon/libvirtd.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9cdf4d9..7cb99b1 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1500,5 +1500,7 @@ cleanup:
daemonConfigFree(config);
+ virStateCleanup(); + return ret; }
Wierd, I wonder when we lost this cleanup ! ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Jan 16, 2013 at 10:53:03AM +0800, Hu Tao wrote:
--- daemon/libvirtd.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9cdf4d9..7cb99b1 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1500,5 +1500,7 @@ cleanup:
daemonConfigFree(config);
+ virStateCleanup(); + return ret; }
Unfortunately this causes libvirtd to segv, if you Ctrl-C the daemon shortly after startup. The problem is that virStateCleanup is running before virStateInitialize has finished its work. We probably need to put a mutex in the virStateInitialize+virStateCleanup functions to make sure they serialize Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Jan 17, 2013 at 05:28:28PM +0000, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:03AM +0800, Hu Tao wrote:
--- daemon/libvirtd.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9cdf4d9..7cb99b1 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1500,5 +1500,7 @@ cleanup:
daemonConfigFree(config);
+ virStateCleanup(); + return ret; }
Unfortunately this causes libvirtd to segv, if you Ctrl-C the daemon shortly after startup. The problem is that virStateCleanup is running before virStateInitialize has finished its work. We probably need to put a mutex in the virStateInitialize+virStateCleanup functions to make sure they serialize
Another problem about lockup is revealed by the method(Ctrl-C shortly after the daemon startup), as below: (gdb) bt #0 0x000000336500ddcd in __lll_lock_wait () from /lib64/libpthread.so.0 #1 0x0000003365009c56 in _L_lock_840 () from /lib64/libpthread.so.0 #2 0x0000003365009b58 in pthread_mutex_lock () from /lib64/libpthread.so.0 #3 0x00007f2d21c2dbbd in virMutexLock (m=<optimized out>) at util/virthreadpthread.c:85 #4 0x00007f2d153e776d in qemuDriverLock (driver=<optimized out>) at qemu/qemu_conf.c:65 #5 0x00007f2d154180ee in qemuShutdown () at qemu/qemu_driver.c:1098 #6 0x00007f2d21c9fdaf in virStateCleanup () at libvirt.c:846 #7 0x000000000040c129 in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1517 (gdb) thread 2 [Switching to thread 2 (Thread 0x7f2d14f43700 (LWP 20335))] #0 0x000000336500b595 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 (gdb) bt #0 0x000000336500b595 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007f2d21c2dcea in virCondWait (c=c@entry=0x7f2d10059668, m=m@entry=0x7f2d10059640) at util/virthreadpthread.c:117 #2 0x00007f2d153fd9cc in qemuMonitorSend (mon=mon@entry=0x7f2d10059630, msg=msg@entry=0x7f2d14f41f30) at qemu/qemu_monitor.c:889 #3 0x00007f2d1540e913 in qemuMonitorJSONCommandWithFd (mon=mon@entry=0x7f2d10059630, cmd=cmd@entry=0x7f2d10014360, scm_fd=scm_fd@entry=-1, reply=reply@entry=0x7f2d14f41fb0) at qemu/qemu_monitor_json.c:265 #4 0x00007f2d1540ea95 in qemuMonitorJSONCommand (mon=mon@entry=0x7f2d10059630, cmd=cmd@entry=0x7f2d10014360, reply=reply@entry=0x7f2d14f41fb0) at qemu/qemu_monitor_json.c:294 #5 0x00007f2d15410027 in qemuMonitorJSONSetCapabilities (mon=mon@entry=0x7f2d10059630) at qemu/qemu_monitor_json.c:991 #6 0x00007f2d153fea3d in qemuMonitorSetCapabilities (mon=mon@entry=0x7f2d10059630) at qemu/qemu_monitor.c:1153 #7 0x00007f2d153bef70 in qemuCapsInitQMP (runGid=0, runUid=0, runDir=<optimized out>, libDir=0x7f2d10066310 "/var/lib/libvirt/qemu", caps=0x7f2d100567f0) at qemu/qemu_capabilities.c:2381 #8 qemuCapsNewForBinary (binary=binary@entry=0x7f2d1005f300 "/usr/local/bin/qemu-system-x86_64", libDir=0x7f2d10066310 "/var/lib/libvirt/qemu", runDir=<optimized out>, runUid=0, runGid=0) at qemu/qemu_capabilities.c:2505 #9 0x00007f2d153c058e in qemuCapsCacheLookup (cache=cache@entry=0x7f2d10002420, binary=0x7f2d1005f300 "/usr/local/bin/qemu-system-x86_64") at qemu/qemu_capabilities.c:2597 #10 0x00007f2d153c08c1 in qemuCapsInitGuest (guestarch=VIR_ARCH_I686, hostarch=VIR_ARCH_X86_64, cache=0x7f2d10002420, caps=0x7f2d100653f0) at qemu/qemu_capabilities.c:685 #11 qemuCapsInit (cache=0x7f2d10002420) at qemu/qemu_capabilities.c:920 #12 0x00007f2d15418436 in qemuCreateCapabilities (driver=driver@entry=0x7f2d100269b0) at qemu/qemu_driver.c:424 #13 0x00007f2d15418d37 in qemuStartup (privileged=<optimized out>, callback=<optimized out>, opaque=<optimized out>) at qemu/qemu_driver.c:874 #14 0x00007f2d21c9fcc0 in virStateInitialize (privileged=true, callback=callback@entry=0x40d480 <daemonInhibitCallback>, opaque=opaque@entry=0x1ea2b40) at libvirt.c:822 #15 0x000000000040d5d5 in daemonRunStateInit (opaque=opaque@entry=0x1ea2b40) at libvirtd.c:877 #16 0x00007f2d21c2d986 in virThreadHelper (data=<optimized out>) at util/virthreadpthread.c:161 #17 0x0000003365007d14 in start_thread () from /lib64/libpthread.so.0 #18 0x0000003364cf167d in clone () from /lib64/libc.so.6 (gdb) -- Thanks, Hu Tao

required by VIR_ENUM_DECL. --- src/util/vircgroup.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 8b6d3b2..05f2e54 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -25,6 +25,8 @@ #ifndef __VIR_CGROUP_H__ # define __VIR_CGROUP_H__ +#include "virutil.h" + struct virCgroup; typedef struct virCgroup *virCgroupPtr; -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:04AM +0800, Hu Tao wrote:
required by VIR_ENUM_DECL. --- src/util/vircgroup.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 8b6d3b2..05f2e54 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -25,6 +25,8 @@ #ifndef __VIR_CGROUP_H__ # define __VIR_CGROUP_H__
+#include "virutil.h" + struct virCgroup; typedef struct virCgroup *virCgroupPtr;
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 01/16/2013 03:12 AM, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:04AM +0800, Hu Tao wrote:
required by VIR_ENUM_DECL. --- src/util/vircgroup.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 8b6d3b2..05f2e54 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -25,6 +25,8 @@ #ifndef __VIR_CGROUP_H__ # define __VIR_CGROUP_H__
+#include "virutil.h" + struct virCgroup; typedef struct virCgroup *virCgroupPtr;
ACK
I had to tweak this to pass 'make syntax-check' when cppi was installed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

required by virSetError. --- src/util/virthread.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virthread.h b/src/util/virthread.h index b11a251..c209440 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -23,6 +23,7 @@ # define __THREADS_H_ # include "internal.h" +# include "virerror.h" typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:05AM +0800, Hu Tao wrote:
required by virSetError. --- src/util/virthread.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h index b11a251..c209440 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -23,6 +23,7 @@ # define __THREADS_H_
# include "internal.h" +# include "virerror.h"
typedef struct virMutex virMutex; typedef virMutex *virMutexPtr;
AFAICT, the header file doesn't need this addition - the .c file needs it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 01/16/2013 03:13 AM, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:05AM +0800, Hu Tao wrote:
required by virSetError. --- src/util/virthread.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h index b11a251..c209440 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -23,6 +23,7 @@ # define __THREADS_H_
# include "internal.h" +# include "virerror.h"
typedef struct virMutex virMutex; typedef virMutex *virMutexPtr;
AFAICT, the header file doesn't need this addition - the .c file needs it.
I disagree. The definition of the VIR_ONCE_GLOBAL_INIT macro uses virErrorPtr, which means if we don't include virerror.h here, _every_ .c file that uses one-shot initialization would then have to pull in virerror.h. It turns out that all existing clients already do, or we would have hit a compilation error sooner; thus this patch is really only mandatory for the new client later in this series - but that commit has already been nack'd on design grounds. Still, requiring a client .c file to do extra includes just to use the macro makes it harder than necessary, so I'm still in favor of making virthread.h self-contained. ACK, after updating the commit message to explain why. I've pushed 1-3 in this series. Like Dan, I agree that the remainder of the series needs a step back to figure out what the real design goal is, before doing any more refactoring. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Jan 16, 2013 at 05:34:52PM -0700, Eric Blake wrote:
On 01/16/2013 03:13 AM, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:05AM +0800, Hu Tao wrote:
required by virSetError. --- src/util/virthread.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h index b11a251..c209440 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -23,6 +23,7 @@ # define __THREADS_H_
# include "internal.h" +# include "virerror.h"
typedef struct virMutex virMutex; typedef virMutex *virMutexPtr;
AFAICT, the header file doesn't need this addition - the .c file needs it.
I disagree. The definition of the VIR_ONCE_GLOBAL_INIT macro uses virErrorPtr, which means if we don't include virerror.h here, _every_ .c file that uses one-shot initialization would then have to pull in virerror.h. It turns out that all existing clients already do, or we would have hit a compilation error sooner; thus this patch is really only mandatory for the new client later in this series - but that commit has already been nack'd on design grounds. Still, requiring a client .c file to do extra includes just to use the macro makes it harder than necessary, so I'm still in favor of making virthread.h self-contained.
ACK, after updating the commit message to explain why. I've pushed 1-3 in this series. Like Dan, I agree that the remainder of the series needs a step back to figure out what the real design goal is, before doing any more refactoring.
Ah, I forgot about the macro expanding to an error call. Makes sense to include this change Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/util/vircgroup.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 48cba93..71d46c5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -113,7 +113,7 @@ bool virCgroupMounted(virCgroupPtr cgroup, int controller) * Process /proc/mounts figuring out what controllers are * mounted and where */ -static int virCgroupDetectMounts(virCgroupPtr group) +static int virCgroupDetectMounts(struct virCgroupController (*controllers)[VIR_CGROUP_CONTROLLER_LAST]) { int i; FILE *mounts = NULL; @@ -148,8 +148,8 @@ static int virCgroupDetectMounts(virCgroupPtr group) * first entry only */ if (typelen == len && STREQLEN(typestr, tmp, len) && - !group->controllers[i].mountPoint && - !(group->controllers[i].mountPoint = strdup(entry.mnt_dir))) + !(*controllers)[i].mountPoint && + !((*controllers)[i].mountPoint = strdup(entry.mnt_dir))) goto no_memory; tmp = next; } @@ -171,7 +171,7 @@ no_memory: * sub-path the current process is assigned to. ie not * necessarily in the root */ -static int virCgroupDetectPlacement(virCgroupPtr group) +static int virCgroupDetectPlacement(struct virCgroupController (*cgroupControllers)[VIR_CGROUP_CONTROLLER_LAST]) { int i; FILE *mapping = NULL; @@ -212,7 +212,7 @@ static int virCgroupDetectPlacement(virCgroupPtr group) len = strlen(tmp); } if (typelen == len && STREQLEN(typestr, tmp, len) && - !(group->controllers[i].placement = strdup(STREQ(path, "/") ? "" : path))) + !((*cgroupControllers)[i].placement = strdup(STREQ(path, "/") ? "" : path))) goto no_memory; tmp = next; @@ -236,7 +236,7 @@ static int virCgroupDetect(virCgroupPtr group) int rc; int i; - rc = virCgroupDetectMounts(group); + rc = virCgroupDetectMounts(&group->controllers); if (rc < 0) { VIR_ERROR(_("Failed to detect mounts for %s"), group->path); return rc; @@ -251,7 +251,7 @@ static int virCgroupDetect(virCgroupPtr group) return -ENXIO; - rc = virCgroupDetectPlacement(group); + rc = virCgroupDetectPlacement(&group->controllers); if (rc == 0) { /* Check that for every mounted controller, we found our placement */ -- 1.8.0.1.240.ge8a1f5a

This patch adds a new structure, virCgroupItem, to represent a cgroup directory(named cgroup item). cgroup directory is created when needed and removed if no one is using it. --- src/libvirt_private.syms | 7 + src/util/vircgroup.c | 411 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/vircgroup.h | 12 ++ 3 files changed, 423 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7be58ee..636c49d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -104,6 +104,12 @@ virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; +virCgroupInit; +virCgroupItemFree; +virCgroupItemKeyPath; +virCgroupItemNew; +virCgroupItemPath; +virCgroupItemType; virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; @@ -123,6 +129,7 @@ virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; virCgroupSetMemSwapHardLimit; +virCgroupUninit; # command.h diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 71d46c5..baa0af7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -37,6 +37,7 @@ #include <libgen.h> #include <dirent.h> +#include "virobject.h" #include "internal.h" #include "virutil.h" #include "viralloc.h" @@ -45,6 +46,7 @@ #include "virfile.h" #include "virhash.h" #include "virhashcode.h" +#include "virthread.h" #define CGROUP_MAX_VAL 512 @@ -58,6 +60,98 @@ struct virCgroupController { char *placement; }; +struct _virCgroupItem { + virObject object; + + char *name; + char *path; + + bool created; /* the path is created or not */ + + virCgroupItemPtr next; + virCgroupItemPtr parent; + virCgroupItemPtr children; + + struct virCgroupController *controller; +}; + +static virClassPtr cgroupItemClass; + +static void cgroupItemDispose(void *obj); + +static int virCgroupItemOnceInit(void) +{ + if (!(cgroupItemClass = virClassNew("cgroupItem", + sizeof(virCgroupItem), + cgroupItemDispose))) + return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(virCgroupItem); + +static struct virCgroupController cgroupControllers[VIR_CGROUP_CONTROLLER_LAST]; +static virCgroupItemPtr rootCgroupItems[VIR_CGROUP_CONTROLLER_LAST]; + +static virCgroupItemPtr virCgroupItemRootNew(int type); +static int virCgroupDetectMounts(struct virCgroupController (*controllers)[VIR_CGROUP_CONTROLLER_LAST]); +static int virCgroupDetectPlacement(struct virCgroupController (*controllers)[VIR_CGROUP_CONTROLLER_LAST]); + +int virCgroupInit(void) +{ + int rc; + int i; + + rc = virCgroupDetectMounts(&cgroupControllers); + if (rc < 0) { + VIR_ERROR(_("Failed to initialize cgroup controllers")); + return rc; + } + + rc = virCgroupDetectPlacement(&cgroupControllers); + + if (rc == 0) { + /* Check that for every mounted controller, we found our placement */ + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + if (!cgroupControllers[i].mountPoint) + continue; + + if (!cgroupControllers[i].placement) { + VIR_ERROR(_("Could not find placement for controller %s at %s"), + virCgroupControllerTypeToString(i), + cgroupControllers[i].placement); + rc = -ENOENT; + break; + } + + VIR_DEBUG("Detected mount/mapping %i:%s at %s in %s", i, + virCgroupControllerTypeToString(i), + cgroupControllers[i].mountPoint, + cgroupControllers[i].placement); + } + } else { + VIR_ERROR(_("Failed to initialize cgroup controllers")); + } + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + rootCgroupItems[i] = virCgroupItemRootNew(i); + } + + return rc; +} + +void virCgroupUninit(void) +{ + int i; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (rootCgroupItems[i]) { + virCgroupItemFree(rootCgroupItems[i]); + rootCgroupItems[i] = NULL; + } + } +} + struct virCgroup { char *path; @@ -74,6 +168,307 @@ typedef enum { * cpuacct and cpuset if possible. */ } virCgroupFlags; +static virCgroupItemPtr virCgroupItemRootNew(int type) +{ + virCgroupItemPtr rootItem = NULL; + + if (type >= VIR_CGROUP_CONTROLLER_LAST) + return NULL; + + if (!cgroupControllers[type].mountPoint) + return NULL; + + if (virCgroupItemInitialize() < 0) + return NULL; + + if (!(rootItem = virObjectNew(cgroupItemClass))) + return NULL; + + rootItem->name = strdup("/"); + rootItem->next = NULL; + rootItem->parent = NULL; + rootItem->children = NULL; + rootItem->controller = &cgroupControllers[type]; + rootItem->created = 1; + + if (virAsprintf(&rootItem->path, "%s%s%s", + rootItem->controller->mountPoint, + rootItem->controller->placement, + rootItem->name) == -1) { + virObjectUnref(rootItem); + rootItem = NULL; + } + + return rootItem; +} + +static virCgroupItemPtr virCgroupHasChildByName(virCgroupItemPtr item, + const char *name) +{ + virCgroupItemPtr child = item->children; + + while (child) { + if (STREQ(child->name, name)) { + virObjectRef(child); + return child; + } + + child = child->next; + } + + return NULL; +} + +virCgroupItemPtr virCgroupItemNew(int type, + const char *name, + virCgroupItemPtr parent) +{ + virCgroupItemPtr cgroupItem = NULL; + virCgroupItemPtr *next = NULL; + + if (type >= VIR_CGROUP_CONTROLLER_LAST) + return NULL; + + if (virCgroupItemInitialize() < 0) + return NULL; + + if (!cgroupControllers[type].mountPoint) + return NULL; + + if (!parent) + parent = rootCgroupItems[type]; + + if (!parent) + return NULL; + + cgroupItem = virCgroupHasChildByName(parent, name); + if (cgroupItem) + return cgroupItem; + + if (!(cgroupItem = virObjectNew(cgroupItemClass))) + return NULL; + + cgroupItem->name = strdup(name); + cgroupItem->parent = parent; + cgroupItem->next = NULL; + cgroupItem->children = NULL; + cgroupItem->controller = parent->controller; + cgroupItem->created = false; + + if (virAsprintf(&cgroupItem->path, "%s/%s", + parent->path, + cgroupItem->name) == -1) { + virObjectUnref(cgroupItem); + cgroupItem = NULL; + return NULL; + } + + virObjectRef(cgroupItem->parent); + + next = &parent->children; + + while (*next) + next = &(*next)->next; + + *next = cgroupItem; + + return cgroupItem; +} + +void virCgroupItemFree(virCgroupItemPtr cgroupItem) +{ + virObjectUnref(cgroupItem); +} + +static void cgroupItemDispose(void *obj) +{ + virCgroupItemPtr cgroupItem = obj; + virCgroupItemPtr parent = cgroupItem->parent; + virCgroupItemPtr *next; + + sa_assert(cgroupItem->children == NULL); + + if (cgroupItem->created && + access(cgroupItem->path, F_OK) == 0) + rmdir(cgroupItem->path); + + if (cgroupItem->parent) { + next = &parent->children; + + while (*next && *next != cgroupItem) + next = &(*next)->next; + + if (*next == cgroupItem) { + *next = cgroupItem->next; + cgroupItem->next = NULL; + cgroupItem->parent = NULL; + virObjectUnref(parent); + } + } + + VIR_FREE(cgroupItem->name); +} + +int virCgroupItemType(virCgroupItemPtr cgroupItem) +{ + return cgroupItem->controller->type; +} + +static int virCgroupItemGetValueStr(virCgroupItemPtr cgroupItem, + const char *key, + char **value) +{ + int rc; + char *keypath = NULL; + + *value = NULL; + + rc = virCgroupItemKeyPath(cgroupItem, key, &keypath); + if (rc != 0) + return rc; + + VIR_DEBUG("Get value %s", keypath); + + rc = virFileReadAll(keypath, 1024*1024, value); + if (rc < 0) { + rc = -errno; + VIR_DEBUG("Failed to read %s: %m\n", keypath); + } else { + /* Terminated with '\n' has sometimes harmful effects to the caller */ + if ((*value)[rc - 1] == '\n') + (*value)[rc - 1] = '\0'; + + rc = 0; + } + + VIR_FREE(keypath); + + return rc; +} + +static int virCgroupItemSetValueStr(virCgroupItemPtr cgroupItem, + const char *key, + const char *value) +{ + int rc = 0; + char *keypath = NULL; + + rc = virCgroupItemKeyPath(cgroupItem, key, &keypath); + if (rc != 0) + return rc; + + VIR_DEBUG("Set value '%s' to '%s'", keypath, value); + rc = virFileWriteStr(keypath, value, 0); + if (rc < 0) { + rc = -errno; + VIR_DEBUG("Failed to write value '%s': %m", value); + } else { + rc = 0; + } + + VIR_FREE(keypath); + return rc; +} + +static int virCgroupItemCpusetInherit(virCgroupItemPtr cgroupItem) +{ + int i; + char *value; + int rc = 0; + const char *inherit_values[] = { + "cpuset.cpus", + "cpuset.mems", + }; + + for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { + rc = virCgroupItemGetValueStr(cgroupItem->parent, + inherit_values[i], + &value); + if (rc != 0) { + VIR_ERROR(_("Failed to get %s %d"), inherit_values[i], rc); + break; + } + + VIR_DEBUG("Inherit %s = %s", inherit_values[i], value); + + rc = virCgroupItemSetValueStr(cgroupItem, + inherit_values[i], + value); + VIR_FREE(value); + + if (rc != 0) { + VIR_ERROR(_("Failed to set %s %d"), inherit_values[i], rc); + break; + } + } + + return rc; +} + +static int virCgroupRemoveRecursively(char *grppath); + +static int virCgroupItemMakePath(virCgroupItemPtr cgroupItem) +{ + int ret = 0; + + if (!cgroupItem->created && access(cgroupItem->path, F_OK) == 0) { + if (virCgroupRemoveRecursively(cgroupItem->path) != 0) { + VIR_ERROR(_("failed to remove historical cgroup directory: %s"), + cgroupItem->path); + return -errno; + } + } + + cgroupItem->created = true; + + if (access(cgroupItem->path, F_OK) != 0) { + if (cgroupItem->parent && + (ret = virCgroupItemMakePath(cgroupItem->parent)) < 0) + return ret; + + if (mkdir(cgroupItem->path, 0755) < 0) { + return -errno; + } + + virCgroupItemCpusetInherit(cgroupItem); + } + + return ret; +} + +int virCgroupItemPath(virCgroupItemPtr cgroupItem, + bool create, + char **path) +{ + if (virAsprintf(path, "%s", cgroupItem->path) == -1) + return -ENOMEM; + + if (create && virCgroupItemMakePath(cgroupItem) < 0) { + VIR_FREE(*path); + return -1; + } + + return 0; +} + +int virCgroupItemKeyPath(virCgroupItemPtr cgroupItem, + const char *key, + char **path) +{ + int ret = 0; + char *cgroupItemPath = NULL; + + ret = virCgroupItemPath(cgroupItem, 1, &cgroupItemPath); + if (ret < 0) + return ret; + + if (virAsprintf(path, "%s/%s", cgroupItemPath, key) == -1) + ret = -ENOMEM; + + VIR_FREE(cgroupItemPath); + return ret; +} + /** * virCgroupFree: * @@ -134,6 +529,7 @@ static int virCgroupDetectMounts(struct virCgroupController (*controllers)[VIR_C const char *typestr = virCgroupControllerTypeToString(i); int typelen = strlen(typestr); char *tmp = entry.mnt_opts; + (*controllers)[i].type = i; while (tmp) { char *next = strchr(tmp, ','); int len; @@ -171,7 +567,7 @@ no_memory: * sub-path the current process is assigned to. ie not * necessarily in the root */ -static int virCgroupDetectPlacement(struct virCgroupController (*cgroupControllers)[VIR_CGROUP_CONTROLLER_LAST]) +static int virCgroupDetectPlacement(struct virCgroupController (*controllers)[VIR_CGROUP_CONTROLLER_LAST]) { int i; FILE *mapping = NULL; @@ -184,24 +580,24 @@ static int virCgroupDetectPlacement(struct virCgroupController (*cgroupControlle } while (fgets(line, sizeof(line), mapping) != NULL) { - char *controllers = strchr(line, ':'); - char *path = controllers ? strchr(controllers+1, ':') : NULL; + char *controllersStr = strchr(line, ':'); + char *path = controllersStr ? strchr(controllersStr+1, ':') : NULL; char *nl = path ? strchr(path, '\n') : NULL; - if (!controllers || !path) + if (!controllersStr || !path) continue; if (nl) *nl = '\0'; *path = '\0'; - controllers++; + controllersStr++; path++; for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { const char *typestr = virCgroupControllerTypeToString(i); int typelen = strlen(typestr); - char *tmp = controllers; + char *tmp = controllersStr; while (tmp) { char *next = strchr(tmp, ','); int len; @@ -212,7 +608,7 @@ static int virCgroupDetectPlacement(struct virCgroupController (*cgroupControlle len = strlen(tmp); } if (typelen == len && STREQLEN(typestr, tmp, len) && - !((*cgroupControllers)[i].placement = strdup(STREQ(path, "/") ? "" : path))) + !((*controllers)[i].placement = strdup(STREQ(path, "/") ? "" : path))) goto no_memory; tmp = next; @@ -230,6 +626,7 @@ no_memory: } + static int virCgroupDetect(virCgroupPtr group) { int any = 0; diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 05f2e54..0ba2430 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -30,6 +30,9 @@ struct virCgroup; typedef struct virCgroup *virCgroupPtr; +typedef struct _virCgroupItem virCgroupItem; +typedef virCgroupItem *virCgroupItemPtr; + enum { VIR_CGROUP_CONTROLLER_CPU, VIR_CGROUP_CONTROLLER_CPUACCT, @@ -166,4 +169,13 @@ int virCgroupKill(virCgroupPtr group, int signum); int virCgroupKillRecursive(virCgroupPtr group, int signum); int virCgroupKillPainfully(virCgroupPtr group); +virCgroupItemPtr virCgroupItemNew(int type, const char *name, virCgroupItemPtr parent); +void virCgroupItemFree(virCgroupItemPtr cgroupItem); +int virCgroupItemType(virCgroupItemPtr cgroupItem); +int virCgroupItemPath(virCgroupItemPtr cgroupItem, bool create, char **path); +int virCgroupItemKeyPath(virCgroupItemPtr cgroupItem, const char *key, char **path); + +int virCgroupInit(void); +void virCgroupUninit(void); + #endif /* __VIR_CGROUP_H__ */ -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:07AM +0800, Hu Tao wrote:
This patch adds a new structure, virCgroupItem, to represent a cgroup directory(named cgroup item). cgroup directory is created when needed and removed if no one is using it. --- src/libvirt_private.syms | 7 + src/util/vircgroup.c | 411 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/vircgroup.h | 12 ++ 3 files changed, 423 insertions(+), 7 deletions(-)
NACK to this whole change. See my comments in patch 6/7 for why this API change to cgroups is a really bad design idea. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This patch deploys the newly introduced virCgroupItem and removes the old cgroup code. --- daemon/libvirtd.c | 4 + src/conf/domain_audit.c | 16 +- src/conf/domain_audit.h | 6 +- src/conf/domain_conf.h | 5 + src/libvirt_private.syms | 10 +- src/lxc/lxc_cgroup.c | 91 ++-- src/lxc/lxc_conf.h | 2 +- src/lxc/lxc_driver.c | 268 ++++------ src/lxc/lxc_process.c | 56 +-- src/qemu/qemu_cgroup.c | 287 ++++------- src/qemu/qemu_cgroup.h | 17 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 478 ++++++------------ src/qemu/qemu_hotplug.c | 44 +- src/qemu/qemu_migration.c | 36 +- src/qemu/qemu_process.c | 29 +- src/util/vircgroup.c | 1181 ++++++++++++--------------------------------- src/util/vircgroup.h | 94 ++-- 18 files changed, 813 insertions(+), 1813 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7cb99b1..92e3292 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1442,6 +1442,9 @@ int main(int argc, char **argv) { VIR_FORCE_CLOSE(statuswrite); } + if (virCgroupInit() < 0) + goto cleanup; + /* Initialize drivers & then start accepting new clients from network */ if (daemonStateInit(srv) < 0) { ret = VIR_DAEMON_ERR_INIT; @@ -1501,6 +1504,7 @@ cleanup: daemonConfigFree(config); virStateCleanup(); + virCgroupUninit(); return ret; } diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 7082804..6c7efe8 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -420,12 +420,12 @@ cleanup: * Log an audit message about an attempted cgroup device ACL change. */ void -virDomainAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, +virDomainAuditCgroup(virDomainObjPtr vm, virCgroupItemPtr cgroup, const char *reason, const char *extra, bool success) { char uuidstr[VIR_UUID_STRING_BUFLEN]; char *vmname; - char *controller = NULL; + char *path = NULL; char *detail; const char *virt; @@ -440,10 +440,8 @@ virDomainAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, virt = "?"; } - ignore_value(virCgroupPathOfController(cgroup, - VIR_CGROUP_CONTROLLER_DEVICES, - NULL, &controller)); - detail = virAuditEncode("cgroup", VIR_AUDIT_STR(controller)); + ignore_value(virCgroupItemPath(cgroup, false, &path)); + detail = virAuditEncode("cgroup", VIR_AUDIT_STR(path)); VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, "virt=%s resrc=cgroup reason=%s %s uuid=%s %s class=%s", @@ -451,7 +449,7 @@ virDomainAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, detail ? detail : "cgroup=?", extra); VIR_FREE(vmname); - VIR_FREE(controller); + VIR_FREE(path); VIR_FREE(detail); } @@ -468,7 +466,7 @@ virDomainAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, * Log an audit message about an attempted cgroup device ACL change. */ void -virDomainAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup, +virDomainAuditCgroupMajor(virDomainObjPtr vm, virCgroupItemPtr cgroup, const char *reason, int maj, const char *name, const char *perms, bool success) { @@ -498,7 +496,7 @@ virDomainAuditCgroupMajor(virDomainObjPtr vm, virCgroupPtr cgroup, * a specific device. */ void -virDomainAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup, +virDomainAuditCgroupPath(virDomainObjPtr vm, virCgroupItemPtr cgroup, const char *reason, const char *path, const char *perms, int rc) { diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 381fe37..573f592 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -66,14 +66,14 @@ void virDomainAuditHostdev(virDomainObjPtr vm, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); void virDomainAuditCgroup(virDomainObjPtr vm, - virCgroupPtr group, + virCgroupItemPtr group, const char *reason, const char *extra, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); void virDomainAuditCgroupMajor(virDomainObjPtr vm, - virCgroupPtr group, + virCgroupItemPtr group, const char *reason, int maj, const char *name, @@ -82,7 +82,7 @@ void virDomainAuditCgroupMajor(virDomainObjPtr vm, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); void virDomainAuditCgroupPath(virDomainObjPtr vm, - virCgroupPtr group, + virCgroupItemPtr group, const char *reason, const char *path, const char *perms, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2ac338c..23ff2c9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -48,6 +48,7 @@ # include "device_conf.h" # include "virbitmap.h" # include "virstoragefile.h" +# include "vircgroup.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -1843,6 +1844,10 @@ struct _virDomainDef { /* Application-specific custom metadata */ xmlNodePtr metadata; + + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr (*vcpuCgroups)[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr emulatorCgroup[VIR_CGROUP_CONTROLLER_LAST]; }; enum virDomainTaintFlags { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 636c49d..82621ab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -72,8 +72,6 @@ virCapabilitiesSetMacPrefix; # cgroup.h -virCgroupAddTask; -virCgroupAddTaskController; virCgroupAllowDevice; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; @@ -83,10 +81,6 @@ virCgroupDenyAllDevices; virCgroupDenyDevice; virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; -virCgroupForDomain; -virCgroupForDriver; -virCgroupForEmulator; -virCgroupForVcpu; virCgroupFree; virCgroupGetAppRoot; virCgroupGetBlkioWeight; @@ -105,6 +99,7 @@ virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; virCgroupInit; +virCgroupItemAddTask; virCgroupItemFree; virCgroupItemKeyPath; virCgroupItemNew; @@ -114,9 +109,8 @@ virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; virCgroupMounted; -virCgroupMoveTask; +virCgroupNew; virCgroupPathOfController; -virCgroupRemove; virCgroupSetBlkioDeviceWeight; virCgroupSetBlkioWeight; virCgroupSetCpuCfsPeriod; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 1984c5f..8363383 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -32,7 +32,7 @@ #define VIR_FROM_THIS VIR_FROM_LXC static int virLXCCgroupSetupCpuTune(virDomainDefPtr def, - virCgroupPtr cgroup) + virCgroupItemPtr cgroup) { int ret = -1; if (def->cputune.shares != 0) { @@ -69,7 +69,7 @@ cleanup: static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, - virCgroupPtr cgroup) + virCgroupItemPtr cgroup) { int ret = -1; @@ -90,7 +90,7 @@ cleanup: static int virLXCCgroupSetupMemTune(virDomainDefPtr def, - virCgroupPtr cgroup) + virCgroupItemPtr cgroup) { int ret = -1; int rc; @@ -139,21 +139,21 @@ cleanup: } -static int virLXCCgroupGetMemSwapUsage(virCgroupPtr cgroup, +static int virLXCCgroupGetMemSwapUsage(virCgroupItemPtr cgroup, virLXCMeminfoPtr meminfo) { return virCgroupGetMemSwapUsage(cgroup, &meminfo->swapusage); } -static int virLXCCgroupGetMemSwapTotal(virCgroupPtr cgroup, +static int virLXCCgroupGetMemSwapTotal(virCgroupItemPtr cgroup, virLXCMeminfoPtr meminfo) { return virCgroupGetMemSwapHardLimit(cgroup, &meminfo->swaptotal); } -static int virLXCCgroupGetMemUsage(virCgroupPtr cgroup, +static int virLXCCgroupGetMemUsage(virCgroupItemPtr cgroup, virLXCMeminfoPtr meminfo) { int ret; @@ -166,14 +166,14 @@ static int virLXCCgroupGetMemUsage(virCgroupPtr cgroup, } -static int virLXCCgroupGetMemTotal(virCgroupPtr cgroup, +static int virLXCCgroupGetMemTotal(virCgroupItemPtr cgroup, virLXCMeminfoPtr meminfo) { return virCgroupGetMemoryHardLimit(cgroup, &meminfo->memtotal); } -static int virLXCCgroupGetMemStat(virCgroupPtr cgroup, +static int virLXCCgroupGetMemStat(virCgroupItemPtr cgroup, virLXCMeminfoPtr meminfo) { int ret = 0; @@ -182,8 +182,7 @@ static int virLXCCgroupGetMemStat(virCgroupPtr cgroup, char *line = NULL; size_t n; - ret = virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_MEMORY, - "memory.stat", &statFile); + ret = virCgroupItemKeyPath(cgroup, "memory.stat", &statFile); if (ret != 0) { virReportSystemError(-ret, "%s", _("cannot get the path of MEMORY cgroup controller")); @@ -240,13 +239,13 @@ cleanup: int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) { int ret; - virCgroupPtr cgroup; + virCgroupItemPtr cgroup; - ret = virCgroupGetAppRoot(&cgroup); - if (ret < 0) { - virReportSystemError(-ret, "%s", + cgroup = virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_MEMORY, false); + if (!cgroup) { + virReportSystemError(-1, "%s", _("Unable to get cgroup for container")); - return ret; + return -1; } ret = virLXCCgroupGetMemStat(cgroup, meminfo); @@ -275,7 +274,6 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) ret = 0; cleanup: - virCgroupFree(&cgroup); return ret; } @@ -296,7 +294,7 @@ virLXCSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, const char *path, void *opaque) { - virCgroupPtr cgroup = opaque; + virCgroupItemPtr cgroup = opaque; int rc; VIR_DEBUG("Process path '%s' for USB device", path); @@ -318,7 +316,7 @@ virLXCTeardownHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, const char *path, void *opaque) { - virCgroupPtr cgroup = opaque; + virCgroupItemPtr cgroup = opaque; int rc; VIR_DEBUG("Process path '%s' for USB device", path); @@ -336,7 +334,7 @@ virLXCTeardownHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, - virCgroupPtr cgroup) + virCgroupItemPtr cgroup) { int ret = -1; int rc; @@ -471,51 +469,52 @@ cleanup: int virLXCCgroupSetup(virDomainDefPtr def) { - virCgroupPtr driver = NULL; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr driver[VIR_CGROUP_CONTROLLER_LAST]; int ret = -1; int rc; + int i; - rc = virCgroupForDriver("lxc", &driver, 1, 0); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("Unable to get cgroup for driver")); - goto cleanup; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + driver[i] = virCgroupItemNew(i, "lxc", virCgroupGetAppRoot(i, true)); + def->cgroup[i] = virCgroupItemNew(i, def->name, driver[i]); } - rc = virCgroupForDomain(driver, def->name, &cgroup, 1); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for domain %s"), - def->name); + if (virLXCCgroupSetupCpuTune(def, + def->cgroup[VIR_CGROUP_CONTROLLER_CPU]) < 0) goto cleanup; - } - if (virLXCCgroupSetupCpuTune(def, cgroup) < 0) + if (virLXCCgroupSetupBlkioTune(def, + def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]) < 0) goto cleanup; - if (virLXCCgroupSetupBlkioTune(def, cgroup) < 0) + if (virLXCCgroupSetupMemTune(def, + def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]) < 0) goto cleanup; - if (virLXCCgroupSetupMemTune(def, cgroup) < 0) + if (virLXCCgroupSetupDeviceACL(def, + def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]) < 0) goto cleanup; - if (virLXCCgroupSetupDeviceACL(def, cgroup) < 0) - goto cleanup; - - rc = virCgroupAddTask(cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to add task %d to cgroup for domain %s"), - getpid(), def->name); - goto cleanup; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + rc = virCgroupItemAddTask(def->cgroup[i], getpid()); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to add task %d to cgroup for domain %s"), + getpid(), def->name); + goto cleanup; + } } ret = 0; cleanup: - virCgroupFree(&cgroup); - virCgroupFree(&driver); + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (ret != 0) { + virCgroupItemFree(def->cgroup[i]); + def->cgroup[i] = NULL; + } + virCgroupItemFree(driver[i]); + } return ret; } diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index d45e0a0..03d9ba3 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -52,7 +52,7 @@ struct _virLXCDriver { virCapsPtr caps; - virCgroupPtr cgroup; + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; size_t nactive; virStateInhibitCallback inhibitCallback; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8050ce6..d6d5a94 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -526,7 +526,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = NULL; int ret = -1, rc; lxcDriverLock(driver); @@ -546,7 +546,8 @@ static int lxcDomainGetInfo(virDomainPtr dom, info->cpuTime = 0; info->memory = vm->def->mem.cur_balloon; } else { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUACCT]; + if (!cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get cgroup for %s"), vm->def->name); goto cleanup; @@ -557,6 +558,14 @@ static int lxcDomainGetInfo(virDomainPtr dom, "%s", _("Cannot read cputime for domain")); goto cleanup; } + + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!cgroup) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to get cgroup for %s"), vm->def->name); + goto cleanup; + } + if ((rc = virCgroupGetMemoryUsage(cgroup, &(info->memory))) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Cannot read memory usage for domain")); @@ -575,8 +584,6 @@ static int lxcDomainGetInfo(virDomainPtr dom, cleanup: lxcDriverUnlock(driver); - if (cgroup) - virCgroupFree(&cgroup); if (vm) virDomainObjUnlock(vm); return ret; @@ -707,7 +714,7 @@ cleanup: static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = NULL; int ret = -1; lxcDriverLock(driver); @@ -733,13 +740,8 @@ 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) { + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get cgroup for %s"), vm->def->name); goto cleanup; @@ -756,8 +758,6 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { cleanup: if (vm) virDomainObjUnlock(vm); - if (cgroup) - virCgroupFree(&cgroup); return ret; } @@ -769,7 +769,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = NULL; virDomainObjPtr vm = NULL; int ret = -1; int rc; @@ -796,7 +796,8 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -831,8 +832,6 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, } cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -847,7 +846,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = NULL; virDomainObjPtr vm = NULL; unsigned long long val; int ret = -1; @@ -873,7 +872,8 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to get cgroup for %s"), vm->def->name); goto cleanup; @@ -930,8 +930,6 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, ret = 0; cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -1407,7 +1405,7 @@ static int lxcStartup(bool privileged, void *opaque ATTRIBUTE_UNUSED) { char *ld; - int rc; + int i; /* Valgrind gets very annoyed when we clone containers, so * disable LXC when under valgrind @@ -1450,14 +1448,9 @@ 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); - 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 - */ + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + lxc_driver->cgroup[i] = virCgroupItemNew(i, "lxc", + virCgroupGetAppRoot(i, privileged)); } /* Call function to load lxc driver configuration information */ @@ -1592,7 +1585,7 @@ static int lxcVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *versio * Return 1 when CFS bandwidth is supported, 0 when CFS bandwidth is not * supported, -1 on error. */ -static int lxcGetCpuBWStatus(virCgroupPtr cgroup) +static int lxcGetCpuBWStatus(virCgroupItemPtr cgroup) { char *cfs_period_path = NULL; int ret = -1; @@ -1600,8 +1593,8 @@ static int lxcGetCpuBWStatus(virCgroupPtr cgroup) if (!cgroup) return 0; - if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", &cfs_period_path) < 0) { + if (virCgroupItemKeyPath(cgroup, "cpu.cfs_period_us", + &cfs_period_path) < 0) { VIR_INFO("cannot get the path of cgroup CPU controller"); ret = 0; goto cleanup; @@ -1626,7 +1619,7 @@ static bool lxcCgroupControllerActive(virLXCDriverPtr driver, return false; if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) return false; - if (!virCgroupMounted(driver->cgroup, controller)) + if (!virCgroupMounted(controller)) return false; #if 0 if (driver->cgroupControllers & (1 << controller)) @@ -1652,7 +1645,7 @@ static char *lxcGetSchedulerType(virDomainPtr domain, } if (nparams) { - rc = lxcGetCpuBWStatus(driver->cgroup); + rc = lxcGetCpuBWStatus(driver->cgroup[VIR_CGROUP_CONTROLLER_CPU]); if (rc < 0) goto cleanup; else if (rc == 0) @@ -1672,7 +1665,7 @@ cleanup: static int -lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, +lxcGetVcpuBWLive(virCgroupItemPtr cgroup, unsigned long long *period, long long *quota) { int rc; @@ -1695,7 +1688,7 @@ lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, } -static int lxcSetVcpuBWLive(virCgroupPtr cgroup, unsigned long long period, +static int lxcSetVcpuBWLive(virCgroupItemPtr cgroup, unsigned long long period, long long quota) { int rc; @@ -1752,7 +1745,7 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; int ret = -1; @@ -1791,18 +1784,12 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; - } } for (i = 0; i < nparams; i++) { @@ -1869,7 +1856,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, cleanup: virDomainDefFree(vmdef); - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -1891,7 +1877,7 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef; unsigned long long shares = 0; @@ -1908,7 +1894,7 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, lxcDriverLock(driver); if (*nparams > 1) { - rc = lxcGetCpuBWStatus(driver->cgroup); + rc = lxcGetCpuBWStatus(driver->cgroup[VIR_CGROUP_CONTROLLER_CPU]); if (rc < 0) goto cleanup; cpu_bw_status = !!rc; @@ -1935,13 +1921,8 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, goto out; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -1988,7 +1969,6 @@ out: ret = 0; cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -2012,7 +1992,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; @@ -2039,14 +2019,9 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, 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) { + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -2097,7 +2072,6 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -2114,7 +2088,7 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, { virLXCDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; @@ -2144,14 +2118,10 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("blkio cgroup isn't mounted")); - goto cleanup; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -2203,8 +2173,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); @@ -2374,7 +2342,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 */ @@ -2382,10 +2350,9 @@ static int lxcFreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) int waited_time = 0; int ret = -1; char *state = NULL; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_FREEZER]; - if (!(driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) + if (!cgroup) return -1; /* From here on, we know that cgroup != NULL. */ @@ -2462,7 +2429,6 @@ error: ret = -1; cleanup: - virCgroupFree(&cgroup); VIR_FREE(state); return ret; } @@ -2492,7 +2458,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; @@ -2517,18 +2483,16 @@ cleanup: return ret; } -static int lxcUnfreezeContainer(virLXCDriverPtr driver, virDomainObjPtr vm) +static int lxcUnfreezeContainer(virDomainObjPtr vm) { int ret; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_FREEZER]; - if (!(driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) + if (!cgroup) return -1; ret = virCgroupSetFreezerState(cgroup, "THAWED"); - virCgroupFree(&cgroup); return ret; } @@ -2557,7 +2521,7 @@ static int lxcDomainResume(virDomainPtr dom) } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (lxcUnfreezeContainer(driver, vm) < 0) { + if (lxcUnfreezeContainer(vm) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Resume operation failed")); goto cleanup; @@ -3100,7 +3064,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainDiskDefPtr def = dev->data.disk; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; int ret = -1; char *dst; struct stat sb; @@ -3185,17 +3149,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, vm->def, def) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, 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; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; if (virCgroupAllowDevicePath(group, def->src, (def->readonly ? @@ -3215,8 +3169,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; @@ -3370,7 +3322,7 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, mode_t mode; bool created = false; usbDevice *usb = NULL; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; if (virDomainHostdevFind(vm->def, def, NULL) >= 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -3405,13 +3357,8 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -3469,7 +3416,6 @@ cleanup: unlink(dstfile); usbFreeDevice(usb); - virCgroupFree(&group); VIR_FREE(src); VIR_FREE(dstfile); VIR_FREE(dstdir); @@ -3485,7 +3431,7 @@ lxcDomainAttachDeviceHostdevStorageLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = dev->data.hostdev; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; int ret = -1; char *dst = NULL; char *vroot = NULL; @@ -3554,13 +3500,9 @@ lxcDomainAttachDeviceHostdevStorageLive(virLXCDriverPtr driver, vm->def, def, vroot) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("devices cgroup isn't mounted")); - goto cleanup; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -3581,8 +3523,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); @@ -3598,7 +3538,7 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = dev->data.hostdev; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; int ret = -1; char *dst = NULL; char *vroot = NULL; @@ -3667,13 +3607,8 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver, vm->def, def, vroot) < 0) goto cleanup; - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -3694,8 +3629,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); @@ -3812,13 +3745,12 @@ 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; + virCgroupItemPtr group = NULL; int i, ret = -1; char *dst; @@ -3844,13 +3776,8 @@ lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -3876,8 +3803,6 @@ lxcDomainDetachDeviceDiskLive(virLXCDriverPtr driver, cleanup: VIR_FREE(dst); - if (group) - virCgroupFree(&group); return ret; } @@ -3955,7 +3880,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, { virLXCDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevDefPtr def = NULL; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; int idx, ret = -1; char *dst; char *vroot; @@ -3983,13 +3908,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -4024,19 +3944,17 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, cleanup: usbFreeDevice(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; + virCgroupItemPtr group = NULL; int i, ret = -1; char *dst = NULL; @@ -4062,13 +3980,8 @@ lxcDomainDetachDeviceHostdevStorageLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -4094,20 +4007,17 @@ 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; + virCgroupItemPtr group = NULL; int i, ret = -1; char *dst = NULL; @@ -4133,13 +4043,8 @@ lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, goto cleanup; } - if (!lxcCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -4165,8 +4070,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virLXCDriverPtr driver, cleanup: VIR_FREE(dst); - if (group) - virCgroupFree(&group); return ret; } @@ -4190,16 +4093,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, @@ -4228,7 +4130,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, @@ -4248,7 +4150,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 1a89e4a..30c8a89 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -218,7 +218,6 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason) { - virCgroupPtr cgroup; int i; virLXCDomainObjPrivatePtr priv = vm->privateData; virNetDevVPortProfilePtr vport = NULL; @@ -274,11 +273,8 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virDomainConfVMNWFilterTeardown(vm); - if (driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) + virCgroupItemFree(vm->def->cgroup[i]); /* now that we know it's stopped call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_LXC)) { @@ -683,8 +679,8 @@ int virLXCProcessStop(virLXCDriverPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason) { - virCgroupPtr group = NULL; int rc; + int i; VIR_DEBUG("Stopping VM name=%s pid=%d reason=%d", vm->def->name, (int)vm->pid, (int)reason); @@ -705,24 +701,26 @@ 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 (rc < 0) { - virReportSystemError(-rc, "%s", - _("Failed to kill container PIDs")); - rc = -1; - goto cleanup; - } - if (rc == 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Some container PIDs refused to die")); - rc = -1; - goto cleanup; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (vm->def->cgroup[i]) { + rc = virCgroupKillPainfully(vm->def->cgroup[i]); + if (rc < 0) { + virReportSystemError(-rc, "%s", + _("Failed to kill container PIDs")); + rc = -1; + goto cleanup; + } + if (rc == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Some container PIDs refused to die")); + rc = -1; + goto cleanup; + } + } else { + /* If cgroup doesn't exist, the VM pids must have already + * died and so we're just cleaning up stale state + */ } - } else { - /* If cgroup doesn't exist, the VM pids must have already - * died and so we're just cleaning up stale state - */ } virLXCProcessCleanup(driver, vm, reason); @@ -730,7 +728,6 @@ int virLXCProcessStop(virLXCDriverPtr driver, rc = 0; cleanup: - virCgroupFree(&group); return rc; } @@ -924,20 +921,17 @@ int virLXCProcessStart(virConnectPtr conn, return -1; } - if (!virCgroupMounted(lxc_driver->cgroup, - VIR_CGROUP_CONTROLLER_CPUACCT)) { + if (!virCgroupMounted(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 (!virCgroupMounted(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 (!virCgroupMounted(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 6527146..c409323 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -48,11 +48,9 @@ static const char *const defaultDeviceACL[] = { bool qemuCgroupControllerActive(virQEMUDriverPtr driver, int controller) { - if (driver->cgroup == NULL) - return false; if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) return false; - if (!virCgroupMounted(driver->cgroup, controller)) + if (driver->cgroup[controller] == NULL) return false; if (driver->cgroupControllers & (1 << controller)) return true; @@ -89,7 +87,7 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, int qemuSetupDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, + virCgroupItemPtr cgroup, virDomainDiskDefPtr disk) { qemuCgroupData data = { vm, cgroup }; @@ -128,7 +126,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, int qemuTeardownDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, + virCgroupItemPtr cgroup, virDomainDiskDefPtr disk) { qemuCgroupData data = { vm, cgroup }; @@ -192,7 +190,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask) { - virCgroupPtr cgroup = NULL; + char *path; + virCgroupItemPtr cgroup = NULL; int rc; unsigned int i; const char *const *deviceACL = @@ -200,17 +199,19 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, (const char *const *)driver->cgroupDeviceACL : defaultDeviceACL; - if (driver->cgroup == NULL) - return 0; /* Not supported, so claim success */ - - 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; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + vm->def->cgroup[i] = virCgroupItemNew(i, vm->def->name, driver->cgroup[i]); + /* XXX: This is a dirty hack to create the cgroup item path before + * any qemu processes bring up. The reason to do so is virCgroup creates + * cgroup item path as needed, and maintains the information. But if the + * path is created by a sub-process(see qemuProcessHook), we can't know it + * and virCgroup will behave incorrectly. + */ + virCgroupItemPath(vm->def->cgroup[i], true, &path); } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); @@ -300,6 +301,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; + if (vm->def->blkio.weight != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { rc = virCgroupSetBlkioWeight(cgroup, vm->def->blkio.weight); @@ -339,6 +342,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { unsigned long long hard_limit = vm->def->mem.hard_limit; @@ -392,6 +397,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name); } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if (vm->def->cputune.shares != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { rc = virCgroupSetCpuShares(cgroup, vm->def->cputune.shares); @@ -407,6 +414,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET]; + if ((vm->def->numatune.memory.nodemask || (vm->def->numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)) && @@ -434,18 +443,13 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } } done: - virCgroupFree(&cgroup); return 0; cleanup: - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } return -1; } -int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, +int qemuSetupCgroupVcpuBW(virCgroupItemPtr cgroup, unsigned long long period, long long quota) { int rc; @@ -493,7 +497,7 @@ cleanup: return -1; } -int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, +int qemuSetupCgroupVcpuPin(virCgroupItemPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, int nvcpupin, int vcpuid) @@ -509,7 +513,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, return -1; } -int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, +int qemuSetupCgroupEmulatorPin(virCgroupItemPtr cgroup, virBitmapPtr cpumask) { int rc = 0; @@ -538,68 +542,48 @@ cleanup: int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) { - virCgroupPtr cgroup = NULL; - virCgroupPtr cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; int rc; unsigned int i, j; unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; - - if ((period || quota) && - (!driver->cgroup || - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cgroup cpu is required for scheduler tuning")); - return -1; - } - - /* We are trying to setup cgroups for CPU pinning, which can also be done - * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal - * here. - */ - if (driver->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; - } + char *vcpuName = NULL; 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; } + if (VIR_ALLOC_N(vm->def->vcpuCgroups, priv->nvcpupids) < 0) { + virReportOOMError(); + goto cleanup; + } + for (i = 0; i < priv->nvcpupids; i++) { - rc = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 1); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to create vcpu cgroup for %s(vcpu:" - " %d)"), - vm->def->name, i); + if (virAsprintf(&vcpuName, "vcpu%d", i) < 0) goto cleanup; - } - /* move the thread for vcpu to sub dir */ - rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); - if (rc < 0) { - virReportSystemError(-rc, - _("unable to add vcpu %d task %d to cgroup"), - i, priv->vcpupids[i]); - goto cleanup; - } + for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) + vm->def->vcpuCgroups[i][j] = virCgroupItemNew(j, + vcpuName, + vm->def->cgroup[j]); + + if ((period || quota) && vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPU]) { + rc = virCgroupItemAddTask(vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPU], + priv->vcpupids[i]); + if (rc < 0) { + virReportSystemError(-rc, + _("unable to add vcpu %d task %d to cgroup"), + i, priv->vcpupids[i]); + goto cleanup; + } - if (period || quota) { - if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) + if (qemuSetupCgroupVcpuBW(vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPU], + period, quota) < 0) goto cleanup; } @@ -611,7 +595,16 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) if (def->cputune.vcpupin[j]->vcpuid != i) continue; - if (qemuSetupCgroupVcpuPin(cgroup_vcpu, + rc = virCgroupItemAddTask(vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPUSET], + priv->vcpupids[i]); + if (rc < 0) { + virReportSystemError(-rc, + _("unable to add vcpu %d task %d to cgroup"), + i, priv->vcpupids[i]); + goto cleanup; + } + + if (qemuSetupCgroupVcpuPin(vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPUSET], def->cputune.vcpupin, def->cputune.nvcpupin, i) < 0) @@ -620,24 +613,17 @@ int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm) break; } } - - virCgroupFree(&cgroup_vcpu); } - virCgroupFree(&cgroup); return 0; cleanup: - if (cgroup_vcpu) { - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } - - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); + if (vm->def->vcpuCgroups) { + for (i = 0; i < priv->nvcpupids; i++) { + for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) + virCgroupItemFree(vm->def->vcpuCgroups[i][j]); + } } - return -1; } @@ -647,52 +633,11 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, { virBitmapPtr cpumask = NULL; virBitmapPtr cpumap = NULL; - virCgroupPtr cgroup = NULL; - virCgroupPtr cgroup_emulator = NULL; virDomainDefPtr def = vm->def; unsigned long long period = vm->def->cputune.emulator_period; long long quota = vm->def->cputune.emulator_quota; - int rc, i; - - if ((period || quota) && - (!driver->cgroup || - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cgroup cpu is required for scheduler tuning")); - return -1; - } - - if (driver->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); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to create emulator cgroup for %s"), - vm->def->name); - goto cleanup; - } - - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - 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; - } - } + int rc = -1; + int i; if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { if (!(cpumap = qemuPrepareCpumap(driver, nodemask))) @@ -704,97 +649,29 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, cpumask = def->cpumask; } - if (cpumask) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { - rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask); - if (rc < 0) - goto cleanup; - } + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + def->emulatorCgroup[i] = virCgroupItemNew(i, "emulator", def->cgroup[i]); + } + + if (cpumask && def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPUSET]) { + rc = qemuSetupCgroupEmulatorPin(def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPUSET], + cpumask); + if (rc < 0) + goto cleanup; cpumask = NULL; /* sanity */ } - if (period || quota) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - if ((rc = qemuSetupCgroupVcpuBW(cgroup_emulator, period, - quota)) < 0) - goto cleanup; - } + if ((period || quota) && def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPU]) { + if ((rc = qemuSetupCgroupVcpuBW(def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPU], + period, quota)) < 0) + goto cleanup; } - virCgroupFree(&cgroup_emulator); - virCgroupFree(&cgroup); - virBitmapFree(cpumap); return 0; cleanup: - virBitmapFree(cpumap); - - if (cgroup_emulator) { - virCgroupRemove(cgroup_emulator); - virCgroupFree(&cgroup_emulator); - } - - if (cgroup) { - virCgroupRemove(cgroup); - virCgroupFree(&cgroup); - } - - return rc; -} - -int qemuRemoveCgroup(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int quiet) -{ - virCgroupPtr cgroup; - int rc; - - if (driver->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; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + virCgroupItemFree(def->emulatorCgroup[i]); } - - rc = virCgroupRemove(cgroup); - virCgroupFree(&cgroup); return rc; } - -int qemuAddToCgroup(virQEMUDriverPtr driver, - virDomainDefPtr def) -{ - virCgroupPtr cgroup = NULL; - int ret = -1; - int rc; - - if (driver->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()); - if (rc != 0) { - virReportSystemError(-rc, - _("unable to add domain %s task %d to cgroup"), - def->name, getpid()); - goto cleanup; - } - - ret = 0; - -cleanup: - virCgroupFree(&cgroup); - return ret; -} diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 75ef514..6444c2a 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -30,17 +30,17 @@ struct _qemuCgroupData { virDomainObjPtr vm; - virCgroupPtr cgroup; + virCgroupItemPtr cgroup; }; typedef struct _qemuCgroupData qemuCgroupData; bool qemuCgroupControllerActive(virQEMUDriverPtr driver, int controller); int qemuSetupDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, + virCgroupItemPtr cgroup, virDomainDiskDefPtr disk); int qemuTeardownDiskCgroup(virDomainObjPtr vm, - virCgroupPtr cgroup, + virCgroupItemPtr cgroup, virDomainDiskDefPtr disk); int qemuSetupHostUsbDeviceCgroup(usbDevice *dev, const char *path, @@ -48,22 +48,17 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev, int qemuSetupCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask); -int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, +int qemuSetupCgroupVcpuBW(virCgroupItemPtr cgroup, unsigned long long period, long long quota); -int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, +int qemuSetupCgroupVcpuPin(virCgroupItemPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, int nvcpupin, int vcpuid); -int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr cpumask); +int qemuSetupCgroupEmulatorPin(virCgroupItemPtr cgroup, virBitmapPtr cpumask); int qemuSetupCgroupForVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm); int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask); -int qemuRemoveCgroup(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int quiet); -int qemuAddToCgroup(virQEMUDriverPtr driver, - virDomainDefPtr def); #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 965eff7..9162914 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -69,7 +69,7 @@ struct _virQEMUDriver { unsigned int qemuVersion; int nextvmid; - virCgroupPtr cgroup; + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; int cgroupControllers; char **cgroupDeviceACL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39175f4..d795abe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -650,11 +650,11 @@ qemuStartup(bool privileged, { char *base; char *driverConf = NULL; - int rc; virConnectPtr conn = NULL; char ebuf[1024]; char *membase = NULL; char *mempath = NULL; + int i; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -795,17 +795,16 @@ qemuStartup(bool privileged, virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) < 0) goto out_of_memory; - rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, 1); - if (rc < 0) { - VIR_INFO("Unable to create cgroup for driver: %s", - virStrerror(-rc, ebuf, sizeof(ebuf))); - } - if (qemuLoadDriverConfig(qemu_driver, driverConf) < 0) { goto error; } VIR_FREE(driverConf); + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + qemu_driver->cgroup[i] = virCgroupItemNew(i, "qemu", + virCgroupGetAppRoot(i, privileged)); + } + /* Allocate bitmap for remote display port reservations. We cannot * do this before the config is loaded properly, since the port * numbers are configurable now */ @@ -1146,10 +1145,14 @@ qemuShutdown(void) { /* Free domain callback list */ virDomainEventStateFree(qemu_driver->domainEventState); - virCgroupFree(&qemu_driver->cgroup); + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) + virCgroupItemFree(qemu_driver->cgroup[i]); virLockManagerPluginUnref(qemu_driver->lockManager); + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) + virCgroupItemFree(qemu_driver->cgroup[i]); + qemuDriverUnlock(qemu_driver); virMutexDestroy(&qemu_driver->lock); virThreadPoolFree(qemu_driver->workerPool); @@ -3652,7 +3655,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, int ncpupids; virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_vcpu = NULL; - bool cgroup_available = false; + //bool cgroup_available = false; qemuDomainObjEnterMonitor(driver, vm); @@ -3697,6 +3700,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, goto cleanup; } +#if 0 /* XXX */ if (ncpupids != vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("got wrong number of vCPU pids from QEMU monitor. " @@ -3808,6 +3812,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } } } +#endif priv->nvcpupids = ncpupids; VIR_FREE(priv->vcpupids); @@ -4014,15 +4019,13 @@ 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 && - qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup" - " for vcpu %d"), vcpu); - goto cleanup; - } + if (vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET] && + qemuSetupCgroupVcpuPin(vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET], + newVcpuPin, newVcpuPinNum, vcpu) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup" + " for vcpu %d"), vcpu); + goto cleanup; } else { if (virProcessSetAffinity(priv->vcpupids[vcpu], pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, @@ -4255,23 +4258,13 @@ qemuDomainPinEmulator(virDomainPtr dom, goto cleanup; } - if (qemuCgroupControllerActive(driver, - 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 (vm->def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPUSET]) { + if (qemuSetupCgroupEmulatorPin(vm->def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPUSET], + newVcpuPin[0]->cpumask) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); + goto cleanup; } } else { if (virProcessSetAffinity(pid, pcpumap) < 0) { @@ -5804,7 +5797,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk = dev->data.disk; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; int ret = -1; if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) { @@ -5822,16 +5815,10 @@ 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 (cgroup) if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) goto end; - } + switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: @@ -5882,8 +5869,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, } end: - if (cgroup) - virCgroupFree(&cgroup); return ret; } @@ -6067,23 +6052,15 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, bool force) { virDomainDiskDefPtr disk = dev->data.disk; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; int ret = -1; 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 (cgroup) if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) goto end; - } switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: @@ -6105,8 +6082,6 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, NULLSTR(disk->src)); } end: - if (cgroup) - virCgroupFree(&cgroup); return ret; } @@ -6693,39 +6668,6 @@ cleanup: } -/* - * check whether the host supports CFS bandwidth - * - * Return 1 when CFS bandwidth is supported, 0 when CFS bandwidth is not - * supported, -1 on error. - */ -static int qemuGetCpuBWStatus(virCgroupPtr cgroup) -{ - char *cfs_period_path = NULL; - int ret = -1; - - if (!cgroup) - return 0; - - if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", &cfs_period_path) < 0) { - VIR_INFO("cannot get the path of cgroup CPU controller"); - ret = 0; - goto cleanup; - } - - if (access(cfs_period_path, F_OK) < 0) { - ret = 0; - } else { - ret = 1; - } - -cleanup: - VIR_FREE(cfs_period_path); - return ret; -} - - static char *qemuGetSchedulerType(virDomainPtr dom, int *nparams) { @@ -6741,10 +6683,10 @@ static char *qemuGetSchedulerType(virDomainPtr dom, } if (nparams) { - rc = qemuGetCpuBWStatus(driver->cgroup); + rc = virCgroupItemKeyPath(driver->cgroup[VIR_CGROUP_CONTROLLER_CPU], + "cpu.cfs_period_us", + NULL); if (rc < 0) - goto cleanup; - else if (rc == 0) *nparams = 1; else *nparams = 5; @@ -6895,7 +6837,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; @@ -6923,23 +6865,14 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; + ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, 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); + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; + if (!group) { + ret = -1; goto cleanup; } - } - ret = 0; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { int rc; virTypedParameterPtr param = ¶ms[i]; @@ -7033,7 +6966,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, } cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7048,7 +6980,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i, j; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; @@ -7084,21 +7016,12 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("blkio cgroup isn't mounted")); - goto cleanup; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!group) goto cleanup; - } - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = ¶ms[i]; val = 0; @@ -7218,8 +7141,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7235,7 +7156,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; int i; virDomainDefPtr persistentDef = NULL; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virTypedParameterPtr hard_limit = NULL; virTypedParameterPtr swap_hard_limit = NULL; @@ -7273,19 +7194,9 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, 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; - } - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!group) + goto cleanup; for (i = 0; i < nparams; i++) { if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { @@ -7388,7 +7299,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, } cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7403,7 +7313,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; @@ -7430,19 +7340,9 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, 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; - } - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!group) + goto cleanup; if ((*nparams) == 0) { /* Current number of memory parameters supported by cgroups */ @@ -7547,8 +7447,6 @@ out: ret = 0; cleanup: - if (group) - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7564,7 +7462,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; int i; virDomainDefPtr persistentDef = NULL; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; @@ -7593,18 +7491,9 @@ qemuDomainSetNumaParameters(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup cpuset controller is not mounted")); + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET]; + if (!group) 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; @@ -7697,7 +7586,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7712,7 +7600,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; char *nodeset = NULL; @@ -7749,18 +7637,9 @@ qemuDomainGetNumaParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (!group) 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++) { @@ -7810,7 +7689,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, cleanup: VIR_FREE(nodeset); - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -7818,13 +7696,13 @@ cleanup: } static int -qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, - unsigned long long period, long long quota) +qemuSetVcpusBWLive(virDomainObjPtr vm, + unsigned long long period, + long long quota) { int i; qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_vcpu = NULL; - int rc; + virCgroupItemPtr cgroup = NULL; if (period == 0 && quota == 0) return 0; @@ -7835,36 +7713,30 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, */ if (priv->nvcpupids != 0 && priv->vcpupids[0] != vm->pid) { for (i = 0; i < priv->nvcpupids; i++) { - rc = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 0); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to find vcpu cgroup for %s(vcpu:" - " %d)"), - vm->def->name, i); - goto cleanup; - } + if (vm->def->vcpuCgroups && + vm->def->vcpuCgroups[i]) + cgroup = vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPU]; - if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) - goto cleanup; + if (!cgroup) + goto cleanup; - virCgroupFree(&cgroup_vcpu); + if (qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0) + goto cleanup; } } return 0; cleanup: - virCgroupFree(&cgroup_vcpu); return -1; } static int -qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, +qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, unsigned long long period, long long quota) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_emulator = NULL; - int rc; + virCgroupItemPtr cgroup = vm->def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPU]; if (period == 0 && quota == 0) return 0; @@ -7873,22 +7745,15 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; } - rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to find emulator cgroup for %s"), - vm->def->name); + if (!cgroup) goto cleanup; - } - if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + if (qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_emulator); return 0; cleanup: - virCgroupFree(&cgroup_emulator); return -1; } @@ -7909,7 +7774,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; int i; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; unsigned long long value_ul; @@ -7954,18 +7819,12 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!qemuCgroupControllerActive(driver, 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; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !group) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), + vm->def->name); + goto cleanup; } for (i = 0; i < nparams; i++) { @@ -7991,7 +7850,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { - if ((rc = qemuSetVcpusBWLive(vm, group, value_ul, 0))) + if ((rc = qemuSetVcpusBWLive(vm, value_ul, 0))) goto cleanup; vm->def->cputune.period = value_ul; @@ -8005,7 +7864,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { - if ((rc = qemuSetVcpusBWLive(vm, group, 0, value_l))) + if ((rc = qemuSetVcpusBWLive(vm, 0, value_l))) goto cleanup; vm->def->cputune.quota = value_l; @@ -8019,7 +7878,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { - if ((rc = qemuSetEmulatorBandwidthLive(vm, group, value_ul, 0))) + if ((rc = qemuSetEmulatorBandwidthLive(vm, value_ul, 0))) goto cleanup; vm->def->cputune.emulator_period = value_ul; @@ -8033,7 +7892,7 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { - if ((rc = qemuSetEmulatorBandwidthLive(vm, group, 0, value_l))) + if ((rc = qemuSetEmulatorBandwidthLive(vm, 0, value_l))) goto cleanup; vm->def->cputune.emulator_quota = value_l; @@ -8061,7 +7920,6 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, cleanup: virDomainDefFree(vmdef); - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -8081,7 +7939,7 @@ qemuSetSchedulerParameters(virDomainPtr dom, } static int -qemuGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, +qemuGetVcpuBWLive(virCgroupItemPtr cgroup, unsigned long long *period, long long *quota) { int rc; @@ -8104,10 +7962,10 @@ 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; + virCgroupItemPtr cgroup = NULL; qemuDomainObjPrivatePtr priv = NULL; int rc; int ret = -1; @@ -8115,41 +7973,30 @@ 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); - if (rc < 0) - goto cleanup; - - if (*quota > 0) - *quota /= vm->def->vcpus; - goto out; - } - - /* get period and quota for vcpu0 */ - rc = virCgroupForVcpu(cgroup, 0, &cgroup_vcpu, 0); - if (!cgroup_vcpu) { - virReportSystemError(-rc, - _("Unable to find vcpu cgroup for %s(vcpu: 0)"), - vm->def->name); - goto cleanup; + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + } else { + /* get period and quota for vcpu0 */ + cgroup = vm->def->vcpuCgroups[0][VIR_CGROUP_CONTROLLER_CPU]; } - rc = qemuGetVcpuBWLive(cgroup_vcpu, period, quota); + rc = qemuGetVcpuBWLive(cgroup, period, quota); if (rc < 0) goto cleanup; -out: + if (*quota > 0) + *quota /= vm->def->vcpus; + ret = 0; cleanup: - virCgroupFree(&cgroup_vcpu); return ret; } static int -qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, +qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, unsigned long long *period, long long *quota) { - virCgroupPtr cgroup_emulator = NULL; + virCgroupItemPtr cgroup = NULL; qemuDomainObjPrivatePtr priv = NULL; int rc; int ret = -1; @@ -8162,23 +8009,23 @@ qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; } + cgroup = vm->def->emulatorCgroup[VIR_CGROUP_CONTROLLER_CPU]; + /* get period and quota for emulator */ - rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); - if (!cgroup_emulator) { - virReportSystemError(-rc, + if (!cgroup) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, _("Unable to find emulator cgroup for %s"), vm->def->name); goto cleanup; } - rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); + rc = qemuGetVcpuBWLive(cgroup, period, quota); if (rc < 0) goto cleanup; ret = 0; cleanup: - virCgroupFree(&cgroup_emulator); return ret; } @@ -8189,7 +8036,7 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; unsigned long long shares; unsigned long long period; @@ -8212,10 +8059,12 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, flags &= ~VIR_TYPED_PARAM_STRING_OKAY; if (*nparams > 1) { - rc = qemuGetCpuBWStatus(driver->cgroup); + rc = virCgroupItemKeyPath(driver->cgroup[VIR_CGROUP_CONTROLLER_CPU], + "cpu.cfs_period_us", + NULL); if (rc < 0) - goto cleanup; - cpu_bw_status = !!rc; + cpu_bw_status = 0; + cpu_bw_status = 1; } vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -8241,13 +8090,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, goto out; } - if (!qemuCgroupControllerActive(driver, 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) { + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -8261,13 +8105,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, &emulator_period, &emulator_quota); if (rc != 0) goto cleanup; @@ -8320,7 +8164,6 @@ out: ret = 0; cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -10373,7 +10216,6 @@ typedef enum { static int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr disk, const char *file, qemuDomainDiskChainMode mode) @@ -10386,6 +10228,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, virStorageFileMetadataPtr origchain = disk->backingChain; bool origreadonly = disk->readonly; int ret = -1; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; disk->src = (char *) file; /* casting away const is safe here */ disk->format = VIR_STORAGE_FILE_RAW; @@ -10840,7 +10683,6 @@ cleanup: static int qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainSnapshotDiskDefPtr snap, virDomainDiskDefPtr disk, virDomainDiskDefPtr persistDisk, @@ -10890,9 +10732,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; } @@ -10934,7 +10776,6 @@ cleanup: static void qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virCgroupPtr cgroup, virDomainDiskDefPtr origdisk, virDomainDiskDefPtr disk, virDomainDiskDefPtr persistDisk, @@ -10951,7 +10792,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) @@ -10988,7 +10829,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, int i; bool persist = false; bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - virCgroupPtr cgroup = NULL; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -10997,7 +10837,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { + !vm->def->cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to find cgroup for %s"), vm->def->name); @@ -11040,7 +10880,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } } - ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, cgroup, + ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, @@ -11069,7 +10909,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, @@ -11080,8 +10920,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, qemuDomainObjExitMonitorWithDriver(driver, vm); cleanup: - virCgroupFree(&cgroup); - if (ret == 0 || !qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) { if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0 || (persist && virDomainSaveConfig(driver->configDir, vm->newDef) < 0)) @@ -12769,7 +12607,7 @@ qemuDomainBlockPivot(virConnectPtr conn, virDomainBlockJobInfo info; const char *format = virStorageFileFormatTypeToString(disk->mirrorFormat); bool resume = false; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; char *oldsrc = NULL; int oldformat; virStorageFileMetadataPtr oldchain = NULL; @@ -12830,7 +12668,7 @@ qemuDomainBlockPivot(virConnectPtr conn, * 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) { + !cgroup < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to find cgroup for %s"), vm->def->name); @@ -12895,8 +12733,6 @@ qemuDomainBlockPivot(virConnectPtr conn, disk->mirroring = false; cleanup: - if (cgroup) - virCgroupFree(&cgroup); if (resume && virDomainObjIsActive(vm) && qemuProcessStartCPUs(driver, vm, conn, VIR_DOMAIN_RUNNING_UNPAUSED, @@ -13127,7 +12963,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, struct stat st; bool need_unlink = false; char *mirror = NULL; - virCgroupPtr cgroup = NULL; + virCgroupItemPtr cgroup = NULL; /* Preliminaries: find the disk we are editing, sanity checks */ virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW | @@ -13142,8 +12978,10 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, _("domain is not running")); goto cleanup; } + + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) < 0) { + !cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to find cgroup for %s"), vm->def->name); @@ -13250,9 +13088,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; } @@ -13264,7 +13102,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; } @@ -13286,8 +13124,6 @@ endjob: } cleanup: - if (cgroup) - virCgroupFree(&cgroup); VIR_FREE(device); if (vm) virDomainObjUnlock(vm); @@ -13342,7 +13178,7 @@ 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; + virCgroupItemPtr cgroup = NULL; bool clean_access = false; virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); @@ -13419,6 +13255,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, goto endjob; } + cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + /* For the commit to succeed, we must allow qemu to open both the * 'base' image and the parent of 'top' as read/write; 'top' might * not have a parent, or might already be read-write. XXX It @@ -13427,17 +13265,17 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, * 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) { + !cgroup) { 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; @@ -13451,15 +13289,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; @@ -14081,7 +13917,7 @@ cleanup: /* qemuDomainGetCPUStats() with start_cpu == -1 */ static int -qemuDomainGetTotalcpuStats(virCgroupPtr group, +qemuDomainGetTotalcpuStats(virCgroupItemPtr group, virTypedParameterPtr params, int nparams) { @@ -14143,28 +13979,24 @@ 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; - virCgroupPtr group_vcpu = NULL; + virCgroupItemPtr group = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; - 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) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("error accessing cgroup cpuacct for vcpu")); - goto cleanup; - } + group = vm->def->vcpuCgroups[i][VIR_CGROUP_CONTROLLER_CPUACCT]; - if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0) + if (virCgroupGetCpuacctPercpuUsage(group, &buf) < 0) goto cleanup; pos = buf; @@ -14177,20 +14009,17 @@ getSumVcpuPercpuStats(virCgroupPtr group, sum_cpu_time[j] += tmp; } - virCgroupFree(&group_vcpu); VIR_FREE(buf); } ret = 0; cleanup: - virCgroupFree(&group_vcpu); VIR_FREE(buf); return ret; } static int qemuDomainGetPercpuStats(virDomainObjPtr vm, - virCgroupPtr group, virTypedParameterPtr params, unsigned int nparams, int start_cpu, @@ -14203,7 +14032,6 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, unsigned long long *sum_cpu_time = NULL; unsigned long long *sum_cpu_pos; unsigned int n = 0; - qemuDomainObjPrivatePtr priv = vm->privateData; virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; @@ -14230,7 +14058,8 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, } /* we get percpu cputime accounting info. */ - if (virCgroupGetCpuacctPercpuUsage(group, &buf)) + if (virCgroupGetCpuacctPercpuUsage(vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUACCT], + &buf)) goto cleanup; pos = buf; memset(params, 0, nparams * ncpus); @@ -14270,7 +14099,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; @@ -14303,7 +14132,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, unsigned int flags) { virQEMUDriverPtr driver = domain->conn->privateData; - virCgroupPtr group = NULL; + virCgroupItemPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; bool isActive; @@ -14326,13 +14155,9 @@ qemuDomainGetCPUStats(virDomainPtr domain, goto cleanup; } - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUACCT)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup CPUACCT controller is not mounted")); - goto cleanup; - } + group = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUACCT]; - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + if (!group) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; @@ -14341,10 +14166,9 @@ qemuDomainGetCPUStats(virDomainPtr domain, if (start_cpu == -1) ret = qemuDomainGetTotalcpuStats(group, params, nparams); else - ret = qemuDomainGetPercpuStats(vm, group, params, nparams, + ret = qemuDomainGetPercpuStats(vm, params, nparams, start_cpu, ncpus); cleanup: - virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 19172e1..3007f06 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1093,24 +1093,16 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, } if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - virCgroupPtr cgroup = NULL; usbDevice *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 = usbGetDevice(hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device, NULL)) == NULL) goto error; data.vm = vm; - data.cgroup = cgroup; + data.cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, &data) < 0) goto error; } @@ -1981,7 +1973,6 @@ int qemuDomainDetachPciDiskDevice(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); @@ -2001,15 +1992,6 @@ int qemuDomainDetachPciDiskDevice(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 (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -2063,8 +2045,10 @@ int qemuDomainDetachPciDiskDevice(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) + if (vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]) { + if (qemuTeardownDiskCgroup(vm, + vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } @@ -2075,7 +2059,6 @@ int qemuDomainDetachPciDiskDevice(virQEMUDriverPtr driver, ret = 0; cleanup: - virCgroupFree(&cgroup); VIR_FREE(drivestr); return ret; } @@ -2087,7 +2070,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); @@ -2114,15 +2096,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", @@ -2155,8 +2128,10 @@ 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) + if (vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]) { + if (qemuTeardownDiskCgroup(vm, + vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } @@ -2168,7 +2143,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 f17c7ff..cb6f6c7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3590,7 +3590,6 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, enum qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup = NULL; int ret = -1; int rc; bool restoreLabel = false; @@ -3623,22 +3622,17 @@ 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 (vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]) { + rc = virCgroupAllowDevicePath(vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + path, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, cgroup, "allow", path, "rw", rc); - if (rc == 1) { - /* path was not a device, no further need for cgroup */ - virCgroupFree(&cgroup); - } else if (rc < 0) { + virDomainAuditCgroupPath(vm, + vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + "allow", + path, + "rw", + rc); + if (rc < 0) { virReportSystemError(-rc, _("Unable to allow device %s for %s"), path, vm->def->name); @@ -3732,14 +3726,16 @@ cleanup: vm->def, path) < 0) VIR_WARN("failed to restore save state label on %s", path); - if (cgroup != NULL) { - rc = virCgroupDenyDevicePath(cgroup, path, + if (vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]) { + rc = virCgroupDenyDevicePath(vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + path, VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(vm, cgroup, "deny", path, "rwm", rc); + virDomainAuditCgroupPath(vm, + vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES], + "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 713670e..95cec7c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2737,6 +2737,7 @@ static int qemuProcessHook(void *data) struct qemuProcessHookData *h = data; int ret = -1; int fd; + int i; /* Some later calls want pid present */ h->vm->pid = getpid(); @@ -2768,8 +2769,9 @@ 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) - goto cleanup; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + virCgroupItemAddTask(h->vm->def->cgroup[i], getpid()); + } /* This must be done after cgroup placement to avoid resetting CPU * affinity */ @@ -3655,11 +3657,6 @@ 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); - for (i = 0 ; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && @@ -4175,13 +4172,11 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainShutoffReason reason, unsigned int flags) { - int ret; - int retries = 0; qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; virDomainDefPtr def; virNetDevVPortProfilePtr vport = NULL; - int i; + int i, j; int logfile = -1; char *timestamp; char ebuf[1024]; @@ -4344,14 +4339,14 @@ void qemuProcessStop(virQEMUDriverPtr driver, networkReleaseActualDevice(net); } -retry: - if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) { - if (ret == -EBUSY && (retries++ < 5)) { - usleep(200*1000); - goto retry; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + virCgroupItemFree(vm->def->cgroup[i]); + virCgroupItemFree(vm->def->emulatorCgroup[i]); + if (vm->def->vcpuCgroups) { + for (j = 0; j < priv->nvcpupids; j++) { + virCgroupItemFree(vm->def->vcpuCgroups[j][i]); + } } - VIR_WARN("Failed to remove cgroup for %s", - vm->def->name); } qemuProcessRemoveDomainStatus(driver, vm); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index baa0af7..c5af032 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -92,6 +92,7 @@ VIR_ONCE_GLOBAL_INIT(virCgroupItem); static struct virCgroupController cgroupControllers[VIR_CGROUP_CONTROLLER_LAST]; static virCgroupItemPtr rootCgroupItems[VIR_CGROUP_CONTROLLER_LAST]; +static virCgroupItemPtr appCgroupItems[2][VIR_CGROUP_CONTROLLER_LAST]; static virCgroupItemPtr virCgroupItemRootNew(int type); static int virCgroupDetectMounts(struct virCgroupController (*controllers)[VIR_CGROUP_CONTROLLER_LAST]); @@ -149,13 +150,21 @@ void virCgroupUninit(void) virCgroupItemFree(rootCgroupItems[i]); rootCgroupItems[i] = NULL; } + if (appCgroupItems[0][i]) { + virCgroupItemFree(appCgroupItems[0][i]); + appCgroupItems[0][i] = NULL; + } + if (appCgroupItems[1][i]) { + virCgroupItemFree(appCgroupItems[1][i]); + appCgroupItems[1][i] = NULL; + } } } -struct virCgroup { - char *path; +struct _virCgroup { + char *name; - struct virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr items[VIR_CGROUP_CONTROLLER_LAST]; }; typedef enum { @@ -236,9 +245,6 @@ virCgroupItemPtr virCgroupItemNew(int type, return NULL; if (!parent) - parent = rootCgroupItems[type]; - - if (!parent) return NULL; cgroupItem = virCgroupHasChildByName(parent, name); @@ -406,6 +412,7 @@ static int virCgroupItemCpusetInherit(virCgroupItemPtr cgroupItem) } static int virCgroupRemoveRecursively(char *grppath); +static int virCgroupItemSetMemoryUseHierarchy(virCgroupItemPtr group); static int virCgroupItemMakePath(virCgroupItemPtr cgroupItem) { @@ -430,7 +437,11 @@ static int virCgroupItemMakePath(virCgroupItemPtr cgroupItem) return -errno; } - virCgroupItemCpusetInherit(cgroupItem); + if (cgroupItem->controller->type == VIR_CGROUP_CONTROLLER_CPUSET) + virCgroupItemCpusetInherit(cgroupItem); + + if (cgroupItem->controller->type == VIR_CGROUP_CONTROLLER_MEMORY) + virCgroupItemSetMemoryUseHierarchy(cgroupItem); } return ret; @@ -470,27 +481,6 @@ int virCgroupItemKeyPath(virCgroupItemPtr cgroupItem, } /** - * virCgroupFree: - * - * @group: The group structure to free - */ -void virCgroupFree(virCgroupPtr *group) -{ - int i; - - if (*group == NULL) - return; - - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - VIR_FREE((*group)->controllers[i].mountPoint); - VIR_FREE((*group)->controllers[i].placement); - } - - VIR_FREE((*group)->path); - VIR_FREE(*group); -} - -/** * virCgroupMounted: query whether a cgroup subsystem is mounted or not * * @cgroup: The group structure to be queried @@ -498,9 +488,9 @@ void virCgroupFree(virCgroupPtr *group) * * Returns true if a cgroup is subsystem is mounted. */ -bool virCgroupMounted(virCgroupPtr cgroup, int controller) +bool virCgroupMounted(int controller) { - return cgroup->controllers[controller].mountPoint != NULL; + return cgroupControllers[controller].mountPoint != NULL; } #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R @@ -627,54 +617,6 @@ no_memory: } -static int virCgroupDetect(virCgroupPtr group) -{ - int any = 0; - int rc; - int i; - - rc = virCgroupDetectMounts(&group->controllers); - if (rc < 0) { - VIR_ERROR(_("Failed to detect mounts for %s"), group->path); - 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 (!any) - return -ENXIO; - - - rc = virCgroupDetectPlacement(&group->controllers); - - if (rc == 0) { - /* Check that for every mounted controller, we found our placement */ - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - if (!group->controllers[i].mountPoint) - continue; - - if (!group->controllers[i].placement) { - VIR_ERROR(_("Could not find placement for controller %s at %s"), - virCgroupControllerTypeToString(i), - group->controllers[i].placement); - rc = -ENOENT; - break; - } - - VIR_DEBUG("Detected mount/mapping %i:%s at %s in %s", i, - virCgroupControllerTypeToString(i), - group->controllers[i].mountPoint, - group->controllers[i].placement); - } - } else { - VIR_ERROR(_("Failed to detect mapping for %s"), group->path); - } - - return rc; -} #endif @@ -683,99 +625,10 @@ int virCgroupPathOfController(virCgroupPtr group, const char *key, char **path) { - if (controller == -1) { - int i; - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - if (group->controllers[i].mountPoint && - group->controllers[i].placement) { - controller = i; - break; - } - } - } - if (controller == -1) - return -ENOSYS; - - if (group->controllers[controller].mountPoint == NULL) - return -ENOENT; - - if (group->controllers[controller].placement == NULL) - return -ENOENT; - - if (virAsprintf(path, "%s%s%s/%s", - group->controllers[controller].mountPoint, - group->controllers[controller].placement, - STREQ(group->path, "/") ? "" : group->path, - key ? key : "") == -1) - return -ENOMEM; - - return 0; -} - - -static int virCgroupSetValueStr(virCgroupPtr group, - int controller, - const char *key, - const char *value) -{ - int rc = 0; - char *keypath = NULL; - - rc = virCgroupPathOfController(group, controller, key, &keypath); - if (rc != 0) - return rc; - - VIR_DEBUG("Set value '%s' to '%s'", keypath, value); - rc = virFileWriteStr(keypath, value, 0); - if (rc < 0) { - rc = -errno; - VIR_DEBUG("Failed to write value '%s': %m", value); - } else { - rc = 0; - } - - VIR_FREE(keypath); - - return rc; -} - -static int virCgroupGetValueStr(virCgroupPtr group, - int controller, - const char *key, - char **value) -{ - int rc; - char *keypath = NULL; - - *value = NULL; - - rc = virCgroupPathOfController(group, controller, key, &keypath); - if (rc != 0) { - VIR_DEBUG("No path of %s, %s", group->path, key); - return rc; - } - - VIR_DEBUG("Get value %s", keypath); - - rc = virFileReadAll(keypath, 1024*1024, value); - if (rc < 0) { - rc = -errno; - VIR_DEBUG("Failed to read %s: %m\n", keypath); - } else { - /* Terminated with '\n' has sometimes harmful effects to the caller */ - if ((*value)[rc - 1] == '\n') - (*value)[rc - 1] = '\0'; - - rc = 0; - } - - VIR_FREE(keypath); - - return rc; + return virCgroupItemKeyPath(group->items[controller], key, path); } -static int virCgroupSetValueU64(virCgroupPtr group, - int controller, +static int virCgroupItemSetValueU64(virCgroupItemPtr group, const char *key, unsigned long long int value) { @@ -785,17 +638,14 @@ static int virCgroupSetValueU64(virCgroupPtr group, if (virAsprintf(&strval, "%llu", value) == -1) return -ENOMEM; - rc = virCgroupSetValueStr(group, controller, key, strval); + rc = virCgroupItemSetValueStr(group, key, strval); VIR_FREE(strval); return rc; } - - -static int virCgroupSetValueI64(virCgroupPtr group, - int controller, +static int virCgroupItemSetValueI64(virCgroupItemPtr group, const char *key, long long int value) { @@ -805,22 +655,21 @@ static int virCgroupSetValueI64(virCgroupPtr group, if (virAsprintf(&strval, "%lld", value) == -1) return -ENOMEM; - rc = virCgroupSetValueStr(group, controller, key, strval); + rc = virCgroupItemSetValueStr(group, key, strval); VIR_FREE(strval); return rc; } -static int virCgroupGetValueI64(virCgroupPtr group, - int controller, +static int virCgroupItemGetValueI64(virCgroupItemPtr group, const char *key, long long int *value) { char *strval = NULL; int rc = 0; - rc = virCgroupGetValueStr(group, controller, key, &strval); + rc = virCgroupItemGetValueStr(group, key, &strval); if (rc != 0) goto out; @@ -832,15 +681,14 @@ out: return rc; } -static int virCgroupGetValueU64(virCgroupPtr group, - int controller, - const char *key, - unsigned long long int *value) +static int virCgroupItemGetValueU64(virCgroupItemPtr group, + const char *key, + unsigned long long int *value) { char *strval = NULL; int rc = 0; - rc = virCgroupGetValueStr(group, controller, key, &strval); + rc = virCgroupItemGetValueStr(group, key, &strval); if (rc != 0) goto out; @@ -852,58 +700,16 @@ out: return rc; } - -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) -{ - int i; - int rc = 0; - const char *inherit_values[] = { - "cpuset.cpus", - "cpuset.mems", - }; - - VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path); - for (i = 0; i < ARRAY_CARDINALITY(inherit_values) ; i++) { - char *value; - - rc = virCgroupGetValueStr(parent, - VIR_CGROUP_CONTROLLER_CPUSET, - inherit_values[i], - &value); - if (rc != 0) { - VIR_ERROR(_("Failed to get %s %d"), inherit_values[i], rc); - break; - } - - VIR_DEBUG("Inherit %s = %s", inherit_values[i], value); - - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - inherit_values[i], - value); - VIR_FREE(value); - - if (rc != 0) { - VIR_ERROR(_("Failed to set %s %d"), inherit_values[i], rc); - break; - } - } - - return rc; -} - -static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) +static int virCgroupItemSetMemoryUseHierarchy(virCgroupItemPtr item) { int rc = 0; unsigned long long value; const char *filename = "memory.use_hierarchy"; - rc = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - filename, &value); + rc = virCgroupItemGetValueU64(item, + filename, &value); if (rc != 0) { - VIR_ERROR(_("Failed to read %s/%s (%d)"), group->path, filename, rc); + VIR_ERROR(_("Failed to read %s/%s (%d)"), item->path, filename, rc); return rc; } @@ -911,109 +717,24 @@ static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) if (value == 1) return 0; - VIR_DEBUG("Setting up %s/%s", group->path, filename); - rc = virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - filename, 1); + VIR_DEBUG("Setting up %s/%s", item->path, filename); + rc = virCgroupItemSetValueU64(item, + filename, 1); if (rc != 0) { - VIR_ERROR(_("Failed to set %s/%s (%d)"), group->path, filename, rc); - } - - return rc; -} - -static int virCgroupMakeGroup(virCgroupPtr parent, - virCgroupPtr group, - bool create, - unsigned int flags) -{ - int i; - int rc = 0; - - VIR_DEBUG("Make group %s", group->path); - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - char *path = NULL; - - /* Skip over controllers that aren't mounted */ - 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; - /* As of Feb 2011, clang can't see that the above function - * call did not modify group. */ - sa_assert(group->controllers[i].mountPoint); - - VIR_DEBUG("Make controller %s", path); - if (access(path, F_OK) != 0) { - if (!create || - mkdir(path, 0755) < 0) { - /* With a kernel that doesn't support multi-level directory - * for blkio controller, libvirt will fail and disable all - * other controllers even though they are available. So - * treat blkio as unmounted if mkdir fails. */ - if (i == VIR_CGROUP_CONTROLLER_BLKIO) { - rc = 0; - VIR_FREE(group->controllers[i].mountPoint); - VIR_FREE(path); - continue; - } else { - rc = -errno; - VIR_FREE(path); - break; - } - } - if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL && - (i == VIR_CGROUP_CONTROLLER_CPUSET || - STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) { - rc = virCgroupCpuSetInherit(parent, group); - if (rc != 0) { - VIR_FREE(path); - break; - } - } - /* - * Note that virCgroupSetMemoryUseHierarchy should always be - * called prior to creating subcgroups and attaching tasks. - */ - if ((flags & VIR_CGROUP_MEM_HIERACHY) && - (group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL) && - (i == VIR_CGROUP_CONTROLLER_MEMORY || - STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { - rc = virCgroupSetMemoryUseHierarchy(group); - if (rc != 0) { - VIR_FREE(path); - break; - } - } - } - - VIR_FREE(path); + VIR_ERROR(_("Failed to set %s/%s (%d)"), item->path, filename, rc); } return rc; } - -static int virCgroupNew(const char *path, - virCgroupPtr *group) +int virCgroupNew(const char *name, + virCgroupPtr parent, + virCgroupPtr *group) { int rc = 0; - char *typpath = NULL; + int i; - VIR_DEBUG("New group %s", path); *group = NULL; if (VIR_ALLOC((*group)) != 0) { @@ -1021,66 +742,48 @@ static int virCgroupNew(const char *path, goto err; } - if (!((*group)->path = strdup(path))) { + if (!((*group)->name = strdup(name))) { rc = -ENOMEM; goto err; } - rc = virCgroupDetect(*group); - if (rc < 0) - goto err; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) + (*group)->items[i] = virCgroupItemNew(i, name, parent->items[i]); return rc; err: - virCgroupFree(group); + VIR_FREE(*group); *group = NULL; - VIR_FREE(typpath); - return rc; } -static int virCgroupAppRoot(bool privileged, - virCgroupPtr *group, - bool create) +/** + * virCgroupFree: + * + * @group: The group to be removed + * + * fre @group and its cgroup items. + * + */ +void virCgroupFree(virCgroupPtr *group) { - virCgroupPtr rootgrp = NULL; - int rc; + int i; - rc = virCgroupNew("/", &rootgrp); - if (rc != 0) - return rc; + if (!group) + return; - if (privileged) { - rc = virCgroupNew("/libvirt", group); - } else { - char *rootname; - char *username; - username = virGetUserName(getuid()); - if (!username) { - rc = -ENOMEM; - goto cleanup; - } - rc = virAsprintf(&rootname, "/libvirt-%s", username); - VIR_FREE(username); - if (rc < 0) { - rc = -ENOMEM; - goto cleanup; - } + if (!*group) + return; - rc = virCgroupNew(rootname, group); - VIR_FREE(rootname); - } - if (rc != 0) - goto cleanup; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) + virCgroupItemFree((*group)->items[i]); - rc = virCgroupMakeGroup(rootgrp, *group, create, VIR_CGROUP_NONE); + VIR_FREE((*group)->name); + VIR_FREE(*group); -cleanup: - virCgroupFree(&rootgrp); - return rc; + return; } -#endif #if defined _DIRENT_HAVE_D_TYPE static int virCgroupRemoveRecursively(char *grppath) @@ -1140,93 +843,22 @@ static int virCgroupRemoveRecursively(char *grppath ATTRIBUTE_UNUSED) #endif /** - * virCgroupRemove: - * - * @group: The group to be removed - * - * It first removes all child groups recursively - * in depth first order and then removes @group - * because the presence of the child groups - * prevents removing @group. + * virCgroupItemAddTask: * - * Returns: 0 on success - */ -int virCgroupRemove(virCgroupPtr group) -{ - int rc = 0; - int i; - char *grppath = NULL; - - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - /* Skip over controllers not mounted */ - if (!group->controllers[i].mountPoint) - continue; - - if (virCgroupPathOfController(group, - i, - NULL, - &grppath) != 0) - continue; - - VIR_DEBUG("Removing cgroup %s and all child cgroups", grppath); - rc = virCgroupRemoveRecursively(grppath); - VIR_FREE(grppath); - } - - return rc; -} - -/** - * virCgroupAddTask: - * - * @group: The cgroup to add a task to + * @item: The cgroup item to add a task to * @pid: The pid of the task to add * * Returns: 0 on success */ -int virCgroupAddTask(virCgroupPtr group, pid_t pid) -{ - int rc = 0; - int i; - - for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { - /* Skip over controllers not mounted */ - if (!group->controllers[i].mountPoint) - continue; - - rc = virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid); - if (rc != 0) - break; - } - - return rc; -} - -/** - * virCgroupAddTaskController: - * - * @group: The cgroup to add a task to - * @pid: The pid of the task to add - * @controller: The cgroup controller to be operated on - * - * Returns: 0 on success or -errno on failure - */ -int virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) +int virCgroupItemAddTask(virCgroupItemPtr item, pid_t pid) { - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) - return -EINVAL; - - if (!group->controllers[controller].mountPoint) - return -EINVAL; - - return virCgroupSetValueU64(group, controller, "tasks", - (unsigned long long)pid); + return virCgroupItemSetValueU64(item, "tasks", + (unsigned long long)pid); } -static int virCgroupAddTaskStrController(virCgroupPtr group, - const char *pidstr, - int controller) +static int virCgroupItemAddTaskStr(virCgroupItemPtr item, + const char *pidstr) { char *str = NULL, *cur = NULL, *next = NULL; unsigned long long p = 0; @@ -1242,7 +874,7 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, if (rc != 0) goto cleanup; - rc = virCgroupAddTaskController(group, p, controller); + rc = virCgroupItemAddTask(item, p); if (rc != 0) goto cleanup; @@ -1261,35 +893,23 @@ cleanup: } /** - * virCgroupMoveTask: + * virCgroupItemMoveTask: * - * @src_group: The source cgroup where all tasks are removed from - * @dest_group: The destination where all tasks are added to - * @controller: The cgroup controller to be operated on + * @src: The source cgroup where all tasks are removed from + * @dest: The destination where all tasks are added to * * Returns: 0 on success or -errno on failure */ -int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group, - int controller) +int virCgroupItemMoveTask(virCgroupItemPtr src, virCgroupItemPtr dest) { int rc = 0, err = 0; char *content = NULL; - if (controller < VIR_CGROUP_CONTROLLER_CPU || - controller > VIR_CGROUP_CONTROLLER_BLKIO) - return -EINVAL; - - if (!src_group->controllers[controller].mountPoint || - !dest_group->controllers[controller].mountPoint) { - VIR_WARN("no vm cgroup in controller %d", controller); - return 0; - } - - rc = virCgroupGetValueStr(src_group, controller, "tasks", &content); + rc = virCgroupItemGetValueStr(src, "tasks", &content); if (rc != 0) return rc; - rc = virCgroupAddTaskStrController(dest_group, content, controller); + rc = virCgroupItemAddTaskStr(dest, content); if (rc != 0) goto cleanup; @@ -1302,229 +922,56 @@ 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); + err = virCgroupItemAddTaskStr(src, content); if (err != 0) VIR_ERROR(_("Cannot recover cgroup %s from %s"), - src_group->controllers[controller].mountPoint, - dest_group->controllers[controller].mountPoint); + src->path, + dest->path); VIR_FREE(content); return rc; } /** - * virCgroupForDriver: - * - * @name: name of this driver (e.g., xen, qemu, lxc) - * @group: Pointer to returned virCgroupPtr - * - * Returns 0 on success - */ -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupForDriver(const char *name, - virCgroupPtr *group, - bool privileged, - bool create) -{ - int rc; - char *path = NULL; - virCgroupPtr rootgrp = NULL; - - rc = virCgroupAppRoot(privileged, &rootgrp, create); - if (rc != 0) - goto out; - - if (virAsprintf(&path, "%s/%s", rootgrp->path, name) < 0) { - rc = -ENOMEM; - goto out; - } - - rc = virCgroupNew(path, group); - VIR_FREE(path); - - if (rc == 0) { - rc = virCgroupMakeGroup(rootgrp, *group, create, VIR_CGROUP_NONE); - if (rc != 0) - virCgroupFree(group); - } - -out: - virCgroupFree(&rootgrp); - - return rc; -} -#else -int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED, - bool privileged ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED) -{ - /* Claim no support */ - return -ENXIO; -} -#endif - -/** * virCgroupGetAppRoot: * -* @group: Pointer to returned virCgroupPtr +* @type: the type of app cgroup item * -* Returns 0 on success +* Returns the app cgroup item of type, or NULL on error */ -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupGetAppRoot(virCgroupPtr *group) -{ - return virCgroupNew("/", group); -} -#else -int virCgroupGetAppRoot(virCgroupPtr *group ATTRIBUTE_UNUSED) -{ - return -ENXIO; -} -#endif - -/** - * virCgroupForDomain: - * - * @driver: group for driver owning the domain - * @name: name of the domain - * @group: Pointer to returned virCgroupPtr - * - * Returns 0 on success - */ -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupForDomain(virCgroupPtr driver, - const char *name, - virCgroupPtr *group, - bool create) +virCgroupItemPtr virCgroupGetAppRoot(int type, bool privileged) { + int p = privileged ? 1 : 0; + char *name; + char *userName; int rc; - char *path; - - if (driver == NULL) - return -EINVAL; - - if (virAsprintf(&path, "%s/%s", driver->path, name) < 0) - return -ENOMEM; - - rc = virCgroupNew(path, group); - VIR_FREE(path); - - if (rc == 0) { - /* - * Create a cgroup with memory.use_hierarchy enabled to - * surely account memory usage of lxc with ns subsystem - * enabled. (To be exact, memory and ns subsystems are - * enabled at the same time.) - * - * The reason why doing it here, not a upper group, say - * a group for driver, is to avoid overhead to track - * cumulative usage that we don't need. - */ - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_MEM_HIERACHY); - if (rc != 0) - virCgroupFree(group); - } - - return rc; -} -#else -int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED, - const char *name ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED) -{ - return -ENXIO; -} -#endif -/** - * virCgroupForVcpu: - * - * @driver: group for the domain - * @vcpuid: id of the vcpu - * @group: Pointer to returned virCgroupPtr - * - * Returns 0 on success - */ -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupForVcpu(virCgroupPtr driver, - int vcpuid, - virCgroupPtr *group, - bool create) -{ - int rc; - char *path; - - if (driver == NULL) - return -EINVAL; - - if (virAsprintf(&path, "%s/vcpu%d", driver->path, vcpuid) < 0) - return -ENOMEM; + if (type < 0 || type >= VIR_CGROUP_CONTROLLER_LAST) + return NULL; - rc = virCgroupNew(path, group); - VIR_FREE(path); + if (virCgroupItemInitialize() < 0) + return NULL; - if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); - if (rc != 0) - virCgroupFree(group); + if (privileged) { + rc = virAsprintf(&name, "libvirt"); + } else { + userName = virGetUserName(getuid()); + if (!userName) + return NULL; + rc = virAsprintf(&name, "libvirt-%s", userName); + VIR_FREE(userName); } - return rc; -} -#else -int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, - int vcpuid ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED) -{ - return -ENXIO; -} -#endif - -/** - * virCgroupForEmulator: - * - * @driver: group for the domain - * @group: Pointer to returned virCgroupPtr - * - * Returns: 0 on success or -errno on failure - */ -#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -int virCgroupForEmulator(virCgroupPtr driver, - virCgroupPtr *group, - bool create) -{ - int rc; - char *path; - - if (driver == NULL) - return -EINVAL; - - if (virAsprintf(&path, "%s/emulator", driver->path) < 0) - return -ENOMEM; - - rc = virCgroupNew(path, group); - VIR_FREE(path); + if (rc < 0) + return NULL; - if (rc == 0) { - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); - if (rc != 0) - virCgroupFree(group); - } + if (!appCgroupItems[p][type]) + appCgroupItems[p][type] = virCgroupItemNew(type, name, + rootCgroupItems[type]); - return rc; -} -#else -int virCgroupForEmulator(virCgroupPtr driver ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED) -{ - return -ENXIO; + return appCgroupItems[p][type]; } -#endif /** * virCgroupSetBlkioWeight: * @@ -1533,15 +980,16 @@ int virCgroupForEmulator(virCgroupPtr driver ATTRIBUTE_UNUSED, * * Returns: 0 on success */ -int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) +int virCgroupSetBlkioWeight(virCgroupItemPtr group, unsigned int weight) { if (weight > 1000 || weight < 100) return -EINVAL; - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.weight", - weight); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_BLKIO); + + return virCgroupItemSetValueU64(group, + "blkio.weight", + weight); } /** @@ -1552,13 +1000,15 @@ int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) * * Returns: 0 on success */ -int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) +int virCgroupGetBlkioWeight(virCgroupItemPtr group, unsigned int *weight) { unsigned long long tmp; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.weight", &tmp); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_BLKIO); + + ret = virCgroupItemGetValueU64(group, + "blkio.weight", &tmp); if (ret == 0) *weight = tmp; return ret; @@ -1577,7 +1027,7 @@ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) * Returns: 0 on success, -errno on failure */ #if defined(major) && defined(minor) -int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, +int virCgroupSetBlkioDeviceWeight(virCgroupItemPtr group, const char *path, unsigned int weight) { @@ -1585,6 +1035,8 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, struct stat sb; int ret; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_BLKIO); + if (weight && (weight > 1000 || weight < 100)) return -EINVAL; @@ -1598,10 +1050,9 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, weight) < 0) return -errno; - ret = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.weight_device", - str); + ret = virCgroupItemSetValueStr(group, + "blkio.weight_device", + str); VIR_FREE(str); return ret; } @@ -1623,22 +1074,22 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group ATTRIBUTE_UNUSED, * * Returns: 0 on success */ -int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemory(virCgroupItemPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + if (kb > maxkb) return -EINVAL; else if (kb == maxkb) - return virCgroupSetValueI64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.limit_in_bytes", - -1); + return virCgroupItemSetValueI64(group, + "memory.limit_in_bytes", + -1); else - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.limit_in_bytes", - kb << 10); + return virCgroupItemSetValueU64(group, + "memory.limit_in_bytes", + kb << 10); } /** @@ -1649,13 +1100,15 @@ int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) * * Returns: 0 on success */ -int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb) +int virCgroupGetMemoryUsage(virCgroupItemPtr group, unsigned long *kb) { long long unsigned int usage_in_bytes; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.usage_in_bytes", &usage_in_bytes); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + + ret = virCgroupItemGetValueU64(group, + "memory.usage_in_bytes", &usage_in_bytes); if (ret == 0) *kb = (unsigned long) usage_in_bytes >> 10; return ret; @@ -1669,7 +1122,7 @@ int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb) * * Returns: 0 on success */ -int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemoryHardLimit(virCgroupItemPtr group, unsigned long long kb) { return virCgroupSetMemory(group, kb); } @@ -1682,13 +1135,15 @@ int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb) * * Returns: 0 on success */ -int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemoryHardLimit(virCgroupItemPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.limit_in_bytes", &limit_in_bytes); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + + ret = virCgroupItemGetValueU64(group, + "memory.limit_in_bytes", &limit_in_bytes); if (ret == 0) *kb = limit_in_bytes >> 10; return ret; @@ -1702,22 +1157,22 @@ int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb) * * Returns: 0 on success */ -int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemorySoftLimit(virCgroupItemPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + if (kb > maxkb) return -EINVAL; else if (kb == maxkb) - return virCgroupSetValueI64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.soft_limit_in_bytes", - -1); + return virCgroupItemSetValueI64(group, + "memory.soft_limit_in_bytes", + -1); else - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.soft_limit_in_bytes", - kb << 10); + return virCgroupItemSetValueU64(group, + "memory.soft_limit_in_bytes", + kb << 10); } @@ -1729,13 +1184,15 @@ int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb) * * Returns: 0 on success */ -int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemorySoftLimit(virCgroupItemPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.soft_limit_in_bytes", &limit_in_bytes); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + + ret = virCgroupItemGetValueU64(group, + "memory.soft_limit_in_bytes", &limit_in_bytes); if (ret == 0) *kb = limit_in_bytes >> 10; return ret; @@ -1749,22 +1206,22 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) * * Returns: 0 on success */ -int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemSwapHardLimit(virCgroupItemPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + if (kb > maxkb) return -EINVAL; else if (kb == maxkb) - return virCgroupSetValueI64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.memsw.limit_in_bytes", - -1); + return virCgroupItemSetValueI64(group, + "memory.memsw.limit_in_bytes", + -1); else - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.memsw.limit_in_bytes", - kb << 10); + return virCgroupItemSetValueU64(group, + "memory.memsw.limit_in_bytes", + kb << 10); } /** @@ -1775,13 +1232,15 @@ int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) * * Returns: 0 on success */ -int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemSwapHardLimit(virCgroupItemPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.memsw.limit_in_bytes", &limit_in_bytes); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + + ret = virCgroupItemGetValueU64(group, + "memory.memsw.limit_in_bytes", &limit_in_bytes); if (ret == 0) *kb = limit_in_bytes >> 10; return ret; @@ -1795,13 +1254,15 @@ int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) * * Returns: 0 on success */ -int virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemSwapUsage(virCgroupItemPtr group, unsigned long long *kb) { long long unsigned int usage_in_bytes; int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.memsw.usage_in_bytes", &usage_in_bytes); + + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_MEMORY); + + ret = virCgroupItemGetValueU64(group, + "memory.memsw.usage_in_bytes", &usage_in_bytes); if (ret == 0) *kb = usage_in_bytes >> 10; return ret; @@ -1815,12 +1276,13 @@ int virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) * * Returns: 0 on success */ -int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) +int virCgroupSetCpusetMems(virCgroupItemPtr group, const char *mems) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.mems", - mems); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUSET); + + return virCgroupItemSetValueStr(group, + "cpuset.mems", + mems); } /** @@ -1831,12 +1293,13 @@ int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) * * Returns: 0 on success */ -int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) +int virCgroupGetCpusetMems(virCgroupItemPtr group, char **mems) { - return virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.mems", - mems); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUSET); + + return virCgroupItemGetValueStr(group, + "cpuset.mems", + mems); } /** @@ -1847,12 +1310,13 @@ int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) * * Retuens: 0 on success */ -int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) +int virCgroupSetCpusetCpus(virCgroupItemPtr group, const char *cpus) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.cpus", - cpus); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUSET); + + return virCgroupItemSetValueStr(group, + "cpuset.cpus", + cpus); } /** @@ -1863,10 +1327,11 @@ int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) * * Retuens: 0 on success */ -int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) +int virCgroupGetCpusetCpus(virCgroupItemPtr group, char **cpus) { - return virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUSET); + + return virCgroupItemGetValueStr(group, "cpuset.cpus", cpus); } @@ -1878,12 +1343,13 @@ int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) * * Returns: 0 on success */ -int virCgroupDenyAllDevices(virCgroupPtr group) +int virCgroupDenyAllDevices(virCgroupItemPtr group) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - "a"); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_DEVICES); + + return virCgroupItemSetValueStr(group, + "devices.deny", + "a"); } /** @@ -1897,12 +1363,14 @@ int virCgroupDenyAllDevices(virCgroupPtr group) * * Returns: 0 on success */ -int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, +int virCgroupAllowDevice(virCgroupItemPtr group, char type, int major, int minor, int perms) { int rc; char *devstr = NULL; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_DEVICES); + if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", @@ -1911,10 +1379,9 @@ int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, goto out; } - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.allow", - devstr); + rc = virCgroupItemSetValueStr(group, + "devices.allow", + devstr); out: VIR_FREE(devstr); @@ -1931,12 +1398,14 @@ out: * * Returns: 0 on success */ -int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, +int virCgroupAllowDeviceMajor(virCgroupItemPtr group, char type, int major, int perms) { int rc; char *devstr = NULL; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_DEVICES); + if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", @@ -1945,10 +1414,9 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, goto out; } - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.allow", - devstr); + rc = virCgroupItemSetValueStr(group, + "devices.allow", + devstr); out: VIR_FREE(devstr); @@ -1969,7 +1437,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, * negative errno value on failure */ #if defined(major) && defined(minor) -int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms) +int virCgroupAllowDevicePath(virCgroupItemPtr group, const char *path, int perms) { struct stat sb; @@ -1986,7 +1454,7 @@ int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms) perms); } #else -int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, +int virCgroupAllowDevicePath(virCgroupItemPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, int perms ATTRIBUTE_UNUSED) { @@ -2006,12 +1474,14 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, * * Returns: 0 on success */ -int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, +int virCgroupDenyDevice(virCgroupItemPtr group, char type, int major, int minor, int perms) { int rc; char *devstr = NULL; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_DEVICES); + if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", @@ -2020,10 +1490,9 @@ int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, goto out; } - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - devstr); + rc = virCgroupItemSetValueStr(group, + "devices.deny", + devstr); out: VIR_FREE(devstr); @@ -2040,12 +1509,14 @@ out: * * Returns: 0 on success */ -int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major, +int virCgroupDenyDeviceMajor(virCgroupItemPtr group, char type, int major, int perms) { int rc; char *devstr = NULL; + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_DEVICES); + if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", @@ -2054,10 +1525,9 @@ int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major, goto out; } - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - devstr); + rc = virCgroupItemSetValueStr(group, + "devices.deny", + devstr); out: VIR_FREE(devstr); @@ -2065,7 +1535,7 @@ int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major, } #if defined(major) && defined(minor) -int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms) +int virCgroupDenyDevicePath(virCgroupItemPtr group, const char *path, int perms) { struct stat sb; @@ -2082,7 +1552,7 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms) perms); } #else -int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, +int virCgroupDenyDevicePath(virCgroupItemPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, int perms ATTRIBUTE_UNUSED) { @@ -2090,18 +1560,20 @@ int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, } #endif -int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares) +int virCgroupSetCpuShares(virCgroupItemPtr group, unsigned long long shares) { - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, - "cpu.shares", shares); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + + return virCgroupItemSetValueU64(group, + "cpu.shares", shares); } -int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares) +int virCgroupGetCpuShares(virCgroupItemPtr group, unsigned long long *shares) { - return virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, - "cpu.shares", shares); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + + return virCgroupItemGetValueU64(group, + "cpu.shares", shares); } /** @@ -2112,16 +1584,17 @@ int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares) * * Returns: 0 on success */ -int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) +int virCgroupSetCpuCfsPeriod(virCgroupItemPtr group, unsigned long long cfs_period) { + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + /* The cfs_period shoule be greater or equal than 1ms, and less or equal * than 1s. */ if (cfs_period < 1000 || cfs_period > 1000000) return -EINVAL; - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, + return virCgroupItemSetValueU64(group, "cpu.cfs_period_us", cfs_period); } @@ -2133,10 +1606,11 @@ int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) * * Returns: 0 on success */ -int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) +int virCgroupGetCpuCfsPeriod(virCgroupItemPtr group, unsigned long long *cfs_period) { - return virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + + return virCgroupItemGetValueU64(group, "cpu.cfs_period_us", cfs_period); } @@ -2149,8 +1623,10 @@ int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) * * Returns: 0 on success */ -int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) +int virCgroupSetCpuCfsQuota(virCgroupItemPtr group, long long cfs_quota) { + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + if (cfs_quota >= 0) { /* The cfs_quota shoule be greater or equal than 1ms */ if (cfs_quota < 1000) @@ -2161,9 +1637,8 @@ int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) return -EINVAL; } - return virCgroupSetValueI64(group, - VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_quota_us", cfs_quota); + return virCgroupItemSetValueI64(group, + "cpu.cfs_quota_us", cfs_quota); } /** @@ -2175,28 +1650,32 @@ int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) * * Returns: 0 on success */ -int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota) +int virCgroupGetCpuCfsQuota(virCgroupItemPtr group, long long *cfs_quota) { - return virCgroupGetValueI64(group, - VIR_CGROUP_CONTROLLER_CPU, + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPU); + + return virCgroupItemGetValueI64(group, "cpu.cfs_quota_us", cfs_quota); } -int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage) +int virCgroupGetCpuacctUsage(virCgroupItemPtr group, unsigned long long *usage) { - return virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_CPUACCT, - "cpuacct.usage", usage); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUACCT); + + return virCgroupItemGetValueU64(group, + "cpuacct.usage", usage); } -int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) +int virCgroupGetCpuacctPercpuUsage(virCgroupItemPtr group, char **usage) { - return virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, - "cpuacct.usage_percpu", usage); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUACCT); + + return virCgroupItemGetValueStr(group, + "cpuacct.usage_percpu", usage); } #ifdef _SC_CLK_TCK -int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, +int virCgroupGetCpuacctStat(virCgroupItemPtr group, unsigned long long *user, unsigned long long *sys) { char *str; @@ -2204,8 +1683,10 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, int ret; static double scale = -1.0; - if ((ret = virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, - "cpuacct.stat", &str)) < 0) + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_CPUACCT); + + if ((ret = virCgroupItemGetValueStr(group, + "cpuacct.stat", &str)) < 0) return ret; if (!(p = STRSKIP(str, "user ")) || virStrToLong_ull(p, &p, 10, user) < 0 || @@ -2234,7 +1715,7 @@ cleanup: return ret; } #else -int virCgroupGetCpuacctStat(virCgroupPtr group ATTRIBUTE_UNUSED, +int virCgroupGetCpuacctStat(virCgroupItemPtr group ATTRIBUTE_UNUSED, unsigned long long *user ATTRIBUTE_UNUSED, unsigned long long *sys ATTRIBUTE_UNUSED) { @@ -2242,33 +1723,33 @@ int virCgroupGetCpuacctStat(virCgroupPtr group ATTRIBUTE_UNUSED, } #endif -int virCgroupSetFreezerState(virCgroupPtr group, const char *state) +int virCgroupSetFreezerState(virCgroupItemPtr group, const char *state) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_FREEZER, - "freezer.state", state); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_FREEZER); + + return virCgroupItemSetValueStr(group, + "freezer.state", state); } -int virCgroupGetFreezerState(virCgroupPtr group, char **state) +int virCgroupGetFreezerState(virCgroupItemPtr group, char **state) { - return virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_FREEZER, - "freezer.state", state); + sa_assert(virCgroupItemType(group) == VIR_CGROUP_CONTROLLER_FREEZER); + + return virCgroupItemGetValueStr(group, + "freezer.state", state); } #if defined HAVE_KILL && defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R -static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids) +static int virCgroupKillInternal(virCgroupItemPtr group, int signum, virHashTablePtr pids) { int rc; int killedAny = 0; char *keypath = NULL; bool done = false; FILE *fp = NULL; - VIR_DEBUG("group=%p path=%s signum=%d pids=%p", - group, group->path, signum, pids); - rc = virCgroupPathOfController(group, -1, "tasks", &keypath); + rc = virCgroupItemKeyPath(group, "tasks", &keypath); if (rc != 0) { VIR_DEBUG("No path of %s, tasks", group->path); return rc; @@ -2345,7 +1826,7 @@ static void *virCgroupPidCopy(const void *name) * 0 : no PIDs killed * 1 : at least one PID killed */ -int virCgroupKill(virCgroupPtr group, int signum) +int virCgroupKill(virCgroupItemPtr group, int signum) { VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); int rc; @@ -2368,17 +1849,17 @@ int virCgroupKill(virCgroupPtr group, int signum) } -static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHashTablePtr pids, bool dormdir) +static int virCgroupKillRecursiveInternal(virCgroupItemPtr group, + int signum, + virHashTablePtr pids) { int rc; - int killedAny = 0; char *keypath = NULL; - DIR *dp; - virCgroupPtr subgroup = NULL; - struct dirent *ent; - VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); + virCgroupItemPtr subgroup = NULL; + VIR_DEBUG("group=%p path=%s signum=%d pids=%p", + group,group->path, signum, pids); - rc = virCgroupPathOfController(group, -1, "", &keypath); + rc = virCgroupItemKeyPath(group, "", &keypath); if (rc != 0) { VIR_DEBUG("No path of %s, tasks", group->path); return rc; @@ -2387,52 +1868,18 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas if ((rc = virCgroupKillInternal(group, signum, pids)) != 0) return rc; - VIR_DEBUG("Iterate over children of %s", keypath); - if (!(dp = opendir(keypath))) { - rc = -errno; - return rc; - } - - while ((ent = readdir(dp))) { - char *subpath; - - if (STREQ(ent->d_name, ".")) - continue; - if (STREQ(ent->d_name, "..")) - continue; - if (ent->d_type != DT_DIR) - continue; - - VIR_DEBUG("Process subdir %s", ent->d_name); - if (virAsprintf(&subpath, "%s/%s", group->path, ent->d_name) < 0) { - rc = -ENOMEM; - goto cleanup; - } - - if ((rc = virCgroupNew(subpath, &subgroup)) != 0) - goto cleanup; - - if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, true)) < 0) - goto cleanup; - if (rc == 1) - killedAny = 1; - - if (dormdir) - virCgroupRemove(subgroup); - - virCgroupFree(&subgroup); + subgroup = group->children; + while (subgroup) { + rc = virCgroupKillRecursiveInternal(subgroup, signum, pids); + if (rc != 0) + return rc; + subgroup = subgroup->next; } - rc = killedAny; - -cleanup: - virCgroupFree(&subgroup); - closedir(dp); - return rc; } -int virCgroupKillRecursive(virCgroupPtr group, int signum) +int virCgroupKillRecursive(virCgroupItemPtr group, int signum) { int rc; VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); @@ -2443,7 +1890,7 @@ int virCgroupKillRecursive(virCgroupPtr group, int signum) virCgroupPidCopy, NULL); - rc = virCgroupKillRecursiveInternal(group, signum, pids, false); + rc = virCgroupKillRecursiveInternal(group, signum, pids); virHashFree(pids); @@ -2451,7 +1898,7 @@ int virCgroupKillRecursive(virCgroupPtr group, int signum) } -int virCgroupKillPainfully(virCgroupPtr group) +int virCgroupKillPainfully(virCgroupItemPtr group) { int i; int rc; @@ -2478,18 +1925,18 @@ int virCgroupKillPainfully(virCgroupPtr group) } #else /* !(HAVE_KILL, HAVE_MNTENT_H, HAVE_GETMNTENT_R) */ -int virCgroupKill(virCgroupPtr group ATTRIBUTE_UNUSED, +int virCgroupKill(virCgroupItemPtr group ATTRIBUTE_UNUSED, int signum ATTRIBUTE_UNUSED) { return -ENOSYS; } -int virCgroupKillRecursive(virCgroupPtr group ATTRIBUTE_UNUSED, +int virCgroupKillRecursive(virCgroupItemPtr group ATTRIBUTE_UNUSED, int signum ATTRIBUTE_UNUSED) { return -ENOSYS; } -int virCgroupKillPainfully(virCgroupPtr group ATTRIBUTE_UNUSED) +int virCgroupKillPainfully(virCgroupItemPtr group ATTRIBUTE_UNUSED) { return -ENOSYS; } diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 0ba2430..a46b76e 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -27,8 +27,8 @@ #include "virutil.h" -struct virCgroup; -typedef struct virCgroup *virCgroupPtr; +typedef struct _virCgroup virCgroup; +typedef virCgroup *virCgroupPtr; typedef struct _virCgroupItem virCgroupItem; typedef virCgroupItem *virCgroupItemPtr; @@ -52,7 +52,7 @@ int virCgroupForDriver(const char *name, bool privileged, bool create); -int virCgroupGetAppRoot(virCgroupPtr *group); +virCgroupItemPtr virCgroupGetAppRoot(int type, bool privileged); int virCgroupForDomain(virCgroupPtr driver, const char *name, @@ -73,33 +73,28 @@ int virCgroupPathOfController(virCgroupPtr group, const char *key, char **path); -int virCgroupAddTask(virCgroupPtr group, pid_t pid); +int virCgroupItemAddTask(virCgroupItemPtr item, pid_t pid); -int virCgroupAddTaskController(virCgroupPtr group, - pid_t pid, - int controller); +int virCgroupItemMoveTask(virCgroupItemPtr src, + virCgroupItemPtr dest); -int virCgroupMoveTask(virCgroupPtr src_group, - virCgroupPtr dest_group, - int controller); +int virCgroupSetBlkioWeight(virCgroupItemPtr group, unsigned int weight); +int virCgroupGetBlkioWeight(virCgroupItemPtr group, unsigned int *weight); -int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); -int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); - -int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, +int virCgroupSetBlkioDeviceWeight(virCgroupItemPtr group, const char *path, unsigned int weight); -int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); -int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb); +int virCgroupSetMemory(virCgroupItemPtr group, unsigned long long kb); +int virCgroupGetMemoryUsage(virCgroupItemPtr group, unsigned long *kb); -int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb); +int virCgroupSetMemoryHardLimit(virCgroupItemPtr group, unsigned long long kb); +int virCgroupGetMemoryHardLimit(virCgroupItemPtr group, unsigned long long *kb); +int virCgroupSetMemorySoftLimit(virCgroupItemPtr group, unsigned long long kb); +int virCgroupGetMemorySoftLimit(virCgroupItemPtr group, unsigned long long *kb); +int virCgroupSetMemSwapHardLimit(virCgroupItemPtr group, unsigned long long kb); +int virCgroupGetMemSwapHardLimit(virCgroupItemPtr group, unsigned long long *kb); +int virCgroupGetMemSwapUsage(virCgroupItemPtr group, unsigned long long *kb); enum { VIR_CGROUP_DEVICE_READ = 1, @@ -109,65 +104,66 @@ enum { VIR_CGROUP_DEVICE_RWM = VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD, }; -int virCgroupDenyAllDevices(virCgroupPtr group); +int virCgroupDenyAllDevices(virCgroupItemPtr group); -int virCgroupAllowDevice(virCgroupPtr group, +int virCgroupAllowDevice(virCgroupItemPtr group, char type, int major, int minor, int perms); -int virCgroupAllowDeviceMajor(virCgroupPtr group, +int virCgroupAllowDeviceMajor(virCgroupItemPtr group, char type, int major, int perms); -int virCgroupAllowDevicePath(virCgroupPtr group, +int virCgroupAllowDevicePath(virCgroupItemPtr group, const char *path, int perms); -int virCgroupDenyDevice(virCgroupPtr group, +int virCgroupDenyDevice(virCgroupItemPtr group, char type, int major, int minor, int perms); -int virCgroupDenyDeviceMajor(virCgroupPtr group, +int virCgroupDenyDeviceMajor(virCgroupItemPtr group, char type, int major, int perms); -int virCgroupDenyDevicePath(virCgroupPtr group, +int virCgroupDenyDevicePath(virCgroupItemPtr group, const char *path, int perms); -int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares); -int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares); +int virCgroupSetCpuShares(virCgroupItemPtr group, unsigned long long shares); +int virCgroupGetCpuShares(virCgroupItemPtr group, unsigned long long *shares); -int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period); -int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period); +int virCgroupSetCpuCfsPeriod(virCgroupItemPtr group, unsigned long long cfs_period); +int virCgroupGetCpuCfsPeriod(virCgroupItemPtr group, unsigned long long *cfs_period); -int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota); -int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota); +int virCgroupSetCpuCfsQuota(virCgroupItemPtr group, long long cfs_quota); +int virCgroupGetCpuCfsQuota(virCgroupItemPtr group, long long *cfs_quota); -int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); -int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage); -int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, +int virCgroupGetCpuacctUsage(virCgroupItemPtr group, unsigned long long *usage); +int virCgroupGetCpuacctPercpuUsage(virCgroupItemPtr group, char **usage); +int virCgroupGetCpuacctStat(virCgroupItemPtr group, unsigned long long *user, unsigned long long *sys); -int virCgroupSetFreezerState(virCgroupPtr group, const char *state); -int virCgroupGetFreezerState(virCgroupPtr group, char **state); +int virCgroupSetFreezerState(virCgroupItemPtr group, const char *state); +int virCgroupGetFreezerState(virCgroupItemPtr group, char **state); -int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems); -int virCgroupGetCpusetMems(virCgroupPtr group, char **mems); +int virCgroupSetCpusetMems(virCgroupItemPtr group, const char *mems); +int virCgroupGetCpusetMems(virCgroupItemPtr group, char **mems); -int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus); -int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus); +int virCgroupSetCpusetCpus(virCgroupItemPtr group, const char *cpus); +int virCgroupGetCpusetCpus(virCgroupItemPtr group, char **cpus); int virCgroupRemove(virCgroupPtr group); +int virCgroupNew(const char *name, virCgroupPtr parent, virCgroupPtr *group); void virCgroupFree(virCgroupPtr *group); -bool virCgroupMounted(virCgroupPtr cgroup, int controller); +bool virCgroupMounted(int controller); -int virCgroupKill(virCgroupPtr group, int signum); -int virCgroupKillRecursive(virCgroupPtr group, int signum); -int virCgroupKillPainfully(virCgroupPtr group); +int virCgroupKill(virCgroupItemPtr group, int signum); +int virCgroupKillRecursive(virCgroupItemPtr group, int signum); +int virCgroupKillPainfully(virCgroupItemPtr group); virCgroupItemPtr virCgroupItemNew(int type, const char *name, virCgroupItemPtr parent); void virCgroupItemFree(virCgroupItemPtr cgroupItem); -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:08AM +0800, Hu Tao wrote:
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7cb99b1..92e3292 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1442,6 +1442,9 @@ int main(int argc, char **argv) { VIR_FORCE_CLOSE(statuswrite); }
+ if (virCgroupInit() < 0) + goto cleanup; +
I don't like this addition. Our aim has been to *remove* the need to global initializers like this, not add new ones. AFAICT the reason you needed to add this is because you removed code from the individual drivers which would initialize the cgroups they required.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2ac338c..23ff2c9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -48,6 +48,7 @@ # include "device_conf.h" # include "virbitmap.h" # include "virstoragefile.h" +# include "vircgroup.h"
/* forward declarations of all device types, required by * virDomainDeviceDef @@ -1843,6 +1844,10 @@ struct _virDomainDef {
/* Application-specific custom metadata */ xmlNodePtr metadata; + + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr (*vcpuCgroups)[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr emulatorCgroup[VIR_CGROUP_CONTROLLER_LAST]; };
Two things here. First, this is driver state and so should *not* be in the virDomainDef struct - it should be in the driver specific private data structs. Second, the new cgroups API you've got here is really bad. It was an explicit design decision in the original API to *not* expose the concept of individual cgroup controllers to the driver APIs. The only time the drivers should can about individual controllers is when they first create the cgroup and decide which controllers they want to use. From then onwards the virCgroupPtr APIs should just 'do the right thing'.
}
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); @@ -300,6 +301,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; + if (vm->def->blkio.weight != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { rc = virCgroupSetBlkioWeight(cgroup, vm->def->blkio.weight); @@ -339,6 +342,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { unsigned long long hard_limit = vm->def->mem.hard_limit;
@@ -392,6 +397,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name); }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if (vm->def->cputune.shares != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { rc = virCgroupSetCpuShares(cgroup, vm->def->cputune.shares); @@ -407,6 +414,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET]; + if ((vm->def->numatune.memory.nodemask || (vm->def->numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)) &&
These 4 additions are a perfect example of what this new API design is awful. The drivers now have to remember which controller they need to use for which tunable. I'm not going to review any more because this change is fatally flawed from a design POV. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Jan 16, 2013 at 10:10:50AM +0000, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:08AM +0800, Hu Tao wrote:
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7cb99b1..92e3292 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1442,6 +1442,9 @@ int main(int argc, char **argv) { VIR_FORCE_CLOSE(statuswrite); }
+ if (virCgroupInit() < 0) + goto cleanup; +
I don't like this addition. Our aim has been to *remove* the need to global initializers like this, not add new ones. AFAICT the reason you needed to add this is because you removed code from the individual drivers which would initialize the cgroups they required.
I think I can eliminate this init/uninit thing.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2ac338c..23ff2c9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -48,6 +48,7 @@ # include "device_conf.h" # include "virbitmap.h" # include "virstoragefile.h" +# include "vircgroup.h"
/* forward declarations of all device types, required by * virDomainDeviceDef @@ -1843,6 +1844,10 @@ struct _virDomainDef {
/* Application-specific custom metadata */ xmlNodePtr metadata; + + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr (*vcpuCgroups)[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr emulatorCgroup[VIR_CGROUP_CONTROLLER_LAST]; };
Two things here. First, this is driver state and so should *not* be in the virDomainDef struct - it should be in the driver specific private data structs.
Agreed.
Second, the new cgroups API you've got here is really bad. It was an explicit design decision in the original API to *not* expose the concept of individual cgroup controllers to the driver APIs. The only time the drivers should can about individual controllers is when they first create the cgroup and decide which controllers they want to use. From then onwards the virCgroupPtr APIs should just 'do the right thing'.
The explanation is helpful. Fortunately I think the new virCgroup can leave the original API unchanged(let me try in v2). What are important in the new virCgroup are: - the lazy creation of cgroup directories, despite of what level they are. - cgroup directories are removed if no one is using the corresponding virCgroup. Do you think it's worth doing it? If yes, can you review patch 5 about the new implementation? (forget about the API change)
}
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_DEVICES]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { qemuCgroupData data = { vm, cgroup }; rc = virCgroupDenyAllDevices(cgroup); @@ -300,6 +301,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_BLKIO]; + if (vm->def->blkio.weight != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { rc = virCgroupSetBlkioWeight(cgroup, vm->def->blkio.weight); @@ -339,6 +342,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_MEMORY]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { unsigned long long hard_limit = vm->def->mem.hard_limit;
@@ -392,6 +397,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name); }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPU]; + if (vm->def->cputune.shares != 0) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { rc = virCgroupSetCpuShares(cgroup, vm->def->cputune.shares); @@ -407,6 +414,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, } }
+ cgroup = vm->def->cgroup[VIR_CGROUP_CONTROLLER_CPUSET]; + if ((vm->def->numatune.memory.nodemask || (vm->def->numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO)) &&
These 4 additions are a perfect example of what this new API design is awful. The drivers now have to remember which controller they need to use for which tunable.
If the original API is kept, these will be the form of: cgroup = vm->cgroup; ... doSomethingWithCgroup(cgroup) ... Is this acceptable?
I'm not going to review any more because this change is fatally flawed from a design POV.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Jan 17, 2013 at 12:02:52PM +0800, Hu Tao wrote:
On Wed, Jan 16, 2013 at 10:10:50AM +0000, Daniel P. Berrange wrote:
On Wed, Jan 16, 2013 at 10:53:08AM +0800, Hu Tao wrote:
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7cb99b1..92e3292 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1442,6 +1442,9 @@ int main(int argc, char **argv) { VIR_FORCE_CLOSE(statuswrite); }
+ if (virCgroupInit() < 0) + goto cleanup; +
I don't like this addition. Our aim has been to *remove* the need to global initializers like this, not add new ones. AFAICT the reason you needed to add this is because you removed code from the individual drivers which would initialize the cgroups they required.
I think I can eliminate this init/uninit thing.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2ac338c..23ff2c9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -48,6 +48,7 @@ # include "device_conf.h" # include "virbitmap.h" # include "virstoragefile.h" +# include "vircgroup.h"
/* forward declarations of all device types, required by * virDomainDeviceDef @@ -1843,6 +1844,10 @@ struct _virDomainDef {
/* Application-specific custom metadata */ xmlNodePtr metadata; + + virCgroupItemPtr cgroup[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr (*vcpuCgroups)[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupItemPtr emulatorCgroup[VIR_CGROUP_CONTROLLER_LAST]; };
Two things here. First, this is driver state and so should *not* be in the virDomainDef struct - it should be in the driver specific private data structs.
Agreed.
Second, the new cgroups API you've got here is really bad. It was an explicit design decision in the original API to *not* expose the concept of individual cgroup controllers to the driver APIs. The only time the drivers should can about individual controllers is when they first create the cgroup and decide which controllers they want to use. From then onwards the virCgroupPtr APIs should just 'do the right thing'.
The explanation is helpful. Fortunately I think the new virCgroup can leave the original API unchanged(let me try in v2).
What are important in the new virCgroup are:
- the lazy creation of cgroup directories, despite of what level they are. - cgroup directories are removed if no one is using the corresponding virCgroup.
Do you think it's worth doing it? If yes, can you review patch 5 about the new implementation? (forget about the API change)
Ok, that sounds more reasonable. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- tests/Makefile.am | 5 +++ tests/vircgrouptest.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 tests/vircgrouptest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 61b0a0c..b2ccdc1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -99,6 +99,7 @@ test_programs = virshtest sockettest \ virlockspacetest \ virstringtest \ sysinfotest \ + vircgrouptest \ $(NULL) if WITH_GNUTLS @@ -640,6 +641,10 @@ utiltest_SOURCES = \ utiltest.c testutils.h testutils.c utiltest_LDADD = $(LDADDS) +vircgrouptest_SOURCES = \ + vircgrouptest.c testutils.h testutils.c +vircgrouptest_LDADD = $(LDADDS) + if WITH_DRIVER_MODULES virdrivermoduletest_SOURCES = \ virdrivermoduletest.c testutils.h testutils.c diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c new file mode 100644 index 0000000..8d0387b --- /dev/null +++ b/tests/vircgrouptest.c @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2012 Fujitsu. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "testutils.h" + +#include "vircgroup.h" + +static int test_cgroup(const void *data ATTRIBUTE_UNUSED) +{ + virCgroupItemPtr cpuset; + + cpuset = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, + "test_cgroup", + virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET, false)); + if (!cpuset) + return -1; + + if (virCgroupSetCpusetCpus(cpuset, "1") < 0) { + virCgroupItemFree(cpuset); + return -1; + } + + virCgroupItemFree(cpuset); + + return 0; +} + +static int test_child_cgroup(const void *data ATTRIBUTE_UNUSED) +{ + int ret = -1; + virCgroupItemPtr item = NULL, item1 = NULL, item2 = NULL; + char *cpus; + + item = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, + "test_child_cgroup", + virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET, false)); + if (!item) + goto out; + + item1 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, "child1", item); + item2 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, "child2", item1); + + if (virCgroupGetCpusetCpus(item2, &cpus) < 0) + goto out; + + VIR_FREE(cpus); + + if (virCgroupSetCpusetCpus(item2, "0") < 0) + goto out; + + if (virCgroupGetCpusetCpus(item2, &cpus) < 0) + goto out; + + VIR_FREE(cpus); + + ret = 0; +out: + if (item) + virCgroupItemFree(item); + if (item1) + virCgroupItemFree(item1); + if (item2) + virCgroupItemFree(item2); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if (virCgroupInit() < 0) + return -1; + + if (virtTestRun("test_cgroup", 1, test_cgroup, NULL) < 0) + ret = -1; + if (virtTestRun("test_child_cgroup", 1, test_child_cgroup, NULL) < 0) + ret = -1; + + virCgroupUninit(); + + return ret; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.0.1.240.ge8a1f5a

On Wed, Jan 16, 2013 at 10:53:09AM +0800, Hu Tao wrote:
--- tests/Makefile.am | 5 +++ tests/vircgrouptest.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 tests/vircgrouptest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am index 61b0a0c..b2ccdc1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -99,6 +99,7 @@ test_programs = virshtest sockettest \ virlockspacetest \ virstringtest \ sysinfotest \ + vircgrouptest \ $(NULL)
if WITH_GNUTLS @@ -640,6 +641,10 @@ utiltest_SOURCES = \ utiltest.c testutils.h testutils.c utiltest_LDADD = $(LDADDS)
+vircgrouptest_SOURCES = \ + vircgrouptest.c testutils.h testutils.c +vircgrouptest_LDADD = $(LDADDS) + if WITH_DRIVER_MODULES virdrivermoduletest_SOURCES = \ virdrivermoduletest.c testutils.h testutils.c diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c new file mode 100644 index 0000000..8d0387b --- /dev/null +++ b/tests/vircgrouptest.c @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2012 Fujitsu. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "testutils.h" + +#include "vircgroup.h" + +static int test_cgroup(const void *data ATTRIBUTE_UNUSED) +{ + virCgroupItemPtr cpuset; + + cpuset = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, + "test_cgroup", + virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET, false)); + if (!cpuset) + return -1; + + if (virCgroupSetCpusetCpus(cpuset, "1") < 0) { + virCgroupItemFree(cpuset); + return -1; + } + + virCgroupItemFree(cpuset); + + return 0; +} + +static int test_child_cgroup(const void *data ATTRIBUTE_UNUSED) +{ + int ret = -1; + virCgroupItemPtr item = NULL, item1 = NULL, item2 = NULL; + char *cpus; + + item = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, + "test_child_cgroup", + virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET, false)); + if (!item) + goto out; + + item1 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, "child1", item); + item2 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, "child2", item1); + + if (virCgroupGetCpusetCpus(item2, &cpus) < 0) + goto out; + + VIR_FREE(cpus); + + if (virCgroupSetCpusetCpus(item2, "0") < 0) + goto out; + + if (virCgroupGetCpusetCpus(item2, &cpus) < 0) + goto out; + + VIR_FREE(cpus); + + ret = 0; +out: + if (item) + virCgroupItemFree(item); + if (item1) + virCgroupItemFree(item1); + if (item2) + virCgroupItemFree(item2); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if (virCgroupInit() < 0) + return -1; + + if (virtTestRun("test_cgroup", 1, test_cgroup, NULL) < 0) + ret = -1; + if (virtTestRun("test_child_cgroup", 1, test_child_cgroup, NULL) < 0) + ret = -1; + + virCgroupUninit(); + + return ret; +} + +VIRT_TEST_MAIN(mymain)
This test appears to be interacting with host system state which is not something we can safely do from unit tests. If you want to test out cgroups, then the only option is to make use of an LD_PRELOAD hack to override the various system calls that the virCgroup APIs do - eg see what we did with tests/securityselinuxhelper.c to fake some system calls. This might be quite alot of hard work for cgroups though. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Jan 16, 2013 at 10:53:02AM +0800, Hu Tao wrote:
Hi,
This series is posted for early review.
This series refactors virCgroup. The changes are:
- virCgroupItem is associated with a cgroup directory. The directory is created only when needed, and removed if no one is using it. - Anyone using cgroups creates instances of virCgroupItem and maintains their lifetime.
Please focus on patch #5, which brings the main change(virCgroupItem), and qemu part in patch #6, which shows the usage of virCgroupItem(I've not tested lxc part yet).
I'm really not to clear on what the actual functional benefit or changes of this series are, so it is hard to know how to suggest changes to it. I do know that I don't like the design you have here though, no matter what the goal. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Hu Tao