[PATCH v4 0/2] block/blkio: support fd passing for virtio-blk-vhost-vdpa driver

[PATCH v4 0/2] block/blkio: support fd passing for virtio-blk-vhost-vdpa driver

[Stefano Garzarella]

v4:
- added patch 02 to allow libvirt to discover we support fdset [Markus]
- modified the commit description of patch 01

v3: https://lore.kernel.org/qemu-devel/20230511091527.46620-1-sgarzare@redhat.com/
- use qemu_open() on `path` to simplify libvirt code [Jonathon]
- remove patch 01 since we are not using monitor_fd_param() anymore

v2: https://lore.kernel.org/qemu-devel/20230504092843.62493-1-sgarzare@redhat.com/
- 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):
  block/blkio: use qemu_open() to support fd passing for virtio-blk
  qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

 meson.build          |  4 ++++
 qapi/block-core.json |  8 ++++++-
 block/blkio.c        | 53 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.40.1

[PATCH v4 1/2] block/blkio: use qemu_open() to support fd passing for virtio-blk

[Stefano Garzarella]

Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd
passing. Let's expose this to the user, so the management layer
can pass the file descriptor of an already opened path.

If the libblkio virtio-blk driver supports fd passing, let's always
use qemu_open() to open the `path`, so we can handle fd passing
from the management layer through the "/dev/fdset/N" special path.

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

Notes:
    v4:
    - modified commit description
    
    v3:
    - use qemu_open() on `path` to simplify libvirt code [Jonathon]

 block/blkio.c | 53 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index 0cdc99a729..6a6f20f923 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -672,25 +672,60 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs,
 {
     const char *path = qdict_get_try_str(options, "path");
     BDRVBlkioState *s = bs->opaque;
-    int ret;
+    bool fd_supported = false;
+    int fd, ret;
 
     if (!path) {
         error_setg(errp, "missing 'path' option");
         return -EINVAL;
     }
 
-    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;
-    }
-
     if (!(flags & BDRV_O_NOCACHE)) {
         error_setg(errp, "cache.direct=off is not supported");
         return -EINVAL;
     }
+
+    if (blkio_get_int(s->blkio, "fd", &fd) == 0) {
+        fd_supported = true;
+    }
+
+    /*
+     * If the libblkio driver supports fd passing, let's always use qemu_open()
+     * to open the `path`, so we can handle fd passing from the management
+     * layer through the "/dev/fdset/N" special path.
+     */
+    if (fd_supported) {
+        int open_flags;
+
+        if (flags & BDRV_O_RDWR) {
+            open_flags = O_RDWR;
+        } else {
+            open_flags = O_RDONLY;
+        }
+
+        fd = qemu_open(path, open_flags, errp);
+        if (fd < 0) {
+            return -EINVAL;
+        }
+
+        ret = blkio_set_int(s->blkio, "fd", fd);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set fd: %s",
+                             blkio_get_error_msg());
+            qemu_close(fd);
+            return ret;
+        }
+    } else {
+        ret = blkio_set_str(s->blkio, "path", path);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set path: %s",
+                             blkio_get_error_msg());
+            return ret;
+        }
+    }
+
+    qdict_del(options, "path");
+
     return 0;
 }
 
-- 
2.40.1

[PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Stefano Garzarella]

The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
passing through the new 'fd' property.

Since now we are using qemu_open() on '@path' if the virtio-blk driver
supports the fd passing, let's announce it.
In this way, 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.

Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
in libblkio supports it.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---

Notes:
    v4:
    - added this patch to allow libvirt to discover we support fdset [Markus]

 meson.build          | 4 ++++
 qapi/block-core.json | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 78890f0155..8ea911f7b4 100644
--- a/meson.build
+++ b/meson.build
@@ -2108,6 +2108,10 @@ config_host_data.set('CONFIG_LZO', lzo.found())
 config_host_data.set('CONFIG_MPATH', mpathpersist.found())
 config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
 config_host_data.set('CONFIG_BLKIO', blkio.found())
+if blkio.found()
+  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
+                       blkio.version().version_compare('>=1.3.0'))
+endif
 config_host_data.set('CONFIG_CURL', curl.found())
 config_host_data.set('CONFIG_CURSES', curses.found())
 config_host_data.set('CONFIG_GBM', gbm.found())
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 98d9116dae..1538d84ef4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3955,10 +3955,16 @@
 #
 # @path: path to the vhost-vdpa character device.
 #
+# Features:
+# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)
+#
 # Since: 7.2
 ##
 { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
-  'data': { 'path': 'str' },
+  'data': { 'path': { 'type': 'str',
+                      'features': [ { 'name' :'fdset',
+                                      'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
+            } },
   'if': 'CONFIG_BLKIO' }
 
 ##
-- 
2.40.1

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Jonathon Jongsma]

On 5/26/23 10:03 AM, Stefano Garzarella wrote:
> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
> passing through the new 'fd' property.
> 
> Since now we are using qemu_open() on '@path' if the virtio-blk driver
> supports the fd passing, let's announce it.
> In this way, 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.
> 
> Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
> in libblkio supports it.
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> 
> Notes:
>      v4:
>      - added this patch to allow libvirt to discover we support fdset [Markus]
> 
>   meson.build          | 4 ++++
>   qapi/block-core.json | 8 +++++++-
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 78890f0155..8ea911f7b4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2108,6 +2108,10 @@ config_host_data.set('CONFIG_LZO', lzo.found())
>   config_host_data.set('CONFIG_MPATH', mpathpersist.found())
>   config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
>   config_host_data.set('CONFIG_BLKIO', blkio.found())
> +if blkio.found()
> +  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
> +                       blkio.version().version_compare('>=1.3.0'))
> +endif
>   config_host_data.set('CONFIG_CURL', curl.found())
>   config_host_data.set('CONFIG_CURSES', curses.found())
>   config_host_data.set('CONFIG_GBM', gbm.found())
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98d9116dae..1538d84ef4 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3955,10 +3955,16 @@
>   #
>   # @path: path to the vhost-vdpa character device.
>   #
> +# Features:
> +# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)
> +#
>   # Since: 7.2
>   ##
>   { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
> -  'data': { 'path': 'str' },
> +  'data': { 'path': { 'type': 'str',
> +                      'features': [ { 'name' :'fdset',
> +                                      'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
> +            } },
>     'if': 'CONFIG_BLKIO' }
>   
>   ##


Take this for what it's worth and do what's best for qemu, but... It's 
easier for libvirt if the 'features' are on the object rather than on 
the 'path' member. Our schema parsing code already supports object 
features but does not yet support features on builtin types.

i.e. something like this just works without any refactoring in libvirt:

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1538d84ef4..78cfd10cdb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3961,11 +3961,11 @@
  # Since: 7.2
  ##
  { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
-  'data': { 'path': { 'type': 'str',
-                      'features': [ { 'name' :'fdset',
-                                      'if': 
'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
-            } },
-  'if': 'CONFIG_BLKIO' }
+  'data': { 'path': 'str' },
+  'features': [ { 'name' :'fdset',
+                'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ],
+  'if': 'CONFIG_BLKIO'
+ }

  ##
  # @IscsiTransport:

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> writes:

> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
> passing through the new 'fd' property.
>
> Since now we are using qemu_open() on '@path' if the virtio-blk driver
> supports the fd passing, let's announce it.
> In this way, 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.
>
> Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
> in libblkio supports it.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>
> Notes:
>     v4:
>     - added this patch to allow libvirt to discover we support fdset [Markus]
>
>  meson.build          | 4 ++++
>  qapi/block-core.json | 8 +++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 78890f0155..8ea911f7b4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2108,6 +2108,10 @@ config_host_data.set('CONFIG_LZO', lzo.found())
>  config_host_data.set('CONFIG_MPATH', mpathpersist.found())
>  config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
>  config_host_data.set('CONFIG_BLKIO', blkio.found())
> +if blkio.found()
> +  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
> +                       blkio.version().version_compare('>=1.3.0'))
> +endif
>  config_host_data.set('CONFIG_CURL', curl.found())
>  config_host_data.set('CONFIG_CURSES', curses.found())
>  config_host_data.set('CONFIG_GBM', gbm.found())
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98d9116dae..1538d84ef4 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3955,10 +3955,16 @@
>  #
>  # @path: path to the vhost-vdpa character device.
>  #
> +# Features:
> +# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)

Slightly long line, break it like this:

   # @fdset: Member @path supports the special "/dev/fdset/N" path
   #     since 8.1)

> +#
>  # Since: 7.2
>  ##
>  { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
> -  'data': { 'path': 'str' },
> +  'data': { 'path': { 'type': 'str',
> +                      'features': [ { 'name' :'fdset',
> +                                      'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
> +            } },
>    'if': 'CONFIG_BLKIO' }
>  
>  ##

Tacking the feature to the member works.

For what it's worth, the existing features serving similar purposes are
all tacked to the command or type.

Do libvirt developers have a preference?

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Stefano Garzarella]

On Fri, May 26, 2023 at 04:20:14PM -0500, Jonathon Jongsma wrote:
>On 5/26/23 10:03 AM, Stefano Garzarella wrote:
>>The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
>>passing through the new 'fd' property.
>>
>>Since now we are using qemu_open() on '@path' if the virtio-blk driver
>>supports the fd passing, let's announce it.
>>In this way, 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.
>>
>>Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
>>in libblkio supports it.
>>
>>Suggested-by: Markus Armbruster <armbru@redhat.com>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>
>>Notes:
>>     v4:
>>     - added this patch to allow libvirt to discover we support fdset [Markus]
>>
>>  meson.build          | 4 ++++
>>  qapi/block-core.json | 8 +++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>>diff --git a/meson.build b/meson.build
>>index 78890f0155..8ea911f7b4 100644
>>--- a/meson.build
>>+++ b/meson.build
>>@@ -2108,6 +2108,10 @@ config_host_data.set('CONFIG_LZO', lzo.found())
>>  config_host_data.set('CONFIG_MPATH', mpathpersist.found())
>>  config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
>>  config_host_data.set('CONFIG_BLKIO', blkio.found())
>>+if blkio.found()
>>+  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
>>+                       blkio.version().version_compare('>=1.3.0'))
>>+endif
>>  config_host_data.set('CONFIG_CURL', curl.found())
>>  config_host_data.set('CONFIG_CURSES', curses.found())
>>  config_host_data.set('CONFIG_GBM', gbm.found())
>>diff --git a/qapi/block-core.json b/qapi/block-core.json
>>index 98d9116dae..1538d84ef4 100644
>>--- a/qapi/block-core.json
>>+++ b/qapi/block-core.json
>>@@ -3955,10 +3955,16 @@
>>  #
>>  # @path: path to the vhost-vdpa character device.
>>  #
>>+# Features:
>>+# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)
>>+#
>>  # Since: 7.2
>>  ##
>>  { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
>>-  'data': { 'path': 'str' },
>>+  'data': { 'path': { 'type': 'str',
>>+                      'features': [ { 'name' :'fdset',
>>+                                      'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
>>+            } },
>>    'if': 'CONFIG_BLKIO' }
>>  ##
>
>
>Take this for what it's worth and do what's best for qemu, but... It's 
>easier for libvirt if the 'features' are on the object rather than on 
>the 'path' member. Our schema parsing code already supports object 
>features but does not yet support features on builtin types.

I had done it that way in the first instance :-), then I saw that the 
members themselves could have their own functionality, and it seemed 
better.

However I agree that if it's easier for libvirt, then better to move the 
feature to the whole object.

I'll change that in v5.

Thanks,
Stefano

>
>i.e. something like this just works without any refactoring in libvirt:
>
>diff --git a/qapi/block-core.json b/qapi/block-core.json
>index 1538d84ef4..78cfd10cdb 100644
>--- a/qapi/block-core.json
>+++ b/qapi/block-core.json
>@@ -3961,11 +3961,11 @@
> # Since: 7.2
> ##
> { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
>-  'data': { 'path': { 'type': 'str',
>-                      'features': [ { 'name' :'fdset',
>-                                      'if': 
>'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
>-            } },
>-  'if': 'CONFIG_BLKIO' }
>+  'data': { 'path': 'str' },
>+  'features': [ { 'name' :'fdset',
>+                'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ],
>+  'if': 'CONFIG_BLKIO'
>+ }
>
> ##
> # @IscsiTransport:
>
>

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Stefano Garzarella]

On Sat, May 27, 2023 at 07:56:13AM +0200, Markus Armbruster wrote:
>Stefano Garzarella <sgarzare@redhat.com> writes:
>
>> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
>> passing through the new 'fd' property.
>>
>> Since now we are using qemu_open() on '@path' if the virtio-blk driver
>> supports the fd passing, let's announce it.
>> In this way, 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.
>>
>> Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
>> in libblkio supports it.
>>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>
>> Notes:
>>     v4:
>>     - added this patch to allow libvirt to discover we support fdset [Markus]
>>
>>  meson.build          | 4 ++++
>>  qapi/block-core.json | 8 +++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 78890f0155..8ea911f7b4 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2108,6 +2108,10 @@ config_host_data.set('CONFIG_LZO', lzo.found())
>>  config_host_data.set('CONFIG_MPATH', mpathpersist.found())
>>  config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
>>  config_host_data.set('CONFIG_BLKIO', blkio.found())
>> +if blkio.found()
>> +  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
>> +                       blkio.version().version_compare('>=1.3.0'))
>> +endif
>>  config_host_data.set('CONFIG_CURL', curl.found())
>>  config_host_data.set('CONFIG_CURSES', curses.found())
>>  config_host_data.set('CONFIG_GBM', gbm.found())
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 98d9116dae..1538d84ef4 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -3955,10 +3955,16 @@
>>  #
>>  # @path: path to the vhost-vdpa character device.
>>  #
>> +# Features:
>> +# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)
>
>Slightly long line, break it like this:
>
>   # @fdset: Member @path supports the special "/dev/fdset/N" path
>   #     since 8.1)
>

Sure, I'll fix it!
For the future, what is the maximum column?

>> +#
>>  # Since: 7.2
>>  ##
>>  { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
>> -  'data': { 'path': 'str' },
>> +  'data': { 'path': { 'type': 'str',
>> +                      'features': [ { 'name' :'fdset',
>> +                                      'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ]
>> +            } },
>>    'if': 'CONFIG_BLKIO' }
>>
>>  ##
>
>Tacking the feature to the member works.
>
>For what it's worth, the existing features serving similar purposes are
>all tacked to the command or type.
>
>Do libvirt developers have a preference?
>

Yep, Jonathon pointed out that for libvirt it's better to move it to the
object level, so I'll do that in the next version.

Thanks,
Stefano

Re: [PATCH v4 1/2] block/blkio: use qemu_open() to support fd passing for virtio-blk

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Fri, May 26, 2023 at 05:03:03PM +0200, Stefano Garzarella wrote:
> Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd
> passing. Let's expose this to the user, so the management layer
> can pass the file descriptor of an already opened path.
> 
> If the libblkio virtio-blk driver supports fd passing, let's always
> use qemu_open() to open the `path`, so we can handle fd passing
> from the management layer through the "/dev/fdset/N" special path.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> 
> Notes:
>     v4:
>     - modified commit description
>     
>     v3:
>     - use qemu_open() on `path` to simplify libvirt code [Jonathon]
> 
>  block/blkio.c | 53 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 44 insertions(+), 9 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On Fri, May 26, 2023 at 05:03:04PM +0200, Stefano Garzarella wrote:
> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
> passing through the new 'fd' property.
> 
> Since now we are using qemu_open() on '@path' if the virtio-blk driver
> supports the fd passing, let's announce it.
> In this way, 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.
> 
> Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
> in libblkio supports it.
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> 
> Notes:
>     v4:
>     - added this patch to allow libvirt to discover we support fdset [Markus]
> 
>  meson.build          | 4 ++++
>  qapi/block-core.json | 8 +++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/2] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> writes:

> On Sat, May 27, 2023 at 07:56:13AM +0200, Markus Armbruster wrote:
>>Stefano Garzarella <sgarzare@redhat.com> writes:
>>
>>> The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
>>> passing through the new 'fd' property.
>>>
>>> Since now we are using qemu_open() on '@path' if the virtio-blk driver
>>> supports the fd passing, let's announce it.
>>> In this way, 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.
>>>
>>> Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
>>> in libblkio supports it.
>>>
>>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

[...]

>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index 98d9116dae..1538d84ef4 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -3955,10 +3955,16 @@
>>>  #
>>>  # @path: path to the vhost-vdpa character device.
>>>  #
>>> +# Features:
>>> +# @fdset: Member @path supports the special "/dev/fdset/N" path (since 8.1)
>>
>>Slightly long line, break it like this:
>>
>>   # @fdset: Member @path supports the special "/dev/fdset/N" path
>>   #     since 8.1)
>>
>
> Sure, I'll fix it!
> For the future, what is the maximum column?

70.  Going over is okay if it improves legibility.  However, when I
reformatted the doc comments, I did not need to even once.

[...]