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

Hyman Huang posted 2 patches 11 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
Posted by Hyman Huang 11 months ago
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));
     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));
         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));
     }
 
     qapi_free_VirtioStatus(s);
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 1dd96ed20f..1660c17653 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -733,12 +733,9 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
     status->name = g_strdup(vdev->name);
     status->device_id = vdev->device_id;
     status->vhost_started = vdev->vhost_started;
-    status->guest_features = qmp_decode_features(vdev->device_id,
-                                                 vdev->guest_features);
-    status->host_features = qmp_decode_features(vdev->device_id,
-                                                vdev->host_features);
-    status->backend_features = qmp_decode_features(vdev->device_id,
-                                                   vdev->backend_features);
+    status->guest_features = vdev->guest_features;
+    status->host_features = vdev->host_features;
+    status->backend_features = vdev->backend_features;
 
     switch (vdev->device_endian) {
     case VIRTIO_DEVICE_ENDIAN_LITTLE:
@@ -753,7 +750,7 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
     }
 
     status->num_vqs = virtio_get_num_queues(vdev);
-    status->status = qmp_decode_status(vdev->status);
+    status->status = vdev->status;
     status->isr = vdev->isr;
     status->queue_sel = vdev->queue_sel;
     status->vm_running = vdev->vm_running;
@@ -775,14 +772,10 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
         status->vhost_dev->n_tmp_sections = hdev->n_tmp_sections;
         status->vhost_dev->nvqs = hdev->nvqs;
         status->vhost_dev->vq_index = hdev->vq_index;
-        status->vhost_dev->features =
-            qmp_decode_features(vdev->device_id, hdev->features);
-        status->vhost_dev->acked_features =
-            qmp_decode_features(vdev->device_id, hdev->acked_features);
-        status->vhost_dev->backend_features =
-            qmp_decode_features(vdev->device_id, hdev->backend_features);
-        status->vhost_dev->protocol_features =
-            qmp_decode_protocols(hdev->protocol_features);
+        status->vhost_dev->features = hdev->features;
+        status->vhost_dev->acked_features = hdev->acked_features;
+        status->vhost_dev->backend_features = hdev->backend_features;
+        status->vhost_dev->protocol_features = hdev->protocol_features;
         status->vhost_dev->max_queues = hdev->max_queues;
         status->vhost_dev->backend_cap = hdev->backend_cap;
         status->vhost_dev->log_enabled = hdev->log_enabled;
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 19c7c36e36..26516fb29c 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -102,10 +102,10 @@
             'n-tmp-sections': 'int',
             'nvqs': 'uint32',
             'vq-index': 'int',
-            'features': 'VirtioDeviceFeatures',
-            'acked-features': 'VirtioDeviceFeatures',
-            'backend-features': 'VirtioDeviceFeatures',
-            'protocol-features': 'VhostDeviceProtocols',
+            'features': 'uint64',
+            'acked-features': 'uint64',
+            'backend-features': 'uint64',
+            'protocol-features': 'uint64',
             'max-queues': 'uint64',
             'backend-cap': 'uint64',
             'log-enabled': 'bool',
@@ -170,11 +170,11 @@
             'device-id': 'uint16',
             'vhost-started': 'bool',
             'device-endian': 'str',
-            'guest-features': 'VirtioDeviceFeatures',
-            'host-features': 'VirtioDeviceFeatures',
-            'backend-features': 'VirtioDeviceFeatures',
+            'guest-features': 'uint64',
+            'host-features': 'uint64',
+            'backend-features': 'uint64',
             'num-vqs': 'int',
-            'status': 'VirtioDeviceStatus',
+            'status': 'uint8',
             'isr': 'uint8',
             'queue-sel': 'uint16',
             'vm-running': 'bool',
@@ -217,41 +217,14 @@
 #          "name": "virtio-crypto",
 #          "started": true,
 #          "device-id": 20,
-#          "backend-features": {
-#              "transports": [],
-#              "dev-features": []
-#          },
+#          "backend-features": 0,
 #          "start-on-kick": false,
 #          "isr": 1,
 #          "broken": false,
-#          "status": {
-#              "statuses": [
-#                  "VIRTIO_CONFIG_S_ACKNOWLEDGE: Valid virtio device found",
-#                  "VIRTIO_CONFIG_S_DRIVER: Guest OS compatible with device",
-#                  "VIRTIO_CONFIG_S_FEATURES_OK: Feature negotiation complete",
-#                  "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"
-#              ]
-#          },
+#          "status": 15,
 #          "num-vqs": 2,
-#          "guest-features": {
-#              "dev-features": [],
-#              "transports": [
-#                  "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                  "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                  "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"
-#              ]
-#          },
-#          "host-features": {
-#              "unknown-dev-features": 1073741824,
-#              "dev-features": [],
-#              "transports": [
-#                  "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                  "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                  "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)",
-#                  "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts",
-#                  "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. descs. on VQ"
-#              ]
-#          },
+#          "guest-features": 5100273664,
+#          "host-features": 6325010432,
 #          "use-guest-notifier-mask": true,
 #          "vm-running": true,
 #          "queue-sel": 1,
@@ -279,147 +252,22 @@
 #              "max-queues": 1,
 #              "backend-cap": 2,
 #              "log-size": 0,
-#              "backend-features": {
-#                  "dev-features": [],
-#                  "transports": []
-#              },
+#              "backend-features": 0,
 #              "nvqs": 2,
-#              "protocol-features": {
-#                  "protocols": []
-#              },
+#              "protocol-features": 0,
 #              "vq-index": 0,
 #              "log-enabled": false,
-#              "acked-features": {
-#                  "dev-features": [
-#                      "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers"
-#                  ],
-#                  "transports": [
-#                      "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                      "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                      "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"
-#                  ]
-#              },
-#              "features": {
-#                  "dev-features": [
-#                      "VHOST_F_LOG_ALL: Logging write descriptors supported",
-#                      "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers"
-#                  ],
-#                  "transports": [
-#                      "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                      "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                      "VIRTIO_F_IOMMU_PLATFORM: Device can be used on IOMMU platform",
-#                      "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)",
-#                      "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts",
-#                      "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. descs. on VQ"
-#                  ]
-#              }
-#          },
-#          "backend-features": {
-#              "dev-features": [
-#                  "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features negotiation supported",
-#                  "VIRTIO_NET_F_GSO: Handling GSO-type packets supported",
-#                  "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control channel",
-#                  "VIRTIO_NET_F_GUEST_ANNOUNCE: Driver sending gratuitous packets supported",
-#                  "VIRTIO_NET_F_CTRL_RX_EXTRA: Extra RX mode control supported",
-#                  "VIRTIO_NET_F_CTRL_VLAN: Control channel VLAN filtering supported",
-#                  "VIRTIO_NET_F_CTRL_RX: Control channel RX mode supported",
-#                  "VIRTIO_NET_F_CTRL_VQ: Control channel available",
-#                  "VIRTIO_NET_F_STATUS: Configuration status field available",
-#                  "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers",
-#                  "VIRTIO_NET_F_HOST_UFO: Device can receive UFO",
-#                  "VIRTIO_NET_F_HOST_ECN: Device can receive TSO with ECN",
-#                  "VIRTIO_NET_F_HOST_TSO6: Device can receive TSOv6",
-#                  "VIRTIO_NET_F_HOST_TSO4: Device can receive TSOv4",
-#                  "VIRTIO_NET_F_GUEST_UFO: Driver can receive UFO",
-#                  "VIRTIO_NET_F_GUEST_ECN: Driver can receive TSO with ECN",
-#                  "VIRTIO_NET_F_GUEST_TSO6: Driver can receive TSOv6",
-#                  "VIRTIO_NET_F_GUEST_TSO4: Driver can receive TSOv4",
-#                  "VIRTIO_NET_F_MAC: Device has given MAC address",
-#                  "VIRTIO_NET_F_CTRL_GUEST_OFFLOADS: Control channel offloading reconfig. supported",
-#                  "VIRTIO_NET_F_GUEST_CSUM: Driver handling packets with partial checksum supported",
-#                  "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum supported"
-#              ],
-#              "transports": [
-#                  "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                  "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                  "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)",
-#                  "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts",
-#                  "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. descs. on VQ"
-#              ]
+#              "acked-features": 5100306432,
+#              "features": 13908344832,
 #          },
+#          "backend-features": 6337593319,
 #          "start-on-kick": false,
 #          "isr": 1,
 #          "broken": false,
-#          "status": {
-#              "statuses": [
-#                  "VIRTIO_CONFIG_S_ACKNOWLEDGE: Valid virtio device found",
-#                  "VIRTIO_CONFIG_S_DRIVER: Guest OS compatible with device",
-#                  "VIRTIO_CONFIG_S_FEATURES_OK: Feature negotiation complete",
-#                  "VIRTIO_CONFIG_S_DRIVER_OK: Driver setup and ready"
-#              ]
-#          },
+#          "status": 15,
 #          "num-vqs": 3,
-#          "guest-features": {
-#              "dev-features": [
-#                  "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control channel",
-#                  "VIRTIO_NET_F_GUEST_ANNOUNCE: Driver sending gratuitous packets supported",
-#                  "VIRTIO_NET_F_CTRL_VLAN: Control channel VLAN filtering supported",
-#                  "VIRTIO_NET_F_CTRL_RX: Control channel RX mode supported",
-#                  "VIRTIO_NET_F_CTRL_VQ: Control channel available",
-#                  "VIRTIO_NET_F_STATUS: Configuration status field available",
-#                  "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers",
-#                  "VIRTIO_NET_F_HOST_UFO: Device can receive UFO",
-#                  "VIRTIO_NET_F_HOST_ECN: Device can receive TSO with ECN",
-#                  "VIRTIO_NET_F_HOST_TSO6: Device can receive TSOv6",
-#                  "VIRTIO_NET_F_HOST_TSO4: Device can receive TSOv4",
-#                  "VIRTIO_NET_F_GUEST_UFO: Driver can receive UFO",
-#                  "VIRTIO_NET_F_GUEST_ECN: Driver can receive TSO with ECN",
-#                  "VIRTIO_NET_F_GUEST_TSO6: Driver can receive TSOv6",
-#                  "VIRTIO_NET_F_GUEST_TSO4: Driver can receive TSOv4",
-#                  "VIRTIO_NET_F_MAC: Device has given MAC address",
-#                  "VIRTIO_NET_F_CTRL_GUEST_OFFLOADS: Control channel offloading reconfig. supported",
-#                  "VIRTIO_NET_F_GUEST_CSUM: Driver handling packets with partial checksum supported",
-#                  "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum supported"
-#              ],
-#              "transports": [
-#                  "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                  "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                  "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)"
-#             ]
-#          },
-#          "host-features": {
-#              "dev-features": [
-#                  "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features negotiation supported",
-#                  "VIRTIO_NET_F_GSO: Handling GSO-type packets supported",
-#                  "VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control channel",
-#                  "VIRTIO_NET_F_GUEST_ANNOUNCE: Driver sending gratuitous packets supported",
-#                  "VIRTIO_NET_F_CTRL_RX_EXTRA: Extra RX mode control supported",
-#                  "VIRTIO_NET_F_CTRL_VLAN: Control channel VLAN filtering supported",
-#                  "VIRTIO_NET_F_CTRL_RX: Control channel RX mode supported",
-#                  "VIRTIO_NET_F_CTRL_VQ: Control channel available",
-#                  "VIRTIO_NET_F_STATUS: Configuration status field available",
-#                  "VIRTIO_NET_F_MRG_RXBUF: Driver can merge receive buffers",
-#                  "VIRTIO_NET_F_HOST_UFO: Device can receive UFO",
-#                  "VIRTIO_NET_F_HOST_ECN: Device can receive TSO with ECN",
-#                  "VIRTIO_NET_F_HOST_TSO6: Device can receive TSOv6",
-#                  "VIRTIO_NET_F_HOST_TSO4: Device can receive TSOv4",
-#                  "VIRTIO_NET_F_GUEST_UFO: Driver can receive UFO",
-#                  "VIRTIO_NET_F_GUEST_ECN: Driver can receive TSO with ECN",
-#                  "VIRTIO_NET_F_GUEST_TSO6: Driver can receive TSOv6",
-#                  "VIRTIO_NET_F_GUEST_TSO4: Driver can receive TSOv4",
-#                  "VIRTIO_NET_F_MAC: Device has given MAC address",
-#                  "VIRTIO_NET_F_CTRL_GUEST_OFFLOADS: Control channel offloading reconfig. supported",
-#                  "VIRTIO_NET_F_GUEST_CSUM: Driver handling packets with partial checksum supported",
-#                  "VIRTIO_NET_F_CSUM: Device handling packets with partial checksum supported"
-#              ],
-#              "transports": [
-#                  "VIRTIO_RING_F_EVENT_IDX: Used & avail. event fields enabled",
-#                  "VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported",
-#                  "VIRTIO_F_VERSION_1: Device compliant for v1 spec (legacy)",
-#                  "VIRTIO_F_ANY_LAYOUT: Device accepts arbitrary desc. layouts",
-#                  "VIRTIO_F_NOTIFY_ON_EMPTY: Notify when device runs out of avail. descs. on VQ"
-#             ]
-#          },
+#          "guest-features": 5111807911,
+#          "host-features": 6337593319,
 #          "use-guest-notifier-mask": true,
 #          "vm-running": true,
 #          "queue-sel": 2,
-- 
2.39.1
Re: [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
Posted by Philippe Mathieu-Daudé 11 months ago
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));
>       }
Re: [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
Posted by Yong Huang 10 months, 4 weeks ago
Thanks, I'll fix that in the next version.

On Fri, Dec 29, 2023 at 5:31 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> 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));
> >       }
>
>

-- 
Best regards