[libvirt] [PATCH v3 0/5] add disk driver metadata_cache_size option

As this patch series only works with -blockdev configurations it is better to be pushed only after -blockdev is supported. Then 'news' and 'xml' patch need to be updated to have correct libvirt version. Diff from v2 [1]: ================= - move caps patch to top - add news patch - add _CAPPED to capabilitity name to distinguish from correspondent qemu capabilitiy - better option explanation in formatdomain.html - misc tests/english fixes as suggested We have come to agreement with John in mailing list to support QEMU versions which do not let use INT64_MAX as "maximum" value. But this will require changing qemuBlockStorageSourceGetFormatQcow2Props and a lot of other functions signature (which was not noticed during conversation). Thus I don't add such a functionality. Anyway the patch requires -blockdev from QEMU so we could only support single QEMU 3.0 version. [1] https://www.redhat.com/archives/libvir-list/2018-November/msg00313.html Nikolay Shirokovskiy (5): qemu: caps: add QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED xml: add disk driver metadata_cache_size option qemu: support metadata-cache-size for blockdev news: add notice for new metadata cache size policy option DO NOT APPLY: add xml2argv test for metadata_cache_size docs/formatdomain.html.in | 11 ++++++ docs/news.xml | 11 ++++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 17 +++++++++ src/conf/domain_conf.h | 9 +++++ src/qemu/qemu_block.c | 5 ++- src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 23 ++++++++++++ src/qemu/qemu_domain.c | 2 ++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++++++++++++++++++++ .../qemuxml2argvdata/disk-metadata_cache_size.xml | 35 ++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../disk-metadata_cache_size.xml | 41 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 17 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml -- 1.8.3.1

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@virtuozzo.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f504db7..e09b13a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -520,6 +520,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 325 */ "memory-backend-file.pmem", "nvdimm.unarmed", + "qcow2.l2-cache-size.capped", ); @@ -4256,6 +4257,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_CAPPED); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6d5ed8a..e17a1dc 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -504,6 +504,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 325 */ QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM, /* -object memory-backend-file,pmem= */ QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm,unarmed= */ + QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED, /* -blockdev supports l2-cache-size with INT64_MAX value */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 1.8.3.1

On 1/10/19 7:15 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@virtuozzo.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+)
I posted a patch series today that would add/update the capabilities for 3.1 (and 4.0) and thus would add this capability to this patch. However, given that the only way you've chosen to support/implement this is via -blockdev, I'm not sure now what the purpose will be unless it's used to determine whether to use INT64_MAX or some other value. I see from previous series that it seems to be possible to have: -drive xxx,format=qcow2,l2-cache-size=VALUE,... However, patch3 notes in the commit message that "-drive configuration is not supported because we can not set l2 cache size down the backing chain in this case." So if -blockdev is going to be your the option for support, then I think it'd be wiser to wait for Peter to finish -blockdev support before acting on this. Since -blockdev support allows some qemu version earlier the 3.1 to be supported, then we revisit this. Additionally, referencing Leonid's qemu.git commit b749562d98 as the "arbiter" of why INT64_MAX must be used (since calculating a max max value isn't possible, IIRC). John
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f504db7..e09b13a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -520,6 +520,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 325 */ "memory-backend-file.pmem", "nvdimm.unarmed", + "qcow2.l2-cache-size.capped", );
@@ -4256,6 +4257,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_CAPPED); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6d5ed8a..e17a1dc 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -504,6 +504,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 325 */ QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM, /* -object memory-backend-file,pmem= */ QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm,unarmed= */ + QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED, /* -blockdev supports l2-cache-size with INT64_MAX value */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;

The options specifies metadata cache size policy for a disk. This is going to be used only for QEMU and only for qcow2 images in next patch to set qcow2 L2 cache size. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 17 +++++++++ src/conf/domain_conf.h | 9 +++++ .../qemuxml2argvdata/disk-metadata_cache_size.xml | 35 ++++++++++++++++++ .../disk-metadata_cache_size.xml | 41 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 126 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f07bb7..fcffa9c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3608,6 +3608,17 @@ virt queues for virtio-blk. (<span class="since">Since 3.9.0</span>) </li> <li> + The optional <code>metadata_cache_size</code> attribute specifies + metadata cache size policy. Possible values are "default" and + "maximum". Using "default" leaves setting cache size to the hypervisor, + Using "maximum" ensures entire disk cache remains in memory, increasing + IO but utilizing more memory. This policy helps if workload's + disk working set (the amount of disk data used intensively) is too large + to be covered by cache size by "default" policy. + (<span class="since">Since 5.0.0, QEMU 3.0</span>). The option makes + sense only for non raw images and supported for qcow2 only now. + </li> + <li> For virtio disks, <a href="#elementsVirtio">Virtio-specific options</a> can also be set. (<span class="since">Since 3.5.0</span>) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index aa50eac..aa8b8ff 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2003,6 +2003,9 @@ <ref name="detect_zeroes"/> </optional> <optional> + <ref name="metadata_cache_size"/> + </optional> + <optional> <attribute name='queues'> <ref name="positiveInteger"/> </attribute> @@ -2103,6 +2106,14 @@ </choice> </attribute> </define> + <define name="metadata_cache_size"> + <attribute name='metadata_cache_size'> + <choice> + <value>default</value> + <value>maximum</value> + </choice> + </attribute> + </define> <define name="controller"> <element name="controller"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 222bb8c..9488c35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -889,6 +889,11 @@ VIR_ENUM_IMPL(virDomainDiskDetectZeroes, VIR_DOMAIN_DISK_DETECT_ZEROES_LAST, "on", "unmap") +VIR_ENUM_IMPL(virDomainDiskMetadataCacheSize, + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST, + "default", + "maximum") + VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, "none", "yes", @@ -9419,6 +9424,14 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, } VIR_FREE(tmp); + if ((tmp = virXMLPropString(cur, "metadata_cache_size")) && + (def->metadata_cache_size = virDomainDiskMetadataCacheSizeTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown driver metadata_cache_size value '%s'"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + ret = 0; cleanup: @@ -24193,6 +24206,10 @@ virDomainDiskDefFormatDriver(virBufferPtr buf, if (disk->queues) virBufferAsprintf(&driverBuf, " queues='%u'", disk->queues); + if (disk->metadata_cache_size) + virBufferAsprintf(&driverBuf, " metadata_cache_size='%s'", + virDomainDiskMetadataCacheSizeTypeToString(disk->metadata_cache_size)); + virDomainVirtioOptionsFormat(&driverBuf, disk->virtio); return virXMLFormatElement(buf, "driver", &driverBuf, NULL); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fae1306..31ce9ab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -567,6 +567,13 @@ typedef enum { VIR_DOMAIN_DISK_DETECT_ZEROES_LAST } virDomainDiskDetectZeroes; +typedef enum { + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_DEFAULT = 0, + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM, + + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST +} virDomainDiskMetadataCacheSize; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -672,6 +679,7 @@ struct _virDomainDiskDef { int discard; /* enum virDomainDiskDiscard */ unsigned int iothread; /* unused = 0, > 0 specific thread # */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ + int metadata_cache_size; /* enum virDomainDiskMetadataCacheSize */ char *domain_name; /* backend domain name */ unsigned int queues; virDomainVirtioOptionsPtr virtio; @@ -3408,6 +3416,7 @@ VIR_ENUM_DECL(virDomainDeviceSGIO) VIR_ENUM_DECL(virDomainDiskTray) VIR_ENUM_DECL(virDomainDiskDiscard) VIR_ENUM_DECL(virDomainDiskDetectZeroes) +VIR_ENUM_DECL(virDomainDiskMetadataCacheSize) VIR_ENUM_DECL(virDomainDiskMirrorState) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModelPCI) diff --git a/tests/qemuxml2argvdata/disk-metadata_cache_size.xml b/tests/qemuxml2argvdata/disk-metadata_cache_size.xml new file mode 100644 index 0000000..121171a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-metadata_cache_size.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>test</name> + <uuid>468404ad-d49c-40f2-9e14-02294f9c1be3</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' metadata_cache_size='maximum'/> + <source file='/var/lib/libvirt/images/f14.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml b/tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml new file mode 100644 index 0000000..2ca7dc4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>test</name> + <uuid>468404ad-d49c-40f2-9e14-02294f9c1be3</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' metadata_cache_size='maximum'/> + <source file='/var/lib/libvirt/images/f14.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 82e2c0e..4aa845c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1265,6 +1265,8 @@ mymain(void) DO_TEST("riscv64-virt", QEMU_CAPS_DEVICE_VIRTIO_MMIO); + DO_TEST("disk-metadata_cache_size", NONE); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 1.8.3.1

On 1/10/19 7:15 AM, Nikolay Shirokovskiy wrote:
The options specifies metadata cache size policy for a disk. This is going to be used only for QEMU and only for qcow2 images in next patch to set qcow2 L2 cache size.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 17 +++++++++ src/conf/domain_conf.h | 9 +++++ .../qemuxml2argvdata/disk-metadata_cache_size.xml | 35 ++++++++++++++++++ .../disk-metadata_cache_size.xml | 41 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 126 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f07bb7..fcffa9c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3608,6 +3608,17 @@ virt queues for virtio-blk. (<span class="since">Since 3.9.0</span>) </li> <li> + The optional <code>metadata_cache_size</code> attribute specifies + metadata cache size policy. Possible values are "default" and + "maximum". Using "default" leaves setting cache size to the hypervisor,
Although "default" is possible for input, on output it's dropped.
+ Using "maximum" ensures entire disk cache remains in memory, increasing + IO but utilizing more memory. This policy helps if workload's + disk working set (the amount of disk data used intensively) is too large + to be covered by cache size by "default" policy. + (<span class="since">Since 5.0.0, QEMU 3.0</span>). The option makes
The usage of the INT64_MAX doesn't happen until at least QEMU 3.1. We don't know when this will be supported though since -blockdev is required and that's not yet in libvirt.
+ sense only for non raw images and supported for qcow2 only now. + </li> + <li> For virtio disks, <a href="#elementsVirtio">Virtio-specific options</a> can also be set. (<span class="since">Since 3.5.0</span>) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index aa50eac..aa8b8ff 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2003,6 +2003,9 @@ <ref name="detect_zeroes"/> </optional> <optional> + <ref name="metadata_cache_size"/> + </optional> + <optional> <attribute name='queues'> <ref name="positiveInteger"/> </attribute> @@ -2103,6 +2106,14 @@ </choice> </attribute> </define> + <define name="metadata_cache_size"> + <attribute name='metadata_cache_size'> + <choice> + <value>default</value> + <value>maximum</value> + </choice> + </attribute> + </define> <define name="controller"> <element name="controller"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 222bb8c..9488c35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -889,6 +889,11 @@ VIR_ENUM_IMPL(virDomainDiskDetectZeroes, VIR_DOMAIN_DISK_DETECT_ZEROES_LAST, "on", "unmap")
+VIR_ENUM_IMPL(virDomainDiskMetadataCacheSize, + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST, + "default", + "maximum") + VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, "none", "yes", @@ -9419,6 +9424,14 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, } VIR_FREE(tmp);
+ if ((tmp = virXMLPropString(cur, "metadata_cache_size")) && + (def->metadata_cache_size = virDomainDiskMetadataCacheSizeTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown driver metadata_cache_size value '%s'"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + ret = 0;
cleanup: @@ -24193,6 +24206,10 @@ virDomainDiskDefFormatDriver(virBufferPtr buf, if (disk->queues) virBufferAsprintf(&driverBuf, " queues='%u'", disk->queues);
+ if (disk->metadata_cache_size) + virBufferAsprintf(&driverBuf, " metadata_cache_size='%s'", + virDomainDiskMetadataCacheSizeTypeToString(disk->metadata_cache_size)); + virDomainVirtioOptionsFormat(&driverBuf, disk->virtio);
return virXMLFormatElement(buf, "driver", &driverBuf, NULL); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fae1306..31ce9ab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -567,6 +567,13 @@ typedef enum { VIR_DOMAIN_DISK_DETECT_ZEROES_LAST } virDomainDiskDetectZeroes;
+typedef enum { + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_DEFAULT = 0,
Because this is 0 that means supplying "default" in the XML is not possible since the only way to Format the data is when metadata_cache_size is present. John
+ VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM, + + VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_LAST +} virDomainDiskMetadataCacheSize; +
[...]

Just set l2-cache-size to INT64_MAX for all format nodes of qcow2 type in block node graph. -drive configuration is not supported because we can not set l2 cache size down the backing chain in this case. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_block.c | 5 ++++- src/qemu/qemu_command.c | 23 +++++++++++++++++++++++ src/qemu/qemu_domain.c | 2 ++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index cbf0aa4..5f98b85 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1322,7 +1322,6 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, * 'pass-discard-snapshot' * 'pass-discard-other' * 'overlap-check' - * 'l2-cache-size' * 'l2-cache-entry-size' * 'refcount-cache-size' * 'cache-clean-interval' @@ -1331,6 +1330,10 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) return -1; + if (src->metadata_cache_size == VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM && + virJSONValueObjectAdd(props, "I:l2-cache-size", INT64_MAX, NULL) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8..96d6601 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1328,6 +1328,20 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, return -1; } + if (disk->metadata_cache_size) { + if (disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set for qcow2 disks")); + return -1; + } + + if (disk->metadata_cache_size != VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set to 'maximum'")); + return -1; + } + } + if (qemuCaps) { if (disk->serial && disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && @@ -1351,6 +1365,15 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, _("detect_zeroes is not supported by this QEMU binary")); return -1; } + + if (disk->metadata_cache_size && + !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting metadata_cache_size is not supported by " + "this QEMU binary")); + return -1; + } } if (disk->serial && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ec6b340..fc5c7c2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9202,6 +9202,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, /* "snapshot" is a libvirt internal field and thus can be changed */ /* startupPolicy is allowed to be updated. Therefore not checked here. */ CHECK_EQ(transient, "transient", true); + CHECK_EQ(metadata_cache_size, "metadata_cache_size", true); /* Note: For some address types the address auto generation for * @disk has still not happened at this point (e.g. driver @@ -13370,6 +13371,7 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, src->iomode = disk->iomode; src->cachemode = disk->cachemode; src->discard = disk->discard; + src->metadata_cache_size = disk->metadata_cache_size; if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) src->floppyimg = true; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index bd4b027..e5660e6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2208,6 +2208,7 @@ virStorageSourceCopy(const virStorageSource *src, ret->cachemode = src->cachemode; ret->discard = src->discard; ret->detect_zeroes = src->detect_zeroes; + ret->metadata_cache_size = src->metadata_cache_size; /* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1d6161a..8bf5fe4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -329,6 +329,7 @@ struct _virStorageSource { int cachemode; /* enum virDomainDiskCache */ int discard; /* enum virDomainDiskDiscard */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ + int metadata_cache_size; /* enum virDomainDiskImageMetadataCacheSize */ bool floppyimg; /* set to true if the storage source is going to be used as a source for floppy drive */ -- 1.8.3.1

On 1/10/19 7:15 AM, Nikolay Shirokovskiy wrote:
Just set l2-cache-size to INT64_MAX for all format nodes of qcow2 type in block node graph.
-drive configuration is not supported because we can not set l2 cache size down the backing chain in this case.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_block.c | 5 ++++- src/qemu/qemu_command.c | 23 +++++++++++++++++++++++ src/qemu/qemu_domain.c | 2 ++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 31 insertions(+), 1 deletion(-)
I guess I find it "strange" to have a driver argument be copied into storage source, but I guess that just an "implementation detail" that I have to live with.
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index cbf0aa4..5f98b85 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1322,7 +1322,6 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, * 'pass-discard-snapshot' * 'pass-discard-other' * 'overlap-check' - * 'l2-cache-size' * 'l2-cache-entry-size' * 'refcount-cache-size' * 'cache-clean-interval' @@ -1331,6 +1330,10 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) return -1;
+ if (src->metadata_cache_size == VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM && + virJSONValueObjectAdd(props, "I:l2-cache-size", INT64_MAX, NULL) < 0) + return -1; + return 0; }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8..96d6601 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1328,6 +1328,20 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, return -1; }
+ if (disk->metadata_cache_size) { + if (disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set for qcow2 disks")); + return -1; + } + + if (disk->metadata_cache_size != VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set to 'maximum'"));
Since the only way to get here is if metadata_cache_size is set and the only way it's set when it's VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM, then I see this as unnecessary... If you think the future holds more than "minimum" (0) and "maximum" (1), then change the condition into here to be metadata_cache_size > VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MINIMUM Even with that I wouldn't bother with this check - leave that for a future change.
+ return -1; + } + } + if (qemuCaps) { if (disk->serial && disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && @@ -1351,6 +1365,15 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, _("detect_zeroes is not supported by this QEMU binary")); return -1; } + + if (disk->metadata_cache_size && + !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED))) {
Even though l2-cache-size needs -blockdev (qemu 3.0+) support to add the field is only qemu 3.1+, so using BLOCKDEV here just becomes a mechanism to note what type of support requires BLOCKDEV. IDC if the check stays - it's just superfluous then. John
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting metadata_cache_size is not supported by " + "this QEMU binary")); + return -1; + } }
if (disk->serial && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ec6b340..fc5c7c2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9202,6 +9202,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, /* "snapshot" is a libvirt internal field and thus can be changed */ /* startupPolicy is allowed to be updated. Therefore not checked here. */ CHECK_EQ(transient, "transient", true); + CHECK_EQ(metadata_cache_size, "metadata_cache_size", true);
/* Note: For some address types the address auto generation for * @disk has still not happened at this point (e.g. driver @@ -13370,6 +13371,7 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, src->iomode = disk->iomode; src->cachemode = disk->cachemode; src->discard = disk->discard; + src->metadata_cache_size = disk->metadata_cache_size;
if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) src->floppyimg = true; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index bd4b027..e5660e6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2208,6 +2208,7 @@ virStorageSourceCopy(const virStorageSource *src, ret->cachemode = src->cachemode; ret->discard = src->discard; ret->detect_zeroes = src->detect_zeroes; + ret->metadata_cache_size = src->metadata_cache_size;
/* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1d6161a..8bf5fe4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -329,6 +329,7 @@ struct _virStorageSource { int cachemode; /* enum virDomainDiskCache */ int discard; /* enum virDomainDiskDiscard */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ + int metadata_cache_size; /* enum virDomainDiskImageMetadataCacheSize */
bool floppyimg; /* set to true if the storage source is going to be used as a source for floppy drive */

On 23.01.2019 00:45, John Ferlan wrote:
On 1/10/19 7:15 AM, Nikolay Shirokovskiy wrote:
Just set l2-cache-size to INT64_MAX for all format nodes of qcow2 type in block node graph.
-drive configuration is not supported because we can not set l2 cache size down the backing chain in this case.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_block.c | 5 ++++- src/qemu/qemu_command.c | 23 +++++++++++++++++++++++ src/qemu/qemu_domain.c | 2 ++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 31 insertions(+), 1 deletion(-)
I guess I find it "strange" to have a driver argument be copied into storage source, but I guess that just an "implementation detail" that I have to live with.
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index cbf0aa4..5f98b85 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1322,7 +1322,6 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, * 'pass-discard-snapshot' * 'pass-discard-other' * 'overlap-check' - * 'l2-cache-size' * 'l2-cache-entry-size' * 'refcount-cache-size' * 'cache-clean-interval' @@ -1331,6 +1330,10 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) return -1;
+ if (src->metadata_cache_size == VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM && + virJSONValueObjectAdd(props, "I:l2-cache-size", INT64_MAX, NULL) < 0) + return -1; + return 0; }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8..96d6601 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1328,6 +1328,20 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, return -1; }
+ if (disk->metadata_cache_size) { + if (disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set for qcow2 disks")); + return -1; + } + + if (disk->metadata_cache_size != VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache_size can only be set to 'maximum'"));
Since the only way to get here is if metadata_cache_size is set and the only way it's set when it's VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MAXIMUM, then I see this as unnecessary...
If you think the future holds more than "minimum" (0) and "maximum" (1), then change the condition into here to be metadata_cache_size > VIR_DOMAIN_DISK_METADATA_CACHE_SIZE_MINIMUM
Even with that I wouldn't bother with this check - leave that for a future change.
Yes, this is future proof. So that if one adds one more value to enum we get error immediately if someone tries to use newly added value. Without the check the qemuBlockStorageSourceGetFormatQcow2Props will simply ignore new value. We could put the check into qemuBlockStorageSourceGetFormatQcow2Props itself but looks like checking functionality resides in qemuCheckDiskConfig. Nikolay
+ return -1; + } + } + if (qemuCaps) { if (disk->serial && disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && @@ -1351,6 +1365,15 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk, _("detect_zeroes is not supported by this QEMU binary")); return -1; } + + if (disk->metadata_cache_size && + !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED))) {
Even though l2-cache-size needs -blockdev (qemu 3.0+) support to add the field is only qemu 3.1+, so using BLOCKDEV here just becomes a mechanism to note what type of support requires BLOCKDEV.
IDC if the check stays - it's just superfluous then.
John
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting metadata_cache_size is not supported by " + "this QEMU binary")); + return -1; + } }
if (disk->serial && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ec6b340..fc5c7c2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9202,6 +9202,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, /* "snapshot" is a libvirt internal field and thus can be changed */ /* startupPolicy is allowed to be updated. Therefore not checked here. */ CHECK_EQ(transient, "transient", true); + CHECK_EQ(metadata_cache_size, "metadata_cache_size", true);
/* Note: For some address types the address auto generation for * @disk has still not happened at this point (e.g. driver @@ -13370,6 +13371,7 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, src->iomode = disk->iomode; src->cachemode = disk->cachemode; src->discard = disk->discard; + src->metadata_cache_size = disk->metadata_cache_size;
if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) src->floppyimg = true; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index bd4b027..e5660e6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2208,6 +2208,7 @@ virStorageSourceCopy(const virStorageSource *src, ret->cachemode = src->cachemode; ret->discard = src->discard; ret->detect_zeroes = src->detect_zeroes; + ret->metadata_cache_size = src->metadata_cache_size;
/* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1d6161a..8bf5fe4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -329,6 +329,7 @@ struct _virStorageSource { int cachemode; /* enum virDomainDiskCache */ int discard; /* enum virDomainDiskDiscard */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ + int metadata_cache_size; /* enum virDomainDiskImageMetadataCacheSize */
bool floppyimg; /* set to true if the storage source is going to be used as a source for floppy drive */

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 8c608cd..e16d82d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -50,6 +50,17 @@ <section title="New features"> <change> <summary> + Add metadata cache size policy for disks + </summary> + <description> + Disk workloads using very big datasets intensively can have bad + perfomance for disks backed by non raw images. The cause is default + limit on disk's metadata cache size. Now one has option to keep all + metadata cache in memory. + </description> + </change> + <change> + <summary> Xen: Add support for openvswitch </summary> <description> -- 1.8.3.1

On 1/10/19 7:15 AM, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 8c608cd..e16d82d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -50,6 +50,17 @@ <section title="New features"> <change> <summary> + Add metadata cache size policy for disks
s/Add/Allow s/disks/qcow2 disks
+ </summary> + <description> + Disk workloads using very big datasets intensively can have bad
s/Disk workloads/Workloads s/big/large ?
+ perfomance for disks backed by non raw images. The cause is default
*performance s/disks/qcow2 disks/
+ limit on disk's metadata cache size. Now one has option to keep all
s/on/on the/ s/Now one has option to/Using the disk driver XML attribute "metadata_cache_size='maximum'" it is now possible to/ John
+ metadata cache in memory. + </description> + </change> + <change> + <summary> Xen: Add support for openvswitch </summary> <description>

This needs next: - turning QEMU_CAPS_BLOCKDEV on - adding caps data for not yet released qemu 3.1 Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 2 files changed, 41 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args diff --git a/tests/qemuxml2argvdata/disk-metadata_cache_size.args b/tests/qemuxml2argvdata/disk-metadata_cache_size.args new file mode 100644 index 0000000..ec90d2f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-metadata_cache_size.args @@ -0,0 +1,39 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test \ +-S \ +-machine pc-0.13,accel=tcg,usb=off,dump-guest-core=off \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 468404ad-d49c-40f2-9e14-02294f9c1be3 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot menu=on \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 \ +-usb \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/f14.img",\ +"node-name":"libvirt-2-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2",\ +"l2-cache-size":9223372036854775807,"file":"libvirt-2-storage"}' \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-2-format,\ +id=virtio-disk0,bootindex=2 \ +-blockdev '{"driver":"file",\ +"filename":"/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso",\ +"node-name":"libvirt-1-storage","read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-drive,bus=ide.1,unit=0,drive=libvirt-1-format,id=ide0-1-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2cb8860..72cd647 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3076,6 +3076,8 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("x86_64-pc-headless", "x86_64"); DO_TEST_CAPS_ARCH_LATEST("x86_64-q35-headless", "x86_64"); + DO_TEST_CAPS_LATEST("disk-metadata_cache_size"); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 1.8.3.1

On 1/10/19 7:15 AM, Nikolay Shirokovskiy wrote:
This needs next: - turning QEMU_CAPS_BLOCKDEV on - adding caps data for not yet released qemu 3.1
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 2 files changed, 41 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args
File name output changes since you wrote this mean the output would be named: disk-metadata_cache_size.x86_64-latest.args oh and more/different output would be on the command line... Waiting for Peter's -blockdev changes would be best still I think. John
diff --git a/tests/qemuxml2argvdata/disk-metadata_cache_size.args b/tests/qemuxml2argvdata/disk-metadata_cache_size.args new file mode 100644 index 0000000..ec90d2f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-metadata_cache_size.args @@ -0,0 +1,39 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test \ +-S \ +-machine pc-0.13,accel=tcg,usb=off,dump-guest-core=off \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 468404ad-d49c-40f2-9e14-02294f9c1be3 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot menu=on \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 \ +-usb \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/f14.img",\ +"node-name":"libvirt-2-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2",\ +"l2-cache-size":9223372036854775807,"file":"libvirt-2-storage"}' \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-2-format,\ +id=virtio-disk0,bootindex=2 \ +-blockdev '{"driver":"file",\ +"filename":"/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso",\ +"node-name":"libvirt-1-storage","read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-drive,bus=ide.1,unit=0,drive=libvirt-1-format,id=ide0-1-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2cb8860..72cd647 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3076,6 +3076,8 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("x86_64-pc-headless", "x86_64"); DO_TEST_CAPS_ARCH_LATEST("x86_64-q35-headless", "x86_64");
+ DO_TEST_CAPS_LATEST("disk-metadata_cache_size"); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir);

Thanx for review, John! Let's wait for finishing -blockdev works indeed. Nikolay On 10.01.2019 15:15, Nikolay Shirokovskiy wrote:
As this patch series only works with -blockdev configurations it is better to be pushed only after -blockdev is supported. Then 'news' and 'xml' patch need to be updated to have correct libvirt version.
Diff from v2 [1]: ================= - move caps patch to top - add news patch - add _CAPPED to capabilitity name to distinguish from correspondent qemu capabilitiy - better option explanation in formatdomain.html - misc tests/english fixes as suggested
We have come to agreement with John in mailing list to support QEMU versions which do not let use INT64_MAX as "maximum" value. But this will require changing qemuBlockStorageSourceGetFormatQcow2Props and a lot of other functions signature (which was not noticed during conversation). Thus I don't add such a functionality. Anyway the patch requires -blockdev from QEMU so we could only support single QEMU 3.0 version.
[1] https://www.redhat.com/archives/libvir-list/2018-November/msg00313.html
Nikolay Shirokovskiy (5): qemu: caps: add QEMU_CAPS_QCOW2_L2_CACHE_SIZE_CAPPED xml: add disk driver metadata_cache_size option qemu: support metadata-cache-size for blockdev news: add notice for new metadata cache size policy option DO NOT APPLY: add xml2argv test for metadata_cache_size
docs/formatdomain.html.in | 11 ++++++ docs/news.xml | 11 ++++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 17 +++++++++ src/conf/domain_conf.h | 9 +++++ src/qemu/qemu_block.c | 5 ++- src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 23 ++++++++++++ src/qemu/qemu_domain.c | 2 ++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + .../qemuxml2argvdata/disk-metadata_cache_size.args | 39 ++++++++++++++++++++ .../qemuxml2argvdata/disk-metadata_cache_size.xml | 35 ++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../disk-metadata_cache_size.xml | 41 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 17 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.args create mode 100644 tests/qemuxml2argvdata/disk-metadata_cache_size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata_cache_size.xml
participants (2)
-
John Ferlan
-
Nikolay Shirokovskiy