[libvirt] [PATCH v4 0/2] Add support for qcow2 cache
Qcow2 small IO random write performance will drop dramatically if the l2 cache table could not cover the whole disk. This will be a lot of l2 cache table RW operations if cache miss happens frequently. This patch exports the qcow2 driver parameter l2-cache-size/refcount-cache-size, first added in Qemu 2.2, and cache-clean-interval, first added in Qemu 2.5, in libvirt. Change since v3: a) copy qcow2 cache configurion from source to backing to backing file source. Liu Qing (2): conf, docs: Add qcow2 cache configuration support qemu: add capability checking for qcow2 cache configuration docs/formatdomain.html.in | 43 ++++++++++ docs/schemas/domaincommon.rng | 35 ++++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++++- src/qemu/qemu_capabilities.c | 9 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 33 ++++++++ src/qemu/qemu_driver.c | 5 ++ src/util/virstoragefile.c | 3 + src/util/virstoragefile.h | 6 ++ tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 + .../caps_2.6.0-gicv2.aarch64.xml | 3 + .../caps_2.6.0-gicv3.aarch64.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-disk-drive-qcow2-cache.args | 28 +++++++ .../qemuxml2argv-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2xmltest.c | 1 + 26 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml -- 1.8.3.1
Random write IOPS will drop dramatically if qcow2 l2 cache could not cover the whole disk. This patch gives libvirt user a chance to adjust the qcow2 cache configuration. Three new qcow2 driver parameters are added. They are l2-cache-size, refcount-cache-size and cache-clean-interval. The following are from qcow2-cache.txt. The amount of virtual disk that can be mapped by the L2 and refcount caches (in bytes) is: disk_size = l2_cache_size * cluster_size / 8 disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits The parameter "cache-clean-interval" defines an interval (in seconds). All cache entries that haven't been accessed during that interval are removed from memory. Signed-off-by: Liu Qing <liuqing@huayun.com> --- Change since v3: a) copy qcow2 cache configurion from source to backing$ to backing file source.$ docs/formatdomain.html.in | 43 ++++++++++ docs/schemas/domaincommon.rng | 35 ++++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++++- src/qemu/qemu_command.c | 6 ++ src/qemu/qemu_driver.c | 5 ++ src/util/virstoragefile.c | 3 + src/util/virstoragefile.h | 6 ++ .../qemuxml2argv-disk-drive-qcow2-cache.xml | 43 ++++++++++ .../qemuxml2xmlout-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8ca7637..245d5c4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3036,6 +3036,49 @@ set. (<span class="since">Since 3.5.0</span>) </li> </ul> + The <code>drive</code> element may contain a qcow2 sub element, which + allows to specifying further details related to qcow2 driver type. + <span class="since">Since 3.8.0</span> + <ul> + <li> + The optional <code>l2_cache_size</code> attribute controls how much + memory will be consumed by qcow2 l2 table cache in bytes. This + option is only valid when the driver type is qcow2. The default + size is 2097152. The amount of virtual disk that can be mapped by + the L2 caches (in bytes) is: + disk_size = l2_cache_size * cluster_size / 8 + <span class='since'>Since 3.8.0</span> + + <b>In general you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </li> + <li> + The optional <code>refcount_cache_size</code> attribute controls + how much memory will be consumed by qcow2 reference count table + cache in bytes. This option is only valid when the driver type is + qcow2. The default size is 262144. The amount of virtual disk that + can be mapped by the refcount caches (in bytes) is: + disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits + <span class='since'>Since 3.8.0</span> + + <b>In general you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </li> + <li> + The optional <code>cache_clean_interval</code> attribute defines + an interval (in seconds). All cache entries that haven't been + accessed during that interval are removed from memory. This option + is only valid when the driver type is qcow2. The default + value is 0, which disables this feature. If the interval is too + short, it will cause frequent cache write back and load, which + impact performance. If the interval is too long, unused cache + will still consume memory until it's been written back to disk. + <span class='since'>Since 3.8.0</span> + + <b>In general you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </li> + </ul> </dd> <dt><code>backenddomain</code></dt> <dd>The optional <code>backenddomain</code> element allows specifying a diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c9a4f7a..e4200fe 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1756,6 +1756,23 @@ </element> </define> <!-- + Parameters for qcow2 driver + --> + <define name="qcow2Driver"> + <element name="qcow2"> + <optional> + <ref name="qcow2_l2_cache_size"/> + </optional> + <optional> + <ref name="qcow2_refcount_cache_size"/> + </optional> + <optional> + <ref name="qcow2_cache_clean_interval"/> + </optional> + </element> + </define> + + <!-- Disk may use a special driver for access. --> <define name="diskDriver"> @@ -1794,6 +1811,9 @@ <ref name="detect_zeroes"/> </optional> <ref name="virtioOptions"/> + <zeroOrMore> + <ref name="qcow2Driver"/> + </zeroOrMore> <empty/> </element> </define> @@ -1889,6 +1909,21 @@ </choice> </attribute> </define> + <define name="qcow2_l2_cache_size"> + <attribute name='l2_cache_size'> + <ref name="unsignedInt"/> + </attribute> + </define> + <define name="qcow2_refcount_cache_size"> + <attribute name='refcount_cache_size'> + <ref name="unsignedInt"/> + </attribute> + </define> + <define name="qcow2_cache_clean_interval"> + <attribute name='cache_clean_interval'> + <ref name="unsignedInt"/> + </attribute> + </define> <define name="controller"> <element name="controller"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a43b25c..4949e8b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5734,6 +5734,28 @@ virDomainDeviceLoadparmIsValid(const char *loadparm) static void +virDoaminQcow2CacheOptionsFormat(virBufferPtr buf, + virDomainDiskDefPtr def) +{ + virBuffer qcow2Buff = VIR_BUFFER_INITIALIZER; + if (def->src->l2_cache_size > 0) + virBufferAsprintf(&qcow2Buff, " l2_cache_size='%llu'", + def->src->l2_cache_size); + if (def->src->refcount_cache_size > 0) + virBufferAsprintf(&qcow2Buff, " refcount_cache_size='%llu'", + def->src->refcount_cache_size); + if (def->src->cache_clean_interval > 0) + virBufferAsprintf(&qcow2Buff, " cache_clean_interval='%llu'", + def->src->cache_clean_interval); + + if (virBufferUse(&qcow2Buff)) { + virBufferAddLit(buf, "<qcow2"); + virBufferAddBuffer(buf, &qcow2Buff); + virBufferAddLit(buf, "/>\n"); + } +} + +static void virDomainVirtioOptionsFormat(virBufferPtr buf, virDomainVirtioOptionsPtr virtio) { @@ -8572,15 +8594,69 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def) } } + if (def->src->format != VIR_STORAGE_FILE_QCOW2 && + (def->src->l2_cache_size > 0 || def->src->refcount_cache_size > 0 || + def->src->cache_clean_interval > 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Setting l2_cache_size, refcount_cache_size, " + "cache_clean_interval is not allowed for types " + "other than QCOW2")); + return -1; + } + return 0; } static int +virDomainDiskDefQcow2ParseXML(virDomainDiskDefPtr def, + xmlNodePtr cur) +{ + char *tmp = NULL; + int ret = -1; + + if ((tmp = virXMLPropString(cur, "l2_cache_size")) && + (virStrToLong_ullp(tmp, NULL, 10, &def->src->l2_cache_size) < 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid l2_cache_size attribute in disk " + "driver element: %s"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + + if ((tmp = virXMLPropString(cur, "refcount_cache_size")) && + (virStrToLong_ullp(tmp, NULL, 10, &def->src->refcount_cache_size) < 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid refcount_cache_size attribute in disk " + "driver element: %s"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + + if ((tmp = virXMLPropString(cur, "cache_clean_interval")) && + (virStrToLong_ullp(tmp, NULL, 10, &def->src->cache_clean_interval) < 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid cache_clean_interval attribute in " + "disk driver element: %s"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + + ret = 0; + + cleanup: + VIR_FREE(tmp); + + return ret; +} + + +static int virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, xmlNodePtr cur) { char *tmp = NULL; + xmlNodePtr child; int ret = -1; def->src->driverName = virXMLPropString(cur, "name"); @@ -8683,6 +8759,12 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, } VIR_FREE(tmp); + for (child = cur->children; child != NULL; child = child->next) { + if (virXMLNodeNameEqual(child, "qcow2") && + virDomainDiskDefQcow2ParseXML(def, child) < 0) { + goto cleanup; + } + } ret = 0; cleanup: @@ -21969,7 +22051,18 @@ virDomainDiskDefFormat(virBufferPtr buf, if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); - virBufferAddLit(buf, "/>\n"); + + if (def->src->l2_cache_size == 0 && + def->src->refcount_cache_size == 0 && + def->src->cache_clean_interval == 0) { + virBufferAddLit(buf, "/>\n"); + } else { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virDoaminQcow2CacheOptionsFormat(buf, def); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</driver>\n"); + } } if (def->src->auth) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d553df5..ba81525 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1433,6 +1433,12 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, qemuformat = "luks"; virBufferAsprintf(buf, "format=%s,", qemuformat); } + if (disk->src->l2_cache_size > 0) + virBufferAsprintf(buf, "l2-cache-size=%llu,", disk->src->l2_cache_size); + if (disk->src->refcount_cache_size > 0) + virBufferAsprintf(buf, "refcount-cache-size=%llu,", disk->src->refcount_cache_size); + if (disk->src->cache_clean_interval > 0) + virBufferAsprintf(buf, "cache-clean-interval=%llu,", disk->src->cache_clean_interval); ret = 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e956839..c3b81e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14318,6 +14318,11 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (!(dd->src = virStorageSourceCopy(snap->def->disks[i].src, false))) goto error; + /* keep the qcow2 cache configuration */ + dd->src->l2_cache_size = vm->def->disks[i]->src->l2_cache_size; + dd->src->refcount_cache_size = vm->def->disks[i]->src->refcount_cache_size; + dd->src->cache_clean_interval = vm->def->disks[i]->src->cache_clean_interval; + if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) goto error; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index e94ad32..f23390f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2038,6 +2038,9 @@ virStorageSourceCopy(const virStorageSource *src, ret->physical = src->physical; ret->readonly = src->readonly; ret->shared = src->shared; + ret->l2_cache_size = src->l2_cache_size; + ret->refcount_cache_size = src->refcount_cache_size; + ret->cache_clean_interval = src->cache_clean_interval; /* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 6c388b1..9b5a5f3 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -280,6 +280,12 @@ struct _virStorageSource { /* metadata that allows identifying given storage source */ char *nodeformat; /* name of the format handler object */ char *nodestorage; /* name of the storage object */ + + unsigned long long l2_cache_size; /* qcow2 l2 cache size */ + /* qcow2 reference count table cache size */ + unsigned long long refcount_cache_size; + /* clean unused cache entries interval */ + unsigned long long cache_clean_interval; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml new file mode 100644 index 0000000..3f464db --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='qcow2' cache='none'> + <qcow2 l2_cache_size='2097152' refcount_cache_size='524288' cache_clean_interval='900'/> + </driver> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <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='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml new file mode 100644 index 0000000..3f464db --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='qcow2' cache='none'> + <qcow2 l2_cache_size='2097152' refcount_cache_size='524288' cache_clean_interval='900'/> + </driver> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <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='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0a87ced..fab1e19 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -461,6 +461,7 @@ mymain(void) DO_TEST("disk-drive-cache-v2-none", NONE); DO_TEST("disk-drive-cache-directsync", NONE); DO_TEST("disk-drive-cache-unsafe", NONE); + DO_TEST("disk-drive-qcow2-cache", NONE); DO_TEST("disk-drive-network-nbd", NONE); DO_TEST("disk-drive-network-nbd-export", NONE); DO_TEST("disk-drive-network-nbd-ipv6", NONE); -- 1.8.3.1
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
Random write IOPS will drop dramatically if qcow2 l2 cache could not cover the whole disk. This patch gives libvirt user a chance to adjust the qcow2 cache configuration.
Three new qcow2 driver parameters are added. They are l2-cache-size, refcount-cache-size and cache-clean-interval.
The following are from qcow2-cache.txt. The amount of virtual disk that can be mapped by the L2 and refcount caches (in bytes) is: disk_size = l2_cache_size * cluster_size / 8 disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits
The parameter "cache-clean-interval" defines an interval (in seconds). All cache entries that haven't been accessed during that interval are removed from memory.
Signed-off-by: Liu Qing <liuqing@huayun.com> --- Change since v3: a) copy qcow2 cache configurion from source to backing$ to backing file source.$
This looks like one of the tuning parameters which really is hard for users to set and thus it should be justified properly if we need it. [1] From the commit message above it looks like that there are guidelines how to set them. Can't we just make them implicit and not expose anything to tune the settings? Does it make sense to do so? Are there any drawbacks? If any of them need to be configured by the user, please describe that in detail why it's necessary. Peter [1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation)
On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
Random write IOPS will drop dramatically if qcow2 l2 cache could not cover the whole disk. This patch gives libvirt user a chance to adjust the qcow2 cache configuration.
Three new qcow2 driver parameters are added. They are l2-cache-size, refcount-cache-size and cache-clean-interval.
The following are from qcow2-cache.txt. The amount of virtual disk that can be mapped by the L2 and refcount caches (in bytes) is: disk_size = l2_cache_size * cluster_size / 8 disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits
The parameter "cache-clean-interval" defines an interval (in seconds). All cache entries that haven't been accessed during that interval are removed from memory.
Signed-off-by: Liu Qing <liuqing@huayun.com> --- Change since v3: a) copy qcow2 cache configurion from source to backing$ to backing file source.$
This looks like one of the tuning parameters which really is hard for users to set and thus it should be justified properly if we need it. [1]
From the commit message above it looks like that there are guidelines how to set them. Can't we just make them implicit and not expose anything to tune the settings? Does it make sense to do so? Are there any drawbacks?
If any of them need to be configured by the user, please describe that in detail why it's necessary.
Peter
[1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation) The guidance could be found in doc/qcow2-cache.txt in qemu source code. As John Ferlan suggested I have added the file locaton in formatdomain.html.in.
On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
[...]
[1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation) The guidance could be found in doc/qcow2-cache.txt in qemu source code. As John Ferlan suggested I have added the file locaton in formatdomain.html.in.
Well, if the guidance is absolute (use this cache size and it's okay) then we should implement it automatically (don't allow users to set it.) I'm asking whether there are some catches why not to do it automatically. E.g whether there's an possibility that users would do something *else* as described by the document and what would be the reasons for it. One of the reasons might be memory consumption of the cache as it looks like you need 1 MiB of memory to fully cover a 8GiB image. Also the problem is that those parameters are qemu-isms so we should be aware when modelling them in the XML since they can change and may not map well to any other technology. Also how does the cache impact read-performance from the backing layers? We might need to set the cache for the backing layers as well.
On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
[...]
[1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation) The guidance could be found in doc/qcow2-cache.txt in qemu source code. As John Ferlan suggested I have added the file locaton in formatdomain.html.in.
Well, if the guidance is absolute (use this cache size and it's okay) then we should implement it automatically (don't allow users to set it.)
One more thing. The design you've proposed is really not user friendly. The user has to read a lenghty document, then inquire the parameters for every single qcow2 image, do the calculations and set it in the XML. This might be okay for higher level management tools, but not for users who use libvirt directly.
On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
[...]
[1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation) The guidance could be found in doc/qcow2-cache.txt in qemu source code. As John Ferlan suggested I have added the file locaton in formatdomain.html.in.
Well, if the guidance is absolute (use this cache size and it's okay) then we should implement it automatically (don't allow users to set it.)
One more thing. The design you've proposed is really not user friendly. The user has to read a lenghty document, then inquire the parameters for every single qcow2 image, do the calculations and set it in the XML. This might be okay for higher level management tools, but not for users who use libvirt directly. I agree this is not user friendly. I only give the user a choice, maybe not the best, right now. Any suggestion will be greatly appreicated. For our company, this feature will be an add-on to Openstack.
On Thu, Sep 14, 2017 at 16:37:03 +0800, Liu Qing wrote:
On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
[...]
One more thing. The design you've proposed is really not user friendly. The user has to read a lenghty document, then inquire the parameters for every single qcow2 image, do the calculations and set it in the XML. This might be okay for higher level management tools, but not for users who use libvirt directly. I agree this is not user friendly. I only give the user a choice, maybe not the best, right now. Any suggestion will be greatly appreicated. For our company, this feature will be an add-on to Openstack.
This means that you will have to deal with everything that I've pointed out. How are you planning to expose this to the user? And which calculations are you going to use?
On Thu, Sep 14, 2017 at 16:37:03 +0800, Liu Qing wrote:
On Thu, Sep 14, 2017 at 10:02:40AM +0200, Peter Krempa wrote:
On Thu, Sep 14, 2017 at 09:29:28 +0200, Peter Krempa wrote:
[...]
One more thing. The design you've proposed is really not user friendly. The user has to read a lenghty document, then inquire the parameters for every single qcow2 image, do the calculations and set it in the XML. This might be okay for higher level management tools, but not for users who use libvirt directly. I agree this is not user friendly. I only give the user a choice, maybe not the best, right now. Any suggestion will be greatly appreicated. For our company, this feature will be an add-on to Openstack.
This means that you will have to deal with everything that I've pointed out. How are you planning to expose this to the user? And which calculations are you going to use? We will not expose this to the user on Openstack level. We have different volume types like performance and capacity. For
On Thu, Sep 14, 2017 at 12:56:30PM +0200, Peter Krempa wrote: performance volumes we will cache as much space as possible. Free memory left in the host will be counted in. For capacity volume we will set a max cache size. The cache setting strategy be implemented in a higher level management tool seems much resaonable to me. As this will be much more flexible.
On Thu, Sep 14, 2017 at 13:02:46 +0800, Liu Qing wrote:
On Wed, Sep 13, 2017 at 01:20:03PM +0200, Peter Krempa wrote:
On Wed, Sep 13, 2017 at 17:21:23 +0800, Liu Qing wrote:
[...]
[1] There's discussion I can link to for other tuning parameters. The gist is that allowing users to set something withoug giving them guidance is pointless since they might not use it. Also if the guidance is strict (e.g. a formula, libvirt or qemu should set the defaults properly and not force users to do the calculation) The guidance could be found in doc/qcow2-cache.txt in qemu source code. As John Ferlan suggested I have added the file locaton in formatdomain.html.in.
Well, if the guidance is absolute (use this cache size and it's okay) then we should implement it automatically (don't allow users to set it.)
I'm asking whether there are some catches why not to do it automatically. E.g whether there's an possibility that users would do something *else* as described by the document and what would be the reasons for it. I think there is some trade off between the cache size and performance. Otherwise qemu does not need to export these parameters to user, they could do the automation in qemu. The guidance only let the user know how much disk space is coverred by caches, and how much memory will be needed to cover all disk spaces. User should do their own decision to choose a right size. But if the user is using the current version of
On Thu, Sep 14, 2017 at 09:29:28AM +0200, Peter Krempa wrote: libvirt, they could not set the value even if they know what's the proper value. For example if the user may need a LUN which need high IOPS, he could set the cache to cover all disk spaces. And in another situation, he needs a large capacity,for example 4T, but low perfermance LUN, then he could set the l2 cache size to a value much less than 512M.
One of the reasons might be memory consumption of the cache as it looks like you need 1 MiB of memory to fully cover a 8GiB image.
Yes, as I said above.
Also the problem is that those parameters are qemu-isms so we should be aware when modelling them in the XML since they can change and may not map well to any other technology.
Currently the new element is added as a child elemnt of driver, and the driver type needs to be qcow2. Also add this kind information in the doc.
Also how does the cache impact read-performance from the backing layers? We might need to set the cache for the backing layers as well.
I have a peek at the latest qemu code and the backing layers will have the same cache size value as the top volume. And this will reduce metadata read count if the table is already in the memory. Also more memory will consumed.
Add qemu capabilities QEMU_CAPS_L2_CACHE_SIZE, QEMU_CAPS_REFCOUNT_CACHE_SIZE, QEMU_CAPS_CACHE_CLEAN_INTERVAL. Add testing for the above qemu capabilities. Signed-off-by: Liu Qing <liuqing@huayun.com> --- src/qemu/qemu_capabilities.c | 9 +++++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 39 ++++++++++++++++++---- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 ++ .../caps_2.6.0-gicv2.aarch64.xml | 3 ++ .../caps_2.6.0-gicv3.aarch64.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 3 ++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 ++ .../qemuxml2argv-disk-drive-qcow2-cache.args | 28 ++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ 17 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e7ea6f4..e8cce38 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -439,6 +439,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-net.tx_queue_size", "chardev-reconnect", "virtio-gpu.max_outputs", + "drive.qcow2.l2-cache-size", + "drive.qcow2.refcount-cache-size", + "drive.qcow2.cache-clean-interval", ); @@ -1811,6 +1814,12 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/options/+gluster/debug-level", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, { "blockdev-add/arg-type/+gluster/debug", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, + { "blockdev-add/arg-type/options/+qcow2/l2-cache-size", QEMU_CAPS_DRIVE_QCOW2_L2_CACHE_SIZE}, + { "blockdev-add/arg-type/options/+qcow2/refcount-cache-size", QEMU_CAPS_DRIVE_QCOW2_REFCOUNT_CACHE_SIZE}, + { "blockdev-add/arg-type/options/+qcow2/cache-clean-interval", QEMU_CAPS_DRIVE_QCOW2_CACHE_CLEAN_INTERVAL}, + { "blockdev-add/arg-type/+qcow2/l2-cache-size", QEMU_CAPS_DRIVE_QCOW2_L2_CACHE_SIZE}, + { "blockdev-add/arg-type/+qcow2/refcount-cache-size", QEMU_CAPS_DRIVE_QCOW2_REFCOUNT_CACHE_SIZE}, + { "blockdev-add/arg-type/+qcow2/cache-clean-interval", QEMU_CAPS_DRIVE_QCOW2_CACHE_CLEAN_INTERVAL}, }; struct virQEMUCapsObjectTypeProps { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f32687d..81fb759 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -425,6 +425,9 @@ typedef enum { QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE, /* virtio-net-*.tx_queue_size */ QEMU_CAPS_CHARDEV_RECONNECT, /* -chardev reconnect */ QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, /* -device virtio-(vga|gpu-*),max-outputs= */ + QEMU_CAPS_DRIVE_QCOW2_L2_CACHE_SIZE, /* -drive support qcow2 l2 cache size */ + QEMU_CAPS_DRIVE_QCOW2_REFCOUNT_CACHE_SIZE, /* -drive support qcow2 refcount cache size */ + QEMU_CAPS_DRIVE_QCOW2_CACHE_CLEAN_INTERVAL, /* -drive qcow2 cache clean interval */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba81525..da91059 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1433,12 +1433,39 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, qemuformat = "luks"; virBufferAsprintf(buf, "format=%s,", qemuformat); } - if (disk->src->l2_cache_size > 0) - virBufferAsprintf(buf, "l2-cache-size=%llu,", disk->src->l2_cache_size); - if (disk->src->refcount_cache_size > 0) - virBufferAsprintf(buf, "refcount-cache-size=%llu,", disk->src->refcount_cache_size); - if (disk->src->cache_clean_interval > 0) - virBufferAsprintf(buf, "cache-clean-interval=%llu,", disk->src->cache_clean_interval); + + if (disk->src->l2_cache_size > 0) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_QCOW2_L2_CACHE_SIZE)) { + virBufferAsprintf(buf, "l2-cache-size=%llu,", + disk->src->l2_cache_size); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("l2-cache-size is not supported by this QEMU")); + goto cleanup; + } + } + + if (disk->src->refcount_cache_size > 0) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_QCOW2_REFCOUNT_CACHE_SIZE)) { + virBufferAsprintf(buf, "refcount-cache-size=%llu,", + disk->src->refcount_cache_size); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("refcount-cache-size is not supported by this QEMU")); + goto cleanup; + } + } + + if (disk->src->cache_clean_interval > 0) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_QCOW2_CACHE_CLEAN_INTERVAL)) { + virBufferAsprintf(buf, "cache-clean-interval=%llu,", + disk->src->cache_clean_interval); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cache-clean-interval is not supported by this QEMU")); + goto cleanup; + } + } ret = 0; diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index 2ba40fc..2ccaca8 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -194,6 +194,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 0b34fa3..2438de2 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -172,6 +172,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index d41d578..b677d40 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -172,6 +172,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index f1c9fc9..a680b62 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -167,6 +167,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index bdf006f..0ebf8a2 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -204,6 +204,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index fe7bca9..44bc54c 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -134,6 +134,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 3fd28f0..3b6c1ed 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -207,6 +207,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 21bbb82..85f39fb 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -136,6 +136,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 761f9d1..d673891 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -209,6 +209,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml index a373a6d..b093244 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml @@ -172,6 +172,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index e80782c..beba716 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -137,6 +137,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 3641d03..64b99b0 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -220,6 +220,9 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='drive.qcow2.l2-cache-size'/> + <flag name='drive.qcow2.refcount-cache-size'/> + <flag name='drive.qcow2.cache-clean-interval'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args new file mode 100644 index 0000000..d13be92 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,l2-cache-size=2097152,\ +refcount-cache-size=524288,cache-clean-interval=900,if=none,id=drive-ide0-0-0,\ +cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\ +id=drive-ide0-1-0,readonly=on \ +-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2c040e4..8762779 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -910,6 +910,10 @@ mymain(void) QEMU_CAPS_DRIVE_CACHE_UNSAFE); DO_TEST("disk-drive-copy-on-read", QEMU_CAPS_DRIVE_COPY_ON_READ); + DO_TEST("disk-drive-qcow2-cache", + QEMU_CAPS_DRIVE_QCOW2_L2_CACHE_SIZE, + QEMU_CAPS_DRIVE_QCOW2_REFCOUNT_CACHE_SIZE, + QEMU_CAPS_DRIVE_QCOW2_CACHE_CLEAN_INTERVAL); DO_TEST("disk-drive-network-nbd", NONE); DO_TEST("disk-drive-network-nbd-export", NONE); DO_TEST("disk-drive-network-nbd-ipv6", NONE); -- 1.8.3.1
On 09/13/2017 05:21 AM, Liu Qing wrote:
Qcow2 small IO random write performance will drop dramatically if the l2 cache table could not cover the whole disk. This will be a lot of l2 cache table RW operations if cache miss happens frequently.
This patch exports the qcow2 driver parameter l2-cache-size/refcount-cache-size, first added in Qemu 2.2, and cache-clean-interval, first added in Qemu 2.5, in libvirt.
Change since v3: a) copy qcow2 cache configurion from source to backing to backing file source.
The difference you list doesn't make any sense. The only difference between this an V3 is removing the qemu_command.c from patch1. I did a git diff between the two trees I have from your patches: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3b81e138..e95683965 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14318,11 +14318,6 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (!(dd->src = virStorageSourceCopy(snap->def->disks[i].src, false))) goto error; - /* keep the qcow2 cache configuration */ - dd->src->l2_cache_size = vm->def->disks[i]->src->l2_cache_size; - dd->src->refcount_cache_size = vm->def->disks[i]->src->refcount_cache_size; - dd->src->cache_clean_interval = vm->def->disks[i]->src->cache_clean_interval; - if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) goto error; I'll stick with the v3 version and adjustments from comments that I made. John
Liu Qing (2): conf, docs: Add qcow2 cache configuration support qemu: add capability checking for qcow2 cache configuration
docs/formatdomain.html.in | 43 ++++++++++ docs/schemas/domaincommon.rng | 35 ++++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++++- src/qemu/qemu_capabilities.c | 9 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 33 ++++++++ src/qemu/qemu_driver.c | 5 ++ src/util/virstoragefile.c | 3 + src/util/virstoragefile.h | 6 ++ tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 + .../caps_2.6.0-gicv2.aarch64.xml | 3 + .../caps_2.6.0-gicv3.aarch64.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-disk-drive-qcow2-cache.args | 28 +++++++ .../qemuxml2argv-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2xmltest.c | 1 + 26 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml
On Wed, Sep 13, 2017 at 06:51:35AM -0400, John Ferlan wrote:
On 09/13/2017 05:21 AM, Liu Qing wrote:
Qcow2 small IO random write performance will drop dramatically if the l2 cache table could not cover the whole disk. This will be a lot of l2 cache table RW operations if cache miss happens frequently.
This patch exports the qcow2 driver parameter l2-cache-size/refcount-cache-size, first added in Qemu 2.2, and cache-clean-interval, first added in Qemu 2.5, in libvirt.
Change since v3: a) copy qcow2 cache configurion from source to backing to backing file source.
The difference you list doesn't make any sense. The only difference between this an V3 is removing the qemu_command.c from patch1. I did a git diff between the two trees I have from your patches:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3b81e138..e95683965 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14318,11 +14318,6 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (!(dd->src = virStorageSourceCopy(snap->def->disks[i].src, false))) goto error;
- /* keep the qcow2 cache configuration */ - dd->src->l2_cache_size = vm->def->disks[i]->src->l2_cache_size; - dd->src->refcount_cache_size = vm->def->disks[i]->src->refcount_cache_size; - dd->src->cache_clean_interval = vm->def->disks[i]->src->cache_clean_interval;
If these three lines are not added, the qcow2 element(child of driver element) will be missed after a external disk snapshot. This is due to the values of the three parameters will be set to 0.
- if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) goto error;
I'll stick with the v3 version and adjustments from comments that I made.
John
Liu Qing (2): conf, docs: Add qcow2 cache configuration support qemu: add capability checking for qcow2 cache configuration
docs/formatdomain.html.in | 43 ++++++++++ docs/schemas/domaincommon.rng | 35 ++++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++++- src/qemu/qemu_capabilities.c | 9 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 33 ++++++++ src/qemu/qemu_driver.c | 5 ++ src/util/virstoragefile.c | 3 + src/util/virstoragefile.h | 6 ++ tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 3 + .../caps_2.6.0-gicv2.aarch64.xml | 3 + .../caps_2.6.0-gicv3.aarch64.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 3 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-disk-drive-qcow2-cache.args | 28 +++++++ .../qemuxml2argv-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-disk-drive-qcow2-cache.xml | 43 ++++++++++ tests/qemuxml2xmltest.c | 1 + 26 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-qcow2-cache.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-qcow2-cache.xml
participants (3)
-
John Ferlan -
Liu Qing -
Peter Krempa