diff mbox series

qemu: virtiofs: support <sandbox mode='chroot'/>

Message ID 6f2cb9738070c4274fcfab387c279c28ed2ff35c.1616773068.git.crobinso@redhat.com
State New
Headers show
Series qemu: virtiofs: support <sandbox mode='chroot'/> | expand

Commit Message

Cole Robinson March 26, 2021, 3:37 p.m. UTC 
Add a new XML element

<filesystem>
  <binary>
    <sandbox mode='chroot|namespace'/>
  </binary>
</filesystem>

Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added
in qemu 5.2.0:

https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 docs/formatdomain.rst                         |  4 ++++
 docs/schemas/domaincommon.rng                 | 12 ++++++++++
 src/conf/domain_conf.c                        | 23 +++++++++++++++++++
 src/conf/domain_conf.h                        | 10 ++++++++
 src/libvirt_private.syms                      |  1 +
 src/qemu/qemu_virtiofs.c                      |  2 ++
 .../vhost-user-fs-fd-memory.xml               |  1 +
 7 files changed, 53 insertions(+)

-- 
2.30.2

Comments

Peter Krempa March 26, 2021, 3:53 p.m. UTC | #1 
On Fri, Mar 26, 2021 at 11:37:48 -0400, Cole Robinson wrote:
> Add a new XML element

> 

> <filesystem>

>   <binary>

>     <sandbox mode='chroot|namespace'/>

>   </binary>

> </filesystem>

> 

> Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added

> in qemu 5.2.0:

> 

> https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>  docs/formatdomain.rst                         |  4 ++++

>  docs/schemas/domaincommon.rng                 | 12 ++++++++++

>  src/conf/domain_conf.c                        | 23 +++++++++++++++++++

>  src/conf/domain_conf.h                        | 10 ++++++++

>  src/libvirt_private.syms                      |  1 +

>  src/qemu/qemu_virtiofs.c                      |  2 ++

>  .../vhost-user-fs-fd-memory.xml               |  1 +

>  7 files changed, 53 insertions(+)


Please split the commit as it's usual for libvirt patches.

Also a test case modifying any of the .args files in qemuxml2argv test
is missing.

> 

> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst

> index 9392c80113..9dda39dbcb 100644

> --- a/docs/formatdomain.rst

> +++ b/docs/formatdomain.rst

> @@ -3234,6 +3234,7 @@ A directory on the host that can be accessed directly from the guest.

>           <driver type='virtiofs' queue='1024'/>

>           <binary path='/usr/libexec/virtiofsd' xattr='on'>

>              <cache mode='always'/>

> +            <sandbox mode='namespace'/>

>              <lock posix='on' flock='on'/>

>           </binary>

>           <source dir='/path'/>

> @@ -3358,6 +3359,9 @@ A directory on the host that can be accessed directly from the guest.

>     ``cache`` element, possible ``mode`` values being ``none`` and ``always``.

>     Locking can be controlled via the ``lock`` element - attributes ``posix`` and

>     ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )

> +   The sandboxing method used by virtiofsd can be configured with the ``sandbox``

> +   element, possible ``mode`` values being ``namespace`` and

> +   ``chroot``. ( :since:`Since 7.2.0` )


Is there any reasonable short explanation of differences? Or perhaps
link to virtiofs docs to clarify what that the modes do?


>  ``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 1dbfc68f18..6404ebf210 100644

> --- a/docs/schemas/domaincommon.rng

> +++ b/docs/schemas/domaincommon.rng

> @@ -2960,6 +2960,18 @@

>              </optional>

>            </element>

>          </optional>

> +        <optional>

> +          <element name="sandbox">

> +            <optional>

> +              <attribute name="mode">

> +                <choice>

> +                  <value>namespace</value>

> +                  <value>chroot</value>

> +                </choice>

> +              </attribute>

> +            </optional>

> +          </element>

> +        </optional>

>          <optional>

>            <element name="lock">

>              <optional>

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

> index b0eba9f7bd..70a900ee25 100644

> --- a/src/conf/domain_conf.c

> +++ b/src/conf/domain_conf.c

> @@ -538,6 +538,13 @@ VIR_ENUM_IMPL(virDomainFSCacheMode,

>                "always",

>  );

>  

> +VIR_ENUM_IMPL(virDomainFSSandboxMode,

> +              VIR_DOMAIN_FS_SANDBOX_MODE_LAST,

> +              "default",

> +              "namespace",

> +              "chroot",

> +);

> +

>  

>  VIR_ENUM_IMPL(virDomainNet,

>                VIR_DOMAIN_NET_TYPE_LAST,

> @@ -10373,6 +10380,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,

>          g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt);

>          g_autofree char *xattr = virXPathString("string(./binary/@xattr)", ctxt);

>          g_autofree char *cache = virXPathString("string(./binary/cache/@mode)", ctxt);

> +        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);

>          int val;

> @@ -10406,6 +10414,16 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,

>              def->cache = val;

>          }

>  

> +        if (sandbox) {

> +            if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) {

> +                virReportError(VIR_ERR_XML_ERROR,

> +                               _("cannot parse sandbox mode '%s' for virtiofs"),

> +                               sandbox);

> +                goto error;

> +            }

> +            def->sandbox = val;

> +        }

> +

>          if (posix_lock) {

>              if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {

>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

> @@ -25483,6 +25501,11 @@ virDomainFSDefFormat(virBufferPtr buf,

>                                virDomainFSCacheModeTypeToString(def->cache));

>          }

>  

> +        if (def->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {

> +            virBufferAsprintf(&binaryBuf, "<sandbox mode='%s'/>\n",

> +                              virDomainFSSandboxModeTypeToString(def->sandbox));

> +        }

> +

>          if (def->posix_lock != VIR_TRISTATE_SWITCH_ABSENT) {

>              virBufferAsprintf(&lockAttrBuf, " posix='%s'",

>                                virTristateSwitchTypeToString(def->posix_lock));

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

> index 0b8895bbdf..d77b04847b 100644

> --- a/src/conf/domain_conf.h

> +++ b/src/conf/domain_conf.h

> @@ -846,6 +846,14 @@ typedef enum {

>      VIR_DOMAIN_FS_CACHE_MODE_LAST

>  } virDomainFSCacheMode;

>  

> +typedef enum {

> +    VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT = 0,

> +    VIR_DOMAIN_FS_SANDBOX_MODE_NAMESPACE,

> +    VIR_DOMAIN_FS_SANDBOX_MODE_CHROOT,

> +

> +    VIR_DOMAIN_FS_SANDBOX_MODE_LAST

> +} virDomainFSSandboxMode;

> +

>  struct _virDomainFSDef {

>      int type;

>      int fsdriver; /* enum virDomainFSDriverType */

> @@ -870,6 +878,7 @@ struct _virDomainFSDef {

>      virDomainFSCacheMode cache;

>      virTristateSwitch posix_lock;

>      virTristateSwitch flock;

> +    virDomainFSSandboxMode sandbox;


validation check rejecting sandbox modes for 9p fs is missing

>      virDomainVirtioOptionsPtr virtio;

>      virObjectPtr privateData;

>  };
Cole Robinson March 26, 2021, 5:17 p.m. UTC | #2 
On 3/26/21 11:53 AM, Peter Krempa wrote:
> On Fri, Mar 26, 2021 at 11:37:48 -0400, Cole Robinson wrote:

>> Add a new XML element

>>

>> <filesystem>

>>   <binary>

>>     <sandbox mode='chroot|namespace'/>

>>   </binary>

>> </filesystem>

>>

>> Which maps to `virtiofsd -o sandbox=chroot|namespace`, which was added

>> in qemu 5.2.0:

>>

>> https://git.qemu.org/?p=qemu.git;a=commit;h=06844584b62a43384642f7243b0fc01c9fff0fc7

>>

>> Signed-off-by: Cole Robinson <crobinso@redhat.com>

>> ---

>>  docs/formatdomain.rst                         |  4 ++++

>>  docs/schemas/domaincommon.rng                 | 12 ++++++++++

>>  src/conf/domain_conf.c                        | 23 +++++++++++++++++++

>>  src/conf/domain_conf.h                        | 10 ++++++++

>>  src/libvirt_private.syms                      |  1 +

>>  src/qemu/qemu_virtiofs.c                      |  2 ++

>>  .../vhost-user-fs-fd-memory.xml               |  1 +

>>  7 files changed, 53 insertions(+)

> 

> Please split the commit as it's usual for libvirt patches.

> 


Okay, fixed in v2. I addressed the docs and validation piece in v2 too

> Also a test case modifying any of the .args files in qemuxml2argv test

> is missing.

> 


This option affects the virtiofsd command line only, so it won't be
reflected in .args files

Thanks,
Cole
diff mbox series

Patch

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 9392c80113..9dda39dbcb 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3234,6 +3234,7 @@  A directory on the host that can be accessed directly from the guest.
          <driver type='virtiofs' queue='1024'/>
          <binary path='/usr/libexec/virtiofsd' xattr='on'>
             <cache mode='always'/>
+            <sandbox mode='namespace'/>
             <lock posix='on' flock='on'/>
          </binary>
          <source dir='/path'/>
@@ -3358,6 +3359,9 @@  A directory on the host that can be accessed directly from the guest.
    ``cache`` element, possible ``mode`` values being ``none`` and ``always``.
    Locking can be controlled via the ``lock`` element - attributes ``posix`` and
    ``flock`` both accepting values ``on`` or ``off``. ( :since:`Since 6.2.0` )
+   The sandboxing method used by virtiofsd can be configured with the ``sandbox``
+   element, possible ``mode`` values being ``namespace`` and
+   ``chroot``. ( :since:`Since 7.2.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 1dbfc68f18..6404ebf210 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2960,6 +2960,18 @@ 
             </optional>
           </element>
         </optional>
+        <optional>
+          <element name="sandbox">
+            <optional>
+              <attribute name="mode">
+                <choice>
+                  <value>namespace</value>
+                  <value>chroot</value>
+                </choice>
+              </attribute>
+            </optional>
+          </element>
+        </optional>
         <optional>
           <element name="lock">
             <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b0eba9f7bd..70a900ee25 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -538,6 +538,13 @@  VIR_ENUM_IMPL(virDomainFSCacheMode,
               "always",
 );
 
+VIR_ENUM_IMPL(virDomainFSSandboxMode,
+              VIR_DOMAIN_FS_SANDBOX_MODE_LAST,
+              "default",
+              "namespace",
+              "chroot",
+);
+
 
 VIR_ENUM_IMPL(virDomainNet,
               VIR_DOMAIN_NET_TYPE_LAST,
@@ -10373,6 +10380,7 @@  virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
         g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt);
         g_autofree char *xattr = virXPathString("string(./binary/@xattr)", ctxt);
         g_autofree char *cache = virXPathString("string(./binary/cache/@mode)", ctxt);
+        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);
         int val;
@@ -10406,6 +10414,16 @@  virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
             def->cache = val;
         }
 
+        if (sandbox) {
+            if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("cannot parse sandbox mode '%s' for virtiofs"),
+                               sandbox);
+                goto error;
+            }
+            def->sandbox = val;
+        }
+
         if (posix_lock) {
             if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -25483,6 +25501,11 @@  virDomainFSDefFormat(virBufferPtr buf,
                               virDomainFSCacheModeTypeToString(def->cache));
         }
 
+        if (def->sandbox != VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT) {
+            virBufferAsprintf(&binaryBuf, "<sandbox mode='%s'/>\n",
+                              virDomainFSSandboxModeTypeToString(def->sandbox));
+        }
+
         if (def->posix_lock != VIR_TRISTATE_SWITCH_ABSENT) {
             virBufferAsprintf(&lockAttrBuf, " posix='%s'",
                               virTristateSwitchTypeToString(def->posix_lock));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0b8895bbdf..d77b04847b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -846,6 +846,14 @@  typedef enum {
     VIR_DOMAIN_FS_CACHE_MODE_LAST
 } virDomainFSCacheMode;
 
+typedef enum {
+    VIR_DOMAIN_FS_SANDBOX_MODE_DEFAULT = 0,
+    VIR_DOMAIN_FS_SANDBOX_MODE_NAMESPACE,
+    VIR_DOMAIN_FS_SANDBOX_MODE_CHROOT,
+
+    VIR_DOMAIN_FS_SANDBOX_MODE_LAST
+} virDomainFSSandboxMode;
+
 struct _virDomainFSDef {
     int type;
     int fsdriver; /* enum virDomainFSDriverType */
@@ -870,6 +878,7 @@  struct _virDomainFSDef {
     virDomainFSCacheMode cache;
     virTristateSwitch posix_lock;
     virTristateSwitch flock;
+    virDomainFSSandboxMode sandbox;
     virDomainVirtioOptionsPtr virtio;
     virObjectPtr privateData;
 };
@@ -3800,6 +3809,7 @@  VIR_ENUM_DECL(virDomainFSAccessMode);
 VIR_ENUM_DECL(virDomainFSWrpolicy);
 VIR_ENUM_DECL(virDomainFSModel);
 VIR_ENUM_DECL(virDomainFSCacheMode);
+VIR_ENUM_DECL(virDomainFSSandboxMode);
 VIR_ENUM_DECL(virDomainNet);
 VIR_ENUM_DECL(virDomainNetBackend);
 VIR_ENUM_DECL(virDomainNetVirtioTxMode);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cb9fe7c80a..04b2bc9dcd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -414,6 +414,7 @@  virDomainFSDriverTypeToString;
 virDomainFSIndexByName;
 virDomainFSInsert;
 virDomainFSRemove;
+virDomainFSSandboxModeTypeToString;
 virDomainFSTypeFromString;
 virDomainFSTypeToString;
 virDomainFSWrpolicyTypeFromString;
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 2e239cad66..988b757d6f 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -131,6 +131,8 @@  qemuVirtioFSBuildCommandLine(virQEMUDriverConfigPtr cfg,
     virQEMUBuildBufferEscapeComma(&opts, fs->src->path);
     if (fs->cache)
         virBufferAsprintf(&opts, ",cache=%s", virDomainFSCacheModeTypeToString(fs->cache));
+    if (fs->sandbox)
+        virBufferAsprintf(&opts, ",sandbox=%s", virDomainFSSandboxModeTypeToString(fs->sandbox));
 
     if (fs->xattr == VIR_TRISTATE_SWITCH_ON)
         virBufferAddLit(&opts, ",xattr");
diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
index 2277850c2c..abddf0870b 100644
--- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
+++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
@@ -30,6 +30,7 @@ 
       <driver type='virtiofs' queue='1024'/>
       <binary path='/usr/libexec/virtiofsd' xattr='on'>
         <cache mode='always'/>
+        <sandbox mode='chroot'/>
         <lock posix='off' flock='off'/>
       </binary>
       <source dir='/path'/>