[v1] support running virtiofsd in session mode

[libvirt PATCHv1 4/8] conf: add idmap element to filesystem

Posted by Ján Tomko 2 years, 4 months ago

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.
 ``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;
+
+            def->idmap.nuidmap = n;
+        }
+        VIR_FREE(nodes);
+
+        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;
+
+            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'/>
-- 
2.41.0

Re: [libvirt PATCHv1 4/8] conf: add idmap element to filesystem

Posted by Michal Prívozník 2 years, 4 months ago

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'/>

[libvirt PATCHv1 1/8] qemu: fix indentation
[libvirt PATCHv1 2/8] conf: move idmap definition earlier
[libvirt PATCHv1 3/8] conf: move idmap parsing earlier
[libvirt PATCHv1 4/8] conf: add idmap element to filesystem
[libvirt PATCHv1 5/8] qemu: format uid/gid map for virtiofs
[libvirt PATCHv1 6/8] qemu: virtiofs: do not force UID 0
[libvirt PATCHv1 7/8] qemu: allow running virtiofsd in session mode
[libvirt PATCHv1 8/8] docs: virtiofs: add section about ID remapping