On Tue, Sep 15, 2015 at 04:44:13PM +0200, Andrea Bolognani wrote:

When looking for a QEMU binary suitable for running ppc64le guests we have to take into account the fact that we use the QEMU target as key for the hash, so direct comparison is not good enough.

Factor out the logic from virQEMUCapsFindBinaryForArch() to a new virQEMUCapsFindTarget() function and use that both when looking for QEMU binaries available on the system and when looking up QEMU capabilities later.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260753 --- src/qemu/qemu_capabilities.c | 100 ++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ad1bdb..6757907 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -386,6 +386,28 @@ static const char *virQEMUCapsArchToString(virArch arch) return virArchToString(arch); }

+/* Given a host and guest architectures, find a suitable QEMU target. + * + * This is meant to be used as a second attempt if qemu-system-$guestarch + * can't be found, eg. on a x86_64 host you want to use qemu-system-i386, + * if available, instead of qemu-system-x86_64 to run i686 guests */ +static virArch +virQEMUCapsFindTarget(virArch hostarch, + virArch guestarch) +{ + /* Both ppc64 and ppc64le guests can use the ppc64 target */ + if (ARCH_IS_PPC64(guestarch)) + guestarch = VIR_ARCH_PPC64; + + /* armv7l guests on aarch64 hosts can use the aarch64 target + * i686 guests on x86_64 hosts can use the x86_64 target */ + if ((guestarch == VIR_ARCH_ARMV7L && hostarch == VIR_ARCH_AARCH64) || + (guestarch == VIR_ARCH_I686 && hostarch == VIR_ARCH_X86_64)) { + return hostarch; + } + + return guestarch; +}

static virCommandPtr virQEMUCapsProbeCommand(const char *qemu, @@ -690,51 +712,52 @@ virQEMUCapsProbeCPUModels(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) return ret; }

- static char * -virQEMUCapsFindBinaryForArch(virArch hostarch, - virArch guestarch) +virQEMUCapsFindBinary(const char *format, + const char *archstr) { - char *ret; - const char *archstr; - char *binary; - - if (ARCH_IS_PPC64(guestarch)) - archstr = virQEMUCapsArchToString(VIR_ARCH_PPC64); - else - archstr = virQEMUCapsArchToString(guestarch); + char *ret = NULL; + char *binary = NULL;

- if (virAsprintf(&binary, "qemu-system-%s", archstr) < 0) - return NULL; + if (virAsprintf(&binary, format, archstr) < 0) + goto out;

ret = virFindFileInPath(binary); VIR_FREE(binary); - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); + if (ret && virFileIsExecutable(ret)) + goto out;

- if (guestarch == VIR_ARCH_ARMV7L && - !ret && - hostarch == VIR_ARCH_AARCH64) { - ret = virFindFileInPath("qemu-system-aarch64"); - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); - } + VIR_FREE(ret);

- if (guestarch == VIR_ARCH_I686 && - !ret && - hostarch == VIR_ARCH_X86_64) { - ret = virFindFileInPath("qemu-system-x86_64"); - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); - } + out: + return ret; +} + +static char * +virQEMUCapsFindBinaryForArch(virArch hostarch, + virArch guestarch) +{ + char *ret = NULL; + const char *archstr; + + /* First attempt: try the guest architecture as it is */ + archstr = virQEMUCapsArchToString(guestarch); + if ((ret = virQEMUCapsFindBinary("qemu-system-%s", archstr)) != NULL) + goto out;

- if (guestarch == VIR_ARCH_I686 && - !ret) { - ret = virFindFileInPath("qemu"); - if (ret && !virFileIsExecutable(ret)) - VIR_FREE(ret); + /* Second attempt: use some arch-specific rules */ + archstr = virQEMUCapsArchToString(virQEMUCapsFindTarget(hostarch, + guestarch));

Nitpick, we could avoid the second search if virQEMUCapsFindTarget() returns a value that == the original guestarch.

+ if ((ret = virQEMUCapsFindBinary("qemu-system-%s", archstr)) != NULL) + goto out; + + /* Third attempt, i686 only: try 'qemu' */ + if (guestarch == VIR_ARCH_I686) { + if ((ret = virQEMUCapsFindBinary("%s", "qemu")) != NULL) + goto out; }

+ out: return ret; }

@@ -3793,10 +3816,17 @@ virQEMUCapsCacheLookupByArch(virQEMUCapsCachePtr cache,

virMutexLock(&cache->lock); ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data); - VIR_DEBUG("Returning caps %p for arch %s", ret, virArchToString(arch)); + if (!ret) { + /* If the first attempt at findind capabilities has failed, try + * again using the QEMU target as lookup key instead */ + data.arch = virQEMUCapsFindTarget(virArchFromHost(), data.arch); + ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data); + }

Here too, skip the hash search if data.arch == data.arch from before. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|