[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] qemu: Enable QEMU_CAPS_PCI_ROMBAR unconditionally



On 05/13/2016 09:47 AM, Andrea Bolognani wrote:
> We already do this when parsing the help string (QEMU versions
> between 0.12.0 and 1.2.0), but not when using QMP.
> 
> Stop checking the QMP data, and enable it unconditionally even
> when using QMP instead.
> 
> The QMP check was never accurate anyway, since it was based on
> the availability of the 'rombar' property for the {kvm-}pci-assign
> device, ignoring the fact that the same property exists and can be
> used for devices such as virtio-net-pci.
> 
> Since a few other capabilities are enabled based on a version
> check, but the version in question is older than 0.12.0, move
> those to a new virQEMUCapsInitHelpBasic() function and get rid
> of the version checks altogether.
> ---
>  src/qemu/qemu_capabilities.c                       | 46 ++++++++++++----------
>  tests/qemucapabilitiesdata/caps_1.2.2-1.x86_64.xml |  1 +
>  .../qemucapabilitiesdata/caps_2.6.0-1.aarch64.xml  |  1 +
>  .../qemucapabilitiesdata/caps_2.6.0-1.ppc64le.xml  |  1 +
>  .../qemucapabilitiesdata/caps_2.6.0-2.aarch64.xml  |  1 +
>  5 files changed, 30 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 1bddf43..a6cce30 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1110,6 +1110,29 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
>  }
>  
>  
> +/* Capabilities that we assume are always enabled
> + * for QEMU >= 0.12.0 */
> +static void
> +virQEMUCapsInitHelpBasic(virQEMUCapsPtr qemuCaps)
> +{
> +
> +    /* Although very new versions of qemu advertise the presence of
> +     * the rombar option in the output of "qemu -device pci-assign,?",
> +     * this advertisement was added to the code long after the option
> +     * itself. According to qemu developers, though, rombar is
> +     * available in all qemu binaries from release 0.12 onward.
> +     * Setting the capability this way makes it available in more
> +     * cases where it might be needed, and shouldn't cause any false
> +     * positives (in the case that it did, qemu would produce an error
> +     * log and refuse to start, so it would be immediately obvious).
> +     */
> +    virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_ROMBAR);
> +
> +    virQEMUCapsSet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO);
> +    virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_HOST);
> +}

As Pavel said, the approach we've taken for other flags is to rename them to
X_$NAME, and remove all code usages. Moving these flags to their own function
confuses that approach, so I suggest either give the X_$NAME treatment to each
of those flags, or just using this series to disable PCI_ROMBAR and leave the
other flags for a later series.

(FWIW there's a list of features needing similar treatment on the
BiteSizedTasks page:
http://wiki.libvirt.org/page/BiteSizedTasks#Remove_unused_QEMU_feature_flags )

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]