On 10.12.2018 19:58, John Ferlan wrote:
>
>
> 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@xxxxxxxxxxxxx>
>> ---
>> 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.
This is a failure rather than enforcement. And AFAIU code that limit cache
to appropriate maximum when INT64_MAX is given in read_cache_sizes:
*l2_cache_size = MIN(max_l2_cache, l2_cache_max_setting);
only appeared after release of 3.0 in
commit b749562d9822d14ef69c9eaa5f85903010b86c30
Author: Leonid Bloch <lbloch@xxxxxxxxxxxxx>
Date: Wed Sep 26 19:04:43 2018 +0300
qcow2: Assign the L2 cache relatively to the image size
ie setting cache size to INT64_MAX before 3.1 will fail. In
other words in earlier versions there were no value to specify that
cache size need to be set to maximum. You can calculate this value
yourself knowing disk size and disk cluster size and set it but
this is not convinient.
>
> 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.
IIUC setting INT64_MAX is ok in this respect. It is not real value for cache
but rather order for QEMU to pick up one.
>
> 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.
Yeah, but 3.1 is not yet released and I need blockdev also as
patch only makes difference in latter case.
Nikolay
>
> 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;
>>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
