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

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



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


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