[PATCH 29/36] qemu: block: Add support for passing FDs of disk images

Peter Krempa posted 36 patches 3 years, 1 month ago
There is a newer version of this series
[PATCH 29/36] qemu: block: Add support for passing FDs of disk images
Posted by Peter Krempa 3 years, 1 month ago
Prepare the internal data for passing FDs instead of having qemu open
the file internally.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_block.c   | 31 ++++++++++++++++++++++++++++---
 src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 8a6f601b29..a672ad6f54 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -673,22 +673,47 @@ qemuBlockStorageSourceGetSshProps(virStorageSource *src)

 static virJSONValue *
 qemuBlockStorageSourceGetFileProps(virStorageSource *src,
-                                   bool onlytarget)
+                                   bool onlytarget,
+                                   virTristateBool *autoReadOnly,
+                                   virTristateBool *readOnly)
 {
+    const char *path = src->path;
     const char *iomode = NULL;
     const char *prManagerAlias = NULL;
     virJSONValue *ret = NULL;

     if (!onlytarget) {
+        qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+
         if (src->pr)
             prManagerAlias = src->pr->mgralias;

         if (src->iomode != VIR_DOMAIN_DISK_IO_DEFAULT)
             iomode = virDomainDiskIoTypeToString(src->iomode);
+
+        if (srcpriv && srcpriv->fdpass) {
+            path = qemuFDPassGetPath(srcpriv->fdpass);
+
+            /* when passing a FD to qemu via the /dev/fdset mechanism qemu
+             * fetches the appropriate FD from the fdset by checking that it has
+             * the correct accessmode. Now with 'auto-read-only' in effect qemu
+             * wants to use a read-only FD first. If the user didn't pass multiple
+             * FDs the feature will not work regardless, so we'll disable it. */
+            if (src->fdtuple->nfds == 1) {
+                *autoReadOnly = VIR_TRISTATE_BOOL_ABSENT;
+
+                /* now we setup the normal readonly flag. If user requested write
+                 * access honour it */
+                if (src->fdtuple->writable)
+                    *readOnly = VIR_TRISTATE_BOOL_NO;
+                else
+                    *readOnly = virTristateBoolFromBool(src->readonly);
+            }
+        }
     }

     ignore_value(virJSONValueObjectAdd(&ret,
-                                       "s:filename", src->path,
+                                       "s:filename", path,
                                        "S:aio", iomode,
                                        "S:pr-manager", prManagerAlias,
                                        NULL) < 0);
@@ -818,7 +843,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource *src,
             driver = "file";
         }

-        if (!(fileprops = qemuBlockStorageSourceGetFileProps(src, onlytarget)))
+        if (!(fileprops = qemuBlockStorageSourceGetFileProps(src, onlytarget, &aro, &ro)))
             return NULL;
         break;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 06e29ff8ae..ed159bc8b1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2146,6 +2146,25 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommand *cmd,
 }


+static int
+qemuBuildDiskSourceCommandLineFDs(virCommand *cmd,
+                                  virDomainDiskDef *disk)
+{
+    virStorageSource *n;
+
+    for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+        qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(n);
+
+        if (!srcpriv || !srcpriv->fdpass)
+            continue;
+
+        qemuFDPassTransferCommand(srcpriv->fdpass, cmd);
+    }
+
+    return 0;
+}
+
+
 static int
 qemuBuildDiskSourceCommandLine(virCommand *cmd,
                                virDomainDiskDef *disk,
@@ -2163,6 +2182,9 @@ qemuBuildDiskSourceCommandLine(virCommand *cmd,
         if (virStorageSourceIsEmpty(disk->src))
             return 0;

+        if (qemuBuildDiskSourceCommandLineFDs(cmd, disk) < 0)
+            return -1;
+
         if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->src)))
             return -1;

-- 
2.38.1
Re: [PATCH 29/36] qemu: block: Add support for passing FDs of disk images
Posted by Pavel Hrdina 3 years, 1 month ago
On Thu, Jan 05, 2023 at 05:30:18PM +0100, Peter Krempa wrote:
> Prepare the internal data for passing FDs instead of having qemu open
> the file internally.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_block.c   | 31 ++++++++++++++++++++++++++++---
>  src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 8a6f601b29..a672ad6f54 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -673,22 +673,47 @@ qemuBlockStorageSourceGetSshProps(virStorageSource *src)
> 
>  static virJSONValue *
>  qemuBlockStorageSourceGetFileProps(virStorageSource *src,
> -                                   bool onlytarget)
> +                                   bool onlytarget,
> +                                   virTristateBool *autoReadOnly,
> +                                   virTristateBool *readOnly)
>  {
> +    const char *path = src->path;
>      const char *iomode = NULL;
>      const char *prManagerAlias = NULL;
>      virJSONValue *ret = NULL;
> 
>      if (!onlytarget) {
> +        qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> +
>          if (src->pr)
>              prManagerAlias = src->pr->mgralias;
> 
>          if (src->iomode != VIR_DOMAIN_DISK_IO_DEFAULT)
>              iomode = virDomainDiskIoTypeToString(src->iomode);
> +
> +        if (srcpriv && srcpriv->fdpass) {
> +            path = qemuFDPassGetPath(srcpriv->fdpass);
> +
> +            /* when passing a FD to qemu via the /dev/fdset mechanism qemu
> +             * fetches the appropriate FD from the fdset by checking that it has
> +             * the correct accessmode. Now with 'auto-read-only' in effect qemu
> +             * wants to use a read-only FD first. If the user didn't pass multiple
> +             * FDs the feature will not work regardless, so we'll disable it. */

What happens if users passes multiple FDs but none of them are
read-only? Otherwise looks good so if this is not an issue

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Re: [PATCH 29/36] qemu: block: Add support for passing FDs of disk images
Posted by Peter Krempa 3 years, 1 month ago
On Fri, Jan 06, 2023 at 14:33:51 +0100, Pavel Hrdina wrote:
> On Thu, Jan 05, 2023 at 05:30:18PM +0100, Peter Krempa wrote:
> > Prepare the internal data for passing FDs instead of having qemu open
> > the file internally.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/qemu/qemu_block.c   | 31 ++++++++++++++++++++++++++++---
> >  src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
> >  2 files changed, 50 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index 8a6f601b29..a672ad6f54 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -673,22 +673,47 @@ qemuBlockStorageSourceGetSshProps(virStorageSource *src)
> > 
> >  static virJSONValue *
> >  qemuBlockStorageSourceGetFileProps(virStorageSource *src,
> > -                                   bool onlytarget)
> > +                                   bool onlytarget,
> > +                                   virTristateBool *autoReadOnly,
> > +                                   virTristateBool *readOnly)
> >  {
> > +    const char *path = src->path;
> >      const char *iomode = NULL;
> >      const char *prManagerAlias = NULL;
> >      virJSONValue *ret = NULL;
> > 
> >      if (!onlytarget) {
> > +        qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> > +
> >          if (src->pr)
> >              prManagerAlias = src->pr->mgralias;
> > 
> >          if (src->iomode != VIR_DOMAIN_DISK_IO_DEFAULT)
> >              iomode = virDomainDiskIoTypeToString(src->iomode);
> > +
> > +        if (srcpriv && srcpriv->fdpass) {
> > +            path = qemuFDPassGetPath(srcpriv->fdpass);
> > +
> > +            /* when passing a FD to qemu via the /dev/fdset mechanism qemu
> > +             * fetches the appropriate FD from the fdset by checking that it has
> > +             * the correct accessmode. Now with 'auto-read-only' in effect qemu
> > +             * wants to use a read-only FD first. If the user didn't pass multiple
> > +             * FDs the feature will not work regardless, so we'll disable it. */
> 
> What happens if users passes multiple FDs but none of them are
> read-only? Otherwise looks good so if this is not an issue

qemu will complain that it can't find the appropriate one. Given that
this is for very specific use cases I didn't want to overdo it with
validation.