qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Adjust the output of x-query-virtio-status
@ 2023-12-28 18:52 Hyman Huang
  2023-12-28 18:52 ` [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output Hyman Huang
  2023-12-28 18:52 ` [PATCH 2/2] hmp: Drop unknown feature and status bits Hyman Huang
  0 siblings, 2 replies; 5+ messages in thread
From: Hyman Huang @ 2023-12-28 18:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael S . Tsirkin, Markus Armbruster, Eric Blake, yong.huang

This patchset is derived from the series:
https://lore.kernel.org/qemu-devel/cover.1699793550.git.yong.huang@smartx.com/
Please go to the link to see more background information.

The following points are what we have done in the patchset:
1. Take the policy of adding human-readable output just in HMP.
2. For the HMP output, display the human-readable information and
   drop the unknown bits in practice.
3. For the QMP output, remove the descriptive strings and only
   display bits encoded as numbers.

Please review, thanks,
Yong

Hyman Huang (2):
  qapi/virtio: Keep feature and status bits in the QMP output
  hmp: Drop unknown feature and status bits

 hw/virtio/virtio-hmp-cmds.c |  38 ++++---
 hw/virtio/virtio-qmp.c      |  23 ++---
 qapi/virtio.json            | 192 ++++--------------------------------
 3 files changed, 45 insertions(+), 208 deletions(-)

-- 
2.39.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
  2023-12-28 18:52 [PATCH 0/2] Adjust the output of x-query-virtio-status Hyman Huang
@ 2023-12-28 18:52 ` Hyman Huang
  2023-12-29  9:31   ` Philippe Mathieu-Daudé
  2023-12-28 18:52 ` [PATCH 2/2] hmp: Drop unknown feature and status bits Hyman Huang
  1 sibling, 1 reply; 5+ messages in thread
From: Hyman Huang @ 2023-12-28 18:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael S . Tsirkin, Markus Armbruster, Eric Blake, yong.huang

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] hmp: Drop unknown feature and status bits
  2023-12-28 18:52 [PATCH 0/2] Adjust the output of x-query-virtio-status Hyman Huang
  2023-12-28 18:52 ` [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output Hyman Huang
@ 2023-12-28 18:52 ` Hyman Huang
  1 sibling, 0 replies; 5+ messages in thread
From: Hyman Huang @ 2023-12-28 18:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael S . Tsirkin, Markus Armbruster, Eric Blake, yong.huang

The QMP command "x-query-virtio-status" outputs the full
feature and status bit information, so there is no need
to maintain it in the HMP output; drop it.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 hw/virtio/virtio-hmp-cmds.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
index 721c630ab0..f9a7384604 100644
--- a/hw/virtio/virtio-hmp-cmds.c
+++ b/hw/virtio/virtio-hmp-cmds.c
@@ -25,10 +25,6 @@ static void hmp_virtio_dump_protocols(Monitor *mon,
         }
     }
     monitor_printf(mon, "\n");
-    if (pcol->has_unknown_protocols) {
-        monitor_printf(mon, "  unknown-protocols(0x%016"PRIx64")\n",
-                       pcol->unknown_protocols);
-    }
 }
 
 static void hmp_virtio_dump_status(Monitor *mon,
@@ -43,10 +39,6 @@ static void hmp_virtio_dump_status(Monitor *mon,
         }
     }
     monitor_printf(mon, "\n");
-    if (status->has_unknown_statuses) {
-        monitor_printf(mon, "  unknown-statuses(0x%016"PRIx32")\n",
-                       status->unknown_statuses);
-    }
 }
 
 static void hmp_virtio_dump_features(Monitor *mon,
@@ -73,11 +65,6 @@ static void hmp_virtio_dump_features(Monitor *mon,
         }
         monitor_printf(mon, "\n");
     }
-
-    if (features->has_unknown_dev_features) {
-        monitor_printf(mon, "  unknown-features(0x%016"PRIx64")\n",
-                       features->unknown_dev_features);
-    }
 }
 
 void hmp_virtio_query(Monitor *mon, const QDict *qdict)
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
  2023-12-28 18:52 ` [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output Hyman Huang
@ 2023-12-29  9:31   ` Philippe Mathieu-Daudé
  2024-01-03  1:30     ` Yong Huang
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-29  9:31 UTC (permalink / raw)
  To: Hyman Huang, qemu-devel
  Cc: Michael S . Tsirkin, Markus Armbruster, Eric Blake

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output
  2023-12-29  9:31   ` Philippe Mathieu-Daudé
@ 2024-01-03  1:30     ` Yong Huang
  0 siblings, 0 replies; 5+ messages in thread
From: Yong Huang @ 2024-01-03  1:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Michael S . Tsirkin, Markus Armbruster, Eric Blake

[-- Attachment #1: Type: text/plain, Size: 4094 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 5647 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-01-03  1:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-28 18:52 [PATCH 0/2] Adjust the output of x-query-virtio-status Hyman Huang
2023-12-28 18:52 ` [PATCH 1/2] qapi/virtio: Keep feature and status bits in the QMP output Hyman Huang
2023-12-29  9:31   ` Philippe Mathieu-Daudé
2024-01-03  1:30     ` Yong Huang
2023-12-28 18:52 ` [PATCH 2/2] hmp: Drop unknown feature and status bits Hyman Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).