Re: [libvirt] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up

Quoting Daniel P. Berrange (berrange(a)redhat.com): ... Thanks, Daniel. You're right! This is fixed in git, by the patch 'lxc: controller: Improve container error reporting' (which does much more than it says :). The following patch is how I had just fixed 0.9.2 this morning. It'll be nicer if I can get the git commit cherrypicked. I can't wait till I can upgrade! thanks, -serge Description: Make lxc driver hold sem until controller is far enough The lxc driver currently does not wait until the container has set up its cgroups before dropping the driver mutex. First, move the controller's accept of the monitor socket wait until it has set up the cgroups. Second, because a connect does not actually wait for an accept to happen, force the driver to wait with a silly two-way read/write handshake. Since the monitor socket is also used elsewhere, make this handshake happen everywhere. Author: Serge Hallyn <serge.hallyn(a)canonical.com&gt; Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nova/+bug/842845 Forwarded: not-needed Index: libvirt-0.9.2/src/lxc/lxc_controller.c =================================================================== --- libvirt-0.9.2.orig/src/lxc/lxc_controller.c 2011-10-03 13:10:31.098934902 -0500 +++ libvirt-0.9.2/src/lxc/lxc_controller.c 2011-10-03 13:10:53.823679619 -0500 @@ -432,6 +432,8 @@ numEvents = epoll_wait(epollFd, &epollEvent, 1, timeout); if (numEvents > 0) { if (epollEvent.data.fd == monitor) { + int ret; + char go[4]; int fd = accept(monitor, NULL, 0); if (fd < 0) { /* First reflex may be simply to declare accept failure @@ -457,6 +459,17 @@ _("epoll_ctl(client) failed")); goto cleanup; } + ret = read(client, go, 3); + if (ret < 0) { + virReportSystemError(errno, "%s", + _("Failed to read 'go' from driver")); + goto cleanup; + } + ret = write(client, "go", 3); + if (ret < 0) { + virReportSystemError(errno, "%s", _("Failed to write 'go' to container")); + goto cleanup; + } } else if (client != -1 && epollEvent.data.fd == client) { if (0 > epoll_ctl(epollFd, EPOLL_CTL_DEL, client, &epollEvent)) { virReportSystemError(errno, "%s", @@ -611,10 +624,9 @@ unsigned int nveths, char **veths, int monitor, - int client, int appPty) { - int rc = -1; + int ret, rc = -1; int control[2] = { -1, -1}; int containerPty = -1; char *containerPtyPath = NULL; @@ -622,6 +634,8 @@ virDomainFSDefPtr root; char *devpts = NULL; char *devptmx = NULL; + char go[4]; + int client; if (socketpair(PF_UNIX, SOCK_STREAM, 0, control) < 0) { virReportSystemError(errno, "%s", @@ -631,8 +645,29 @@ root = virDomainGetRootFilesystem(def); + VIR_DEBUG("About to set resources and cgroups\n"); if (lxcSetContainerResources(def) < 0) goto cleanup; + VIR_DEBUG("Done setting resources and cgroups\n"); + + /* Accept initial client which is the libvirtd daemon */ + if ((client = accept(monitor, NULL, 0)) < 0) { + virReportSystemError(errno, "%s", + _("Failed to accept a connection from driver")); + goto cleanup; + } + VIR_DEBUG("Accepted monitor fd from driver\n"); + ret = read(client, go, 3); + if (ret < 0) { + virReportSystemError(errno, "%s", + _("Failed to read 'go' from driver")); + goto cleanup; + } + ret = write(client, "go", 3); + if (ret < 0) { + virReportSystemError(errno, "%s", _("Failed to write 'go' to container")); + goto cleanup; + } /* * If doing a chroot style setup, we need to prepare @@ -765,7 +800,6 @@ { pid_t pid; int rc = 1; - int client; char *name = NULL; int nveths = 0; char **veths = NULL; @@ -922,14 +956,7 @@ /* Initialize logging */ virLogSetFromEnv(); - /* Accept initial client which is the libvirtd daemon */ - if ((client = accept(monitor, NULL, 0)) < 0) { - virReportSystemError(errno, "%s", - _("Failed to accept a connection from driver")); - goto cleanup; - } - - rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty); + rc = lxcControllerRun(def, nveths, veths, monitor, appPty); cleanup: Index: libvirt-0.9.2/src/lxc/lxc_driver.c =================================================================== --- libvirt-0.9.2.orig/src/lxc/lxc_driver.c 2011-10-03 13:10:27.608663571 -0500 +++ libvirt-0.9.2/src/lxc/lxc_driver.c 2011-10-03 13:10:53.823679619 -0500 @@ -1160,8 +1160,9 @@ virDomainObjPtr vm) { char *sockpath = NULL; - int fd; + int fd, r; struct sockaddr_un addr; + char go[4]; if (virAsprintf(&sockpath, "%s/%s.sock", driver->stateDir, vm->def->name) < 0) { @@ -1189,6 +1190,17 @@ goto error; } + r = write(fd, "go", 3); + if (r < 0) { + virReportSystemError(errno, "%s", _("Failed to write 'go' to container")); + goto error; + } + r = read(fd, go, 3); + if (r < 0) { + virReportSystemError(errno, "%s", _("Failed to read 'go' from container")); + goto error; + } + VIR_FREE(sockpath); return fd; @@ -1491,6 +1503,7 @@ * pid file out to disk */ if ((priv->monitor = lxcMonitorClient(driver, vm)) < 0) goto cleanup; + VIR_DEBUG("driver: got the monitor socket from client\n"); /* And get its pid */ if ((r = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) != 0) {