[libvirt] [PATCH 6/6] Convert QEMU capabilities code to use virArch

[Jiri Denemark]

On Tue, Dec 11, 2012 at 14:53:41 +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 94 ++++++++++++++++++--------------------------
>  src/qemu/qemu_capabilities.h |  2 +-
>  2 files changed, 40 insertions(+), 56 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index f6b53ca..a0fe1c5 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
...
> @@ -599,6 +598,8 @@ qemuCapsFindBinaryForArch(virArch hostarch,
>              ret = virFindFileInPath("qemu");
>      } else if (guestarch == VIR_ARCH_ITANIUM) {
>          ret = virFindFileInPath("qemu-system-ia64");
> +    } else if (guestarch == VIR_ARCH_ARMV7L) {
> +        ret = virFindFileInPath("qemu-system-arm");
>      } else {
>          /* Default case we have matching arch strings */
>          char *bin;

I think this hunk should go to 4/6. Actually, it would be even better to
create a reverse mapping function to qemuCapsArchFromString and call it
here to keep the qemu special naming at one place.

...
> @@ -2120,6 +2102,18 @@ int qemuCapsProbeQMP(qemuCapsPtr caps,
>  }
>  
>  
> +static virArch qemuCapsArchFromString(const char *arch)
> +{
> +    if (STREQ(arch, "ia64"))
> +        return VIR_ARCH_ITANIUM;
> +    if (STREQ(arch, "i386"))
> +        return VIR_ARCH_I686;
> +    if (STREQ(arch, "arm"))
> +        return VIR_ARCH_ARMV7L;
> +
> +    return virArchFromString(arch);
> +}
> +
>  #define QEMU_SYSTEM_PREFIX "qemu-system-"
>  
>  static int
...
> @@ -2381,26 +2367,24 @@ qemuCapsInitQMP(qemuCapsPtr caps,
>  
>      qemuCapsInitQMPBasic(caps);
>  
> -    if (!(caps->arch = qemuMonitorGetTargetArch(mon)))
> +    archstr = qemuMonitorGetTargetArch(mon);

This ignores possible failure in qemuMonitorGetTargetArch(); neither
qemuCapsArchFromString nor virArchFromString work with NULL arch.

> +    if ((caps->arch = qemuCapsArchFromString(archstr)) == VIR_ARCH_NONE) {
> +        VIR_DEBUG("Unknown QEMU arch %s", archstr);
> +        ret = 0;

Why is this non-fatal?

> +        VIR_FREE(archstr);
>          goto cleanup;
> -
> -    /* Map i386, i486, i586 to i686.  */
> -    if (caps->arch[0] == 'i' &&
> -        caps->arch[1] != '\0' &&
> -        caps->arch[2] == '8' &&
> -        caps->arch[3] == '6' &&
> -        caps->arch[4] == '\0')
> -        caps->arch[1] = '6';
> +    }
> +    VIR_FREE(archstr);
>  
>      /* Currently only x86_64 and i686 support PCI-multibus. */
> -    if (STREQLEN(caps->arch, "x86_64", 6) ||
> -        STREQLEN(caps->arch, "i686", 4)) {
> +    if (caps->arch == VIR_ARCH_X86_64 ||
> +        caps->arch == VIR_ARCH_I686)
>          qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS);
> -    }
>  
>      /* S390 and probably other archs do not support no-acpi -
>         maybe the qemu option parsing should be re-thought. */
> -    if (STRPREFIX(caps->arch, "s390"))
> +    if (caps->arch == VIR_ARCH_S390 ||
> +        caps->arch == VIR_ARCH_S390X)
>          qemuCapsClear(caps, QEMU_CAPS_NO_ACPI);
>  
>      if (qemuCapsProbeQMPCommands(caps, mon) < 0)
...

Jirka