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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list