[libvirt] [PATCH 3/3] qemu: caps: Advertise arm 32-on-64 KVM option
Daniel P. Berrange
berrange at redhat.com
Mon Jun 8 08:46:21 UTC 2015
On Sat, Jun 06, 2015 at 07:06:42PM -0400, Cole Robinson wrote:
> On 05/28/2015 07:31 AM, Daniel P. Berrange wrote:
> > On Thu, May 21, 2015 at 07:03:28PM -0400, Cole Robinson wrote:
> >> We need to use qemu-system-aarch64 to run armv7l KVM VMs on an aarch64
> >> host.
> >> ---
> >> src/qemu/qemu_capabilities.c | 42 ++++++++++++++++++++++--------------------
> >> 1 file changed, 22 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> >> index 1e7bddb..7181865 100644
> >> --- a/src/qemu/qemu_capabilities.c
> >> +++ b/src/qemu/qemu_capabilities.c
> >> @@ -723,19 +723,6 @@ virQEMUCapsFindBinaryForArch(virArch hostarch,
> >> return ret;
> >> }
> >>
> >> -
> >> -static bool
> >> -virQEMUCapsIsValidForKVM(virArch hostarch,
> >> - virArch guestarch)
> >> -{
> >> - if (hostarch == guestarch)
> >> - return true;
> >> - if (hostarch == VIR_ARCH_X86_64 &&
> >> - guestarch == VIR_ARCH_I686)
> >> - return true;
> >> - return false;
> >> -}
> >> -
> >> static int
> >> virQEMUCapsInitGuest(virCapsPtr caps,
> >> virQEMUCapsCachePtr cache,
> >> @@ -747,6 +734,7 @@ virQEMUCapsInitGuest(virCapsPtr caps,
> >> char *binary = NULL;
> >> virQEMUCapsPtr qemubinCaps = NULL;
> >> virQEMUCapsPtr kvmbinCaps = NULL;
> >> + bool native_kvm, x86_32on64_kvm, arm_32on64_kvm;
> >> int ret = -1;
> >>
> >> /* Check for existence of base emulator, or alternate base
> >> @@ -764,16 +752,30 @@ virQEMUCapsInitGuest(virCapsPtr caps,
> >>
> >> /* qemu-kvm/kvm binaries can only be used if
> >> * - host & guest arches match
> >> - * Or
> >> - * - hostarch is x86_64 and guest arch is i686
> >> - * The latter simply needs "-cpu qemu32"
> >> + * - hostarch is x86_64 and guest arch is i686 (needs -cpu qemu32)
> >> + * - hostarch is aarch64 and guest arch is armv7l (needs -cpu aarch64=off)
> >> */
> >> - if (virQEMUCapsIsValidForKVM(hostarch, guestarch)) {
> >> - const char *const kvmbins[] = { "/usr/libexec/qemu-kvm", /* RHEL */
> >> - "qemu-kvm", /* Fedora */
> >> - "kvm" }; /* Upstream .spec */
> >> + native_kvm = (hostarch == guestarch);
> >> + x86_32on64_kvm = (hostarch == VIR_ARCH_X86_64 &&
> >> + guestarch == VIR_ARCH_I686);
> >> + arm_32on64_kvm = (hostarch == VIR_ARCH_AARCH64 &&
> >> + guestarch == VIR_ARCH_ARMV7L);
> >> +
> >> + if (native_kvm || x86_32on64_kvm || arm_32on64_kvm) {
> >> + const char *kvmbins[] = {
> >> + "/usr/libexec/qemu-kvm", /* RHEL */
> >> + "qemu-kvm", /* Fedora */
> >> + "kvm", /* Debian/Ubuntu */
> >> + NULL,
> >> + };
> >> +
> >> + if (arm_32on64_kvm)
> >> + kvmbins[3] = "qemu-system-aarch64";
> >
> > I'm unclear why you need to be adding this. We don't need it for
> > the equivalent i686 with qemu-system-x86_64, as the earlier call
> > to virQEMUCapsFindBinaryForArch() will already return the binary
> > qemu-system-x86_64. IIUC, it should have returned the binary
> > qemu-system-aarch64 too, so this just seems to duplicate the
> > check for that binary.
>
> We need this in the case you are running on an aarch64 host and have both
> qemu-system-arm and qemu-system-aarch64 installed. In this case, when you want
> to use KVM for arm32, you _have_ to use qemu-system-aarch64, qemu-system-arm
> does not work. What you suggest would mean that qemu-system-arm is grabbed
> from the caps cache.
>
> x86 doesn't have this problem because qemu-system-i386, qemu-system-x86_64 and
> by extension qemu-kvm can all be used to do 32-on-64 KVM.
Ok, I think we need to have this explained in the code comments, because
that is none obvious from reading the code & we don't want to be wondering
why we did this when looking at the code again in a year :-)
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 :|
More information about the libvir-list
mailing list