[libvirt PATCH 1/2] conf: add killpriv v2 attribute for virtiofs
Daniel P. Berrangé
berrange at redhat.com
Wed Jan 5 15:30:07 UTC 2022
On Wed, Jan 05, 2022 at 04:06:49PM +0100, Ján Tomko wrote:
> Add a new attribute to control the killpriv feature:
>
> <filesystem>
> ...
> <binary>
> <killpriv v2='off'/>
> </binary>
> </filesystem>
Ewww, ewww, ewww.
This is a horrible element & attribute name. Even with the docs I have
little clue as to why we would ever need this or what its real effects
are. Having now read the libvirt and related QEMU bugzilla, I now see
that killpriv v2 is enabled by default in virtiofs because it is
faster and more reliable. A limitation in NFS, however, means that
some specific syscalls don't work correctly with it, so QEMU folks
are saying killpriv v2 should not be used with NFS.
At this point I ask why doesn't virtiofsd just do the right thing.
It can statfs() the root of the export it is configured with and see
that it is NFS. I'm not convinced by a need to expose this knob in
libvirt, especially with such an unintelligible name & difficult to
understand behavioural impact.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1972571
>
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
> docs/formatdomain.rst | 4 ++++
> docs/schemas/domaincommon.rng | 7 +++++++
> src/conf/domain_conf.c | 15 +++++++++++++++
> src/conf/domain_conf.h | 1 +
> .../qemuxml2argvdata/vhost-user-fs-fd-memory.xml | 1 +
> 5 files changed, 28 insertions(+)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index d4f30bb8af..73ff8bce51 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3313,6 +3313,7 @@ A directory on the host that can be accessed directly from the guest.
> <binary path='/usr/libexec/virtiofsd' xattr='on'>
> <cache mode='always'/>
> <sandbox mode='namespace'/>
> + <killpriv v2='on'/>
> <lock posix='on' flock='on'/>
> </binary>
> <source dir='/path'/>
> @@ -3447,6 +3448,9 @@ A directory on the host that can be accessed directly from the guest.
> ``chroot``, see the
> `virtiofsd documentation <https://qemu.readthedocs.io/en/latest/tools/virtiofsd.html>`__
> for more details. ( :since:`Since 7.2.0` )
> + The ``killpriv`` element with the attribute ``v2`` (values: ``on`` or ``off``)
> + can be used to disable the killpriv capability which is used to improve performance
> + by expecting writes to reset some security attributes. ( :since:`Since 8.0.0` )
> ``source``
> The resource on the host that is being accessed in the guest. The ``name``
> attribute must be used with ``type='template'``, and the ``dir`` attribute
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 7fa5c2b8b5..5701bbe193 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3057,6 +3057,13 @@
> </optional>
> </element>
> </optional>
> + <optional>
> + <element name="killpriv">
> + <attribute name="v2">
> + <ref name="virOnOff"/>
> + </attribute>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5691b8d2d5..2a1802a3d5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9874,6 +9874,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
> g_autofree char *sandbox = virXPathString("string(./binary/sandbox/@mode)", ctxt);
> g_autofree char *posix_lock = virXPathString("string(./binary/lock/@posix)", ctxt);
> g_autofree char *flock = virXPathString("string(./binary/lock/@flock)", ctxt);
> + g_autofree char *killpriv_v2 = virXPathString("string(./binary/killpriv/@v2)", ctxt);
> int val;
>
> if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) {
> @@ -9932,6 +9933,15 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
> }
> def->flock = val;
> }
> +
> + if (killpriv_v2) {
> + if ((val = virTristateSwitchTypeFromString(killpriv_v2)) <= 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown killpriv_v2 value '%s'"), killpriv_v2);
> + goto error;
> + }
> + def->killpriv_v2 = val;
> + }
> }
>
> if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM
> @@ -24197,6 +24207,11 @@ virDomainFSDefFormat(virBuffer *buf,
> virTristateSwitchTypeToString(def->flock));
> }
>
> + if (def->killpriv_v2 != VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(&binaryBuf, "<killpriv v2='%s'/>\n",
> + virTristateSwitchTypeToString(def->killpriv_v2));
> + }
> +
> virXMLFormatElement(&binaryBuf, "lock", &lockAttrBuf, NULL);
> }
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 144ba4dd12..4619fcbfd1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -892,6 +892,7 @@ struct _virDomainFSDef {
> virTristateSwitch posix_lock;
> virTristateSwitch flock;
> virDomainFSSandboxMode sandbox;
> + virTristateSwitch killpriv_v2;
> virDomainVirtioOptions *virtio;
> virObject *privateData;
> };
> diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
> index abddf0870b..2f44a1593a 100644
> --- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
> +++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
> @@ -31,6 +31,7 @@
> <binary path='/usr/libexec/virtiofsd' xattr='on'>
> <cache mode='always'/>
> <sandbox mode='chroot'/>
> + <killpriv v2='on'/>
> <lock posix='off' flock='off'/>
> </binary>
> <source dir='/path'/>
> --
> 2.31.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list