[PATCH v2 0/2] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[PATCH v2 0/2] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[Stefano Garzarella]

v2:
- added patch 01 to use monitor_fd_param() in the blkio module
- use monitor_fd_param() to parse the fd like vhost devices [Stefan]

v1: https://lore.kernel.org/qemu-devel/20230502145050.224615-1-sgarzare@redhat.com/

The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the new
'fd' property. Let's expose this to the user, so the management layer
can pass the file descriptor of an already opened vhost-vdpa character
device. This is useful especially when the device can only be accessed
with certain privileges.

Stefano Garzarella (2):
  stubs/monitor: add monitor_fd_param()
  block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

 qapi/block-core.json |  6 ++++-
 block/blkio.c        | 53 +++++++++++++++++++++++++++++++++++++++++++-
 stubs/monitor.c      |  6 +++++
 3 files changed, 63 insertions(+), 2 deletions(-)

-- 
2.40.1

[PATCH v2 1/2] stubs/monitor: add monitor_fd_param()

[Stefano Garzarella]

The blkio block driver will use monitor_fd_param() to support
file descriptor passing. This is possible in builds (e.g. softmmu)
where the monitor API is available.

Add the monitor_fd_param() stub so tools and tests that link the
block layer can build successfully.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---

Notes:
    v2:
    - added this patch to use monitor_fd_param() in the blkio module

 stubs/monitor.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/stubs/monitor.c b/stubs/monitor.c
index 20786ac4ff..0bcd49e41e 100644
--- a/stubs/monitor.c
+++ b/stubs/monitor.c
@@ -9,6 +9,12 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
     return -1;
 }
 
+int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp)
+{
+    error_setg(errp, "only QEMU supports file descriptor passing");
+    return -1;
+}
+
 void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
 {
 }
-- 
2.40.1

[PATCH v2 2/2] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[Stefano Garzarella]

The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the new
'fd' property. Let's expose this to the user, so the management layer
can pass the file descriptor of an already opened vhost-vdpa character
device. This is useful especially when the device can only be accessed
with certain privileges.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---

Notes:
    v2:
    - used monitor_fd_param() to parse the fd like vhost devices [Stefan]

 qapi/block-core.json |  6 ++++-
 block/blkio.c        | 53 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index b57978957f..9f70777d49 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3841,10 +3841,14 @@
 #
 # @path: path to the vhost-vdpa character device.
 #
+# @fd: file descriptor of an already opened vhost-vdpa character device.
+#      (Since 8.1)
+#
 # Since: 7.2
 ##
 { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
-  'data': { 'path': 'str' },
+  'data': { '*path': 'str',
+            '*fd': 'str' },
   'if': 'CONFIG_BLKIO' }
 
 ##
diff --git a/block/blkio.c b/block/blkio.c
index 0cdc99a729..66b3dd2cd0 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -18,6 +18,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qemu/module.h"
 #include "exec/memory.h" /* for ram_block_discard_disable() */
+#include "monitor/monitor.h"
 
 #include "block/block-io.h"
 
@@ -694,6 +695,56 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs,
     return 0;
 }
 
+static int blkio_virtio_blk_vhost_vdpa_open(BlockDriverState *bs,
+        QDict *options, int flags, Error **errp)
+{
+    const char *path = qdict_get_try_str(options, "path");
+    const char *fd_str = qdict_get_try_str(options, "fd");
+    BDRVBlkioState *s = bs->opaque;
+    int ret;
+
+    if (path && fd_str) {
+        error_setg(errp, "'path' and 'fd' options are mutually exclusive");
+        return -EINVAL;
+    }
+
+    if (!path && !fd_str) {
+        error_setg(errp, "none of 'path' or 'fd' options was specified");
+        return -EINVAL;
+    }
+
+    if (path) {
+        ret = blkio_set_str(s->blkio, "path", path);
+        qdict_del(options, "path");
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set path: %s",
+                             blkio_get_error_msg());
+            return ret;
+        }
+    } else {
+        int fd = monitor_fd_param(monitor_cur(), fd_str, errp);
+
+        if (fd < 0) {
+            error_prepend(errp, "unable to parse 'fd' option: ");
+            return -EINVAL;
+        }
+
+        ret = blkio_set_int(s->blkio, "fd", fd);
+        qdict_del(options, "fd");
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set fd: %s",
+                             blkio_get_error_msg());
+            return ret;
+        }
+    }
+
+    if (!(flags & BDRV_O_NOCACHE)) {
+        error_setg(errp, "cache.direct=off is not supported");
+        return -EINVAL;
+    }
+    return 0;
+}
+
 static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,
                            Error **errp)
 {
@@ -717,7 +768,7 @@ static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,
     } else if (strcmp(blkio_driver, DRIVER_VIRTIO_BLK_VHOST_USER) == 0) {
         ret = blkio_virtio_blk_common_open(bs, options, flags, errp);
     } else if (strcmp(blkio_driver, DRIVER_VIRTIO_BLK_VHOST_VDPA) == 0) {
-        ret = blkio_virtio_blk_common_open(bs, options, flags, errp);
+        ret = blkio_virtio_blk_vhost_vdpa_open(bs, options, flags, errp);
     } else {
         g_assert_not_reached();
     }
-- 
2.40.1

Re: [PATCH v2 0/2] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[Jonathon Jongsma]

On 5/4/23 4:28 AM, Stefano Garzarella wrote:
> v2:
> - added patch 01 to use monitor_fd_param() in the blkio module
> - use monitor_fd_param() to parse the fd like vhost devices [Stefan]
> 
> v1: https://lore.kernel.org/qemu-devel/20230502145050.224615-1-sgarzare@redhat.com/
> 
> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the new
> 'fd' property. Let's expose this to the user, so the management layer
> can pass the file descriptor of an already opened vhost-vdpa character
> device. This is useful especially when the device can only be accessed
> with certain privileges.
> 
> Stefano Garzarella (2):
>    stubs/monitor: add monitor_fd_param()
>    block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver
> 
>   qapi/block-core.json |  6 ++++-
>   block/blkio.c        | 53 +++++++++++++++++++++++++++++++++++++++++++-
>   stubs/monitor.c      |  6 +++++
>   3 files changed, 63 insertions(+), 2 deletions(-)
> 


I mentioned this briefly off-list, but I'm following up here just to 
provide a bit more visibility. From libvirt's point of view, it is 
actually much easier if we could pass the fd via the existing 'path' 
parameter (using /dev/fdset/N as the path, for example) due to how we 
construct the commandline for qemu. So if I get a vote, I would 
definitely vote for that approach over adding a new 'fd' property.

Thanks,
Jonathon

Re: [PATCH v2 0/2] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[Stefano Garzarella]

On Wed, May 10, 2023 at 6:03 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
>
> On 5/4/23 4:28 AM, Stefano Garzarella wrote:
> > v2:
> > - added patch 01 to use monitor_fd_param() in the blkio module
> > - use monitor_fd_param() to parse the fd like vhost devices [Stefan]
> >
> > v1: https://lore.kernel.org/qemu-devel/20230502145050.224615-1-sgarzare@redhat.com/
> >
> > The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the new
> > 'fd' property. Let's expose this to the user, so the management layer
> > can pass the file descriptor of an already opened vhost-vdpa character
> > device. This is useful especially when the device can only be accessed
> > with certain privileges.
> >
> > Stefano Garzarella (2):
> >    stubs/monitor: add monitor_fd_param()
> >    block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver
> >
> >   qapi/block-core.json |  6 ++++-
> >   block/blkio.c        | 53 +++++++++++++++++++++++++++++++++++++++++++-
> >   stubs/monitor.c      |  6 +++++
> >   3 files changed, 63 insertions(+), 2 deletions(-)
> >
>
>
> I mentioned this briefly off-list, but I'm following up here just to
> provide a bit more visibility. From libvirt's point of view, it is
> actually much easier if we could pass the fd via the existing 'path'
> parameter (using /dev/fdset/N as the path, for example) due to how we
> construct the commandline for qemu. So if I get a vote, I would
> definitely vote for that approach over adding a new 'fd' property.

Thanks for pointing that out.
Since libvirt will be the main user of this feature, I think it is
fine to support fd passing via `path`, using qemu_open().

I'll send a v3 with that change.

Thanks,
Stefano