[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp comman
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command |
Date: |
Fri, 15 Dec 2017 15:41:01 +0100 |
On Fri, 15 Dec 2017 00:06:06 +0300
Jan Dakinevich <address@hidden> wrote:
> On Thu, 14 Dec 2017 12:05:19 +0100
> Cornelia Huck <address@hidden> wrote:
>
> > On Tue, 12 Dec 2017 16:54:50 +0300
> > Jan Dakinevich <address@hidden> 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.
>
>
>