[libvirt] [PATCH v4] Add new attribute wrpolicy to <driver> element

This introduces new attribute wrpolicy with only supported value as immediate. This will be an optional attribute with no defaults. This helps specify whether to skip the host page cache. When wrpolicy is specified, meaning when wrpolicy=immediate a writeback is explicitly initiated for the dirty pages in the host page cache as part of the guest file write operation. Usage: <filesystem type='mount' accessmode='passthrough'> <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem> Currently this only works with type='mount' for the QEMU/KVM driver. Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com> --- v2: - added writeout as a qemu cap - cosmetic changes in comments - moved to using VIR_ERR_CONFIG_UNSUPPORTED - corrected doc v3: - changed attr name from writeout to wrpolicy - moved attr to be part of driver xml element v4: - implemented review comments given by eric blake against v3 - added wrpolicy testcase docs/formatdomain.html.in | 9 ++++++-- docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++++++- src/conf/domain_conf.h | 10 +++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 12 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-fs9p.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml | 4 ++-- tests/qemuxml2argvtest.c | 2 +- 11 files changed, 71 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index faf42df..875ac22 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1328,7 +1328,7 @@ <target dir='/'/> </filesystem> <filesystem type='mount' accessmode='passthrough'> - <driver type='path'/> + <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> <readonly/> @@ -1355,7 +1355,12 @@ sub-element <code>driver</code>, with an attribute <code>type='path'</code> or <code>type='handle'</code> <span class="since">(since - 0.9.7)</span>. + 0.9.7)</span>. The driver block has an optional attribute + <code>wrpolicy</code> that further controls interaction with + the host page cache; omitting the attribute gives default behavior, + while the value <code>immediate</code> means that a host writeback + is immediately triggered for all pages touched during a guest file + write operation <span class="since">(since 0.9.10)</span>. </dd> <dt><code>type='template'</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 243ff93..f72f7d1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1162,6 +1162,11 @@ <value>handle</value> </choice> </attribute> + <optional> + <attribute name="wrpolicy"> + <value>immediate</value> + </attribute> + </optional> <empty/> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c42f36..5fa23dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -265,6 +265,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "mapped", "squash") +VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, + "default", + "immediate") VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "user", @@ -3459,6 +3462,7 @@ virDomainFSDefParseXML(xmlNodePtr node, char *source = NULL; char *target = NULL; char *accessmode = NULL; + char *wrpolicy = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3508,6 +3512,7 @@ virDomainFSDefParseXML(xmlNodePtr node, def->readonly = 1; } else if ((fsdriver == NULL) && (xmlStrEqual(cur->name, BAD_CAST "driver"))) { fsdriver = virXMLPropString(cur, "type"); + wrpolicy = virXMLPropString(cur, "wrpolicy"); } } cur = cur->next; @@ -3521,6 +3526,16 @@ virDomainFSDefParseXML(xmlNodePtr node, } } + if (wrpolicy) { + if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown filesystem write policy '%s'"), wrpolicy); + goto error; + } + } else { + def->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT; + } + if (source == NULL) { virDomainReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); @@ -3547,6 +3562,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(accessmode); + VIR_FREE(wrpolicy); return def; @@ -10165,6 +10181,7 @@ virDomainFSDefFormat(virBufferPtr buf, const char *type = virDomainFSTypeToString(def->type); const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode); const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy); if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -10184,7 +10201,15 @@ virDomainFSDefFormat(virBufferPtr buf, type, accessmode); if (def->fsdriver) { - virBufferAsprintf(buf, " <driver type='%s'/>\n", fsdriver); + virBufferAsprintf(buf, " <driver type='%s'", fsdriver); + + /* Don't generate anything if wrpolicy is set to default */ + if (def->wrpolicy) { + virBufferAsprintf(buf, " wrpolicy='%s'", wrpolicy); + } + + virBufferAddLit(buf, "/>\n"); + } if (def->src) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7c7e93b..ac8e768 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -491,12 +491,21 @@ enum virDomainFSAccessMode { VIR_DOMAIN_FS_ACCESSMODE_LAST }; +/* Filesystem Write policy */ +enum virDomainFSWrpolicy { + VIR_DOMAIN_FS_WRPOLICY_DEFAULT = 0, + VIR_DOMAIN_FS_WRPOLICY_IMMEDIATE, + + VIR_DOMAIN_FS_WRPOLICY_LAST +}; + typedef struct _virDomainFSDef virDomainFSDef; typedef virDomainFSDef *virDomainFSDefPtr; struct _virDomainFSDef { int type; int fsdriver; int accessmode; + int wrpolicy; /* enum virDomainFSWrpolicy */ char *src; char *dst; unsigned int readonly : 1; @@ -1984,6 +1993,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB) VIR_ENUM_DECL(virDomainFS) VIR_ENUM_DECL(virDomainFSDriverType) VIR_ENUM_DECL(virDomainFSAccessMode) +VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) VIR_ENUM_DECL(virDomainNetVirtioTxMode) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cfb88f8..3e1ba0c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -313,6 +313,8 @@ virDomainDiskTypeToString; virDomainFSDefFree; virDomainFSTypeFromString; virDomainFSTypeToString; +virDomainFSWrpolicyTypeFromString; +virDomainFSWrpolicyTypeToString; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 530bbb1..3069dbe 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -148,6 +148,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "virtio-blk-pci.scsi", "blk-sg-io", "drive-copy-on-read", + "fsdev-writeout", ); struct qemu_feature_flags { @@ -1089,6 +1090,8 @@ qemuCapsComputeCmdFlags(const char *help, qemuCapsSet(flags, QEMU_CAPS_FSDEV); if (strstr(fsdev, "readonly")) qemuCapsSet(flags, QEMU_CAPS_FSDEV_READONLY); + if (strstr(fsdev, "writeout")) + qemuCapsSet(flags, QEMU_CAPS_FSDEV_WRITEOUT); } if (strstr(help, "-smbios type")) qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ee9d5ab..4823aae 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -121,6 +121,7 @@ enum qemuCapsFlags { QEMU_CAPS_VIRTIO_BLK_SCSI = 80, /* virtio-blk-pci.scsi */ QEMU_CAPS_VIRTIO_BLK_SG_IO = 81, /* support for SG_IO commands, reportedly added in 0.11 */ QEMU_CAPS_DRIVE_COPY_ON_READ = 82, /* -drive copy-on-read */ + QEMU_CAPS_FSDEV_WRITEOUT = 83, /* -fsdev writeout supported */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 98824ac..77f2896 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2129,6 +2129,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2161,6 +2162,17 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, goto error; } } + + if (fs->wrpolicy) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) { + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + } else { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem writeout not supported")); + goto error; + } + } + virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args index 8579810..f244114 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args @@ -4,10 +4,10 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -fsdev local,security_model=passthrough,id=fsdev-fs0,\ path=/export/to/guest -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,\ mount_tag=/import/from/host,bus=pci.0,addr=0x3 \ --fsdev local,security_model=mapped,id=fsdev-fs1,\ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ path=/export/to/guest2 -device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,\ mount_tag=/import/from/host2,bus=pci.0,addr=0x4 \ --fsdev handle,id=fsdev-fs2,\ +-fsdev handle,writeout=immediate,id=fsdev-fs2,\ path=/export/to/guest3 -device virtio-9p-pci,id=fs2,fsdev=fsdev-fs2,\ mount_tag=/import/from/host3,bus=pci.0,addr=0x5 \ -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml index 07d7e8a..e31db48 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.xml @@ -25,12 +25,12 @@ <target dir='/import/from/host'/> </filesystem> <filesystem accessmode='mapped'> - <driver type='path'/> + <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest2'/> <target dir='/import/from/host2'/> </filesystem> <filesystem> - <driver type='handle'/> + <driver type='handle' wrpolicy='immediate'/> <source dir='/export/to/guest3'/> <target dir='/import/from/host3'/> </filesystem> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7beaa4b..8feda9e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -631,7 +631,7 @@ mymain(void) DO_TEST("sound-device", false, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_HDA_DUPLEX); DO_TEST("fs9p", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV); + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT); DO_TEST("hostdev-usb-address", false, NONE); DO_TEST("hostdev-usb-address-device", false,

On 01/17/2012 05:44 AM, Deepak C Shetty wrote:
This introduces new attribute wrpolicy with only supported value as immediate. This will be an optional attribute with no defaults. This helps specify whether to skip the host page cache.
When wrpolicy is specified, meaning when wrpolicy=immediate a writeback is explicitly initiated for the dirty pages in the host page cache as part of the guest file write operation.
Usage: <filesystem type='mount' accessmode='passthrough'> <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='mount_tag'/> </filesystem>
Currently this only works with type='mount' for the QEMU/KVM driver.
Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com>
You had lots of trailing whitespace, caught by 'make syntax-check'.
+++ b/src/qemu/qemu_capabilities.c @@ -148,6 +148,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "virtio-blk-pci.scsi", "blk-sg-io", "drive-copy-on-read", + "fsdev-writeout",
Trivial merge conflict here; nothing too hard to figure out.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args @@ -4,10 +4,10 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -fsdev local,security_model=passthrough,id=fsdev-fs0,\ path=/export/to/guest -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,\ mount_tag=/import/from/host,bus=pci.0,addr=0x3 \ --fsdev local,security_model=mapped,id=fsdev-fs1,\ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ path=/export/to/guest2 -device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,\
Excellent - that's what I was looking for in an added test. ACK after fixing up the whitespace and one other test failure, so I pushed with this squashed in: diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 1d6484a..de9b480 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -1424,10 +1424,10 @@ attribute <code>type='path'</code> or <code>type='handle'</code> <span class="since">(since 0.9.7)</span>. The driver block has an optional attribute - <code>wrpolicy</code> that further controls interaction with - the host page cache; omitting the attribute gives default behavior, - while the value <code>immediate</code> means that a host writeback - is immediately triggered for all pages touched during a guest file + <code>wrpolicy</code> that further controls interaction with + the host page cache; omitting the attribute gives default behavior, + while the value <code>immediate</code> means that a host writeback + is immediately triggered for all pages touched during a guest file write operation <span class="since">(since 0.9.10)</span>. </dd> <dt><code>type='template'</code></dt> diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 28f9b1b..f97014e 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -10210,7 +10210,6 @@ virDomainFSDefFormat(virBufferPtr buf, } virBufferAddLit(buf, "/>\n"); - } if (def->src) { diff --git i/tests/qemuhelptest.c w/tests/qemuhelptest.c index 164707d..8802271 100644 --- i/tests/qemuhelptest.c +++ w/tests/qemuhelptest.c @@ -666,7 +666,8 @@ mymain(void) QEMU_CAPS_FSDEV_READONLY, QEMU_CAPS_VIRTIO_BLK_SCSI, QEMU_CAPS_VIRTIO_BLK_SG_IO, - QEMU_CAPS_CPU_HOST); + QEMU_CAPS_CPU_HOST, + QEMU_CAPS_FSDEV_WRITEOUT); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/18/2012 04:07 AM, Eric Blake wrote:
You had lots of trailing whitespace, caught by 'make syntax-check'.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fs9p.args @@ -4,10 +4,10 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -fsdev local,security_model=passthrough,id=fsdev-fs0,\ path=/export/to/guest -device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,\ mount_tag=/import/from/host,bus=pci.0,addr=0x3 \ --fsdev local,security_model=mapped,id=fsdev-fs1,\ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ path=/export/to/guest2 -device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,\ Excellent - that's what I was looking for in an added test.
ACK after fixing up the whitespace and one other test failure, so I pushed with this squashed in:
Thanks Eric, will make sure I run the syntax-check next time.
participants (2)
-
Deepak C Shetty
-
Eric Blake