10:45 p.m.
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 v4: removed unnecessary cache error check
Liu Qing (2):
conf, docs: Add qcow2 cache configuration support
qemu: add capability checking for qcow2 cache configuration
docs/formatdomain.html.in | 41 +++++++++
docs/schemas/domaincommon.rng | 35 ++++++++
src/conf/domain_conf.c | 97 ++++++++++++++++++++--
src/qemu/qemu_capabilities.c | 11 +++
src/qemu/qemu_capabilities.h | 5 ++
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, 384 insertions(+), 7 deletions(-)
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
10:45 p.m.
New subject: [libvirt] [PATCH v6 1/2] conf, docs: Add qcow2 cache configuration support
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 (l2-cache-size, refcount-cache-size
and cache-clean-interval) are added as attributes to a new <qcow2>
subelement for a <driver name='qemu' type='qcow2'...> of a
<disk>
element. The QEMU source docs/qcow2-cache.txt provides the guidelines
for defining/configuring values for each.
Signed-off-by: Liu Qing <liuqing(a)huayun.com>
---
change since v4: removed unnecessary cache error check
docs/formatdomain.html.in | 41 +++++++++
docs/schemas/domaincommon.rng | 35 ++++++++
src/conf/domain_conf.c | 97 ++++++++++++++++++++--
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 +
9 files changed, 267 insertions(+), 7 deletions(-)
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..4574e3a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3036,6 +3036,47 @@
set. (<span class="since">Since 3.5.0</span>)
</li>
</ul>
+ The <code>driver</code> element may contain a
<code>qcow2</code> sub
+ element, which to specifying further details related to a qcow2 disk.
+ For recommended setting guidelines refer to the QEMU source file
+ <code>docs/qcow2-cache.txt</code>.
+ <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.
+ <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.
+ <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..0e25f44 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"/>
+ <optional>
+ <ref name="qcow2Driver"/>
+ </optional>
<empty/>
</element>
</define>
@@ -1889,6 +1909,21 @@
</choice>
</attribute>
</define>
+ <define name="qcow2_l2_cache_size">
+ <attribute name='l2_cache_size'>
+ <ref name="unsignedLong"/>
+ </attribute>
+ </define>
+ <define name="qcow2_refcount_cache_size">
+ <attribute name='refcount_cache_size'>
+ <ref name="unsignedLong"/>
+ </attribute>
+ </define>
+ <define name="qcow2_cache_clean_interval">
+ <attribute name='cache_clean_interval'>
+ <ref name="unsignedLong"/>
+ </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..f225a7f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5734,6 +5734,30 @@ virDomainDeviceLoadparmIsValid(const char *loadparm)
static void
+virDomainQcow2CacheOptionsFormat(virBufferPtr buf,
+ virDomainDiskDefPtr def)
+{
+ if (def->src->l2_cache_size == 0 &&
+ def->src->refcount_cache_size == 0 &&
+ def->src->cache_clean_interval == 0)
+ return;
+
+ virBufferAddLit(buf, "<qcow2");
+
+ if (def->src->l2_cache_size > 0)
+ virBufferAsprintf(buf, " l2_cache_size='%llu'",
+ def->src->l2_cache_size);
+ if (def->src->refcount_cache_size > 0)
+ virBufferAsprintf(buf, " refcount_cache_size='%llu'",
+ def->src->refcount_cache_size);
+ if (def->src->cache_clean_interval > 0)
+ virBufferAsprintf(buf, " cache_clean_interval='%llu'",
+ def->src->cache_clean_interval);
+ virBufferAddLit(buf, "/>\n");
+}
+
+
+static void
virDomainVirtioOptionsFormat(virBufferPtr buf,
virDomainVirtioOptionsPtr virtio)
{
@@ -8572,15 +8596,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 +8761,13 @@ 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:
@@ -21885,6 +21970,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
const char *discard = virDomainDiskDiscardTypeToString(def->discard);
const char *detect_zeroes =
virDomainDiskDetectZeroesTypeToString(def->detect_zeroes);
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+ virBuffer driverQcow2Buf = VIR_BUFFER_INITIALIZER;
if (!type || !def->src->type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -21963,14 +22049,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
- if (virBufferCheckError(&driverBuf) < 0)
- return -1;
+ virBufferSetChildIndent(&driverQcow2Buf, buf);
+ virDomainQcow2CacheOptionsFormat(&driverQcow2Buf, def);
- if (virBufferUse(&driverBuf)) {
- virBufferAddLit(buf, "<driver");
- virBufferAddBuffer(buf, &driverBuf);
- virBufferAddLit(buf, "/>\n");
- }
+ if (virXMLFormatElement(buf, "driver", &driverBuf, &driverQcow2Buf)
< 0)
+ return -1;
if (def->src->auth) {
if (virStorageAuthDefFormat(buf, def->src->auth) < 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
10:45 p.m.
New subject: [libvirt] [PATCH v6 2/2] qemu: add capability checking for qcow2 cache configuration
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(a)huayun.com>
---
src/qemu/qemu_capabilities.c | 11 ++++++++
src/qemu/qemu_capabilities.h | 5 ++++
src/qemu/qemu_command.c | 33 ++++++++++++++++++++++
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, 117 insertions(+)
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..2db6af1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -439,6 +439,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"virtio-net.tx_queue_size",
"chardev-reconnect",
"virtio-gpu.max_outputs",
+
+ /* 270 */
+ "drive.qcow2.l2-cache-size",
+ "drive.qcow2.refcount-cache-size",
+ "drive.qcow2.cache-clean-interval",
);
@@ -1811,6 +1816,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..021297e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -426,6 +426,11 @@ typedef enum {
QEMU_CAPS_CHARDEV_RECONNECT, /* -chardev reconnect */
QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, /* -device virtio-(vga|gpu-*),max-outputs= */
+ /* 270 */
+ 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 d553df5..da91059 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1434,6 +1434,39 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
virBufferAsprintf(buf, "format=%s,", qemuformat);
}
+ 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;
cleanup:
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
4:06 p.m.
On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
Liu Qing (2):
conf, docs: Add qcow2 cache configuration support
According to what you added for the v5 - there's an upstream bz that
could be associated with this work. Is this something that should be
added to the commit message so that we can "track" where the idea comes
from?
Beyond that - I agree setting policy is not the business we want to be
in. I just figured I'd throw it out there. I didn't read all of the qemu
links from the bz about thoughts on this - too many links too much to
follow not enough time/desire to think about it.
I'm not against pushing these patches, but would like to at least make
sure Peter isn't 100% against them. I agree the attributes are really
painful to understand, but it's not the first time we've added some
attribute that comes with a beware of using this unless you know what
you're doing.
John
qemu: add capability checking for qcow2 cache configuration
docs/formatdomain.html.in | 41 +++++++++
docs/schemas/domaincommon.rng | 35 ++++++++
src/conf/domain_conf.c | 97 ++++++++++++++++++++--
src/qemu/qemu_capabilities.c | 11 +++
src/qemu/qemu_capabilities.h | 5 ++
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, 384 insertions(+), 7 deletions(-)
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
9:08 p.m.
On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
>
> Liu Qing (2):
> conf, docs: Add qcow2 cache configuration support
According to what you added for the v5 - there's an upstream bz that
could be associated with this work. Is this something that should be
added to the commit message so that we can "track" where the idea comes
from?
John could you help to add the bz in the commit message while pushing the
patch, of course, with Peter's support on pushing. The bz link is:
https://bugzilla.redhat.com/show_bug.cgi?id=1377735
If you need me to do the change, just let me know. Thanks.
Beyond that - I agree setting policy is not the business we want to be
in. I just figured I'd throw it out there. I didn't read all of the qemu
links from the bz about thoughts on this - too many links too much to
follow not enough time/desire to think about it.
Thanks for the understanding.
I'm not against pushing these patches, but would like to at least make
sure Peter isn't 100% against them. I agree the attributes are really
painful to understand, but it's not the first time we've added some
attribute that comes with a beware of using this unless you know what
you're doing.
John
> qemu: add capability checking for qcow2 cache configuration
>
> docs/formatdomain.html.in | 41 +++++++++
> docs/schemas/domaincommon.rng | 35 ++++++++
> src/conf/domain_conf.c | 97 ++++++++++++++++++++--
> src/qemu/qemu_capabilities.c | 11 +++
> src/qemu/qemu_capabilities.h | 5 ++
> 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, 384 insertions(+), 7 deletions(-)
> 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
>
7:24 a.m.
On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
>
> Liu Qing (2):
> conf, docs: Add qcow2 cache configuration support
According to what you added for the v5 - there's an upstream bz that
could be associated with this work. Is this something that should be
added to the commit message so that we can "track" where the idea comes
from?
Beyond that - I agree setting policy is not the business we want to be
in. I just figured I'd throw it out there. I didn't read all of the qemu
links from the bz about thoughts on this - too many links too much to
follow not enough time/desire to think about it.
I'm not against pushing these patches, but would like to at least make
sure Peter isn't 100% against them. I agree the attributes are really
painful to understand, but it's not the first time we've added some
attribute that comes with a beware of using this unless you know what
you're doing.
I'm still not convinced that this is something we would like to expose
to users. It's way too low-level for libvirt XML. I like the idea to
allow 'max' as possible value in QEMU and that could be reused by
libvirt.
One may argue that having two options as "max storage" or
"max performace" isn't good enough and they would like to be able to
tune it for they exact needs. Most of the users don't care about the
specifics and they just need a performance or not.
I attempted to introduce "polling" feature [1] for iothreads but that
was a bad idea and too low-level for libvirt users and configuring exact
values for qcow2 cache seems to be the same case.
Adding an element/attribute into XML where you would specify whether you
care about performance or not isn't a policy. Libvirt will simply use
some defaults or configure the maximum performance. For fine-grained
tuning you can use <qemu:arg value='...'/> like it was suggested for
"polling" feature.
Pavel
7:42 a.m.
On Wed, Sep 27, 2017 at 02:24:12PM +0200, Pavel Hrdina wrote:
On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
>
>
> On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
> >
> > Liu Qing (2):
> > conf, docs: Add qcow2 cache configuration support
>
> According to what you added for the v5 - there's an upstream bz that
> could be associated with this work. Is this something that should be
> added to the commit message so that we can "track" where the idea comes
> from?
>
> Beyond that - I agree setting policy is not the business we want to be
> in. I just figured I'd throw it out there. I didn't read all of the qemu
> links from the bz about thoughts on this - too many links too much to
> follow not enough time/desire to think about it.
>
> I'm not against pushing these patches, but would like to at least make
> sure Peter isn't 100% against them. I agree the attributes are really
> painful to understand, but it's not the first time we've added some
> attribute that comes with a beware of using this unless you know what
> you're doing.
I'm still not convinced that this is something we would like to expose
to users. It's way too low-level for libvirt XML. I like the idea to
allow 'max' as possible value in QEMU and that could be reused by
libvirt.
One may argue that having two options as "max storage" or
"max performace" isn't good enough and they would like to be able to
tune it for they exact needs. Most of the users don't care about the
specifics and they just need a performance or not.
I attempted to introduce "polling" feature [1] for iothreads but that
was a bad idea and too low-level for libvirt users and configuring exact
values for qcow2 cache seems to be the same case.
Adding an element/attribute into XML where you would specify whether you
care about performance or not isn't a policy. Libvirt will simply use
some defaults or configure the maximum performance. For fine-grained
tuning you can use <qemu:arg value='...'/> like it was suggested for
"polling" feature.
Pavel
Ehm, I forgot to include the link to the "polling" discussion:
[1] <https://www.redhat.com/archives/libvir-list/2017-February/msg01084.html>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
1:07 p.m.
On 09/27/2017 08:42 AM, Pavel Hrdina wrote:
On Wed, Sep 27, 2017 at 02:24:12PM +0200, Pavel Hrdina wrote:
> On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
>>
>>
>> On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
>>>
>>> Liu Qing (2):
>>> conf, docs: Add qcow2 cache configuration support
>>
>> According to what you added for the v5 - there's an upstream bz that
>> could be associated with this work. Is this something that should be
>> added to the commit message so that we can "track" where the idea
comes
>> from?
>>
>> Beyond that - I agree setting policy is not the business we want to be
>> in. I just figured I'd throw it out there. I didn't read all of the qemu
>> links from the bz about thoughts on this - too many links too much to
>> follow not enough time/desire to think about it.
>>
>> I'm not against pushing these patches, but would like to at least make
>> sure Peter isn't 100% against them. I agree the attributes are really
>> painful to understand, but it's not the first time we've added some
>> attribute that comes with a beware of using this unless you know what
>> you're doing.
>
> I'm still not convinced that this is something we would like to expose
> to users. It's way too low-level for libvirt XML. I like the idea to
> allow 'max' as possible value in QEMU and that could be reused by
> libvirt.
>
> One may argue that having two options as "max storage" or
> "max performace" isn't good enough and they would like to be able to
> tune it for they exact needs. Most of the users don't care about the
> specifics and they just need a performance or not.
>
> I attempted to introduce "polling" feature [1] for iothreads but that
> was a bad idea and too low-level for libvirt users and configuring exact
> values for qcow2 cache seems to be the same case.
>
> Adding an element/attribute into XML where you would specify whether you
> care about performance or not isn't a policy. Libvirt will simply use
> some defaults or configure the maximum performance. For fine-grained
> tuning you can use <qemu:arg value='...'/> like it was suggested for
> "polling" feature.
>
> Pavel
Ehm, I forgot to include the link to the "polling" discussion:
[1] <https://www.redhat.com/archives/libvir-list/2017-February/msg01084.html>
I scanned the .1 discussion... it seems as though there were adjustments
made to the default iothreads to make them useful or at least enabled by
default a better set of values which could be disabled via some
<qemu:arg value='-newarg'/> setting.
From the discussion thus far for qcow2 cache the difference here is
that
the default values QEMU uses have been inadequate for real world type
usages and it doesn't appear convergence is possible because the
possible adjustments has vary degrees of issues. I think it's nicely
summed up here :
https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c2 and
https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c3
Following the link in the bz from comment 5 to the qemu patches shows
only a couple of attempts at trying something before the idea was
seemingly dropped. I looked forward and only recently was the idea
resurrected, but perhaps in perhaps a bit different format.
https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
In any case, I've sent an email to the author of those patches to get
insights regarding adding exposure from libvirt is a good or not and to
determine if the new proposal somehow overrides any settins that would
be made. It'd be awful if a value was provided, but how that value is
used gets changed...
John
10:23 p.m.
On Wed, Sep 27, 2017 at 02:07:02PM -0400, John Ferlan wrote:
On 09/27/2017 08:42 AM, Pavel Hrdina wrote:
> On Wed, Sep 27, 2017 at 02:24:12PM +0200, Pavel Hrdina wrote:
>> On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
>>>
>>>
>>> On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
>>>>
>>>> Liu Qing (2):
>>>> conf, docs: Add qcow2 cache configuration support
>>>
>>> According to what you added for the v5 - there's an upstream bz that
>>> could be associated with this work. Is this something that should be
>>> added to the commit message so that we can "track" where the idea
comes
>>> from?
>>>
>>> Beyond that - I agree setting policy is not the business we want to be
>>> in. I just figured I'd throw it out there. I didn't read all of the
qemu
>>> links from the bz about thoughts on this - too many links too much to
>>> follow not enough time/desire to think about it.
>>>
>>> I'm not against pushing these patches, but would like to at least make
>>> sure Peter isn't 100% against them. I agree the attributes are really
>>> painful to understand, but it's not the first time we've added some
>>> attribute that comes with a beware of using this unless you know what
>>> you're doing.
>>
>> I'm still not convinced that this is something we would like to expose
>> to users. It's way too low-level for libvirt XML. I like the idea to
>> allow 'max' as possible value in QEMU and that could be reused by
>> libvirt.
>>
>> One may argue that having two options as "max storage" or
>> "max performace" isn't good enough and they would like to be able
to
>> tune it for they exact needs. Most of the users don't care about the
>> specifics and they just need a performance or not.
>>
>> I attempted to introduce "polling" feature [1] for iothreads but that
>> was a bad idea and too low-level for libvirt users and configuring exact
>> values for qcow2 cache seems to be the same case.
>>
>> Adding an element/attribute into XML where you would specify whether you
>> care about performance or not isn't a policy. Libvirt will simply use
>> some defaults or configure the maximum performance. For fine-grained
>> tuning you can use <qemu:arg value='...'/> like it was suggested
for
>> "polling" feature.
>>
>> Pavel
>
> Ehm, I forgot to include the link to the "polling" discussion:
>
> [1] <https://www.redhat.com/archives/libvir-list/2017-February/msg01084.html>
>
I scanned the .1 discussion... it seems as though there were adjustments
made to the default iothreads to make them useful or at least enabled by
default a better set of values which could be disabled via some
<qemu:arg value='-newarg'/> setting.
From the discussion thus far for qcow2 cache the difference here is that
the default values QEMU uses have been inadequate for real world type
usages and it doesn't appear convergence is possible because the
possible adjustments has vary degrees of issues. I think it's nicely
summed up here :
https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c2 and
https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c3
Following the link in the bz from comment 5 to the qemu patches shows
only a couple of attempts at trying something before the idea was
seemingly dropped. I looked forward and only recently was the idea
resurrected, but perhaps in perhaps a bit different format.
https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
Looked into the
thread, not sure whether the author will change the originial
qcow2 cache configuration. Personally I don't think so, he may add a new
parameter which could be used to configure how much memory will be consumed
for each "slice" as you suggested.
If the cache is separated to size smaller than cluster, this will lead
to more frequent IO access. Athough memory could be used more
efficiently. Mmm... this should not be in this thread.
In any case, I've sent an email to the author of those patches to get
insights regarding adding exposure from libvirt is a good or not and to
determine if the new proposal somehow overrides any settins that would
be made. It'd be awful if a value was provided, but how that value is
used gets changed...
John
4:51 p.m.
On 09/27/2017 11:23 PM, Liu Qing wrote:
On Wed, Sep 27, 2017 at 02:07:02PM -0400, John Ferlan wrote:
>
>
> On 09/27/2017 08:42 AM, Pavel Hrdina wrote:
>> On Wed, Sep 27, 2017 at 02:24:12PM +0200, Pavel Hrdina wrote:
>>> On Tue, Sep 26, 2017 at 05:06:37PM -0400, John Ferlan wrote:
>>>>
>>>>
>>>> On 09/18/2017 11:45 PM, 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 v4: removed unnecessary cache error check
>>>>>
>>>>> Liu Qing (2):
>>>>> conf, docs: Add qcow2 cache configuration support
>>>>
>>>> According to what you added for the v5 - there's an upstream bz that
>>>> could be associated with this work. Is this something that should be
>>>> added to the commit message so that we can "track" where the
idea comes
>>>> from?
>>>>
>>>> Beyond that - I agree setting policy is not the business we want to be
>>>> in. I just figured I'd throw it out there. I didn't read all of
the qemu
>>>> links from the bz about thoughts on this - too many links too much to
>>>> follow not enough time/desire to think about it.
>>>>
>>>> I'm not against pushing these patches, but would like to at least
make
>>>> sure Peter isn't 100% against them. I agree the attributes are
really
>>>> painful to understand, but it's not the first time we've added
some
>>>> attribute that comes with a beware of using this unless you know what
>>>> you're doing.
>>>
>>> I'm still not convinced that this is something we would like to expose
>>> to users. It's way too low-level for libvirt XML. I like the idea to
>>> allow 'max' as possible value in QEMU and that could be reused by
>>> libvirt.
>>>
>>> One may argue that having two options as "max storage" or
>>> "max performace" isn't good enough and they would like to be
able to
>>> tune it for they exact needs. Most of the users don't care about the
>>> specifics and they just need a performance or not.
>>>
>>> I attempted to introduce "polling" feature [1] for iothreads but
that
>>> was a bad idea and too low-level for libvirt users and configuring exact
>>> values for qcow2 cache seems to be the same case.
>>>
>>> Adding an element/attribute into XML where you would specify whether you
>>> care about performance or not isn't a policy. Libvirt will simply use
>>> some defaults or configure the maximum performance. For fine-grained
>>> tuning you can use <qemu:arg value='...'/> like it was
suggested for
>>> "polling" feature.
>>>
>>> Pavel
>>
>> Ehm, I forgot to include the link to the "polling" discussion:
>>
>> [1]
<https://www.redhat.com/archives/libvir-list/2017-February/msg01084.html>
>>
>
> I scanned the .1 discussion... it seems as though there were adjustments
> made to the default iothreads to make them useful or at least enabled by
> default a better set of values which could be disabled via some
> <qemu:arg value='-newarg'/> setting.
>
> From the discussion thus far for qcow2 cache the difference here is that
> the default values QEMU uses have been inadequate for real world type
> usages and it doesn't appear convergence is possible because the
> possible adjustments has vary degrees of issues. I think it's nicely
> summed up here :
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c2 and
> https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c3
>
> Following the link in the bz from comment 5 to the qemu patches shows
> only a couple of attempts at trying something before the idea was
> seemingly dropped. I looked forward and only recently was the idea
> resurrected, but perhaps in perhaps a bit different format.
>
> https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
Looked into the thread, not sure whether the author will change the originial
qcow2 cache configuration. Personally I don't think so, he may add a new
parameter which could be used to configure how much memory will be consumed
for each "slice" as you suggested.
If the cache is separated to size smaller than cluster, this will lead
to more frequent IO access. Athough memory could be used more
efficiently. Mmm... this should not be in this thread.
>
> In any case, I've sent an email to the author of those patches to get
> insights regarding adding exposure from libvirt is a good or not and to
> determine if the new proposal somehow overrides any settins that would
> be made. It'd be awful if a value was provided, but how that value is
> used gets changed...
>
> John
I've had my head down in other rabbit holes, but this one has surfaced
again and I would like to think we could come to a consensus and/or
conclusion early in this 3.9.0 release so that we can make a decision to
merge these patches or officially NACK them.
I contacted the QEMU author (email attached for those interested in more
reading materials and he's also CC'd in this response) to allow him to
read the context of the concerns thus far and of course provide any
other thoughts or answer questions directly if necessary.
I certainly understand the position to not include something that's
difficult to describe/configure; however, if by all accounts certain
configurations can receive greater performance/throughput given the
ability to configure the parameters, then I'd hedge on the side of I
think we should merge the patches (with some adjustments that I'm
willing to make to use 3.9.0 instead of 3.8.0 in the documentation of
where the patches got merged).
While looking at something else recently - it dawned on me that these
changes do not include any tests for hot plug... and then of course is
that something that would (be expected to) work and whether that option
was tested by the author.
If we do merge them it doesn't seem from Berto's response that future
QEMU adjustments in this area will be affected if someone does configure
and provide the parameters.
John
2605
days inactive
2620
days old
9 comments
3 participants
participants (3)
-
John Ferlan -
Liu Qing -
Pavel Hrdina