On 06/21/2011 10:47 AM, Cole Robinson wrote:
Currently a user can connect to lxc:/// if cgroups aren't mounted, but they can't do a whole lot: starting and even stopping guests doesn't work (the latter only if cgroups were unmounted behind libvirts back). To make
s/libvirts/libvirt's/
matters worse, even after mounting cgroups, libvirt must be restarted to actually notice.
This is frustrating for users who may make it all the way to the end of the virt-manager 'New VM' wizard only to receive an error that requires a libvirtd restart.
Fix this by checking for cgroup mounts at lxc:/// connect time, and if none are found, fail the connection.
I'm not sure if there are any negative consequences to putting this logic in lxcOpen...
I'm not thinking of any; at any rate, this seems like a reasonable change. But there are some issues that probably require a v2:
+++ b/src/lxc/lxc_driver.c @@ -107,6 +107,22 @@ static void lxcDomainEventFlush(int timer, void *opaque); static void lxcDomainEventQueue(lxc_driver_t *driver, virDomainEventPtr event);
+static int lxcGetCgroup(lxc_driver_t *driver) +{ + int privileged = 1;
Why create this variable, if it never changes from the constant of 1? Is it even possible to have a non-privileged lxc driver instance, and if so, shouldn't we be getting this value from driver?
+ + if (driver->cgroup) + return 0; + + int rc = virCgroupForDriver("lxc", &driver->cgroup, privileged, 1); + if (rc < 0) { + char buf[1024]; + VIR_DEBUG("Unable to create cgroup for LXC driver: %s", + virStrerror(-rc, buf, sizeof(buf)));
Unrelated to your patch, but I can't help but wonder if we should change virStrerror into taking one argument and always returning a thread-local string, rather than making callers pass a stack-allocated buffer.
@@ -2113,11 +2136,7 @@ static int lxcStartup(int 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]; - VIR_DEBUG("Unable to create cgroup for LXC driver: %s", - virStrerror(-rc, buf, sizeof(buf))); + if (lxcGetCgroup(lxc_driver) < 0) {
Hmm, this makes it look like lxcGetCgroup should take a second parameter, privileged. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org