Re: [Qemu-devel] [RFC v6 1/2] virtio: introduce `query-virtio' QMP command

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 20 Dec 2017 11:16:46 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes:
> 
> > On Tue, 19 Dec 2017 15:57:18 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:

> >> Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes:

> Taking a step back: what's the intended purpose of query-virtio?  The
> cover letter doesn't really say:
> 
>     After some discussion, I am going to suggest reworked QMP/HMP for
>     gathering virtio info. It would provide the following monitor output.
> 
>     (qemu) info virtio
>     virtio-blk-device at 0000:00:02.0
>       QOM path: /machine/peripheral-anon/device[0]/virtio-backend
>       status: 0x0f
>         VIRTIO_CONFIG_S_ACKNOWLEDGE
>         VIRTIO_CONFIG_S_DRIVER
>         VIRTIO_CONFIG_S_DRIVER_OK
>         VIRTIO_CONFIG_S_FEATURES_OK
>       host features:  0x0000000179000e54
>       guest features: 0x0000000130000e54
>       common features:
>         VIRTIO_F_NOTIFY_ON_EMPTY
>         VIRTIO_F_ANY_LAYOUT
>         VIRTIO_RING_F_INDIRECT_DESC              acked
>         VIRTIO_RING_F_EVENT_IDX                  acked
>         VIRTIO_F_BAD_FEATURE
>         VIRTIO_F_VERSION_1                       acked
>       device features:
>         VIRTIO_BLK_F_SEG_MAX                     acked
>         VIRTIO_BLK_F_BLK_SIZE                    acked
>         VIRTIO_BLK_F_FLUSH                       acked
>         VIRTIO_BLK_F_TOPOLOGY                    acked
> 
> The first sentence carries no useful information; suggest to scratch it.
> The rest is an HMP example.  Examples are great, but we really need to
> understand *why* you need this command.

From my side, I see at least two use cases:

- Monitoring the state of virtio devices. If I can see from outside
  that VIRTIO_CONFIG_S_FAILED or VIRTIO_CONFIG_S_NEEDS_RESET has been
  set for a device, I know that something went wrong for that device.
- Debugging. This command gives me a nice way to find out why something
  is not working as it is supposed to, or to verify that something
  indeed does work as intended (I would have found such a command quite
  useful when implementing virtio-1).

> 
> >> In case symbolic names may not be known at QEMU compile time (me
> >> having to wonder at v6 is a sign of rather ineffective patch review,
> >> sorry about that): you have to explain this in the doc comment, with a
> >> reference to where the bits are specified.
> >>   
> >
> > ok  
> 
> Can you think of a scenario where status or feature bits occur that are
> not known to QEMU at compile time?

Could happen for status bits (although very unlikely). For feature
bits, the guest is supposed to ack a subset of the offered bits, so
this would mean a fundamentally broken guest (also unlikely, but not
impossible).

> 
> >> > +#
> >> > +# @status-names: names of checked bits in status bitmask  
> >> 
> >> How are the strings in @status-names connected to the bits in @status?
> >> Spell it out in this doc string, please.
> >>   
> >
> > ok
> >
> > ...but, strictly saying, I was not going to show in QAPI how these names
> > are mapped into bits, QMP answer would not contain how these names are
> > connected to bits. Otherwise, It would be reinvention of v4.  
> 
> To avoid going in circles some more, let's take a step back and examine
> what exactly you're trying to accomplish.  Give us use cases: this is my
> need, and this is how you use the proposed command to satisfy my need.
> Also give us design limitations: what the command is *not* trying to do.