[libvirt] [PATCH 1/4] qemu: Advertise ACPI support for aarch64 guests

Andrea Bolognani abologna at redhat.com
Mon Mar 27 13:57:34 UTC 2017


On Mon, 2017-03-27 at 08:03 -0400, John Ferlan wrote:
[...]
> > +    /* Older QEMU versions reported -no-acpi in the output of -help even
> > +     * though it was not supported by the architecture. The issue has since
> > +     * been fixed, but to maintain compatibility with all release we still
> 
> "releases"

Good catch!

> > +     * need to filter out the capability for architectures that we know
> > +     * don't support the feature, eg. anything but x86 and aarch64 */
> > +    if (!ARCH_IS_X86(qemuCaps->arch) &&
> > +        qemuCaps->arch != VIR_ARCH_AARCH64) {
> >          virQEMUCapsClear(qemuCaps, QEMU_CAPS_NO_ACPI);
> > +    }
> 
> Some day maybe we'll be able to stop parsing the help output. Still
> makes me wonder is AARCH64 even support on those older versions and thus
> is this necessary?  IDC either way as I suppose this is preventative or
> "more complete".

Probably not, but I don't think having different
arch-specific handling in the two code paths is a good
idea: we should stay consistent, if anything not to confuse
our future selves :)

[...]
> > @@ -4222,9 +4231,14 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
> >          goto cleanup;
> >      }
> >  
> > -    /* ACPI/HPET/KVM PIT are x86 specific */
> > -    if (ARCH_IS_X86(qemuCaps->arch)) {
> > +    /* ACPI only works on x86 and aarch64 */
> > +    if (ARCH_IS_X86(qemuCaps->arch) ||
> > +        qemuCaps->arch == VIR_ARCH_AARCH64) {
> >          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
> > +    }
> > +
> > +    /* HPET and KVM PIT are x86 specific */
> > +    if (ARCH_IS_X86(qemuCaps->arch)) {
> >          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_HPET);
> >          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
> >      }
> 
> Considering on what's coming in patch 2, this would be better as a
> virQEMUCapsSetFirmwareCaps? "utility" function...  That way the added
> comments in both places referencing the other place could be dropped.

HPET and KVM PIT are not firmware-related, though.

How about I move setting the arch based on the monitor to
a separate virQEMUCapsInitQMPArch() and leave only setting
the actual arch-dependent capabilities in this function?

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list