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