Re: [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 28/12/23 19:52, Hyman Huang wrote:
> Maintain the feature and status bits in the x-query-virtio-status
> output and, as usual, add human-readable output only in HMP.
> 
> Applications may find it useful to compare features and status
> information directly. An upper application, for example, could
> use the QMP command x-query-virtio-status to retrieve vhost-user
> net device features and the "ovs-vsctl list interface" command to
> retrieve interface features (in number format) in order to verify
> the correctness of the virtio negotiation between guest, QEMU,
> and OVS-DPDK. The application could then compare the two features
> directly, without the need for additional feature encoding.
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>   hw/virtio/virtio-hmp-cmds.c |  25 +++--
>   hw/virtio/virtio-qmp.c      |  23 ++---
>   qapi/virtio.json            | 192 ++++--------------------------------
>   3 files changed, 45 insertions(+), 195 deletions(-)
> 
> diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
> index 477c97dea2..721c630ab0 100644
> --- a/hw/virtio/virtio-hmp-cmds.c
> +++ b/hw/virtio/virtio-hmp-cmds.c
> @@ -6,6 +6,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "virtio-qmp.h"
>   #include "monitor/hmp.h"
>   #include "monitor/monitor.h"
>   #include "qapi/qapi-commands-virtio.h"
> @@ -145,13 +146,17 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
>       monitor_printf(mon, "  endianness:              %s\n",
>                      s->device_endian);
>       monitor_printf(mon, "  status:\n");
> -    hmp_virtio_dump_status(mon, s->status);
> +    hmp_virtio_dump_status(mon,
> +        qmp_decode_status(s->status));

Why not let the callee do this call?

>       monitor_printf(mon, "  Guest features:\n");
> -    hmp_virtio_dump_features(mon, s->guest_features);
> +    hmp_virtio_dump_features(mon,
> +        qmp_decode_features(s->device_id, s->guest_features));
>       monitor_printf(mon, "  Host features:\n");
> -    hmp_virtio_dump_features(mon, s->host_features);
> +    hmp_virtio_dump_features(mon,
> +        qmp_decode_features(s->device_id, s->host_features));
>       monitor_printf(mon, "  Backend features:\n");
> -    hmp_virtio_dump_features(mon, s->backend_features);
> +    hmp_virtio_dump_features(mon,
> +        qmp_decode_features(s->device_id, s->backend_features));
>   
>       if (s->vhost_dev) {
>           monitor_printf(mon, "  VHost:\n");
> @@ -172,13 +177,17 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
>           monitor_printf(mon, "    log_size:       %"PRId64"\n",
>                          s->vhost_dev->log_size);
>           monitor_printf(mon, "    Features:\n");
> -        hmp_virtio_dump_features(mon, s->vhost_dev->features);
> +        hmp_virtio_dump_features(mon,
> +            qmp_decode_features(s->device_id, s->vhost_dev->features));

Ditto.

>           monitor_printf(mon, "    Acked features:\n");
> -        hmp_virtio_dump_features(mon, s->vhost_dev->acked_features);
> +        hmp_virtio_dump_features(mon,
> +            qmp_decode_features(s->device_id, s->vhost_dev->acked_features));
>           monitor_printf(mon, "    Backend features:\n");
> -        hmp_virtio_dump_features(mon, s->vhost_dev->backend_features);
> +        hmp_virtio_dump_features(mon,
> +            qmp_decode_features(s->device_id, s->vhost_dev->backend_features));
>           monitor_printf(mon, "    Protocol features:\n");
> -        hmp_virtio_dump_protocols(mon, s->vhost_dev->protocol_features);
> +        hmp_virtio_dump_protocols(mon,
> +            qmp_decode_protocols(s->vhost_dev->protocol_features));
>       }