[PATCH 0/5] qemu: Introduce control of qcow2 metadata cache maximum size
See patch 3/5 for explanation. Peter Krempa (5): virDomainDiskDefFormatDriver: Rename 'driverBuf' to 'attrBuf' virDomainSnapshotDiskDefFormat: Use virXMLFormatElement conf: Introduce <metadata_cache> subelement of <disk><driver> conf: snapshot: Add support for <metadata_cache> qemu: Implement '<metadata_cache><max_size>' control for qcow2 docs/formatdomain.rst | 43 ++++++++++ docs/formatsnapshot.html.in | 4 + docs/schemas/domaincommon.rng | 20 ++++- docs/schemas/domainsnapshot.rng | 10 ++- src/conf/domain_conf.c | 81 ++++++++++++++----- src/conf/snapshot_conf.c | 50 ++++++++---- src/qemu/qemu_block.c | 11 +++ src/qemu/qemu_domain.c | 15 ++++ src/qemu/qemu_snapshot.c | 14 ++++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qcow2-metadata-cache.xml | 14 ++++ .../qcow2-metadata-cache.xml | 18 +++++ tests/qemudomainsnapshotxml2xmltest.c | 3 + .../disk-metadata-cache.x86_64-latest.args | 57 +++++++++++++ .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++ tests/qemuxml2argvtest.c | 1 + .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 411 insertions(+), 38 deletions(-) create mode 100644 tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml -- 2.29.2
Unify the code with other places using virXMLFormatElement. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 453e06491e..3e1dccb1b8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24145,59 +24145,59 @@ static void virDomainDiskDefFormatDriver(virBufferPtr buf, virDomainDiskDefPtr disk) { - g_auto(virBuffer) driverBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - virBufferEscapeString(&driverBuf, " name='%s'", virDomainDiskGetDriver(disk)); + virBufferEscapeString(&attrBuf, " name='%s'", virDomainDiskGetDriver(disk)); if (disk->src->format > 0) - virBufferAsprintf(&driverBuf, " type='%s'", + virBufferAsprintf(&attrBuf, " type='%s'", virStorageFileFormatTypeToString(disk->src->format)); if (disk->cachemode) - virBufferAsprintf(&driverBuf, " cache='%s'", + virBufferAsprintf(&attrBuf, " cache='%s'", virDomainDiskCacheTypeToString(disk->cachemode)); if (disk->error_policy) - virBufferAsprintf(&driverBuf, " error_policy='%s'", + virBufferAsprintf(&attrBuf, " error_policy='%s'", virDomainDiskErrorPolicyTypeToString(disk->error_policy)); if (disk->rerror_policy) - virBufferAsprintf(&driverBuf, " rerror_policy='%s'", + virBufferAsprintf(&attrBuf, " rerror_policy='%s'", virDomainDiskErrorPolicyTypeToString(disk->rerror_policy)); if (disk->iomode) - virBufferAsprintf(&driverBuf, " io='%s'", + virBufferAsprintf(&attrBuf, " io='%s'", virDomainDiskIoTypeToString(disk->iomode)); if (disk->ioeventfd) - virBufferAsprintf(&driverBuf, " ioeventfd='%s'", + virBufferAsprintf(&attrBuf, " ioeventfd='%s'", virTristateSwitchTypeToString(disk->ioeventfd)); if (disk->event_idx) - virBufferAsprintf(&driverBuf, " event_idx='%s'", + virBufferAsprintf(&attrBuf, " event_idx='%s'", virTristateSwitchTypeToString(disk->event_idx)); if (disk->copy_on_read) - virBufferAsprintf(&driverBuf, " copy_on_read='%s'", + virBufferAsprintf(&attrBuf, " copy_on_read='%s'", virTristateSwitchTypeToString(disk->copy_on_read)); if (disk->discard) - virBufferAsprintf(&driverBuf, " discard='%s'", + virBufferAsprintf(&attrBuf, " discard='%s'", virDomainDiskDiscardTypeToString(disk->discard)); if (disk->iothread) - virBufferAsprintf(&driverBuf, " iothread='%u'", disk->iothread); + virBufferAsprintf(&attrBuf, " iothread='%u'", disk->iothread); if (disk->detect_zeroes) - virBufferAsprintf(&driverBuf, " detect_zeroes='%s'", + virBufferAsprintf(&attrBuf, " detect_zeroes='%s'", virDomainDiskDetectZeroesTypeToString(disk->detect_zeroes)); if (disk->queues) - virBufferAsprintf(&driverBuf, " queues='%u'", disk->queues); + virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues); - virDomainVirtioOptionsFormat(&driverBuf, disk->virtio); + virDomainVirtioOptionsFormat(&attrBuf, disk->virtio); - virXMLFormatElement(buf, "driver", &driverBuf, NULL); + virXMLFormatElement(buf, "driver", &attrBuf, NULL); } -- 2.29.2
Refactor the code to use modern XML formatting approach. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/snapshot_conf.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index f896fd1cf2..757af681cd 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -772,33 +772,30 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainSnapshotDiskDefPtr disk, virDomainXMLOptionPtr xmlopt) { - int type = disk->src->type; + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); if (!disk->name) return 0; - virBufferEscapeString(buf, "<disk name='%s'", disk->name); + virBufferEscapeString(&attrBuf, " name='%s'", disk->name); if (disk->snapshot > 0) - virBufferAsprintf(buf, " snapshot='%s'", + virBufferAsprintf(&attrBuf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(disk->snapshot)); - if (!disk->src->path && disk->src->format == 0) { - virBufferAddLit(buf, "/>\n"); - return 0; - } + if (disk->src->path || disk->src->format != 0) { + virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->src->type)); - virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type)); - virBufferAdjustIndent(buf, 2); + if (disk->src->format > 0) + virBufferEscapeString(&childBuf, "<driver type='%s'/>\n", + virStorageFileFormatTypeToString(disk->src->format)); - if (disk->src->format > 0) - virBufferEscapeString(buf, "<driver type='%s'/>\n", - virStorageFileFormatTypeToString(disk->src->format)); - if (virDomainDiskSourceFormat(buf, disk->src, "source", 0, false, 0, - false, false, xmlopt) < 0) + if (virDomainDiskSourceFormat(&childBuf, disk->src, "source", 0, false, 0, + false, false, xmlopt) < 0) return -1; + } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</disk>\n"); + virXMLFormatElement(buf, "disk", &attrBuf, &childBuf); return 0; } -- 2.29.2
In certain specific cases it might be beneficial to be able to control the metadata caching of storage image format drivers of a hypervisor. Introduce XML machinery to set the maximum size of the metadata cache which will be used by qemu's qcow2 driver. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 43 ++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++- src/conf/domain_conf.c | 51 ++++++++++++++-- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++++++ .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1189795974..abf85064fa 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2735,6 +2735,11 @@ paravirtualized driver is specified via the ``disk`` element. ``format`` The ``format`` element contains ``type`` attribute which specifies the internal format of the backing store, such as ``raw`` or ``qcow2``. + + The ``format`` element can contain ``metadata_cache`` subelement, which + has identical semantics to the identically named subelement of ``driver`` + of a ``disk``. + ``source`` This element has the same structure as the ``source`` element in ``disk``. It specifies which file, device, or network location contains the data of @@ -2947,6 +2952,44 @@ paravirtualized driver is specified via the ``disk`` element. virtio-blk. ( :since:`Since 3.9.0` ) - For virtio disks, `Virtio-specific options <#elementsVirtio>`__ can also be set. ( :since:`Since 3.5.0` ) + - The optional ``metadata_cache`` subelement controls aspects related to the + format specific caching of storage image metadata. Note that this setting + applies only on the top level image; the identically named sublelement of + ``backingStore``'s ``format`` element can be used to specify cache + settings for the backing image. + + :since:`Since 7.0.0` the maximum size of the metadata cache of ``qcow2`` + format driver of the ``qemu`` hypervisor can be controlled via the + ``max_size`` subelement (see example below). + + In the majority of cases the default configuration used by the hypervisor + is sufficient so modifying this setting should not be necessary. For + specifics on how the metadata cache of ``qcow2`` in ``qemu`` behaves refer + to the ``qemu`` + `qcow2 cache docs <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt>`__ + + **Example:** + +:: + + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </driver> + <source file='/var/lib/libvirt/images/domain.qcow'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </format> + <source file='/var/lib/libvirt/images/snapshot.qcow'/> + <backingStore/> + </backingStore> + <target dev='vdd' bus='virtio'/> + </disk> ``backenddomain`` The optional ``backenddomain`` element allows specifying a backend domain diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 24b4994670..56946a621a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1600,7 +1600,15 @@ <attribute name="type"> <ref name="storageFormat"/> </attribute> - <empty/> + <optional> + <element name="metadata_cache"> + <optional> + <element name="max_size"> + <ref name="scaledInteger"/> + </element> + </optional> + </element> + </optional> </element> </define> @@ -2245,7 +2253,15 @@ </attribute> </optional> <ref name="virtioOptions"/> - <empty/> + <optional> + <element name="metadata_cache"> + <optional> + <element name="max_size"> + <ref name="scaledInteger"/> + </element> + </optional> + </element> + </optional> </element> </define> <define name="driverFormat"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e1dccb1b8..ddacc6443b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8589,6 +8589,12 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, if (!(backingStore = virDomainStorageSourceParseBase(type, format, idx))) return -1; + if (virParseScaledValue("./format/metadata_cache/max_size", NULL, + ctxt, + &backingStore->metadataCacheMaxSize, + 1, ULLONG_MAX, false) < 0) + return -1; + /* backing store is always read-only */ backingStore->readonly = true; @@ -8939,9 +8945,13 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def) static int virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, - xmlNodePtr cur) + xmlNodePtr cur, + xmlXPathContextPtr ctxt) { g_autofree char *tmp = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + ctxt->node = cur; def->driverName = virXMLPropString(cur, "name"); @@ -9051,6 +9061,12 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, return -1; } + if (virParseScaledValue("./metadata_cache/max_size", NULL, + ctxt, + &def->src->metadataCacheMaxSize, + 1, ULLONG_MAX, false) < 0) + return -1; + return 0; } @@ -9207,7 +9223,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) return NULL; - if (virDomainDiskDefDriverParseXML(def, cur) < 0) + if (virDomainDiskDefDriverParseXML(def, cur, ctxt) < 0) return NULL; } else if (!def->mirror && virXMLNodeNameEqual(cur, "mirror") && @@ -24050,6 +24066,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) formatAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) formatChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE; virStorageSourcePtr backingStore = src->backingStore; @@ -24077,8 +24095,22 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, if (backingStore->id != 0) virBufferAsprintf(&attrBuf, " index='%u'", backingStore->id); - virBufferAsprintf(&childBuf, "<format type='%s'/>\n", + virBufferAsprintf(&formatAttrBuf, " type='%s'", virStorageFileFormatTypeToString(backingStore->format)); + + if (backingStore->metadataCacheMaxSize > 0) { + g_auto(virBuffer) metadataCacheChildBuf = VIR_BUFFER_INIT_CHILD(&formatChildBuf); + + virBufferAsprintf(&metadataCacheChildBuf, + "<max_size unit='bytes'>%llu</max_size>\n", + backingStore->metadataCacheMaxSize); + + virXMLFormatElement(&formatChildBuf, "metadata_cache", NULL, &metadataCacheChildBuf); + } + + virXMLFormatElement(&childBuf, "format", &formatAttrBuf, &formatChildBuf); + + if (virDomainDiskSourceFormat(&childBuf, backingStore, "source", 0, false, flags, false, false, xmlopt) < 0) return -1; @@ -24146,6 +24178,7 @@ virDomainDiskDefFormatDriver(virBufferPtr buf, virDomainDiskDefPtr disk) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); virBufferEscapeString(&attrBuf, " name='%s'", virDomainDiskGetDriver(disk)); @@ -24197,7 +24230,17 @@ virDomainDiskDefFormatDriver(virBufferPtr buf, virDomainVirtioOptionsFormat(&attrBuf, disk->virtio); - virXMLFormatElement(buf, "driver", &attrBuf, NULL); + if (disk->src->metadataCacheMaxSize > 0) { + g_auto(virBuffer) metadataCacheChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + + virBufferAsprintf(&metadataCacheChildBuf, + "<max_size unit='bytes'>%llu</max_size>\n", + disk->src->metadataCacheMaxSize); + + virXMLFormatElement(&childBuf, "metadata_cache", NULL, &metadataCacheChildBuf); + } + + virXMLFormatElement(buf, "driver", &attrBuf, &childBuf); } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3db85d8b89..f870657dd8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2216,6 +2216,7 @@ virStorageSourceCopy(const virStorageSource *src, def->sslverify = src->sslverify; def->readahead = src->readahead; def->timeout = src->timeout; + def->metadataCacheMaxSize = src->metadataCacheMaxSize; /* storage driver metadata are not copied */ def->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 2452b967b2..205db1b997 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -321,6 +321,8 @@ struct _virStorageSource { unsigned long long clusterSize; /* in bytes, 0 if unknown */ bool has_allocation; /* Set to true when provided in XML */ + unsigned long long metadataCacheMaxSize; /* size of the metadata cache in bytes */ + size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; diff --git a/tests/qemuxml2argvdata/disk-metadata-cache.xml b/tests/qemuxml2argvdata/disk-metadata-cache.xml new file mode 100644 index 0000000000..d79f194eee --- /dev/null +++ b/tests/qemuxml2argvdata/disk-metadata-cache.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'> + <metadata_cache> + <max_size>12345</max_size> + </metadata_cache> + </driver> + <source file='/tmp/QEMUGuest1.img'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/tmp/QEMUGuest2.img'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='kiB'>1024</max_size> + </metadata_cache> + </format> + <source file='/tmp/backing-store.qcow'/> + <backingStore/> + </backingStore> + <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml new file mode 100644 index 0000000000..7104151a10 --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml @@ -0,0 +1,58 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'> + <metadata_cache> + <max_size unit='bytes'>12345</max_size> + </metadata_cache> + </driver> + <source file='/tmp/QEMUGuest1.img'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/tmp/QEMUGuest2.img'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1048576</max_size> + </metadata_cache> + </format> + <source file='/tmp/backing-store.qcow'/> + <backingStore/> + </backingStore> + <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </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='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f8bca9f559..093fceea3e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -294,6 +294,7 @@ mymain(void) DO_TEST_CAPS_VER("disk-cache", "2.7.0"); DO_TEST_CAPS_VER("disk-cache", "2.12.0"); DO_TEST_CAPS_LATEST("disk-cache"); + DO_TEST_CAPS_LATEST("disk-metadata-cache"); DO_TEST("disk-network-nbd", NONE); DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_BLOCK); -- 2.29.2
On a Thursday in 2021, Peter Krempa wrote:
In certain specific cases it might be beneficial to be able to control the metadata caching of storage image format drivers of a hypervisor.
Introduce XML machinery to set the maximum size of the metadata cache which will be used by qemu's qcow2 driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 43 ++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++- src/conf/domain_conf.c | 51 ++++++++++++++-- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++++++ .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1189795974..abf85064fa 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2735,6 +2735,11 @@ paravirtualized driver is specified via the ``disk`` element. ``format`` The ``format`` element contains ``type`` attribute which specifies the internal format of the backing store, such as ``raw`` or ``qcow2``. + + The ``format`` element can contain ``metadata_cache`` subelement, which + has identical semantics to the identically named subelement of ``driver`` + of a ``disk``. + ``source`` This element has the same structure as the ``source`` element in ``disk``. It specifies which file, device, or network location contains the data of @@ -2947,6 +2952,44 @@ paravirtualized driver is specified via the ``disk`` element. virtio-blk. ( :since:`Since 3.9.0` ) - For virtio disks, `Virtio-specific options <#elementsVirtio>`__ can also be set. ( :since:`Since 3.5.0` ) + - The optional ``metadata_cache`` subelement controls aspects related to the + format specific caching of storage image metadata. Note that this setting + applies only on the top level image; the identically named sublelement of + ``backingStore``'s ``format`` element can be used to specify cache + settings for the backing image. + + :since:`Since 7.0.0` the maximum size of the metadata cache of ``qcow2`` + format driver of the ``qemu`` hypervisor can be controlled via the + ``max_size`` subelement (see example below). + + In the majority of cases the default configuration used by the hypervisor + is sufficient so modifying this setting should not be necessary. For + specifics on how the metadata cache of ``qcow2`` in ``qemu`` behaves refer + to the ``qemu`` + `qcow2 cache docs <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt>`__ + + **Example:** + +:: + + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </driver> + <source file='/var/lib/libvirt/images/domain.qcow'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache>
I don't like introducing a snake_case element under a camelCase one. But regardless of that, our XML is a mix of both of those and skewer-case. See also: https://www.redhat.com/archives/libvir-list/2021-January/msg00371.html Jano
+ </format> + <source file='/var/lib/libvirt/images/snapshot.qcow'/> + <backingStore/> + </backingStore> + <target dev='vdd' bus='virtio'/> + </disk>
``backenddomain`` The optional ``backenddomain`` element allows specifying a backend domain
On a Thursday in 2021, Peter Krempa wrote:
In certain specific cases it might be beneficial to be able to control the metadata caching of storage image format drivers of a hypervisor.
Introduce XML machinery to set the maximum size of the metadata cache which will be used by qemu's qcow2 driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 43 ++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++- src/conf/domain_conf.c | 51 ++++++++++++++-- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++++++ .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1189795974..abf85064fa 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2735,6 +2735,11 @@ paravirtualized driver is specified via the ``disk`` element. ``format`` The ``format`` element contains ``type`` attribute which specifies the internal format of the backing store, such as ``raw`` or ``qcow2``. + + The ``format`` element can contain ``metadata_cache`` subelement, which + has identical semantics to the identically named subelement of ``driver`` + of a ``disk``. + ``source`` This element has the same structure as the ``source`` element in ``disk``. It specifies which file, device, or network location contains the data of @@ -2947,6 +2952,44 @@ paravirtualized driver is specified via the ``disk`` element. virtio-blk. ( :since:`Since 3.9.0` ) - For virtio disks, `Virtio-specific options <#elementsVirtio>`__ can also be set. ( :since:`Since 3.5.0` ) + - The optional ``metadata_cache`` subelement controls aspects related to the + format specific caching of storage image metadata. Note that this setting
+ applies only on the top level image; the identically named sublelement of
Also: s/lelement/element/ Jano
+ ``backingStore``'s ``format`` element can be used to specify cache + settings for the backing image. + + :since:`Since 7.0.0` the maximum size of the metadata cache of ``qcow2`` + format driver of the ``qemu`` hypervisor can be controlled via the + ``max_size`` subelement (see example below). + + In the majority of cases the default configuration used by the hypervisor + is sufficient so modifying this setting should not be necessary. For + specifics on how the metadata cache of ``qcow2`` in ``qemu`` behaves refer + to the ``qemu`` + `qcow2 cache docs <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt>`__
On Thu, Jan 07, 2021 at 03:59:36PM +0100, Peter Krempa wrote:
In certain specific cases it might be beneficial to be able to control the metadata caching of storage image format drivers of a hypervisor.
Introduce XML machinery to set the maximum size of the metadata cache which will be used by qemu's qcow2 driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 43 ++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++- src/conf/domain_conf.c | 51 ++++++++++++++-- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++++++ .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
+ <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/tmp/QEMUGuest2.img'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='kiB'>1024</max_size> + </metadata_cache>
I only wonder if this is uneccessarily verbose XML nesting ? Not a blocker though. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Jan 08, 2021 at 13:13:30 +0000, Daniel Berrange wrote:
On Thu, Jan 07, 2021 at 03:59:36PM +0100, Peter Krempa wrote:
In certain specific cases it might be beneficial to be able to control the metadata caching of storage image format drivers of a hypervisor.
Introduce XML machinery to set the maximum size of the metadata cache which will be used by qemu's qcow2 driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 43 ++++++++++++++ docs/schemas/domaincommon.rng | 20 ++++++- src/conf/domain_conf.c | 51 ++++++++++++++-- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++++++ .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
+ <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/tmp/QEMUGuest2.img'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='kiB'>1024</max_size> + </metadata_cache>
I only wonder if this is uneccessarily verbose XML nesting ?
I actually had the same feeling, but adding it just as an attribute to <disk><driver ... >(the other case besides <backingStore> felt worse since we have a huge list of attributes there. I specifically want that it points to metadata cache maximum size so that there isn't any unnecessary ambiguity. Possibilities are to merge the <max_size> subelement into the parent element so that we decrease the amount of lines. Or possibly go with Jano's suggestion of dropping 'size' since that is obvious when unit='' is present (but note that unit is optional on input).
Similarly to the domain config code it may be beneficial to control the cache size of images introduced as snapshots into the backing chain. Wire up handling of the 'metadata_cache' element. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatsnapshot.html.in | 4 ++++ docs/schemas/domainsnapshot.rng | 10 +++++++- src/conf/snapshot_conf.c | 23 ++++++++++++++++++- .../qcow2-metadata-cache.xml | 14 +++++++++++ .../qcow2-metadata-cache.xml | 18 +++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 3 +++ 6 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index d640deb86d..e481284aa8 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -186,6 +186,10 @@ with an attribute <code>type</code> giving the driver type (such as qcow2), of the new file created by the external snapshot of the new file. + + Optionally <code>metadata_cache</code> sub-element can be used + with same semantics as the identically named subelement of the + domain definition disk's driver. </dd> <dt><code>seclabel</code></dt> </dl> diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index e1fb4f7cea..58c370878d 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -212,7 +212,15 @@ <ref name="storageFormatBacking"/> </attribute> </optional> - <empty/> + <optional> + <element name="metadata_cache"> + <optional> + <element name="max_size"> + <ref name="scaledInteger"/> + </element> + </optional> + </element> + </optional> </element> </optional> </define> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 757af681cd..673282be7a 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -190,6 +190,12 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, goto cleanup; } + if (virParseScaledValue("./driver/metadata_cache/max_size", NULL, + ctxt, + &def->src->metadataCacheMaxSize, + 1, ULLONG_MAX, false) < 0) + goto cleanup; + /* validate that the passed path is absolute */ if (virStorageSourceIsRelative(def->src)) { virReportError(VIR_ERR_XML_ERROR, @@ -784,12 +790,27 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainSnapshotLocationTypeToString(disk->snapshot)); if (disk->src->path || disk->src->format != 0) { + g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) driverChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->src->type)); if (disk->src->format > 0) - virBufferEscapeString(&childBuf, "<driver type='%s'/>\n", + virBufferEscapeString(&driverAttrBuf, " type='%s'", virStorageFileFormatTypeToString(disk->src->format)); + if (disk->src->metadataCacheMaxSize > 0) { + g_auto(virBuffer) metadataCacheChildBuf = VIR_BUFFER_INIT_CHILD(&driverChildBuf); + + virBufferAsprintf(&metadataCacheChildBuf, + "<max_size unit='bytes'>%llu</max_size>\n", + disk->src->metadataCacheMaxSize); + + virXMLFormatElement(&driverChildBuf, "metadata_cache", NULL, &metadataCacheChildBuf); + } + + virXMLFormatElement(&childBuf, "driver", &driverAttrBuf, &driverChildBuf); + if (virDomainDiskSourceFormat(&childBuf, disk->src, "source", 0, false, 0, false, false, xmlopt) < 0) return -1; diff --git a/tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml b/tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml new file mode 100644 index 0000000000..92440aa0ae --- /dev/null +++ b/tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml @@ -0,0 +1,14 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <disks> + <disk name='hdd' snapshot='external'> + <source file='/path/to/new'/> + <driver type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </driver> + </disk> + </disks> +</domainsnapshot> diff --git a/tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml b/tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml new file mode 100644 index 0000000000..def2a8ffce --- /dev/null +++ b/tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml @@ -0,0 +1,18 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <creationTime>1386166249</creationTime> + <disks> + <disk name='hdd' snapshot='external' type='file'> + <driver type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </driver> + <source file='/path/to/new'/> + </disk> + </disks> + <domain> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> + </domain> +</domainsnapshot> diff --git a/tests/qemudomainsnapshotxml2xmltest.c b/tests/qemudomainsnapshotxml2xmltest.c index 4b92967339..2a1fe1f52d 100644 --- a/tests/qemudomainsnapshotxml2xmltest.c +++ b/tests/qemudomainsnapshotxml2xmltest.c @@ -181,6 +181,9 @@ mymain(void) DO_TEST_IN("description_only", NULL); DO_TEST_IN("name_only", NULL); + DO_TEST_INOUT("qcow2-metadata-cache", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", + 1386166249, 0); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.29.2
qemu's qcow2 driver allows control of the metadata cache of qcow2 driver by the 'cache-size' property. Wire it up to the recently introduced elements. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 11 ++++ src/qemu/qemu_domain.c | 15 +++++ src/qemu/qemu_snapshot.c | 14 +++++ .../disk-metadata-cache.x86_64-latest.args | 57 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 98 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4640e339c0..857c5aa8d0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1343,6 +1343,17 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) return -1; + /* 'cache-size' controls the maximum size of l2 and refcount caches. + * see: qemu.git/docs/qcow2-cache.txt + * https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt + */ + if (src->metadataCacheMaxSize > 0) { + if (virJSONValueObjectAdd(props, + "U:cache-size", src->metadataCacheMaxSize, + NULL) < 0) + return -1; + } + return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bfb6e23942..38defdab4b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4626,6 +4626,21 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + /* metadata cache size control is currently supported only for qcow2 */ + if (src->metadataCacheMaxSize > 0) { + if (src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metdata cache max size control is supported only with qcow2 images")); + return -1; + } + + if (!blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metdata cache max size control is not supported with this QEMU binary")); + return -1; + } + } + return 0; } diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 15494c3415..e7e7e7babc 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -717,6 +717,20 @@ qemuSnapshotPrepare(virDomainObjPtr vm, return -1; } + if (disk->src->metadataCacheMaxSize > 0) { + if (disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metdata cache max size control is supported only with qcow2 images")); + return -1; + } + + if (!blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metdata cache max size control is not supported with this QEMU binary")); + return -1; + } + } + if (qemuSnapshotPrepareDiskExternal(vm, dom_disk, disk, active, reuse, blockdev) < 0) return -1; diff --git a/tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args b/tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args new file mode 100644 index 0000000000..3e520664ab --- /dev/null +++ b/tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args @@ -0,0 +1,57 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-cpu qemu64 \ +-m 214 \ +-object memory-backend-ram,id=pc.ram,size=224395264 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"file","filename":"/tmp/QEMUGuest1.img",\ +"node-name":"libvirt-3-storage","cache":{"direct":true,"no-flush":false},\ +"auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,\ +"cache":{"direct":true,"no-flush":false},"driver":"qcow2","cache-size":12345,\ +"file":"libvirt-3-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-3-format,id=ide0-0-0,bootindex=1,\ +write-cache=on \ +-blockdev '{"driver":"file","filename":"/tmp/backing-store.qcow",\ +"node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},\ +"auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,\ +"cache":{"direct":true,"no-flush":false},"driver":"qcow2","cache-size":1048576,\ +"file":"libvirt-2-storage","backing":null}' \ +-blockdev '{"driver":"file","filename":"/tmp/QEMUGuest2.img",\ +"node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},\ +"auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,\ +"cache":{"direct":true,"no-flush":false},"driver":"qcow2",\ +"file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ +-device ide-hd,bus=ide.0,unit=1,drive=libvirt-1-format,id=ide0-0-1,\ +write-cache=on \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d2712e0dce..d6fa81078f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1212,6 +1212,7 @@ mymain(void) DO_TEST_CAPS_VER("disk-cache", "2.7.0"); DO_TEST_CAPS_VER("disk-cache", "2.12.0"); DO_TEST_CAPS_LATEST("disk-cache"); + DO_TEST_CAPS_LATEST("disk-metadata-cache"); DO_TEST_CAPS_ARCH_VER_PARSE_ERROR("disk-transient", "x86_64", "4.1.0"); DO_TEST_CAPS_LATEST("disk-transient"); DO_TEST("disk-network-nbd", NONE); -- 2.29.2
On a Thursday in 2021, Peter Krempa wrote:
qemu's qcow2 driver allows control of the metadata cache of qcow2 driver by the 'cache-size' property. Wire it up to the recently introduced elements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 11 ++++ src/qemu/qemu_domain.c | 15 +++++ src/qemu/qemu_snapshot.c | 14 +++++ .../disk-metadata-cache.x86_64-latest.args | 57 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 98 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4640e339c0..857c5aa8d0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1343,6 +1343,17 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) return -1;
+ /* 'cache-size' controls the maximum size of l2 and refcount caches.
s/l2/L2/
+ * see: qemu.git/docs/qcow2-cache.txt + * https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt + */ + if (src->metadataCacheMaxSize > 0) { + if (virJSONValueObjectAdd(props, + "U:cache-size", src->metadataCacheMaxSize, + NULL) < 0) + return -1; + } + return 0; }
Jano
On a Thursday in 2021, Peter Krempa wrote:
See patch 3/5 for explanation.
Peter Krempa (5): virDomainDiskDefFormatDriver: Rename 'driverBuf' to 'attrBuf' virDomainSnapshotDiskDefFormat: Use virXMLFormatElement conf: Introduce <metadata_cache> subelement of <disk><driver> conf: snapshot: Add support for <metadata_cache> qemu: Implement '<metadata_cache><max_size>' control for qcow2
docs/formatdomain.rst | 43 ++++++++++ docs/formatsnapshot.html.in | 4 + docs/schemas/domaincommon.rng | 20 ++++- docs/schemas/domainsnapshot.rng | 10 ++- src/conf/domain_conf.c | 81 ++++++++++++++----- src/conf/snapshot_conf.c | 50 ++++++++---- src/qemu/qemu_block.c | 11 +++ src/qemu/qemu_domain.c | 15 ++++ src/qemu/qemu_snapshot.c | 14 ++++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qcow2-metadata-cache.xml | 14 ++++ .../qcow2-metadata-cache.xml | 18 +++++ tests/qemudomainsnapshotxml2xmltest.c | 3 + .../disk-metadata-cache.x86_64-latest.args | 57 +++++++++++++ .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++ tests/qemuxml2argvtest.c | 1 + .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 411 insertions(+), 38 deletions(-) create mode 100644 tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
Okay, so IIUC the difference to the previous attempts [0] is that this series that were rejected is, that this only uses 'cache-size' instead of the finer options. Fine by me: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano [0] [libvirt] [PATCH v6 0/2] Add support for qcow2 cache by Liu Qing: https://www.redhat.com/archives/libvir-list/2017-September/msg00553.html [libvirt] [RFC] add option to configure qcow2 cache sizes in domain xml https://www.redhat.com/archives/libvir-list/2018-April/msg00773.html
On Thu, Jan 07, 2021 at 22:11:42 +0100, Ján Tomko wrote:
On a Thursday in 2021, Peter Krempa wrote:
See patch 3/5 for explanation.
Peter Krempa (5): virDomainDiskDefFormatDriver: Rename 'driverBuf' to 'attrBuf' virDomainSnapshotDiskDefFormat: Use virXMLFormatElement conf: Introduce <metadata_cache> subelement of <disk><driver> conf: snapshot: Add support for <metadata_cache> qemu: Implement '<metadata_cache><max_size>' control for qcow2
docs/formatdomain.rst | 43 ++++++++++ docs/formatsnapshot.html.in | 4 + docs/schemas/domaincommon.rng | 20 ++++- docs/schemas/domainsnapshot.rng | 10 ++- src/conf/domain_conf.c | 81 ++++++++++++++----- src/conf/snapshot_conf.c | 50 ++++++++---- src/qemu/qemu_block.c | 11 +++ src/qemu/qemu_domain.c | 15 ++++ src/qemu/qemu_snapshot.c | 14 ++++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 2 + .../qcow2-metadata-cache.xml | 14 ++++ .../qcow2-metadata-cache.xml | 18 +++++ tests/qemudomainsnapshotxml2xmltest.c | 3 + .../disk-metadata-cache.x86_64-latest.args | 57 +++++++++++++ .../qemuxml2argvdata/disk-metadata-cache.xml | 46 +++++++++++ tests/qemuxml2argvtest.c | 1 + .../disk-metadata-cache.x86_64-latest.xml | 58 +++++++++++++ tests/qemuxml2xmltest.c | 1 + 19 files changed, 411 insertions(+), 38 deletions(-) create mode 100644 tests/qemudomainsnapshotxml2xmlin/qcow2-metadata-cache.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/qcow2-metadata-cache.xml create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-metadata-cache.xml create mode 100644 tests/qemuxml2xmloutdata/disk-metadata-cache.x86_64-latest.xml
Okay, so IIUC the difference to the previous attempts [0] is that this series that were rejected is, that this only uses 'cache-size' instead
Yes. Specifically, we now declare it as a metadata cache size maximum size knob. This comes also with a change in qemu which allocates the cache on-demand.
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa