On Donnerstag, 19. März 2020 14:10:26 CET Ján Tomko wrote:
On a Tuesday in 2020, Christian Schoenebeck wrote:
Introduce new 'multidevs' option for filesystem.
<filesystem type='mount' accessmode='mapped' multidevs='remap'>
I don't like the 'multidevs' name, but cannot think of anything beter.
'collisions' maybe?
Not sure if 'collisions' is better, e.g. collisions='remap' sounds scary. :) And which collision would that be? At least IMO 'multidevs' is less ambigious. I have no problem though to change it to whatever name you might come up with. Just keep the resulting key-value pair set in mind: multidevs='default' multidevs='remap' multidevs='forbid' multidevs='warn' vs. collisions='default' collisions='remap' <- probably misleading what 'remap' means in this case collisions='forbid' collisions='warn' <- wrong, it warns about multiple devices, not about file ID collisions. So different key-name might also require different value-names. Another option would be the long form 'multi-devices=...'
<source dir='/path'/> <target dir='mount_tag'>
</filesystem>
This option prevents misbheaviours on guest if a 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 | 47 ++++++++++++++++++++++++++++++++++- docs/schemas/domaincommon.rng | 10 ++++++++ src/conf/domain_conf.c | 30 ++++++++++++++++++++++ src/conf/domain_conf.h | 13 ++++++++++ src/qemu/qemu_command.c | 7 ++++++ 5 files changed, 106 insertions(+), 1 deletion(-)
Please split the XML changes from the qemu driver changes.
Ok
Also missing: * qemu_capabilities addition
AFAICS the common procedure is to add new capabilities always to the end of the enum list. So I guess I will do that as well.
* qemuDomainDeviceDefValidateFS in qemu_domain.c - check for the capability, reject this setting for virtiofs
Good to know where that check is supposed to go to, thanks!
* qemuxml2xmltest addition * qemuxml2argvtest addition
Ok, I have to read up how those tests work. AFAICS I need to add xml files to their data subdirs. Separate patches required for those 2 tests?
(no changes required for virschematest - it checks all the XML files in the directories used by the above tests against the schema)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 594146009d..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'/>
@@ -4084,13 +4084,58 @@
</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> +
Unrelated change that can be split out.
Ok, I'll make that the 1st preparatory patch then including ...
+ <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> + The <code>filesystem</code> element may contain the following subelements: + </p> +
And so can this one.
... this one.
<dt><code>driver</code></dt> <dd>
The optional driver element allows specifying further details
@@ -25422,6 +25449,9 @@ virDomainFSDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " model='%s'",
virDomainFSModelTypeToString(def->model));
}
+ if (def->multidevs) { + virBufferAsprintf(buf, " multidevs='%s'", multidevs); + }
make syntax-check complains here: Curly brackets around single-line body: ../src/conf/domain_conf.c:25452-25454: if (def->multidevs) { virBufferAsprintf(buf, " multidevs='%s'", multidevs); }
Jano
Sorry for that, I assumed same code style as qemu. I'll do the automated syntax checks from now on. Best regards, Christian Schoenebeck