On Thu, Jul 09, 2009 at 08:09:45PM +0100, Mark McLoughlin wrote:
> 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
Ahh, good forward thinking - I would not have remembered that the
state file needed changing for this.
> ---
> 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;
>
ACK
Daniel
--
|: 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