On 9/11/23 15:51, Ján Tomko wrote:

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 98 +++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2c8727de54..dd67e7f21b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8581,6 +8581,55 @@ virDomainNetGenerateMAC(virDomainXMLOption *xmlopt, }

+static int virDomainIdMapEntrySort(const void *a, const void *b) +{ + const virDomainIdMapEntry *entrya = a; + const virDomainIdMapEntry *entryb = b; + + if (entrya->start > entryb->start) + return 1; + else if (entrya->start < entryb->start) + return -1; + else + return 0; +} +

Please put an extra empty line here.

+/* Parse the XML definition for user namespace id map. + * + * idmap has the form of + * + * <uid start='0' target='1000' count='10'/> + * <gid start='0' target='1000' count='10'/> + */ +static virDomainIdMapEntry * +virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr *node, + size_t num) +{ + size_t i; + virDomainIdMapEntry *idmap = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + idmap = g_new0(virDomainIdMapEntry, num); + + for (i = 0; i < num; i++) { + ctxt->node = node[i]; + if (virXPathUInt("string(./@start)", ctxt, &idmap[i].start) < 0 || + virXPathUInt("string(./@target)", ctxt, &idmap[i].target) < 0 || + virXPathUInt("string(./@count)", ctxt, &idmap[i].count) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid idmap start/target/count settings")); + VIR_FREE(idmap); + return NULL; + } + } + + qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort); + + return idmap; +} + +

Michal

On 9/11/23 15:51, Ján Tomko wrote:

Let unprivileged virtiofs use user namespace.

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.rst | 7 +++ src/conf/domain_conf.c | 51 +++++++++++++++++++ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 3 ++ .../vhost-user-fs-fd-memory.xml | 4 ++ 5 files changed, 66 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index bc469e5f9f..0f10b3043f 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3501,6 +3501,10 @@ A directory on the host that can be accessed directly from the guest. </binary> <source dir='/path'/> <target dir='mount_tag'/> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> </filesystem> <filesystem type='mount'> <driver type='virtiofs' queue='1024'/> @@ -3650,6 +3654,9 @@ A directory on the host that can be accessed directly from the guest. Where the ``source`` can be accessed in the guest. For most drivers this is an automatic mount point, but for QEMU/KVM this is merely an arbitrary string tag that is exported to the guest as a hint for where to mount. +``idmap`` + For ``virtiofs``, an ``idmap`` element can be specified to map IDs in the user + namespace. See the `Container boot`_ section for the syntax of the element.

:since:

``readonly`` Enables exporting filesystem as a readonly mount for guest, by default read-write access is given (currently only works for QEMU/KVM driver). diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dd67e7f21b..2379a9204f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2585,6 +2585,8 @@ void virDomainFSDefFree(virDomainFSDef *def) virObjectUnref(def->privateData); g_free(def->binary); g_free(def->sock); + g_free(def->idmap.uidmap); + g_free(def->idmap.gidmap);

g_free(def); } @@ -8767,6 +8769,8 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt); xmlNodePtr binary_sandbox_node = virXPathNode("./binary/sandbox", ctxt); + ssize_t n; + xmlNodePtr *nodes = NULL;

if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) { virReportError(VIR_ERR_XML_ERROR, @@ -8812,6 +8816,30 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, VIR_XML_PROP_NONZERO, &def->sandbox) < 0) goto error; + + if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) + return NULL; + + if (n) { + def->idmap.uidmap = virDomainIdmapDefParseXML(ctxt, nodes, n); + if (!def->idmap.uidmap) + return NULL;

If this return is taken, then ...

+ + def->idmap.nuidmap = n; + } + VIR_FREE(nodes);

... this is never called.

+ + if ((n = virXPathNodeSet("./idmap/gid", ctxt, &nodes)) < 0) + return NULL; + + if (n) { + def->idmap.gidmap = virDomainIdmapDefParseXML(ctxt, nodes, n); + if (!def->idmap.gidmap) + return NULL;

Same here.

+ + def->idmap.ngidmap = n; + } + VIR_FREE(nodes); }

if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM @@ -23164,6 +23192,29 @@ virDomainFSDefFormat(virBuffer *buf, virXMLFormatElement(buf, "driver", &driverAttrBuf, &driverBuf); virXMLFormatElement(buf, "binary", &binaryAttrBuf, &binaryBuf);

+ if (def->idmap.uidmap) { + size_t i; + + virBufferAddLit(buf, "<idmap>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->idmap.nuidmap; i++) { + virBufferAsprintf(buf, + "<uid start='%u' target='%u' count='%u'/>\n", + def->idmap.uidmap[i].start, + def->idmap.uidmap[i].target, + def->idmap.uidmap[i].count); + } + for (i = 0; i < def->idmap.ngidmap; i++) { + virBufferAsprintf(buf, + "<gid start='%u' target='%u' count='%u'/>\n", + def->idmap.gidmap[i].start, + def->idmap.gidmap[i].target, + def->idmap.gidmap[i].count); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</idmap>\n"); + } + switch (def->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: case VIR_DOMAIN_FS_TYPE_BIND: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8937968e3b..b84719b01d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -925,6 +925,7 @@ struct _virDomainFSDef { virTristateSwitch flock; virDomainFSSandboxMode sandbox; int thread_pool_size; + virDomainIdMapDef idmap; virDomainVirtioOptions *virtio; virObject *privateData; }; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2f9ba31c0a..2ca0e92f00 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3052,6 +3052,9 @@ </choice> <empty/> </element> + <optional> + <ref name="idmap"/> + </optional> <ref name="filesystemCommon"/> </interleave> </group> diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml index 81de8c0dd7..1d0bc26c46 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml +++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml @@ -34,6 +34,10 @@ <lock posix='off' flock='off'/> <thread_pool size='16'/> </binary> + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> <source dir='/path'/> <target dir='mount_tag'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>

On Mon, Sep 11, 2023 at 03:51:28PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/kbase/virtiofs.rst | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)

diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index 5940092db5..ecfb8e4236 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -59,6 +59,35 @@ Sharing a host directory with a guest

Note: this requires virtiofs support in the guest kernel (Linux v5.4 or later)

+ID mapping +========== + +In unprivileged mode (``qemu:///session``), mapping user/group IDs is available +(since libvirt version TBD). After reserving an ID range from the host for your +regular user

Is the GUID/GID mapping available as in optional, or available as in mandatory ? I would expect libvirt to "do the right thing" and automatically load the /etc/subuid data for the current user and NOT require any extra XML mapping to be set for unprivileged usage. By all means have an XML config for it, but it should be optional and something that is essentially never used except for niche scenarios

+ +:: + + $ cat /etc/subuid + jtomko:100000:65536 + $ cat /etc/subgid + jtomko:100000:65536 + +you can let virtiofsd map guest UIDs from 0 to 65535 +to host IDs 100000 to 165535 for example: + +:: + + <filesystem type='mount' accessmode='passthrough'> + <driver type='virtiofs' queue='1024'/> + ... + <idmap> + <uid start='0' target='100000' count='65535'/> + <gid start='0' target='100000' count='65535'/> + </idmap> + </filesystem> + + Optional parameters ===================

-- 2.41.0

With 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 :|

On Tue, Sep 12, 2023 at 04:05:04PM +0200, Ján Tomko wrote:

On a Monday in 2023, Daniel P. Berrangé wrote:

...

On Mon, Sep 11, 2023 at 03:51:28PM +0200, Ján Tomko wrote:

...

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/kbase/virtiofs.rst | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)

diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst index 5940092db5..ecfb8e4236 100644 --- a/docs/kbase/virtiofs.rst +++ b/docs/kbase/virtiofs.rst @@ -59,6 +59,35 @@ Sharing a host directory with a guest

Note: this requires virtiofs support in the guest kernel (Linux v5.4 or later)

+ID mapping +========== + +In unprivileged mode (``qemu:///session``), mapping user/group IDs is available +(since libvirt version TBD). After reserving an ID range from the host for your +regular user

Is the GUID/GID mapping available as in optional, or available as in mandatory ?

In this series, optional.

My reasoning was that someone might want to not do it and would prefer to run virtiofsd as their own user.

On second thought, that is not what accessmode='passthrough' means, so for non-mapping non-privileged use, a different/new accessmode attribute will be needed.

...

I would expect libvirt to "do the right thing" and automatically load the /etc/subuid data for the current user and NOT require any extra XML mapping to be set for unprivileged usage.

So, by default libvirt would assume that unprivileged accessmode='passthrough' means "use the whole range for my user from /etc/subuid"?

Podman treats /etc/subuid as a pool and chooses a 64K range that is (to its knowledge) unused. I'm undecided whether that would also be a reasonable option for a default.

I thought podman simply used the entry that is in /etc/subuid as is: $ grep $LOGNAME /etc/subuid berrange:165536:65536 $ podman run -it centos:stream9 cat /proc/self/uid_map 0 1001 1 1 165536 65536 Maps "root" to my original unpriv login UID, and maps everything else to the 64k IDs reserved in /etc/subuid With 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 :|