[libvirt] [PATCH 1/4] qemu: get arch name from <cpu> element

[Jiri Denemark]

On Mon, Jan 23, 2012 at 14:11:08 +0100, Paolo Bonzini wrote:
> The qemu32 CPU model is chosen based on the <os arch=...> name when
> creating the QEMU command line.  Reflect the kvm32/kvm64/qemu32/qemu64
> CPU models in the <os> element when doing the opposite transformation.
> To do this, we need to not look at def->os.arch until after the
> command-line has been parsed.
> 
> At the same time, avoid creating an empty <cpu> element when the QEMU
> command-line specifies the default CPU model for the guest arch.
> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  src/qemu/qemu_command.c |   70 +++++++++++++++++++++++++++++++----------------
>  1 files changed, 46 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index aaccf62..7c4460e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
...
> @@ -6831,6 +6832,21 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
>          }
>      } while ((p = next));
>  
> +    if (model) {
> +        if (STREQ(model, "qemu32") || STREQ(model, "kvm32")) {
> +            dom->os.arch = strdup("i686");
> +            VIR_FREE(model);
> +        } else if (STREQ(model, "qemu64") || STREQ(model, "kvm64")) {
> +            dom->os.arch = strdup("x86_64");
> +            VIR_FREE(model);
> +        } else {
> +            if (!cpu && !(cpu = qemuInitGuestCPU(dom)))
> +                goto error;
> +
> +            cpu->model = model;
> +        }
> +    }
> +

I think we could just set cpu->model even if the model used in qemu command
line was {qemu,kvm}{32,64}. That would probably mean we need to add some of
the models in cpu_map.xml, though.

>      return 0;
>  
>  syntax:
...
> @@ -7542,6 +7544,26 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>          }
>      }
>  
> +    if (!def->os.arch) {
> +        if (STRPREFIX(def->emulator, "qemu"))
> +            path = def->emulator;
> +        else
> +            path = strstr(def->emulator, "qemu");
> +        if (path &&
> +            STRPREFIX(path, "qemu-system-"))
> +            def->os.arch = strdup(path + strlen("qemu-system-"));
> +        else
> +            def->os.arch = strdup("i686");
> +        if (!def->os.arch)
> +            goto no_memory;
> +    }
> +
> +    if (STREQ(def->os.arch, "i686")||STREQ(def->os.arch, "x86_64"))
> +        def->features = (1 << VIR_DOMAIN_FEATURE_ACPI)
> +        /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;

I know you were just copy&pasting, but while doing that, you could also make
this last statement a bit more readable and add spaces around "||" and either
use {} or remove the last line (the comment).

> +
> +    def->features &= ~disabled_features;
> +
>  #undef WANT_VALUE
>      if (def->ndisks > 0 && ceph_args) {
>          char *hosts, *port, *saveptr = NULL, *token;

Otherwise the patch looks good.

Jirka