Previously, it was suggested to use QAPI unions/enums to declare feature 
bits. Some of weak parts of my code (such `get_feature_name' callback) 
went away. But a lot of of dummy code with feature declaration is still 
here, it just migrated to json file.

So, I would be glad to get a responce if my understanging of using QAPI
structures was correct, or there was another way to do that cleaner?

v5:
 Use QAPI enums/unions to declare virtio feature bits

v4: http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg00393.html
v3: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07565.html
v2: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07527.html
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html

Jan Dakinevich (2):
  virtio: introduce `query-virtio' QMP command
  virtio: add `info virtio' HMP command

 hmp-commands-info.hx    |  14 ++
 hmp.c                   | 122 +++++++++++++
 hmp.h                   |   1 +
 hw/virtio/Makefile.objs |   3 +
 hw/virtio/virtio-qmp.c  | 223 +++++++++++++++++++++++
 hw/virtio/virtio-stub.c |   9 +
 qapi-schema.json        | 466 ++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 838 insertions(+)
 create mode 100644 hw/virtio/virtio-qmp.c
 create mode 100644 hw/virtio/virtio-stub.c

-- 
2.1.4

I am going to reanimate works under this QMP/HMP. First of all, it
could be meaningful to settle what output would provide the QMP. I would
like to suggest the following description:

##
# @VirtioFeature:
##
{
    'struct': 'VirtioFeature',
    'data': {
        'name': 'str',
        'acked': 'bool'
    }
}

##
# @VirtioInfo:
##
{
    'struct': 'VirtioInfo',
    'data': {
        'qom-path': 'str',
        
        'status': 'uint8',
        'host-features': 'uint64',
        'guest-features': 'uint64',

        'status-names': ['str'],
        'common-features-names': ['VirtioFeature'],
        'device-features-names': ['VirtioFeature']
    }
}

##
# @query-virtio:
##
{
    'command': 'query-virtio',
    'data': {'*path': 'str'},
    'returns': ['VirtioInfo']
}

My final goal is to implement HMP which will print all exposed virtio
features (both common and device-specific) with their acknowledgements,
and virtio device configuration status. These are provided by last 3
fields in @VirtioInfo.

For these who are going make own decision on features and status
bitmask, respective fields with raw values are preserved.

So, I expect following data on the wire in response to `query-virtio'
command:

{
    "return": [
        {
            "qom-path": "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15,
            "host-features": 6325010438,
            "guest-features": 5100273670,
            "status-names": [
                "acknowledge", 
                "driver", 
                "driver-ok", 
                "features-ok"
            ],
            "common-features-names": [
                {"name": "notify-on-empty", "acked": false},
                {"name": "any-layout", "acked": false},
                {"name": "indirect-desc", "acked": true},
                {"name": "event-idx", "acked": true},
                {"name": "bad-feature", "acked": false},
                {"name": "version-1", "acked": true}
            ],
            "device-features-names": [
                {"name": "hotplug", "acked": true},
                {"name": "change", "acked": true}
            ]
        }
    ]
}


Eric Blake, returning to your question which would probably appear again
after this mail:

>> +##
>> +# @query-virtio:
>> ...
>> +##
>> +{
>> +    'command': 'query-virtio',
>> +    'data': { '*path': 'str' },  
>
> Do we need filterable queries, or is it better to just have the
> command return info on all virtio devices at once and let the client
> filter the results as desired?

I think it would be better to do here. I suppose, the client which uses
HMP will not be happy on filtering monitor output.

-- 
Best regards
Jan Dakinevich

On Tue, 12 Dec 2017 16:54:50 +0300
Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote:

> I am going to reanimate works under this QMP/HMP. First of all, it
> could be meaningful to settle what output would provide the QMP. I would
> like to suggest the following description:
> 
> ##
> # @VirtioFeature:
> ##
> {
>     'struct': 'VirtioFeature',
>     'data': {
>         'name': 'str',
>         'acked': 'bool'
>     }
> }
> 
> ##
> # @VirtioInfo:
> ##
> {
>     'struct': 'VirtioInfo',
>     'data': {
>         'qom-path': 'str',
>         
>         'status': 'uint8',
>         'host-features': 'uint64',
>         'guest-features': 'uint64',
> 
>         'status-names': ['str'],
>         'common-features-names': ['VirtioFeature'],
>         'device-features-names': ['VirtioFeature']
>     }
> }
> 
> ##
> # @query-virtio:
> ##
> {
>     'command': 'query-virtio',
>     'data': {'*path': 'str'},
>     'returns': ['VirtioInfo']
> }
> 
> My final goal is to implement HMP which will print all exposed virtio
> features (both common and device-specific) with their acknowledgements,
> and virtio device configuration status. These are provided by last 3
> fields in @VirtioInfo.
> 
> For these who are going make own decision on features and status
> bitmask, respective fields with raw values are preserved.

Looks sensible. What will you return an for the *-features-names fields
if the status field indicates that negotiation is not yet done?
(This has some fun interaction with the VERSION_1 feature bit...)

> 
> So, I expect following data on the wire in response to `query-virtio'
> command:
> 
> {
>     "return": [
>         {
>             "qom-path": "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15,
>             "host-features": 6325010438,
>             "guest-features": 5100273670,
>             "status-names": [
>                 "acknowledge", 
>                 "driver", 
>                 "driver-ok", 
>                 "features-ok"
>             ],
>             "common-features-names": [
>                 {"name": "notify-on-empty", "acked": false},
>                 {"name": "any-layout", "acked": false},
>                 {"name": "indirect-desc", "acked": true},
>                 {"name": "event-idx", "acked": true},
>                 {"name": "bad-feature", "acked": false},
>                 {"name": "version-1", "acked": true}
>             ],
>             "device-features-names": [
>                 {"name": "hotplug", "acked": true},
>                 {"name": "change", "acked": true}

I suggest to use the #defines as names, especially as they are also
used in the spec. Makes grepping easier.

>             ]
>         }
>     ]
> }
> 
> 
> Eric Blake, returning to your question which would probably appear again
> after this mail:
> 
> >> +##
> >> +# @query-virtio:
> >> ...
> >> +##
> >> +{
> >> +    'command': 'query-virtio',
> >> +    'data': { '*path': 'str' },    
> >
> > Do we need filterable queries, or is it better to just have the
> > command return info on all virtio devices at once and let the client
> > filter the results as desired?  
> 
> I think it would be better to do here. I suppose, the client which uses
> HMP will not be happy on filtering monitor output.

I think being able to optionally specify a path is the most flexible
solution.

On Thu, 14 Dec 2017 12:05:19 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 12 Dec 2017 16:54:50 +0300
> Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote:
> 
> > I am going to reanimate works under this QMP/HMP. First of all, it
> > could be meaningful to settle what output would provide the QMP. I
> > would like to suggest the following description:
> > 
> > ##
> > # @VirtioFeature:
> > ##
> > {
> >     'struct': 'VirtioFeature',
> >     'data': {
> >         'name': 'str',
> >         'acked': 'bool'
> >     }
> > }
> > 
> > ##
> > # @VirtioInfo:
> > ##
> > {
> >     'struct': 'VirtioInfo',
> >     'data': {
> >         'qom-path': 'str',
> >         
> >         'status': 'uint8',
> >         'host-features': 'uint64',
> >         'guest-features': 'uint64',
> > 
> >         'status-names': ['str'],
> >         'common-features-names': ['VirtioFeature'],
> >         'device-features-names': ['VirtioFeature']
> >     }
> > }
> > 
> > ##
> > # @query-virtio:
> > ##
> > {
> >     'command': 'query-virtio',
> >     'data': {'*path': 'str'},
> >     'returns': ['VirtioInfo']
> > }
> > 
> > My final goal is to implement HMP which will print all exposed
> > virtio features (both common and device-specific) with their
> > acknowledgements, and virtio device configuration status. These are
> > provided by last 3 fields in @VirtioInfo.
> > 
> > For these who are going make own decision on features and status
> > bitmask, respective fields with raw values are preserved.
> 
> Looks sensible. What will you return an for the *-features-names
> fields if the status field indicates that negotiation is not yet done?
> (This has some fun interaction with the VERSION_1 feature bit...)
> 

Hmm... I was going to return current features and theirs
acknowledgments regardless if they were negotiated. Thus,
*-features-names would contain all exposed host features with `false'
in `acked' field.

> > 
> > So, I expect following data on the wire in response to
> > `query-virtio' command:
> > 
> > {
> >     "return": [
> >         {
> >             "qom-path":
> > "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15,
> > "host-features": 6325010438, "guest-features": 5100273670,
> >             "status-names": [
> >                 "acknowledge", 
> >                 "driver", 
> >                 "driver-ok", 
> >                 "features-ok"
> >             ],
> >             "common-features-names": [
> >                 {"name": "notify-on-empty", "acked": false},
> >                 {"name": "any-layout", "acked": false},
> >                 {"name": "indirect-desc", "acked": true},
> >                 {"name": "event-idx", "acked": true},
> >                 {"name": "bad-feature", "acked": false},
> >                 {"name": "version-1", "acked": true}
> >             ],
> >             "device-features-names": [
> >                 {"name": "hotplug", "acked": true},
> >                 {"name": "change", "acked": true}
> 
> I suggest to use the #defines as names, especially as they are also
> used in the spec. Makes grepping easier.
> 

You mean, for example "VIRTIO_RING_F_EVENT_IDX" instead of "event-idx"
should be used. Right?

> >             ]
> >         }
> >     ]
> > }
> > 
> > 
> > Eric Blake, returning to your question which would probably appear
> > again after this mail:
> > 
> > >> +##
> > >> +# @query-virtio:
> > >> ...
> > >> +##
> > >> +{
> > >> +    'command': 'query-virtio',
> > >> +    'data': { '*path': 'str' },    
> > >
> > > Do we need filterable queries, or is it better to just have the
> > > command return info on all virtio devices at once and let the
> > > client filter the results as desired?  
> > 
> > I think it would be better to do here. I suppose, the client which
> > uses HMP will not be happy on filtering monitor output.
> 
> I think being able to optionally specify a path is the most flexible
> solution.



-- 
Best regards
Jan Dakinevich

On Fri, 15 Dec 2017 00:06:06 +0300
Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote:

> On Thu, 14 Dec 2017 12:05:19 +0100
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > On Tue, 12 Dec 2017 16:54:50 +0300
> > Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote:
> >   
> > > I am going to reanimate works under this QMP/HMP. First of all, it
> > > could be meaningful to settle what output would provide the QMP. I
> > > would like to suggest the following description:
> > > 
> > > ##
> > > # @VirtioFeature:
> > > ##
> > > {
> > >     'struct': 'VirtioFeature',
> > >     'data': {
> > >         'name': 'str',
> > >         'acked': 'bool'
> > >     }
> > > }
> > > 
> > > ##
> > > # @VirtioInfo:
> > > ##
> > > {
> > >     'struct': 'VirtioInfo',
> > >     'data': {
> > >         'qom-path': 'str',
> > >         
> > >         'status': 'uint8',
> > >         'host-features': 'uint64',
> > >         'guest-features': 'uint64',
> > > 
> > >         'status-names': ['str'],
> > >         'common-features-names': ['VirtioFeature'],
> > >         'device-features-names': ['VirtioFeature']
> > >     }
> > > }
> > > 
> > > ##
> > > # @query-virtio:
> > > ##
> > > {
> > >     'command': 'query-virtio',
> > >     'data': {'*path': 'str'},
> > >     'returns': ['VirtioInfo']
> > > }
> > > 
> > > My final goal is to implement HMP which will print all exposed
> > > virtio features (both common and device-specific) with their
> > > acknowledgements, and virtio device configuration status. These are
> > > provided by last 3 fields in @VirtioInfo.
> > > 
> > > For these who are going make own decision on features and status
> > > bitmask, respective fields with raw values are preserved.  
> > 
> > Looks sensible. What will you return an for the *-features-names
> > fields if the status field indicates that negotiation is not yet done?
> > (This has some fun interaction with the VERSION_1 feature bit...)
> >   
> 
> Hmm... I was going to return current features and theirs
> acknowledgments regardless if they were negotiated. Thus,
> *-features-names would contain all exposed host features with `false'
> in `acked' field.

acked=false is probably the most sensible approach.

> 
> > > 
> > > So, I expect following data on the wire in response to
> > > `query-virtio' command:
> > > 
> > > {
> > >     "return": [
> > >         {
> > >             "qom-path":
> > > "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15,
> > > "host-features": 6325010438, "guest-features": 5100273670,
> > >             "status-names": [
> > >                 "acknowledge", 
> > >                 "driver", 
> > >                 "driver-ok", 
> > >                 "features-ok"
> > >             ],
> > >             "common-features-names": [
> > >                 {"name": "notify-on-empty", "acked": false},
> > >                 {"name": "any-layout", "acked": false},
> > >                 {"name": "indirect-desc", "acked": true},
> > >                 {"name": "event-idx", "acked": true},
> > >                 {"name": "bad-feature", "acked": false},
> > >                 {"name": "version-1", "acked": true}
> > >             ],
> > >             "device-features-names": [
> > >                 {"name": "hotplug", "acked": true},
> > >                 {"name": "change", "acked": true}  
> > 
> > I suggest to use the #defines as names, especially as they are also
> > used in the spec. Makes grepping easier.
> >   
> 
> You mean, for example "VIRTIO_RING_F_EVENT_IDX" instead of "event-idx"
> should be used. Right?

Right, that's what I meant.

> 
> > >             ]
> > >         }
> > >     ]
> > > }
> > > 
> > > 
> > > Eric Blake, returning to your question which would probably appear
> > > again after this mail:
> > >   
> > > >> +##
> > > >> +# @query-virtio:
> > > >> ...
> > > >> +##
> > > >> +{
> > > >> +    'command': 'query-virtio',
> > > >> +    'data': { '*path': 'str' },      
> > > >
> > > > Do we need filterable queries, or is it better to just have the
> > > > command return info on all virtio devices at once and let the
> > > > client filter the results as desired?    
> > > 
> > > I think it would be better to do here. I suppose, the client which
> > > uses HMP will not be happy on filtering monitor output.  
> > 
> > I think being able to optionally specify a path is the most flexible
> > solution.  
> 
> 
>