Re: [libvirt] [PATCH 2/2] lxc: Fail connection attempt if cgroups not mounted
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)