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

[Philippe Mathieu-Daudé]

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.hu...@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));
      }