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

[Jonathon Jongsma <jjongsma@redhat.com>]

On 5/25/23 1:30 PM, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
>> On Wed, May 17, 2023 at 09:19:26AM +0200, Stefano Garzarella wrote:
>>> CCing Markus for some advice.
>>>
>>> On Tue, May 16, 2023 at 11:04:21AM -0500, Jonathon Jongsma wrote:
> 
> [...]
> 
>>>> 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?
>>>
>>> The only thing I can think of is to make a mix between v2 and v3. I mean add
>>> both the new `fd` parameter, and support qemu_open() on `path`.
>>>
>>> That way libvirt (or other users) can check that fd passing is supported and
>>> use `fd` or fdset with `path`.
>>>
>>> Obviously I would have liked to implement only one of the two methods, but
>>> if this helps, maybe it makes sense to support both.
>>>
>>> What do you think?
>>
>> Markus: Is a preferred way to make this new path handling behavior
>> introspectable? I vaguely remember a way for QMP clients to query
>> strings that describe QMP behavior that's not otherwise
>> introspectable...
> 
> Let me try to answer this without first reading the entire thread.
> 
> QMP introspection lets you find out things like whether a command is
> there, or whether an an argument is there, and what its type is.
> Suffices most of the time.
> 
> However, behavior can certainly change while the introspection data
> remains the same.  When a management application needs to know about the
> change, we better expose the change in introspection somehow.
> 
> The "obvious" way to do that would be some arbitrary change that *is*
> visible in introspection.  Meh.
> 
> The modern way is to add a suitable "feature".
> docs/devel/qapi-code-gen.rst:
> 
>      Features
>      --------
> 
>      Syntax::
> 
>          FEATURES = [ FEATURE, ... ]
>          FEATURE = STRING
>                  | { 'name': STRING, '*if': COND }
> 
>      Sometimes, the behaviour of QEMU changes compatibly, but without a
>      change in the QMP syntax (usually by allowing values or operations
>      that previously resulted in an error).  QMP clients may still need to
>      know whether the extension is available.
> 
>      For this purpose, a list of features can be specified for definitions,
>      enumeration values, and struct members.  Each feature list member can
>      either be ``{ 'name': STRING, '*if': COND }``, or STRING, which is
>      shorthand for ``{ 'name': STRING }``.
> 
>      The optional 'if' member specifies a conditional.  See `Configuring
>      the schema`_ below for more on this.
> 
>      Example::
> 
>       { 'struct': 'TestType',
>         'data': { 'number': 'int' },
>         'features': [ 'allow-negative-numbers' ] }
> 
>      The feature strings are exposed to clients in introspection, as
>      explained in section `Client JSON Protocol introspection`_.
> 
>      Intended use is to have each feature string signal that this build of
>      QEMU shows a certain behaviour.
> 
> For a real example, see commit c6bdc312f30 (qapi: Add
> '@allow-write-only-overlay' feature for 'blockdev-snapshot').
> 
> Does this answer your question?
> 

That sounds perfect for libvirt's needs.

Jonathon