[libvirt] [PATCH v2 2/4] qemu: caps: add QEMU_CAPS_QCOW2_L2_CACHE_SIZE

[John Ferlan]

On 11/8/18 8:02 AM, Nikolay Shirokovskiy wrote:
> For qemu capable of setting l2-cache-size for qcow2 images
> to INT64_MAX and semantics of upper limit on l2 cache
> size. We can only check this by qemu version (3.1.0) now.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/qemu/qemu_capabilities.c | 5 +++++
>  src/qemu/qemu_capabilities.h | 1 +
>  2 files changed, 6 insertions(+)
> 

This patch needs to be updated to top of tree.

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 2ca5af3..49a3b60 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -509,6 +509,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "vfio-pci.display",
>                "blockdev",
>                "vfio-ap",
> +              "qcow2.l2-cache-size",

When you do update, you will need to fix the comma-less entry for
"egl-headless.rendernode" as well, unless someone else gets to it first.

>      );
>  
>  
> @@ -4117,6 +4118,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
>      }
>  
> +    /* l2-cache-size before 3001000 does not accept INT64_MAX */
> +    if (qemuCaps->version >= 3001000)
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE);
> +

Not a fan of this.

The 'l2-cache-size' was supported since QEMU 2.2. A "newer" algorithm
for cache adjustment has been supported since QEMU 2.12 (and there's
l2-cache-entry-size that "could be used" to know that). Then there's
some unreferenced commit indicating usage of INT64_MAX. Tracking that
down, I find qemu.git commit 6c1c8d5d from Max which enforces MAX_INT.

Still does that really matter? Let QEMU pick their own max and don't
have libvirt be the arbiter of that (like I noted in my 1/4 review). My
reasoning is that there's been quite a few "adjustments" to the
algorithms along the way. Those adjustments are one of the concerns
voiced in the past why making any "semi-permanent" change to the value
in some libvirt XML format has been NACKed historically. THus by placing
boundaries that are true today we limit ourselves for the future.

BTW: If 3.1 was the "base" from which you want to work, then adjustments
to the tests/qemucapabilitiesdata/*3_1*{.replies|.xml } files would be
required as evidenced by your patch 4. The *.xml file would need to have
the correct <version>3001000</version> setting which should allow patch4
to be merged into patch3.

John

>      if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
>          goto cleanup;
>  
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 6bb9a2c..bb2ac17 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -493,6 +493,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>      QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
>      QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
>      QEMU_CAPS_DEVICE_VFIO_AP, /* -device vfio-ap */
> +    QEMU_CAPS_QCOW2_L2_CACHE_SIZE, /* -blockdev supports l2-cache-size with INT64_MAX value */
>  
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
>