Re: [PATCH v3 14/14] vdpa: Add x-svq to NetdevVhostVDPAOptions

[Markus Armbruster]

Eugenio Perez Martin <eperezma@redhat.com> writes:

> On Thu, Mar 3, 2022 at 7:09 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Eugenio Pérez <eperezma@redhat.com> writes:
>>
>> > Finally offering the possibility to enable SVQ from the command line.
>> >
>> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>> > ---
>> >  qapi/net.json    |  5 ++++-
>> >  net/vhost-vdpa.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
>> >  2 files changed, 44 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/qapi/net.json b/qapi/net.json
>> > index 7fab2e7cd8..d243701527 100644
>> > --- a/qapi/net.json
>> > +++ b/qapi/net.json
>> > @@ -445,12 +445,15 @@
>> >  # @queues: number of queues to be created for multiqueue vhost-vdpa
>> >  #          (default: 1)
>> >  #
>> > +# @x-svq: Start device with (experimental) shadow virtqueue. (Since 7.0)
>> > +#
>> >  # Since: 5.1
>> >  ##
>> >  { 'struct': 'NetdevVhostVDPAOptions',
>> >    'data': {
>> >      '*vhostdev':     'str',
>> > -    '*queues':       'int' } }
>> > +    '*queues':       'int',
>> > +    '*x-svqs':        'bool' } }
>>
>> Experimental members *must* be tagged with feature @unstable.  Their
>> name *may* start with 'x-' to help human users, at the cost of renames
>> when the member becomes stable.
>>
>
> Hi Markus,
>
> Thank you very much for the warning. I'll add the unstable feature tag.
>
> If I understood correctly this needs to be done as x-perf at
> BackupCommon struct. Could you confirm to me that it marks only the
> x-perf member as unstable? Without reading the actual comment it might
> seem as if it marks all the whole BackupCommon struct as unstable.
>
> # ...
> # @filter-node-name: the node name that should be assigned to the
> #                    filter driver that the backup job inserts into the graph
> #                    above node specified by @drive. If this option is
> not given,
> #                    a node name is autogenerated. (Since: 4.2)
> #
> # @x-perf: Performance options. (Since 6.0)
> #
> # Features:
> # @unstable: Member @x-perf is experimental.
> #
> # Note: @on-source-error and @on-target-error only affect background
> #       I/O.  If an error occurs during a guest write request, the device's
> #       rerror/werror actions will be used.
> #
> # Since: 4.2
> ##
> { 'struct': 'BackupCommon',
>   'data': { ...
>             '*filter-node-name': 'str',
>             '*x-perf': { 'type': 'BackupPerf',
>                          'features': [ 'unstable' ] } } }

This tacks features to member @x-perf, i.e. they apply just to member
@x-perf.

Features can also be tacked to the struct type, like this:

  { 'struct': 'BackupCommon',
    'data': { ...
              '*filter-node-name': 'str',
              '*x-perf': 'BackupPerf' },
    'features': [ 'unstable' ] }

Now they apply to type BackupCommon as a whole.

BlockdevOptionsFile in block-core.json actually makes use of both ways:

{ 'struct': 'BlockdevOptionsFile',
  'data': { 'filename': 'str',
            '*pr-manager': 'str',
            '*locking': 'OnOffAuto',
            '*aio': 'BlockdevAioOptions',
            '*aio-max-batch': 'int',
            '*drop-cache': {'type': 'bool',
                            'if': 'CONFIG_LINUX'},
            '*x-check-cache-dropped': { 'type': 'bool',
                                        'features': [ 'unstable' ] } },
  'features': [ { 'name': 'dynamic-auto-read-only',
                  'if': 'CONFIG_POSIX' } ] }

Feature @dynamic-auto-read-only applies to the type, and feature
@unstable applies to member @x-check-cache-dropped.

Questions?