Misc User Mode Linux startup/shutdown bugs

diff --git a/ChangeLog b/ChangeLog
index 9cd271648f6e58e6fecf3121d53fc117db81d80c..d8d8c1a0269aafcee8a1d6ca24dd041598ba7d48 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Wed Jun  3 11:53:52 BST 2009 Daniel P. Berrange <berrange@redhat.com>
+
+       Misc User Mode Linux startup/shutdown bugs
+       * src/util.c: Make virKillProcess refuse to kill PID==1 too
+       * src/uml_driver.c: Increment retries counter when starting
+       guest. Check for monitor command failure. Use unique source
+       socket per domain for talking to monitor.  Use virKillProcess
+       for killing guests. Fix guest cleanup when getting inotify
+
 Wed Jun  3 11:32:52 BST 2009 Daniel P. Berrange <berrange@redhat.com>
 
        * qemud/qemud.c: Honour LIBVIRT_LOG_FILTERS/OUTPUTS env variables

diff --git a/src/uml_driver.c b/src/uml_driver.c
index 4cb8eaabde8fb14babc9cbc5dc66faa37bbff505..be03567ab7db65807b33178054af313c1b04515a 100644 (file)
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -166,9 +166,10 @@ umlIdentifyOneChrPTY(virConnectPtr conn,
         return -1;
     }
 requery:
-    umlMonitorCommand(NULL, driver, dom, cmd, &res);
+    if (umlMonitorCommand(NULL, driver, dom, cmd, &res) < 0)
+        return -1;
 
-    if (STRPREFIX(res, "pts:")) {
+    if (res && STRPREFIX(res, "pts:")) {
         VIR_FREE(def->data.file.path);
         if ((def->data.file.path = strdup(res + 4)) == NULL) {
             virReportOOMError(conn);
@@ -176,7 +177,7 @@ requery:
             VIR_FREE(cmd);
             return -1;
         }
-    } else if (STRPREFIX(res, "pts")) {
+    } else if (!res || STRPREFIX(res, "pts")) {
         /* It can take a while to startup, so retry for
            upto 5 seconds */
         /* XXX should do this in a better non-blocking
@@ -263,19 +264,15 @@ reread:
         }
 
         if (e->mask & IN_DELETE) {
+            VIR_DEBUG("Got inotify domain shutdown '%s'", name);
             if (!virDomainIsActive(dom)) {
                 virDomainObjUnlock(dom);
                 continue;
             }
 
-            dom->def->id = -1;
-            dom->pid = -1;
-            if (dom->newDef) {
-                virDomainDefFree(dom->def);
-                dom->def = dom->newDef;
-            }
-            dom->state = VIR_DOMAIN_SHUTOFF;
+            umlShutdownVMDaemon(NULL, driver, dom);
         } else if (e->mask & (IN_CREATE | IN_MODIFY)) {
+            VIR_DEBUG("Got inotify domain startup '%s'", name);
             if (virDomainIsActive(dom)) {
                 virDomainObjUnlock(dom);
                 continue;
@@ -289,11 +286,13 @@ reread:
             dom->def->id = driver->nextvmid++;
             dom->state = VIR_DOMAIN_RUNNING;
 
-            if (umlOpenMonitor(NULL, driver, dom) < 0)
+            if (umlOpenMonitor(NULL, driver, dom) < 0) {
+                VIR_WARN0("Could not open monitor for new domain");
                 umlShutdownVMDaemon(NULL, driver, dom);
-
-            if (umlIdentifyChrPTY(NULL, driver, dom) < 0)
+            } else if (umlIdentifyChrPTY(NULL, driver, dom) < 0) {
+                VIR_WARN0("Could not identify charater devices for new domain");
                 umlShutdownVMDaemon(NULL, driver, dom);
+            }
         }
         virDomainObjUnlock(dom);
     }
@@ -383,6 +382,7 @@ umlStartup(void) {
         goto error;
     }
 
+    VIR_INFO("Adding inotify watch on %s", uml_driver->monitorDir);
     if (inotify_add_watch(uml_driver->inotifyFD,
                           uml_driver->monitorDir,
                           IN_CREATE | IN_MODIFY | IN_DELETE) < 0) {
@@ -595,10 +595,11 @@ static int umlOpenMonitor(virConnectPtr conn,
     if (umlMonitorAddress(conn, driver, vm, &addr) < 0)
         return -1;
 
+    VIR_DEBUG("Dest address for monitor is '%s'", addr.sun_path);
 restat:
     if (stat(addr.sun_path, &sb) < 0) {
         if (errno == ENOENT &&
-            retries < 50) {
+            retries++ < 50) {
             usleep(1000 * 100);
             goto restat;
         }
@@ -612,7 +613,8 @@ restat:
     }
 
     memset(addr.sun_path, 0, sizeof addr.sun_path);
-    sprintf(addr.sun_path + 1, "%u", getpid());
+    sprintf(addr.sun_path + 1, "libvirt-uml-%u", vm->pid);
+    VIR_DEBUG("Reply address for monitor is '%s'", addr.sun_path+1);
     if (bind(vm->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) {
         virReportSystemError(conn, errno,
                              "%s", _("cannot bind socket"));
@@ -657,6 +659,8 @@ static int umlMonitorCommand(virConnectPtr conn,
     struct sockaddr_un addr;
     unsigned int addrlen;
 
+    VIR_DEBUG("Run command '%s'", cmd);
+
     *reply = NULL;
 
     if (umlMonitorAddress(conn, driver, vm, &addr) < 0)
@@ -706,6 +710,8 @@ static int umlMonitorCommand(virConnectPtr conn,
 
     } while (res.extra);
 
+    VIR_DEBUG("Command reply is '%s'", NULLSTR(retdata));
+
     *reply = retdata;
 
     return ret;
@@ -852,12 +858,10 @@ static void umlShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
                                 virDomainObjPtr vm)
 {
     int ret;
-    if (!virDomainIsActive(vm) ||
-        vm->pid <= 1)
+    if (!virDomainIsActive(vm))
         return;
 
-
-    kill(vm->pid, SIGTERM);
+    virKillProcess(vm->pid, SIGTERM);
 
     if (vm->monitor != -1)
         close(vm->monitor);

diff --git a/src/util.c b/src/util.c
index b7fedc58b8860a7d52a5d52bd3de26367d2b0acf..3a8c105c76e875df70e0e9e24c029854fcea1f4e 100644 (file)
--- a/src/util.c
+++ b/src/util.c
@@ -1680,7 +1680,7 @@ char *virGetHostname(void)
 /* send signal to a single process */
 int virKillProcess(pid_t pid, int sig)
 {
-    if (pid < 1) {
+    if (pid <= 1) {
         errno = ESRCH;
         return -1;
     }