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