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

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

 



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 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]