Re: [Qemu-devel] [PATCH] virtio: introduce `info virtio' hmp command

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 26 Sep 2017 19:20:20 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 26.09.2017 um 18:13 hat Jan Dakinevich geschrieben:
> > The command is intended for exposing device specific virtio feature bits
> > and their negotiation status. It is convenient and useful for debug
> > purpose.
> > 
> > Names of features are taken from a devices via get_feature_name() within
> > VirtioDeviceClass. If certain device doesn't implement it, the command
> > will print only hexadecimal value of feature mask.
> > 
> > Cc: Denis V. Lunev <den@virtuozzo.com>
> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com>
> > ---
> > The patch suggests an another approach for this:
> > 
> > "virtio: show guest features in 'info qtree'"
> > http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg01609.html  
> 
> Actually, I think the original approach is much nicer because it uses
> generic infrastructure rather than introducing a one-off monitor command
> that is specific to a certain class of devices.

Probably unsurprisingly, I like this approach much better.

> 
> But at the same time I can see Cornelia's point that this would create a
> whole lot of properties and if we did the same thing consistently for
> all devices, the output of 'info qtree' would be completely cluttered
> up.
> 
> Maybe it would make sense to have properties that can be queried with
> QOM, but that don't show up in 'info qtree'? If we then add some HMP
> 'qom-get' command that allows dumping a whole device as a QOM object
> instead of having to query property by property, it should be quite
> convenient to use.

It's not just that you have to add a property for every feature bit.
They would also need either more state (to cover host offered and guest
accepted bits), or they would need to be duplicated for host and guest
bits. Additionally, we need to consider that feature negotiation is only
final for virtio-1 devices if the feature accepted status has been set
-- that's why I think providing the current status makes sense as well.

I do like your idea of queryable properties and HMP-driven qom-get,
though.