Re: [PATCH 1/1] conf: qemu 9pfs: add 'multidevs' option

[Ján Tomko]

On a Thursday in 2020, Christian Schoenebeck wrote:
> 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=...'

Okay, let's leave it at 'multidevs'.

> > >    <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 <address@hidden>
> > >---
> > >
> > > 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?

Usually xml2xmltest is in the same test as the XML parser/formatter
and xml2argvtest is a part of the qemu driver patch.

Jano

signature.asc
Description: PGP signature