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

Re: [libvirt] [PATCH 3/3] Get QEMU pty paths from the monitor



On Mon, Nov 23, 2009 at 12:30:29PM +0000, Matthew Booth wrote:
> This change makes the QEMU driver get pty paths from the output of the monitor
> 'info chardev' command. This output is structured, and contains both the name of
> the device and the path on the same line. This is considerably more reliable
> than parsing the startup log output, which requires the parsing code to know
> which order QEMU will print pty information in.
> 
> Note that we still need to parse the log output as the monitor itself may be on
> a pty. This should be rare, however, and the new code will replace all pty paths
> parsed by the log output method once the monitor is available.
> 
> * src/qemu/qemu_monitor.(c|h) src/qemu_monitor_text.(c|h): Implement
>   qemuMonitorGetPtyPaths().
> * src/qemu/qemu_driver.c: Get pty path information using
>   qemuMonitorGetPtyPaths().
> ---
>  src/qemu/qemu_driver.c       |   68 +++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_monitor.c      |    9 +++++
>  src/qemu/qemu_monitor.h      |    3 ++
>  src/qemu/qemu_monitor_text.c |   71 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_text.h |    4 ++
>  5 files changed, 153 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ebf44b0..90dd9cd 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1239,6 +1239,40 @@ qemudExtractTTYPath(virConnectPtr conn,
>  }
>  
>  static int
> +qemudFindCharDevicePTYsMonitor(virConnectPtr conn,
> +                               virDomainObjPtr vm,
> +                               virHashTablePtr paths)
> +{
> +    int i;
> +
> +#define LOOKUP_PTYS(array, arraylen, idprefix) \
> +    for (i = 0 ; i < (arraylen) ; i++) { \
> +        virDomainChrDefPtr chr = (array)[i]; \
> +        if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { \
> +            char id[16]; \
> +\
> +            if (snprintf(id, sizeof(id), idprefix "%i", i) >= sizeof(id)) \
> +                return -1; \
> +\
> +            const char *path = (const char *) virHashLookup(paths, id); \
> +            if (path == NULL) { \
> +                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, \
> +                                 _("no assigned pty for device %s"), id); \
> +                return -1; \
> +            } \
> +\
> +            chr->data.file.path = strdup(path); \
> +        } \
> +    }

Can you indent the  \  to they all line up in the right hand side. 

> +
> +    LOOKUP_PTYS(vm->def->serials,   vm->def->nserials,   "serial");
> +    LOOKUP_PTYS(vm->def->parallels, vm->def->nparallels, "parallel");
> +    LOOKUP_PTYS(vm->def->channels,  vm->def->nchannels,  "channel");
> +

#undef LOOKUP_PTYS

> +    return 0;
> +}
> +
> +static int
>  qemudFindCharDevicePTYs(virConnectPtr conn,
>                          virDomainObjPtr vm,
>                          const char *output,
> @@ -1284,6 +1318,11 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
>      return 0;
>  }
>  
> +static void qemudFreePtyPath(void *payload, const char *name ATTRIBUTE_UNUSED)
> +{
> +    free(payload);

VIR_FREE

> +}
> +
>  static int
>  qemudWaitForMonitor(virConnectPtr conn,
>                      struct qemud_driver* driver,
> @@ -1291,7 +1330,7 @@ qemudWaitForMonitor(virConnectPtr conn,
>  {
>      char buf[4096]; /* Plenty of space to get startup greeting */
>      int logfd;
> -    int ret;
> +    int ret = -1;
>  
>      if ((logfd = qemudLogReadFD(conn, driver->logDir, vm->def->name, pos))
>          < 0)
> @@ -1317,7 +1356,32 @@ qemudWaitForMonitor(virConnectPtr conn,
>      if (qemuConnectMonitor(vm) < 0)
>          return -1;
>  
> -    return 0;
> +    /* Try to get the pty path mappings again via the monitor. This is much more
> +     * reliable if it's available.
> +     * Note that the monitor itself can be on a pty, so we still need to try the
> +     * log output method. */
> +    virHashTablePtr paths = virHashCreate(0);
> +    if (paths == NULL) {
> +        virReportOOMError(NULL);
> +        goto cleanup;
> +    }
> +
> +    qemuDomainObjEnterMonitor(vm);

This needs to be EnterMonitorWithDriver(driver, vm), since the 'driver'
is locked in this context

> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    ret = qemuMonitorGetPtyPaths(priv->mon, paths);
> +    qemuDomainObjExitMonitor(vm);

And ExitMonitorWithDriver

> +
> +    VIR_DEBUG("qemuMonitorGetPtyPaths returned %i", ret);
> +    if (ret == 0) {
> +        ret = qemudFindCharDevicePTYsMonitor(conn, vm, paths);
> +    }
> +
> +cleanup:
> +    if (paths) {
> +        virHashFree(paths, qemudFreePtyPath);
> +    }
> +
> +    return ret;
>  }


> +
> +
> +/* Parse the output of "info chardev" and return a hash of pty paths.
> + *
> + * Output is:
> + * foo: filename=pty:/dev/pts/7
> + * monitor: filename=stdio
> + * serial0: filename=vc
> + * parallel0: filename=vc
> + *
> + * Non-pty lines are ignored. In the above example, key is 'foo', value is
> + * '/dev/pty/7'. The hash will contain only a single value.
> + */
> +
> +int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon,
> +                               virHashTablePtr paths)
> +{
> +    const char *cmd = "info chardev";
> +    char *reply = NULL;
> +    int ret = -1;
> +
> +    if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
> +        qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> +                         _("failed to retrieve chardev info in qemu with '%s'"),
> +                         cmd);
> +        goto cleanup;
> +    }
> +
> +    char *pos = reply;                  /* The current start of searching */
> +    char *end = pos + strlen(reply);    /* The end of the reply string */
> +    char *eol;                   /* The character which ends the current line */
> +
> +    while (pos < end) {
> +        /* Split the output into lines */
> +        eol = memchr(pos, '\n', end - pos);
> +        if (eol == NULL)
> +            eol = end;
> +
> +        /* Look for 'filename=pty:' */
> +#define NEEDLE "filename=pty:"
> +        char *needle = memmem(pos, eol - pos, NEEDLE, strlen(NEEDLE));
> +
> +        /* If it's not there we can ignore this line */
> +        if (!needle)
> +            goto next;
> +
> +        /* id is everthing from the beginning of the line to the ':'
> +         * find ':' and turn it into a terminator */
> +        char *colon = memchr(pos, ':', needle - pos);
> +        if (colon == NULL)
> +            goto next;
> +        *colon = '\0';
> +        char *id = pos;
> +
> +        /* Path is everything after needle to the end of the line */
> +        *eol = '\0';
> +        char *path = needle + strlen(NEEDLE);
> +
> +        virHashAddEntry(paths, id, strdup(path));

Not checking OOM on strdup() here, or for failure of virHashAddEntry()

> +#undef NEEDLE
> +
> +    next:
> +        pos = eol + 1;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(reply);
> +    return ret;
> +}

Regards,
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]