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

[Jonathon Jongsma <jjongsma@redhat.com>]

On 5/15/23 5:10 AM, Stefano Garzarella wrote:
> On Thu, May 11, 2023 at 11:03:22AM -0500, Jonathon Jongsma wrote:
>> On 5/11/23 4:15 AM, Stefano Garzarella wrote:
>>> 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.
>>>
>>> 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:
>>>     v3:
>>>     - use qemu_open() on `path` to simplify libvirt code [Jonathon]
>>
>>
>> Thanks
>>
>> The one drawback now is that it doesn't seem possible for libvirt to 
>> introspect whether or not qemu supports passing an fd to the driver or 
>> not.
> 
> Yep, this was because the libblkio library did not support this new way.
> 
>> When I was writing my initial patch (before I realized that it was 
>> missing fd-passing), I just checked for the existence of the 
>> virtio-blk-vhost-vdpa device. But we actually need to know both that 
>> this device exists and supports fd passing.
> 
> Yep, this was one of the advantages of using the new `fd` parameter.
> Can't libvirt handle the later failure?

Not very well. libvirt tries to provide useful errors to the user. So 
for example if the qemu executable doesn't support a device, we would 
want to provide an error indicating that the device is not supported 
rather than a possibly-inscrutable qemu error.

For example, in this scenario, we would want an error such as:

error: unsupported configuration: vhostvdpa disk is not supported with 
this QEMU binary

Instead of:

error: internal error: qemu unexpectedly closed the monitor: 
2023-05-16T15:17:36.666129Z qemu-system-x86_64: -blockdev 
{"driver":"virtio-blk-vhost-vdpa","path":"/dev/fdset/0","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}: 
blkio_connect failed: Failed to connect to vDPA device: Input/output error

And we can only do that if we can determine that the binary has the 
proper support for fds.

> 
>> As far as I can tell, versions 7.2.0 and 8.0.0 include this device but 
>> won't accept fds.
> 
> Right.
> 
> How do you suggest to proceed?

I need some way to determine that the particular qemu binary can accept 
a /dev/fdset/ path for vdpa block devices. libvirt uses a variety of 
methods to determine capabilities for a given qemu binary, including 
querying the qmp schema, commands, object types, specific device/object 
properties, etc. For example, right now I can determine (via querying 
the qmp schema) whether virtio-blk-vhost-vdpa is a valid type for the 
blockdev-add command by querying the qmp schema. I need something more 
than that but I'm not sure how to do it without introducing a separate 
'fd' parameter. Any ideas?

Thanks,
Jonathon