This option allows or disallows detection of zero-writes if it is set to "on" or "off", respectively. It can be also set to "unmap" in which case it will try discarding that part of image based on the value of the "discard" option. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 12 ++++++ src/conf/domain_conf.c | 19 ++++++++- src/conf/domain_conf.h | 11 ++++++ src/libvirt_private.syms | 2 + .../qemuxml2argv-disk-drive-detect-zeroes.xml | 45 ++++++++++++++++++++++ .../qemuxml2xmlout-disk-drive-detect-zeroes.xml | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e737e39d384f..199900963938 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2574,6 +2574,17 @@ <code>bus</code> and "pci" or "ccw" <code>address</code> types. <span class='since'>Since 1.2.8 (QEMU 2.1)</span> </li> + <li> + The optional <code>detect_zeroes</code> attribute controls whether + to detect zero write requests. The value can be "off", "on" or + "unmap". First two values turn the detection off and on, + respectively. Detecting such writes can take some time, however it + can save space for spare images, the third value ("unmap") turns the + detection on and additionally tries to discard such areas from the + image based on the value of <code>discard</code> above (it will act + as "on" if <code>discard</code> is set to "ignore"). + <span class='since'>Since 1.3.6</span> + </li> </ul> </dd> <dt><code>backenddomain</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d14c1c5ecffd..da8de14a8afd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1618,6 +1618,9 @@ <optional> <ref name="driverIOThread"/> </optional> + <optional> + <ref name="detect_zeroes"/> + </optional> <empty/> </element> </define> @@ -1702,6 +1705,15 @@ <ref name="unsignedInt"/> </attribute> </define> + <define name="detect_zeroes"> + <attribute name='detect_zeroes'> + <choice> + <value>off</value> + <value>on</value> + <value>unmap</value> + </choice> + </attribute> + </define> <define name="controller"> <element name="controller"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9f9fdf24190e..dbff1cfa0399 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -806,6 +806,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, "unmap", "ignore") +VIR_ENUM_IMPL(virDomainDiskDetectZeroes, VIR_DOMAIN_DISK_DETECT_ZEROES_LAST, + "default", + "off", + "on", + "unmap") + VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, "none", "yes", @@ -7203,6 +7209,14 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, VIR_FREE(tmp); } + if ((tmp = virXMLPropString(cur, "detect_zeroes")) && + (def->detect_zeroes = virDomainDiskDetectZeroesTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown driver detect_zeroes value '%s'"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + ret = 0; cleanup: @@ -19368,6 +19382,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *copy_on_read = virTristateSwitchTypeToString(def->copy_on_read); const char *sgio = virDomainDeviceSGIOTypeToString(def->sgio); const char *discard = virDomainDiskDiscardTypeToString(def->discard); + const char *detect_zeroes = virDomainDiskDetectZeroesTypeToString(def->detect_zeroes); if (!type || !def->src->type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -19422,7 +19437,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->src->driverName || def->src->format > 0 || def->cachemode || def->error_policy || def->rerror_policy || def->iomode || def->ioeventfd || def->event_idx || def->copy_on_read || - def->discard || def->iothread) { + def->discard || def->iothread || def->detect_zeroes) { virBufferAddLit(buf, "<driver"); virBufferEscapeString(buf, " name='%s'", def->src->driverName); if (def->src->format > 0) @@ -19446,6 +19461,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " discard='%s'", discard); if (def->iothread) virBufferAsprintf(buf, " iothread='%u'", def->iothread); + if (def->detect_zeroes) + virBufferAsprintf(buf, " detect_zeroes='%s'", detect_zeroes); virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b1953b31431a..f44daa0b6b2e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -527,6 +527,15 @@ typedef enum { VIR_DOMAIN_DISK_DISCARD_LAST } virDomainDiskDiscard; +typedef enum { + VIR_DOMAIN_DISK_DETECT_ZEROES_DEFAULT = 0, + VIR_DOMAIN_DISK_DETECT_ZEROES_OFF, + VIR_DOMAIN_DISK_DETECT_ZEROES_ON, + VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP, + + VIR_DOMAIN_DISK_DETECT_ZEROES_LAST +} virDomainDiskDetectZeroes; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -605,6 +614,7 @@ struct _virDomainDiskDef { int sgio; /* enum virDomainDeviceSGIO */ int discard; /* enum virDomainDiskDiscard */ unsigned int iothread; /* unused = 0, > 0 specific thread # */ + int detect_zeroes; /* enum virDomainDiskDetectZeroes */ char *domain_name; /* backend domain name */ }; @@ -2910,6 +2920,7 @@ VIR_ENUM_DECL(virDomainDiskIo) VIR_ENUM_DECL(virDomainDeviceSGIO) VIR_ENUM_DECL(virDomainDiskTray) VIR_ENUM_DECL(virDomainDiskDiscard) +VIR_ENUM_DECL(virDomainDiskDetectZeroes) VIR_ENUM_DECL(virDomainDiskMirrorState) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModelPCI) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 333bf7c8b2db..b07e90bf9fec 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -266,6 +266,8 @@ virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDefNew; virDomainDiskDefSourceParse; +virDomainDiskDetectZeroesTypeFromString; +virDomainDiskDetectZeroesTypeToString; virDomainDiskDeviceTypeToString; virDomainDiskDiscardTypeToString; virDomainDiskErrorPolicyTypeFromString; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml new file mode 100644 index 000000000000..8953f50f3f92 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>test</name> + <uuid>92d7a226-cfae-425b-a6d3-00bbf9ec5c9e</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.13'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' discard='unmap' detect_zeroes='unmap'/> + <source file='/var/lib/libvirt/images/f14.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver discard='ignore' detect_zeroes='off'/> + <source file='/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso'/> + <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='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml new file mode 120000 index 000000000000..afa3199d4a7e --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e9d1f938360d..d8810f791f27 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -558,6 +558,7 @@ mymain(void) DO_TEST("disk-source-pool-mode"); DO_TEST("disk-drive-discard"); + DO_TEST("disk-drive-detect-zeroes"); DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd"); -- 2.8.3

On Wed, Jun 08, 2016 at 07:27:47PM -0400, John Ferlan wrote: Never again =) And since this is HTML, it will be rendered as a one space, no matter how many whitespaces there are ;) Funny typo, thanks for catching that. I believe we discussed that already. I don't know with whom that was or in what version; maybe it was on another subject altogether. Nevertheless we can't say that not supplying the attribute means "off" and that's for one simple reason. For QEMU that would work, but if there is some other hypervisor (or maybe a new one in the future) that does zero detection when we don't specify anything, then we're screwed. Or when QEMU changes it's default behaviour (very unlikely, I know, but better safe than sorry). That's why not specifying anything means "let the hypervisor decide" and we can support that until eternity and be sure we won't break anything like that. If we decide that not specifying anything means "off", then we can always add that default value in the XML automatically. I added this in the docs because I was unable to come up with such nicely worded explanation ;) Differently than what I described in the documentation? I mentioned it there and we discussed whether we should follow that behaviour or not in previous round of patches I believe. I'm thinking about doing some write-ups about suggestions and explanation of scenarios that people ask about often. But that's a story for another day and it doesn't fit into documentation nor manuals (maybe some example section in the manual but I don't know who'd be looking for that there). [...] [...] Bear in mind that there are freaking weird hypervisors out there and the fact that QEMU behaves "sanely" (in this particular scenario) doesn't mean others will follow. And libvirt being all about the abstraction should make sure we won't have bunch of new XML elements just to say the same thing (which happened way too many times already). Wow, 1 crown is double the amount of 2 cents ;)

On Thu, Jun 09, 2016 at 07:33:30AM -0400, John Ferlan wrote: Especially since it's six months since I sent the first series to the list, right? =D And sure enough, you are right. I updated my qemu and found out that since the first posting some things have changed. It looks like it's hard keeping out-of-tree patches in sync across bunch of releases =) I looked at the code, and at qemu and so on and I've found out that this needs few more changes. So I fixed that and also incorporated one small piece of logic in there which does not assume any hypervisor defaults. Have a look at the next version and see. But don't worry, it's not like something tragic will happen if it slips one more release. Or five =D I'll try to post is ASAP.

On 06/04/2016 08:46 PM, Martin Kletzander wrote: Would not apply - merge conflicts too difficult for git am -3 to handle before falling flat on it's face. I'm not convinced we need a "detect_zeroes=off", but an "on" would be good. John