[libvirt] [PATCH] Canonicalize qemu machine types

[Daniel P. Berrange]

On Tue, Jul 21, 2009 at 04:01:37PM +0100, Mark McLoughlin wrote:
> In qemu-0.11 there is a 'pc-0.10' machine type which allows you to run
> guests with a machine which is compatible with the pc machine in
> qemu-0.10 - e.g. using the original PCI class for virtio-blk and
> virtio-console and disabling MSI support in virtio-net. The idea here
> is that we don't want to surprise guests by changing the hardware when
> qemu is updated.
> 
> I've just posted some patches for qemu-0.11 which allows libvirt to
> canonicalize the 'pc' machine alias to the latest machine version.
> 
> This patches makes us use that so that when a guest is configured to
> use the 'pc' machine type, we resolve that to 'pc-0.11' machine and
> save that in the guest XML.

We currently also list "valid" machine types in the capabilities
XML for each driver. I use quotes, because this list is currently
dubiously hardcoded. I'm thinking it might be better to do the
parsing of -M ? output as part of the capaiblities intiialization
code. The canonicalize method would then just need to query the 
existing capabilities data & could even be  done in the domain XML
parser code instead of qemu driver.


> ---
>  src/qemu_conf.c   |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu_conf.h   |    3 +
>  src/qemu_driver.c |    3 +
>  3 files changed, 120 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index 4043d70..3f4edfa 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -470,6 +470,120 @@ virCapsPtr qemudCapsInit(void) {
>      return NULL;
>  }
>  
> +/* Format is:
> + * <machine> <desc> [(alias of <machine>)]
> + */
> +static int
> +qemudParseMachineTypesStr(const char *machines ATTRIBUTE_UNUSED,
> +                          const char *machine ATTRIBUTE_UNUSED,
> +                          char **canonical)
> +{
> +    const char *p = machines;
> +
> +    *canonical = NULL;
> +
> +    do {
> +        const char *eol;
> +        char *s;
> +
> +        if (!(eol = strchr(p, '\n')))
> +            return -1; /* eof file without finding @machine */
> +
> +        if (!STRPREFIX(p, machine)) {
> +            p = eol + 1;
> +            continue; /* doesn't match @machine */
> +        }
> +
> +        p += strlen(machine);
> +
> +        if (*p != ' ') {
> +            p = eol + 1;
> +            continue; /* not a complete match of @machine */
> +        }
> +
> +        do {
> +            p++;
> +        } while (*p == ' ');
> +
> +        p = strstr(p, "(alias of ");
> +        if (!p || p > eol)
> +            return 0; /* not an alias, name is canonical */
> +
> +        *canonical = strndup(p + strlen("(alias of "), eol - p);


Need to check for NULL here.

> +
> +        s = strchr(*canonical, ')');
> +        if (!s) {
> +            VIR_FREE(*canonical);
> +            *canonical = NULL;
> +            return -1; /* output is screwed up */
> +        }
> +
> +        *s = '\0';
> +        break;
> +    } while (1);
> +
> +    return 0;
> +}
> +
> +int
> +qemudCanonicalizeMachine(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                         virDomainDefPtr def)
> +{
> +    const char *const qemuarg[] = { def->emulator, "-M", "?", NULL };
> +    const char *const qemuenv[] = { "LC_ALL=C", NULL };
> +    char *machines, *canonical;
> +    enum { MAX_MACHINES_OUTPUT_SIZE = 1024*4 };
> +    pid_t child;
> +    int newstdout = -1, len;
> +    int ret = -1, status;
> +
> +    if (virExec(NULL, qemuarg, qemuenv, NULL,
> +                &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0)
> +        return -1;
> +
> +    len = virFileReadLimFD(newstdout, MAX_MACHINES_OUTPUT_SIZE, &machines);
> +    if (len < 0) {
> +        virReportSystemError(NULL, errno, "%s",
> +                             _("Unable to read 'qemu -M ?' output"));
> +        goto cleanup;
> +    }
> +
> +    if (qemudParseMachineTypesStr(machines, def->os.machine, &canonical) < 0)
> +        goto cleanup2;
> +
> +    if (canonical) {
> +        VIR_FREE(def->os.machine);
> +        def->os.machine = canonical;
> +    }
> +
> +    ret = 0;
> +
> +cleanup2:
> +    VIR_FREE(machines);
> +cleanup:
> +    if (close(newstdout) < 0)
> +        ret = -1;
> +
> +rewait:
> +    if (waitpid(child, &status, 0) != child) {
> +        if (errno == EINTR)
> +            goto rewait;
> +
> +        VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"),
> +                  WEXITSTATUS(status), (unsigned long)child);
> +        ret = -1;
> +    }
> +    /* Check & log unexpected exit status, but don't fail,
> +     * as there's really no need to throw an error if we did
> +     * actually read a valid version number above */
> +    if (WEXITSTATUS(status) != 0) {
> +        VIR_WARN(_("Unexpected exit status '%d', qemu probably failed"),
> +                 WEXITSTATUS(status));
> +    }
> +
> +    return ret;
> +}
> +
>  static unsigned int qemudComputeCmdFlags(const char *help,
>                                           unsigned int version,
>                                           unsigned int is_kvm,
> diff --git a/src/qemu_conf.h b/src/qemu_conf.h
> index fbf2ab9..b668669 100644
> --- a/src/qemu_conf.h
> +++ b/src/qemu_conf.h
> @@ -123,6 +123,9 @@ int         qemudExtractVersionInfo     (const char *qemu,
>                                           unsigned int *version,
>                                           unsigned int *flags);
>  
> +int         qemudCanonicalizeMachine    (virConnectPtr conn,
> +                                         virDomainDefPtr def);
> +
>  int         qemudParseHelpStr           (const char *str,
>                                           unsigned int *flags,
>                                           unsigned int *version,
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index d2db1a2..98f8e95 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -4102,6 +4102,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
>          }
>      }
>  
> +    if (qemudCanonicalizeMachine(conn, def) < 0)
> +        goto cleanup;
> +
>      if (!(vm = virDomainAssignDef(conn,
>                                    &driver->domains,
>                                    def))) {
> -- 
> 1.6.2.5


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