[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] PATCH: Fix infinite loop when QEMU quits at startup



The recent refactoring of the QEMU startup process now reads the monitor
TTY from the logfile. Unfortunately in this refactoring we lost the check
for the 'ret == 0' scenario in the read() return value. So if QEMU quits
at startup, eg  due to missing disk image, we loop forever on read() == 0
because we hit end-of-file and QEMU has quit. 

This patch adds back handling for this scenario, and takes care to 
propagate the contents of the log to the user as an error message

 # start demo
libvir: QEMU error : internal error unable to start guest: qemu: could 
  not open disk image /home/berrange/Fedora-9-i686-Live.iso
error: Failed to start domain demo

In addition, there were a couple of other bugs


 - a memory leak where we set the 'monitorpath' variable, even 
   though we'd just set it moments before.

 - a missing check for whether the driver VNC password was present
   when initializing passwords at VM startupo

 - missing initialization of the monitor_watch field, and missing
   checking for whether the watch was set before removing it.

 - a gratuitous LOG_INFO when shutting down any VM, which could
   just be LOG_DEBUG.

Daniel

diff -r 826e6ed70ee0 src/domain_conf.c
--- a/src/domain_conf.c	Fri Jan 30 10:58:34 2009 +0000
+++ b/src/domain_conf.c	Fri Jan 30 11:00:43 2009 +0000
@@ -497,6 +497,7 @@ virDomainObjPtr virDomainAssignDef(virCo
     virDomainObjLock(domain);
     domain->state = VIR_DOMAIN_SHUTOFF;
     domain->def = def;
+    domain->monitor_watch = -1;
 
     if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
         virReportOOMError(conn);
diff -r 826e6ed70ee0 src/qemu_driver.c
--- a/src/qemu_driver.c	Fri Jan 30 10:58:34 2009 +0000
+++ b/src/qemu_driver.c	Fri Jan 30 11:00:43 2009 +0000
@@ -355,10 +355,9 @@ qemudReconnectVMs(struct qemud_driver *d
             qemudLog(QEMUD_ERR, _("Failed to reconnect monitor for %s: %d\n"),
                      vm->def->name, rc);
             goto next_error;
-        } else
-            vm->monitorpath = status->monitorpath;
-
-        if((vm->logfile = qemudLogFD(NULL, driver->logDir, vm->def->name)) < 0)
+        }
+
+        if ((vm->logfile = qemudLogFD(NULL, driver->logDir, vm->def->name)) < 0)
             return -1;
 
         if (vm->def->id >= driver->nextvmid)
@@ -376,6 +375,8 @@ next_error:
         vm->newDef = NULL;
 next:
         virDomainObjUnlock(vm);
+        if (status)
+            VIR_FREE(status->monitorpath);
         VIR_FREE(status);
         VIR_FREE(config);
     }
@@ -617,6 +618,9 @@ typedef int qemudHandlerMonitorOutput(vi
                                       const char *output,
                                       int fd);
 
+/*
+ * Returns -1 for error, 0 on end-of-file, 1 for success
+ */
 static int
 qemudReadMonitorOutput(virConnectPtr conn,
                        virDomainObjPtr vm,
@@ -630,7 +634,7 @@ qemudReadMonitorOutput(virConnectPtr con
     int got = 0;
     buf[0] = '\0';
 
-   /* Consume & discard the initial greeting */
+    /* Consume & discard the initial greeting */
     while (got < (buflen-1)) {
         int ret;
 
@@ -670,11 +674,17 @@ qemudReadMonitorOutput(virConnectPtr con
                                  _("Failure while reading %s startup output"), what);
                 return -1;
             }
+        } else if (ret == 0) {
+            return 0;
         } else {
             got += ret;
             buf[got] = '\0';
-            if ((ret = func(conn, vm, buf, fd)) != 1)
-                return ret;
+            ret = func(conn, vm, buf, fd);
+            if (ret == -1)
+                return -1;
+            if (ret == 1)
+                continue;
+            return 1;
         }
     }
 
@@ -724,11 +734,14 @@ static int qemudOpenMonitor(virConnectPt
     }
 
     if (!reconnect) {
-        ret = qemudReadMonitorOutput(conn,
-                                     vm, monfd,
-                                     buf, sizeof(buf),
-                                     qemudCheckMonitorPrompt,
-                                     "monitor", 10000);
+        if (qemudReadMonitorOutput(conn,
+                                   vm, monfd,
+                                   buf, sizeof(buf),
+                                   qemudCheckMonitorPrompt,
+                                   "monitor", 10000) <= 0)
+            ret = -1;
+        else
+            ret = 0;
     } else {
         vm->monitor = monfd;
         ret = 0;
@@ -858,7 +871,7 @@ static int qemudWaitForMonitor(virConnec
 
     if ((logfd = qemudLogReadFD(conn, driver->logDir, vm->def->name, pos))
         < 0)
-        return logfd;
+        return -1;
 
     ret = qemudReadMonitorOutput(conn, vm, logfd, buf, sizeof(buf),
                                  qemudFindCharDevicePTYs,
@@ -866,7 +879,17 @@ static int qemudWaitForMonitor(virConnec
     if (close(logfd) < 0)
         qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
                  strerror(errno));
-    return ret;
+
+    if (ret == 1) /* Success */
+        return 0;
+
+    if (ret == -1)
+        return -1;
+
+    /* Unexpected end of file - inform user of QEMU log data */
+    qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                     _("unable to start guest: %s"), buf);
+    return -1;
 }
 
 static int
@@ -1033,7 +1056,7 @@ qemudInitPasswords(virConnectPtr conn,
 
     if (vm->def->graphics &&
         vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
-        vm->def->graphics->data.vnc.passwd) {
+        (vm->def->graphics->data.vnc.passwd || driver->vncPassword)) {
 
         if (qemudMonitorCommandExtra(vm, "change vnc password",
                                      vm->def->graphics->data.vnc.passwd ?
@@ -1212,14 +1235,25 @@ static int qemudStartVMDaemon(virConnect
     /* wait for qemu process to to show up */
     if (ret == 0) {
         int retries = 100;
-        while (retries) {
-            if ((ret = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
-                break;
-            usleep(100*1000);
-            retries--;
-        }
-        if (ret)
-            qemudLog(QEMUD_WARN, _("Domain %s didn't show up\n"), vm->def->name);
+        int childstat;
+
+        while (waitpid(child, &childstat, 0) == -1 &&
+               errno == EINTR);
+
+        if (childstat == 0) {
+            while (retries) {
+                if ((ret = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
+                    break;
+                usleep(100*1000);
+                retries--;
+            }
+            if (ret)
+                qemudLog(QEMUD_WARN, _("Domain %s didn't show up\n"), vm->def->name);
+        } else {
+            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                             _("Unable to daemonize QEMU process"));
+            ret = -1;
+        }
     }
 
     if (ret == 0) {
@@ -1262,14 +1296,17 @@ static void qemudShutdownVMDaemon(virCon
     if (!virDomainIsActive(vm))
         return;
 
-    qemudLog(QEMUD_INFO, _("Shutting down VM '%s'\n"), vm->def->name);
+    qemudLog(QEMUD_DEBUG, _("Shutting down VM '%s'\n"), vm->def->name);
 
     if (virKillProcess(vm->pid, 0) == 0 &&
         virKillProcess(vm->pid, SIGTERM) < 0)
         qemudLog(QEMUD_ERROR, _("Failed to send SIGTERM to %s (%d): %s\n"),
                  vm->def->name, vm->pid, strerror(errno));
 
-    virEventRemoveHandle(vm->monitor_watch);
+    if (vm->monitor_watch != -1) {
+        virEventRemoveHandle(vm->monitor_watch);
+        vm->monitor_watch = -1;
+    }
 
     if (close(vm->logfile) < 0)
         qemudLog(QEMUD_WARN, _("Unable to close logfile %d: %s\n"),


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]