Re: [libvirt] [PATCH 3/4] Add the monitor type to the domain state XML

There are no functional changes in this patch apart from adding the monitor type to the state XML. The patch mostly consists of switching to use virDomainChrDef every where to describe the monitor. * src/domain_conf.h: replace monitorpath with monitor_chr * src/domain_conf.c: handle parsing the monitor type and initializing monitor chr * src/qemu_conf.[ch]: make qemudBuildCommandLine take a virDomainChrDefPtr and use that to build the -monitor parameter * src/qemu_driver.c: split pty specific and common code from qemudOpenMonitor, have qemudStartVMDaemon() initialize monitor_chr * tests/qemuxml2argvtest.c: update for qemudBuildCommandLine() change

--- src/domain_conf.c | 46 ++++++++++++++++++++-- src/domain_conf.h | 2 +- src/qemu_conf.c | 12 +++++- src/qemu_conf.h | 1 + src/qemu_driver.c | 96 ++++++++++++++++++++++++++++++++------------- tests/qemuxml2argvtest.c | 6 ++- 6 files changed, 127 insertions(+), 36 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index cc8c3ef..8e8b076 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -516,7 +516,8 @@ void virDomainObjFree(virDomainObjPtr dom) virDomainDefFree(dom->def); virDomainDefFree(dom->newDef); - VIR_FREE(dom->monitorpath); + virDomainChrDefFree(dom->monitor_chr); + VIR_FREE(dom->vcpupids); virMutexDestroy(&dom->lock); @@ -2890,6 +2891,7 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn, xmlNodePtr config; xmlNodePtr oldnode; virDomainObjPtr obj; + char *monitorpath; if (!(obj = virDomainObjNew(conn))) return NULL; @@ -2927,16 +2929,41 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn, } obj->pid = (pid_t)val; - if(!(obj->monitorpath = - virXPathString(conn, "string(./monitor[1]/@path)", ctxt))) { + if (VIR_ALLOC(obj->monitor_chr) < 0) { + virReportOOMError(conn); + goto error; + } + + if (!(monitorpath = + virXPathString(conn, "string(./monitor[1]/@path)", ctxt))) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("no monitor path")); goto error; } + tmp = virXPathString(conn, "string(./monitor[1]/@type)", ctxt); + if (tmp) + obj->monitor_chr->type = virDomainChrTypeFromString(tmp); + else + obj->monitor_chr->type = VIR_DOMAIN_CHR_TYPE_PTY; + VIR_FREE(tmp); + + switch (obj->monitor_chr->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + obj->monitor_chr->data.file.path = monitorpath; + break; + default: + VIR_FREE(monitorpath); + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unsupported monitor type '%s'"), + virDomainChrTypeToString(obj->monitor_chr->type)); + break; + } + return obj; error: + virDomainChrDefFree(obj->monitor_chr); virDomainObjFree(obj); return NULL; } @@ -4134,11 +4161,22 @@ char *virDomainObjFormat(virConnectPtr conn, { char *config_xml = NULL, *xml = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *monitorpath; virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n", virDomainStateTypeToString(obj->state), obj->pid); - virBufferEscapeString(&buf, " <monitor path='%s'/>\n", obj->monitorpath); + + switch (obj->monitor_chr->type) { + default: + case VIR_DOMAIN_CHR_TYPE_PTY: + monitorpath = obj->monitor_chr->data.file.path; + break; + } + + virBufferEscapeString(&buf, " <monitor path='%s'", monitorpath); + virBufferVSprintf(&buf, " type='%s'/>\n", + virDomainChrTypeToString(obj->monitor_chr->type)); if (!(config_xml = virDomainDefFormat(conn, obj->def, diff --git a/src/domain_conf.h b/src/domain_conf.h index 51dd6d3..6e111fa 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -538,7 +538,7 @@ struct _virDomainObj { virMutex lock; int monitor; - char *monitorpath; + virDomainChrDefPtr monitor_chr; int monitorWatch; int pid; int state; diff --git a/src/qemu_conf.c b/src/qemu_conf.c index afa6b3e..414b71b 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -888,6 +888,7 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, int qemudBuildCommandLine(virConnectPtr conn, struct qemud_driver *driver, virDomainDefPtr def, + virDomainChrDefPtr monitor_chr, unsigned int qemuCmdFlags, const char ***retargv, const char ***retenv, @@ -1118,8 +1119,15 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->graphics) ADD_ARG_LIT("-nographic"); - ADD_ARG_LIT("-monitor"); - ADD_ARG_LIT("pty"); + if (monitor_chr) { + char buf[4096]; + + if (qemudBuildCommandLineChrDevStr(monitor_chr, buf, sizeof(buf)) < 0) + goto error; + + ADD_ARG_LIT("-monitor"); + ADD_ARG_LIT(buf); + } if (def->localtime) ADD_ARG_LIT("-localtime"); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 9065821..175173d 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -129,6 +129,7 @@ int qemudParseHelpStr (const char *str, int qemudBuildCommandLine (virConnectPtr conn, struct qemud_driver *driver, virDomainDefPtr def, + virDomainChrDefPtr monitor_chr, unsigned int qemuCmdFlags, const char ***retargv, const char ***retenv, diff --git a/src/qemu_driver.c b/src/qemu_driver.c index d74596f..eee8857 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -283,7 +283,6 @@ cleanup: static int qemudOpenMonitor(virConnectPtr conn, struct qemud_driver* driver, virDomainObjPtr vm, - const char *monitor, int reconnect); @@ -297,7 +296,7 @@ qemuReconnectDomain(struct qemud_driver *driver, { int rc; - if ((rc = qemudOpenMonitor(NULL, driver, obj, obj->monitorpath, 1)) != 0) { + if ((rc = qemudOpenMonitor(NULL, driver, obj, 1)) != 0) { VIR_ERROR(_("Failed to reconnect monitor for %s: %d\n"), obj->def->name, rc); goto error; @@ -821,30 +820,24 @@ qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -qemudOpenMonitor(virConnectPtr conn, - struct qemud_driver* driver, - virDomainObjPtr vm, - const char *monitor, - int reconnect) +qemudOpenMonitorCommon(virConnectPtr conn, + struct qemud_driver* driver, + virDomainObjPtr vm, + int monfd, + int reconnect) { - int monfd; char buf[1024]; - int ret = -1; + int ret; - if ((monfd = open(monitor, O_RDWR)) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Unable to open monitor path %s"), monitor); - return -1; - } if (virSetCloseExec(monfd) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to set monitor close-on-exec flag")); - goto error; + return -1; } if (virSetNonBlock(monfd) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to put monitor into non-blocking mode")); - goto error; + return -1; } if (!reconnect) { @@ -862,21 +855,58 @@ qemudOpenMonitor(virConnectPtr conn, } if (ret != 0) - goto error; + return ret; if ((vm->monitorWatch = virEventAddHandle(vm->monitor, 0, qemudDispatchVMEvent, driver, NULL)) < 0) - goto error; + return -1; + return 0; +} - /* Keep monitor open upon success */ - if (ret == 0) - return ret; +static int +qemudOpenMonitorPty(virConnectPtr conn, + struct qemud_driver* driver, + virDomainObjPtr vm, + const char *monitor, + int reconnect) +{ + int monfd; - error: + if ((monfd = open(monitor, O_RDWR)) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unable to open monitor path %s"), monitor); + return -1; + } + + if (qemudOpenMonitorCommon(conn, driver, vm, monfd, reconnect) < 0) + goto error; + + return 0; + +error: close(monfd); - return ret; + return -1; +} + +static int +qemudOpenMonitor(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + int reconnect) +{ + switch (vm->monitor_chr->type) { + case VIR_DOMAIN_CHR_TYPE_PTY: + return qemudOpenMonitorPty(conn, driver, vm, + vm->monitor_chr->data.file.path, + reconnect); + default: + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unable to handle monitor type: %s"), + virDomainChrTypeToString(vm->monitor_chr->type)); + return -1; + } } /* Returns -1 for error, 0 success, 1 continue reading */ @@ -966,10 +996,11 @@ qemudFindCharDevicePTYs(virConnectPtr conn, } /* Got them all, so now open the monitor console */ - if ((ret = qemudOpenMonitor(conn, driver, vm, monitor, 0)) != 0) - goto cleanup; + vm->monitor_chr->data.file.path = monitor; + monitor = NULL; - vm->monitorpath = monitor; + if ((ret = qemudOpenMonitor(conn, driver, vm, 0)) != 0) + goto cleanup; return 0; @@ -1414,6 +1445,13 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (qemuPrepareHostDevices(conn, vm->def) < 0) goto cleanup; + if (VIR_ALLOC(vm->monitor_chr) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + vm->monitor_chr->type = VIR_DOMAIN_CHR_TYPE_PTY; + if ((ret = virFileDeletePid(driver->stateDir, vm->def->name)) != 0) { virReportSystemError(conn, ret, _("Cannot remove stale PID file for %s"), @@ -1428,7 +1466,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, } vm->def->id = driver->nextvmid++; - if (qemudBuildCommandLine(conn, driver, vm->def, + if (qemudBuildCommandLine(conn, driver, vm->def, vm->monitor_chr, qemuCmdFlags, &argv, &progenv, &tapfds, &ntapfds, migrateFrom) < 0) goto cleanup; @@ -3405,6 +3443,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, unsigned int flags ATTRIBUTE_UNUSED) { struct qemud_driver *driver = conn->privateData; virDomainDefPtr def = NULL; + virDomainChrDef monitor_chr; const char *emulator; unsigned int qemuCmdFlags; struct stat sb; @@ -3483,9 +3522,10 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, goto cleanup; } + monitor_chr.type = VIR_DOMAIN_CHR_TYPE_PTY; if (qemudBuildCommandLine(conn, driver, def, - qemuCmdFlags, + &monitor_chr, qemuCmdFlags, &retargv, &retenv, NULL, NULL, /* Don't want it to create TAP devices */ NULL) < 0) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2a93018..ea29200 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -34,6 +34,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char **tmp = NULL; int ret = -1, len, flags; virDomainDefPtr vmdef = NULL; + virDomainChrDef monitor_chr; if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) goto fail; @@ -47,12 +48,15 @@ static int testCompareXMLToArgvFiles(const char *xml, else vmdef->id = -1; + monitor_chr.type = VIR_DOMAIN_CHR_TYPE_PTY; + flags = QEMUD_CMD_FLAG_VNC_COLON | QEMUD_CMD_FLAG_NO_REBOOT | extraFlags; if (qemudBuildCommandLine(NULL, &driver, - vmdef, flags, &argv, &qenv, + vmdef, &monitor_chr, flags, + &argv, &qenv, NULL, NULL, migrateFrom) < 0) goto fail;