[PATCH v2 0/4] add support for QEMU 9pfs 'multidevs' option

QEMU 4.2 added a new option 'multidevs' for 9pfs. The following patch adds support for this new option to libvirt. In short, what is this about: to distinguish files uniquely from each other in general, numeric file IDs are typically used for comparison, which in practice is the combination of a file's device ID and the file's inode number. Unfortunately 9p protocol's QID field used for this purpose, currently is too small to fit both the device ID and inode number in, which hence is a problem if one 9pfs export contains multiple devices and may thus lead to misbheaviours on guest (e.g. with SAMBA file servers) in that case due to potential file ID collisions. To mitigate this problem with 9pfs a 'multidevs' option was introduced in QEMU 4.2 for defining how to deal with this, e.g. multidevs=remap will cause QEMU's 9pfs implementation to remap all inodes from host side to different inode numbers on guest side in a way that prevents file ID collisions. NOTE: In the libvirt docs changes of this libvirt patch I simply assumed "since 6.2.0". So the final libvirt version number would need to be adjusted in that text if necessary. See QEMU discussion with following Message-ID for details: 8a2ffe17fda3a86b9a5a437e1245276881f1e235.1567680121.git.qemu_oss@crudebyte.com v1->v2: * Unrelated docs/formatdomain.html.in changes to separate patch. [patch 1] * Added new capability QEMU_CAPS_VIRTFS_MULTIDEVS. [patch 2] * XML changes as isolated patch. [patch 3] * Code style fix. [patch 3] * QEMU 'multidevs' command handling as isolated patch. [patch 4] * Error out if not QEMU_CAPS_VIRTFS_MULTIDEVS capability. [patch 4] * Error out on virtiofs (since it does not have the 'multidevs' option). [patch 4] TODO: * Capabilities test cases would fail if <flag name='virtfs-multidevs'/> was added to the other architectures' test case xml files, why? [patch 2] * The requested test cases to add: Sorry, the libvirt test case environment is yet a mystery to me, I would not even know where to start here. Message-ID of v1: E1jEFpL-00028n-Qj@lizzy.crudebyte.com Christian Schoenebeck (4): docs: virtfs: add section separators qemu: capabilities: add QEMU_CAPS_VIRTFS_MULTIDEVS conf: add 'multidevs' option qemu: add support for 'multidevs' option docs/formatdomain.html.in | 47 ++++++++++++++++++- docs/schemas/domaincommon.rng | 10 ++++ src/conf/domain_conf.c | 29 ++++++++++++ src/conf/domain_conf.h | 13 +++++ src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 12 +++++ .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + 11 files changed, 126 insertions(+), 1 deletion(-) -- 2.20.1

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- docs/formatdomain.html.in | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 594146009d..cc2c671c14 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4084,13 +4084,20 @@ </dd> </dl> + <p> <span class="since">Since 5.2.0</span>, the filesystem element has an optional attribute <code>model</code> with supported values "virtio-transitional", "virtio-non-transitional", or "virtio". See <a href="#elementsVirtioTransitional">Virtio transitional devices</a> for more details. + </p> + </dd> + <p> + The <code>filesystem</code> element may contain the following subelements: + </p> + <dt><code>driver</code></dt> <dd> The optional driver element allows specifying further details -- 2.20.1

On a Friday in 2020, Christian Schoenebeck wrote:
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- docs/formatdomain.html.in | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> and pushed Jano

The QEMU 9pfs 'multidevs' option exists since QEMU 4.2, so just set this capability based on that QEMU version. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + 5 files changed, 9 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a95a60c36a..68b6e166e9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -567,6 +567,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "query-named-block-nodes.flat", "blockdev-snapshot.allow-write-only-overlay", "blockdev-reopen", + "virtfs-multidevs", ); @@ -4837,6 +4838,10 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) ARCH_IS_PPC64(qemuCaps->arch)) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); } + + /* -virtfs multidevs option is supported since QEMU 4.2 */ + if (qemuCaps->version >= 4002000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VIRTFS_MULTIDEVS); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f0961e273c..a6025312be 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -548,6 +548,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_QMP_QUERY_NAMED_BLOCK_NODES_FLAT, /* query-named-block-nodes supports the 'flat' option */ QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY, /* blockdev-snapshot has the 'allow-write-only-overlay' feature */ QEMU_CAPS_BLOCKDEV_REOPEN, /* 'blockdev-reopen' qmp command is supported */ + QEMU_CAPS_VIRTFS_MULTIDEVS, /* -virtfs multidevs supported by virtio-9p */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 83e804ea36..d8b0de46cd 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -223,6 +223,7 @@ <flag name='rng-builtin'/> <flag name='virtio-net.failover'/> <flag name='vhost-user-fs'/> + <flag name='virtfs-multidevs'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index e52c60607d..3a695fbe79 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -181,6 +181,7 @@ <flag name='virtio-net.failover'/> <flag name='cpu.kvm-no-adjvtime'/> <flag name='vhost-user-fs'/> + <flag name='virtfs-multidevs'/> <version>4002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index d773f7e356..95fa0813dd 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -226,6 +226,7 @@ <flag name='vhost-user-fs'/> <flag name='query-named-block-nodes.flat'/> <flag name='blockdev-snapshot.allow-write-only-overlay'/> + <flag name='virtfs-multidevs'/> <version>4002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> -- 2.20.1

On a Friday in 2020, Christian Schoenebeck wrote:
The QEMU 9pfs 'multidevs' option exists since QEMU 4.2, so just set this capability based on that QEMU version.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + 5 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a95a60c36a..68b6e166e9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -567,6 +567,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "query-named-block-nodes.flat", "blockdev-snapshot.allow-write-only-overlay", "blockdev-reopen", + "virtfs-multidevs", );
@@ -4837,6 +4838,10 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) ARCH_IS_PPC64(qemuCaps->arch)) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT); } + + /* -virtfs multidevs option is supported since QEMU 4.2 */ + if (qemuCaps->version >= 4002000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VIRTFS_MULTIDEVS); }
The preferred way is to set the capabilities based on what QEMU actually knows - that way it works correctly even on newer QEMUs with the feature compiled out. This option shows up in the output of 'query-command-line-options', so all that's needed is adding it to the correct array: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 68b6e166e9..9402581a9d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3157,6 +3157,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS }, { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT }, { "smp-opts", "dies", QEMU_CAPS_SMP_DIES }, + { "virtfs", "multidevs", QEMU_CAPS_VIRTFS_MULTIDEVS }, }; static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f0961e273c..a6025312be 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -548,6 +548,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_QMP_QUERY_NAMED_BLOCK_NODES_FLAT, /* query-named-block-nodes supports the 'flat' option */ QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY, /* blockdev-snapshot has the 'allow-write-only-overlay' feature */ QEMU_CAPS_BLOCKDEV_REOPEN, /* 'blockdev-reopen' qmp command is supported */ + QEMU_CAPS_VIRTFS_MULTIDEVS, /* -virtfs multidevs supported by virtio-9p */
The brief version should be enough: /* virtfs.multidevs */ Jano
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;

Introduce new 'multidevs' option for filesystem. <filesystem type='mount' accessmode='mapped' multidevs='remap'> <source dir='/path'/> <target dir='mount_tag'> </filesystem> This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- docs/formatdomain.html.in | 40 ++++++++++++++++++++++++++++++++++- docs/schemas/domaincommon.rng | 10 +++++++++ src/conf/domain_conf.c | 29 +++++++++++++++++++++++++ src/conf/domain_conf.h | 13 ++++++++++++ 4 files changed, 91 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cc2c671c14..13c506988b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3967,7 +3967,7 @@ <source name='my-vm-template'/> <target dir='/'/> </filesystem> - <filesystem type='mount' accessmode='passthrough'> + <filesystem type='mount' accessmode='passthrough' multidevs='remap'> <driver type='path' wrpolicy='immediate'/> <source dir='/export/to/guest'/> <target dir='/import/from/host'/> @@ -4092,6 +4092,44 @@ for more details. </p> + <p> + The filesystem element has an optional attribute <code>multidevs</code> + which specifies how to deal with a filesystem export containing more than + one device, in order to avoid file ID collisions on guest when using 9pfs + (<span class="since">since 6.2.0, requires QEMU 4.2</span>). + This attribute is not available for virtiofs. The possible values are: + </p> + + <dl> + <dt><code>default</code></dt> + <dd> + Use QEMU's default setting (which currently is <code>warn</code>). + </dd> + <dt><code>remap</code></dt> + <dd> + This setting allows guest to access multiple devices per export without + encountering misbehaviours. Inode numbers from host are automatically + remapped on guest to actively prevent file ID collisions if guest + accesses one export containing multiple devices. + </dd> + <dt><code>forbid</code></dt> + <dd> + Only allow to access one device per export by guest. Attempts to access + additional devices on the same export will cause the individual + filesystem access by guest to fail with an error and being logged (once) + as error on host side. + </dd> + <dt><code>warn</code></dt> + <dd> + This setting resembles the behaviour of 9pfs prior to QEMU 4.2, that is + no action is performed to prevent any potential file ID collisions if an + export contains multiple devices, with the only exception: a warning is + logged (once) on host side now. This setting may lead to misbehaviours + on guest side if more than one device is exported per export, due to the + potential file ID collisions this may cause on guest side in that case. + </dd> + </dl> + </dd> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6805420451..9b37740e30 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2676,6 +2676,16 @@ </choice> </attribute> </optional> + <optional> + <attribute name="multidevs"> + <choice> + <value>default</value> + <value>remap</value> + <value>forbid</value> + <value>warn</value> + </choice> + </attribute> + </optional> <optional> <element name='readonly'> <empty/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 71535f53f5..6a9a7dd0bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -501,6 +501,14 @@ VIR_ENUM_IMPL(virDomainFSModel, "virtio-non-transitional", ); +VIR_ENUM_IMPL(virDomainFSMultidevs, + VIR_DOMAIN_FS_MULTIDEVS_LAST, + "default", + "remap", + "forbid", + "warn", +); + VIR_ENUM_IMPL(virDomainFSCacheMode, VIR_DOMAIN_FS_CACHE_MODE_LAST, "default", @@ -11376,6 +11384,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt, g_autofree char *usage = NULL; g_autofree char *units = NULL; g_autofree char *model = NULL; + g_autofree char *multidevs = NULL; ctxt->node = node; @@ -11414,6 +11423,17 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt, } } + multidevs = virXMLPropString(node, "multidevs"); + if (multidevs) { + if ((def->multidevs = virDomainFSMultidevsTypeFromString(multidevs)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown multidevs '%s'"), multidevs); + goto error; + } + } else { + def->multidevs = VIR_DOMAIN_FS_MULTIDEVS_DEFAULT; + } + if (virDomainParseScaledValue("./space_hard_limit[1]", NULL, ctxt, &def->space_hard_limit, 1, ULLONG_MAX, false) < 0) @@ -25397,6 +25417,7 @@ virDomainFSDefFormat(virBufferPtr buf, const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode); const char *fsdriver = virDomainFSDriverTypeToString(def->fsdriver); const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy); + const char *multidevs = virDomainFSMultidevsTypeToString(def->multidevs); const char *src = def->src->path; g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) driverBuf = VIR_BUFFER_INIT_CHILD(buf); @@ -25415,6 +25436,12 @@ virDomainFSDefFormat(virBufferPtr buf, return -1; } + if (!multidevs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected multidevs %d"), def->multidevs); + return -1; + } + virBufferAsprintf(buf, "<filesystem type='%s' accessmode='%s'", type, accessmode); @@ -25422,6 +25449,8 @@ virDomainFSDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " model='%s'", virDomainFSModelTypeToString(def->model)); } + if (def->multidevs) + virBufferAsprintf(buf, " multidevs='%s'", multidevs); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 91b776c28a..23b7924781 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -796,6 +796,18 @@ typedef enum { VIR_DOMAIN_FS_WRPOLICY_LAST } virDomainFSWrpolicy; +/* How to handle exports containing multiple devices. */ +typedef enum { + VIR_DOMAIN_FS_MULTIDEVS_DEFAULT = 0, /* Use QEMU's default setting */ + VIR_DOMAIN_FS_MULTIDEVS_REMAP, /* Remap inodes from host to guest */ + VIR_DOMAIN_FS_MULTIDEVS_FORBID, /* Prohibit more than one device */ + VIR_DOMAIN_FS_MULTIDEVS_WARN, /* Just log a warning if multiple devices */ + + VIR_DOMAIN_FS_MULTIDEVS_LAST +} virDomainFSMultidevs; + +VIR_ENUM_DECL(virDomainFSMultidevs); + typedef enum { VIR_DOMAIN_FS_MODEL_DEFAULT = 0, VIR_DOMAIN_FS_MODEL_VIRTIO, @@ -820,6 +832,7 @@ struct _virDomainFSDef { int wrpolicy; /* enum virDomainFSWrpolicy */ int format; /* virStorageFileFormat */ int model; /* virDomainFSModel */ + int multidevs; /* virDomainFSMultidevs */ unsigned long long usage; /* in bytes */ virStorageSourcePtr src; char *dst; -- 2.20.1

On a Friday in 2020, Christian Schoenebeck wrote:
Introduce new 'multidevs' option for filesystem.
<filesystem type='mount' accessmode='mapped' multidevs='remap'> <source dir='/path'/> <target dir='mount_tag'> </filesystem>
This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- docs/formatdomain.html.in | 40 ++++++++++++++++++++++++++++++++++- docs/schemas/domaincommon.rng | 10 +++++++++ src/conf/domain_conf.c | 29 +++++++++++++++++++++++++ src/conf/domain_conf.h | 13 ++++++++++++ 4 files changed, 91 insertions(+), 1 deletion(-)
Looks good, missing an addition of qemuxml2xmltest. Jano

This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9790c92cf8..7020e5448c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2632,6 +2632,13 @@ qemuBuildFSStr(virDomainFSDefPtr fs) } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { virBufferAddLit(&opt, ",security_model=none"); } + if (fs->multidevs == VIR_DOMAIN_FS_MULTIDEVS_REMAP) { + virBufferAddLit(&opt, ",multidevs=remap"); + } else if (fs->multidevs == VIR_DOMAIN_FS_MULTIDEVS_FORBID) { + virBufferAddLit(&opt, ",multidevs=forbid"); + } else if (fs->multidevs == VIR_DOMAIN_FS_MULTIDEVS_WARN) { + virBufferAddLit(&opt, ",multidevs=warn"); + } } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE) { /* removed since qemu 4.0.0 see v3.1.0-29-g93aee84f57 */ virBufferAddLit(&opt, "handle"); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index edc8ba2ddb..c54c64fadb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8529,6 +8529,13 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs, _("only supports mount filesystem type")); return -1; } + if (fs->multidevs != VIR_DOMAIN_FS_MODEL_DEFAULT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTFS_MULTIDEVS)) + { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("multidevs is not supported with this QEMU binary")); + return -1; + } switch ((virDomainFSDriverType) fs->fsdriver) { case VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT: @@ -8581,6 +8588,11 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs, _("virtiofs is not supported with this QEMU binary")); return -1; } + if (fs->multidevs != VIR_DOMAIN_FS_MULTIDEVS_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtiofs does not support multidevs")); + return -1; + } if (qemuDomainDefValidateVirtioFSSharedMemory(def) < 0) return -1; break; -- 2.20.1

On a Friday in 2020, Christian Schoenebeck wrote:
This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)
A change to qemuxml2argvtest is needed. If you already added the XML file to qemuxml2argvdata in the previous commit, all you need to to to generate the output file is: * add a new DO_TEST_CAPS_LATEST line to qemuxml2argvtest.c * run the test with VIR_TEST_REGENERATE_OUTPUT=1 Otherwise LGTM Jano

On Freitag, 27. März 2020 18:16:37 CEST Ján Tomko wrote:
On a Friday in 2020, Christian Schoenebeck wrote:
This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> ---
src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)
A change to qemuxml2argvtest is needed.
If you already added the XML file to qemuxml2argvdata in the previous commit, all you need to to to generate the output file is: * add a new DO_TEST_CAPS_LATEST line to qemuxml2argvtest.c
Looking at the existing test cases, it probably makes sense to extend qemuxml2argvdata/virtio-options.xml instead of adding a separate XML file, that is: diff --git a/tests/qemuxml2argvdata/virtio-options.xml b/tests/ qemuxml2argvdata/virtio-options.xml index dd9a4f4a01..7ea624dfad 100644 --- a/tests/qemuxml2argvdata/virtio-options.xml +++ b/tests/qemuxml2argvdata/virtio-options.xml @@ -47,6 +47,21 @@ <target dir='fs2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='remap'> + <source dir='/export/fs3'/> + <target dir='fs3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='forbid'> + <source dir='/export/fs4'/> + <target dir='fs4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='warn'> + <source dir='/export/fs5'/> + <target dir='fs5'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x13' function='0x0'/> + </filesystem> <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/>
* run the test with VIR_TEST_REGENERATE_OUTPUT=1
Ah, that's the one! :) I was already wondering whether other people are auto generating the files I manually changed so far. I'll post a rebased v3 with the things discussed so far. Thanks Ján! Best regards, Christian Schoenebeck

On a Monday in 2020, Christian Schoenebeck wrote:
On Freitag, 27. März 2020 18:16:37 CEST Ján Tomko wrote:
On a Friday in 2020, Christian Schoenebeck wrote:
This option prevents misbehaviours on guest if a qemu 9pfs export contains multiple devices, due to the potential file ID collisions this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> ---
src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 19 insertions(+)
A change to qemuxml2argvtest is needed.
If you already added the XML file to qemuxml2argvdata in the previous commit, all you need to to to generate the output file is: * add a new DO_TEST_CAPS_LATEST line to qemuxml2argvtest.c
Looking at the existing test cases, it probably makes sense to extend qemuxml2argvdata/virtio-options.xml instead of adding a separate XML file, that is:
That one is aiming at testing the iommu and ats options common for virtio devices. The 'fs9p' test would be a better candidate for adding these, but I'd still rather add a new file - to show that the command line for the old config remains unchanged. Jano
diff --git a/tests/qemuxml2argvdata/virtio-options.xml b/tests/ qemuxml2argvdata/virtio-options.xml index dd9a4f4a01..7ea624dfad 100644 --- a/tests/qemuxml2argvdata/virtio-options.xml +++ b/tests/qemuxml2argvdata/virtio-options.xml @@ -47,6 +47,21 @@ <target dir='fs2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='remap'> + <source dir='/export/fs3'/> + <target dir='fs3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='forbid'> + <source dir='/export/fs4'/> + <target dir='fs4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped' multidevs='warn'> + <source dir='/export/fs5'/> + <target dir='fs5'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x13' function='0x0'/> + </filesystem> <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/>
* run the test with VIR_TEST_REGENERATE_OUTPUT=1
Ah, that's the one! :) I was already wondering whether other people are auto generating the files I manually changed so far.
I'll post a rebased v3 with the things discussed so far. Thanks Ján!
Best regards, Christian Schoenebeck

On Montag, 30. März 2020 16:45:08 CEST Ján Tomko wrote:
A change to qemuxml2argvtest is needed.
If you already added the XML file to qemuxml2argvdata in the previous commit, all you need to to to generate the output file is: * add a new DO_TEST_CAPS_LATEST line to qemuxml2argvtest.c
Looking at the existing test cases, it probably makes sense to extend qemuxml2argvdata/virtio-options.xml instead of adding a separate XML file,
that is: That one is aiming at testing the iommu and ats options common for virtio devices.
The 'fs9p' test would be a better candidate for adding these, but I'd still rather add a new file - to show that the command line for the old config remains unchanged.
Convinced. I change it. Best regards, Christian Schoenebeck
participants (2)
-
Christian Schoenebeck
-
Ján Tomko