* [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command
@ 2017-10-24 17:50 Jan Dakinevich
2017-10-24 17:50 ` [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jan Dakinevich @ 2017-10-24 17:50 UTC (permalink / raw)
To: qemu-devel
Cc: Jan Dakinevich, Dr. David Alan Gilbert, Michael S. Tsirkin,
Eric Blake, Markus Armbruster, Kevin Wolf, Denis V. Lunev,
Cornelia Huck, Max Reitz, Stefan Hajnoczi, Paolo Bonzini
Previously, it was suggested to use QAPI unions/enums to declare feature
bits. Some of weak parts of my code (such `get_feature_name' callback)
went away. But a lot of of dummy code with feature declaration is still
here, it just migrated to json file.
So, I would be glad to get a responce if my understanging of using QAPI
structures was correct, or there was another way to do that cleaner?
v5:
Use QAPI enums/unions to declare virtio feature bits
v4: http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg00393.html
v3: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07565.html
v2: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07527.html
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
Jan Dakinevich (2):
virtio: introduce `query-virtio' QMP command
virtio: add `info virtio' HMP command
hmp-commands-info.hx | 14 ++
hmp.c | 122 +++++++++++++
hmp.h | 1 +
hw/virtio/Makefile.objs | 3 +
hw/virtio/virtio-qmp.c | 223 +++++++++++++++++++++++
hw/virtio/virtio-stub.c | 9 +
qapi-schema.json | 466 ++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 838 insertions(+)
create mode 100644 hw/virtio/virtio-qmp.c
create mode 100644 hw/virtio/virtio-stub.c
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command 2017-10-24 17:50 [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich @ 2017-10-24 17:50 ` Jan Dakinevich 2017-11-02 18:52 ` Eric Blake 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 2/2] virtio: add `info virtio' HMP command Jan Dakinevich 2017-12-12 13:54 ` [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2 siblings, 1 reply; 8+ messages in thread From: Jan Dakinevich @ 2017-10-24 17:50 UTC (permalink / raw) To: qemu-devel Cc: Jan Dakinevich, Denis V. Lunev, Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Cornelia Huck, Max Reitz, Stefan Hajnoczi, Paolo Bonzini The command is intended for gathering virtio information such as status, feature bits, negotiation status. It is convenient and useful for debug purpose. The commands returns generic virtio information for virtio such as common feature names and status bits names and information for all attached to current machine devices. Cc: Denis V. Lunev <den@virtuozzo.com> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> --- hw/virtio/Makefile.objs | 3 + hw/virtio/virtio-qmp.c | 223 +++++++++++++++++++++++ hw/virtio/virtio-stub.c | 9 + qapi-schema.json | 466 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 701 insertions(+) create mode 100644 hw/virtio/virtio-qmp.c create mode 100644 hw/virtio/virtio-stub.c diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index 765d363..3524aa7 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -3,12 +3,15 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o +common-obj-y += virtio-qmp.o obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o obj-y += virtio-crypto.o obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o +else +obj-y += virtio-stub.o endif common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c new file mode 100644 index 0000000..8192c33 --- /dev/null +++ b/hw/virtio/virtio-qmp.c @@ -0,0 +1,223 @@ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qmp-commands.h" + +#include "hw/virtio/virtio.h" +#include "hw/virtio/virtio-net.h" +#include "hw/virtio/virtio-blk.h" +#include "hw/virtio/virtio-serial.h" +#include "hw/virtio/virtio-balloon.h" +#include "hw/virtio/virtio-scsi.h" +#ifdef CONFIG_VIRTFS +#include "hw/9pfs/virtio-9p.h" +#endif +#include "hw/virtio/virtio-gpu.h" + +#define qmp_query_virtio_features(__vdev, __info, __offset, __count,\ + __name_kind, __name_type, __name_field) ({ \ + unsigned __idx; \ + for (__idx = (__count); __idx--; ) { \ + unsigned int __fbit = __idx + (__offset); \ + bool __host = virtio_host_has_feature(__vdev, __fbit); \ + bool __guest = virtio_vdev_has_feature(__vdev, __fbit); \ + VirtioFeatureList *__feature; \ + \ + if (!__host && !__guest) { \ + continue; \ + } \ + \ + __feature = g_new0(VirtioFeatureList, 1); \ + __feature->value = g_new0(VirtioFeature, 1); \ + __feature->value->name = g_new0(VirtioFeatureName, 1); \ + \ + __feature->value->host = __host; \ + __feature->value->guest = __guest; \ + \ + __feature->value->name->type = (__name_kind); \ + __feature->value->name->u.__name_field.data = (__name_type)__idx; \ + \ + __feature->next = (__info)->features; \ + (__info)->features = __feature; \ + } \ +}) + +#define qmp_query_virtio_features_common(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), VIRTIO_FEATURE_OTHER__MAX, \ + VIRTIO_FEATURE_COMMON__MAX, VIRTIO_FEATURE_NAME_KIND_COMMON, \ + VirtioFeatureCommon, common) + +#define qmp_query_virtio_features_net(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_NET__MAX, VIRTIO_FEATURE_NAME_KIND_NET, \ + VirtioFeatureNet, net) + +#define qmp_query_virtio_features_blk(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_BLK__MAX, VIRTIO_FEATURE_NAME_KIND_BLK, \ + VirtioFeatureBlk, blk) + +#define qmp_query_virtio_features_serial(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_SERIAL__MAX, VIRTIO_FEATURE_NAME_KIND_SERIAL, \ + VirtioFeatureSerial, serial) + +#define qmp_query_virtio_features_balloon(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_BALLOON__MAX, VIRTIO_FEATURE_NAME_KIND_BALLOON, \ + VirtioFeatureBalloon, balloon) + +#define qmp_query_virtio_features_scsi(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_SCSI__MAX, VIRTIO_FEATURE_NAME_KIND_SCSI, \ + VirtioFeatureScsi, scsi) + +#ifdef CONFIG_VIRTFS +#define qmp_query_virtio_features_virtfs(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_VIRTFS__MAX, VIRTIO_FEATURE_NAME_KIND_VIRTFS, \ + VirtioFeatureVirtfs, virtfs) +#endif + +#define qmp_query_virtio_features_gpu(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_GPU__MAX, VIRTIO_FEATURE_NAME_KIND_GPU, \ + VirtioFeatureGpu, gpu) + +#define qmp_query_virtio_features_other(__vdev, __info) \ + qmp_query_virtio_features((__vdev), (__info), 0, \ + VIRTIO_FEATURE_OTHER__MAX, VIRTIO_FEATURE_NAME_KIND_OTHER, \ + VirtioFeatureOther, other) + + +static VirtioInfoList *qmp_query_virtio_one(VirtIODevice *vdev) +{ + VirtioInfoList *list; + VirtioInfo *info; + unsigned int idx; + + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_COMMON__MAX != 40); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_NET__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_BLK__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_SCSI__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_BALLOON__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_SERIAL__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_GPU__MAX != 24); + QEMU_BUILD_BUG_ON(VIRTIO_FEATURE_OTHER__MAX != 24); + + list = g_new0(VirtioInfoList, 1); + list->value = g_new0(VirtioInfo, 1); + + info = list->value; + info->qom_path = object_get_canonical_path(OBJECT(vdev)); + info->status = vdev->status; + info->host_features = vdev->host_features; + info->guest_features = vdev->guest_features; + + /* device status */ + for (idx = VIRTIO_STATUS__MAX; idx--; ) { + VirtioStatusList *status; + + if (!(vdev->status & (1 << idx))) { + continue; + } + + status = g_new0(VirtioStatusList, 1); + status->value = (VirtioStatus)idx; + + status->next = info->status_names; + info->status_names = status; + } + + /* device-specific features */ + if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_NET)) { + qmp_query_virtio_features_net(vdev, info); + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_BLK)) { + qmp_query_virtio_features_blk(vdev, info); + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_SERIAL)) { + qmp_query_virtio_features_serial(vdev, info); + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_BALLOON)) { + qmp_query_virtio_features_balloon(vdev, info); + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_SCSI)) { + qmp_query_virtio_features_scsi(vdev, info); +#ifdef CONFIG_VIRTFS + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_9P)) { + qmp_query_virtio_features_virtfs(vdev, info); +#endif + } else if (object_dynamic_cast(OBJECT(vdev), TYPE_VIRTIO_GPU)) { + qmp_query_virtio_features_gpu(vdev, info); + } else { + qmp_query_virtio_features_other(vdev, info); + } + + /* common features */ + qmp_query_virtio_features_common(vdev, info); + + return list; +} + +typedef struct QueryVirtioEntry { + VirtIODevice *vdev; + QTAILQ_ENTRY(QueryVirtioEntry) link; +} QueryVirtioEntry; + +typedef QTAILQ_HEAD(, QueryVirtioEntry) QueryVirtioHead; + +static void qmp_query_virtio_recursive(QueryVirtioHead *head, BusState *bus) +{ + BusChild *kid; + + QTAILQ_FOREACH(kid, &bus->children, sibling) { + DeviceState *dev = kid->child; + BusState *child; + + if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_DEVICE)) { + QueryVirtioEntry *entry = g_new0(QueryVirtioEntry, 1); + + entry->vdev = VIRTIO_DEVICE(dev); + QTAILQ_INSERT_TAIL(head, entry, link); + } + QLIST_FOREACH(child, &dev->child_bus, sibling) { + qmp_query_virtio_recursive(head, child); + } + } +} + +VirtioInfoList *qmp_query_virtio(bool has_path, const char *path, Error **errp) +{ + BusState *bus = sysbus_get_default(); + VirtioInfoList *list = NULL; + + if (!bus) { + return NULL; + } + + if (has_path) { + Object *obj = object_resolve_path(path, NULL); + if (!obj) { + return NULL; + } + if (!object_dynamic_cast(OBJECT(obj), TYPE_VIRTIO_DEVICE)) { + return NULL; + } + + list = qmp_query_virtio_one(VIRTIO_DEVICE(obj)); + } else { + QueryVirtioHead head; + QueryVirtioEntry *query, *tmp; + + QTAILQ_INIT(&head); + qmp_query_virtio_recursive(&head, bus); + + QTAILQ_FOREACH_SAFE(query, &head, link, tmp) { + VirtioInfoList *entry = qmp_query_virtio_one(query->vdev); + + QTAILQ_REMOVE(&head, query, link); + g_free(query); + + entry->next = list; + list = entry; + } + } + + return list; +} diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c new file mode 100644 index 0000000..185d4bd --- /dev/null +++ b/hw/virtio/virtio-stub.c @@ -0,0 +1,9 @@ +#include "qemu/osdep.h" +#include "qmp-commands.h" +#include "qapi/qmp/qerror.h" + +VirtioInfoList *qmp_query_virtio(bool has_path, const char *path, Error **errp) +{ + error_setg(errp, QERR_UNSUPPORTED); + return NULL; +} diff --git a/qapi-schema.json b/qapi-schema.json index a9dd043..b1645f3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3200,3 +3200,469 @@ # Since: 2.11 ## { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} } + +## +# @VirtioStatus: +# +# Virtio device status bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioStatus', + 'data': [ + 'acknowledge', + 'driver', + 'driver-ok', + 'features-ok', + 'status-reserved-4', + 'status-reserved-5', + 'needs-reset', + 'failed' + ] +} + +## +# @VirtioFeatureCommon: +# +# Virtio feature bits common to all devices +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureCommon', + 'data': [ + 'notify-on-empty', + 'common-reserved-25', + 'common-reserved-26', + 'any-layout', + 'indirect-desc', + 'event-idx', + 'bad-feature', + 'common-reserved-31', + 'version-1', + 'iommu-platform', + 'common-reserved-34', + 'common-reserved-35', + 'common-reserved-36', + 'common-reserved-37', + 'common-reserved-38', + 'common-reserved-39', + 'common-reserved-40', + 'common-reserved-41', + 'common-reserved-42', + 'common-reserved-43', + 'common-reserved-44', + 'common-reserved-45', + 'common-reserved-46', + 'common-reserved-47', + 'common-reserved-48', + 'common-reserved-49', + 'common-reserved-50', + 'common-reserved-51', + 'common-reserved-52', + 'common-reserved-53', + 'common-reserved-54', + 'common-reserved-55', + 'common-reserved-56', + 'common-reserved-57', + 'common-reserved-58', + 'common-reserved-59', + 'common-reserved-60', + 'common-reserved-61', + 'common-reserved-62', + 'common-reserved-63' + ] +} + +## +# @VirtioFeatureNet: +# +# Virtio network device feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureNet', + 'data': [ + 'csum', + 'guest-csum', + 'ctrl-guest-offloads', + 'mtu', + 'net-reserved-4', + 'mac', + 'gso', + 'guest-tso4', + 'guest-tso6', + 'guest-ecn', + 'guest-ufo', + 'host-tso4', + 'host-tso6', + 'host-ecn', + 'host-ufo', + 'mrg-rxbuf', + 'status', + 'ctrl-vq', + 'ctrl-rx', + 'ctrl-vlan', + 'ctrl-rx-extra', + 'guest-announce', + 'mq', + 'ctrl-mac-addr' + ] +} + +## +# @VirtioFeatureBlk: +# +# Virtio block device feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureBlk', + 'data': [ + 'barrier', + 'size-max', + 'seg-max', + 'blk-reserved-3', + 'geometry', + 'ro', + 'blk-size', + 'scsi', + 'blk-reserved-8', + 'flush', + 'topology', + 'config-wce', + 'mq', + 'blk-reserved-13', + 'blk-reserved-14', + 'blk-reserved-15', + 'blk-reserved-16', + 'blk-reserved-17', + 'blk-reserved-18', + 'blk-reserved-19', + 'blk-reserved-20', + 'blk-reserved-21', + 'blk-reserved-22', + 'blk-reserved-23' + ] +} + +## +# @VirtioFeatureSerial: +# +# Virtio serial device (console) feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureSerial', + 'data': [ + 'size', + 'multiport', + 'emerg-write', + 'serial-reserved-3', + 'serial-reserved-4', + 'serial-reserved-5', + 'serial-reserved-6', + 'serial-reserved-7', + 'serial-reserved-8', + 'serial-reserved-9', + 'serial-reserved-10', + 'serial-reserved-11', + 'serial-reserved-12', + 'serial-reserved-13', + 'serial-reserved-14', + 'serial-reserved-15', + 'serial-reserved-16', + 'serial-reserved-17', + 'serial-reserved-18', + 'serial-reserved-19', + 'serial-reserved-20', + 'serial-reserved-21', + 'serial-reserved-22', + 'serial-reserved-23' + ] +} + +## +# @VirtioFeatureBalloon: +# +# Virtio balloon feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureBalloon', + 'data': [ + 'must-tell-host', + 'stats-vq', + 'deflate-on-oom', + 'balloon-reserved-3', + 'balloon-reserved-4', + 'balloon-reserved-5', + 'balloon-reserved-6', + 'balloon-reserved-7', + 'balloon-reserved-8', + 'balloon-reserved-9', + 'balloon-reserved-10', + 'balloon-reserved-11', + 'balloon-reserved-12', + 'balloon-reserved-13', + 'balloon-reserved-14', + 'balloon-reserved-15', + 'balloon-reserved-16', + 'balloon-reserved-17', + 'balloon-reserved-18', + 'balloon-reserved-19', + 'balloon-reserved-20', + 'balloon-reserved-21', + 'balloon-reserved-22', + 'balloon-reserved-23' + ] +} + +## +# @VirtioFeatureScsi: +# +# Virtio scsi controller feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureScsi', + 'data': [ + 'inout', + 'hotplug', + 'change', + 't10-pi', + 'scsi-reserved-4', + 'scsi-reserved-5', + 'scsi-reserved-6', + 'scsi-reserved-7', + 'scsi-reserved-8', + 'scsi-reserved-9', + 'scsi-reserved-10', + 'scsi-reserved-11', + 'scsi-reserved-12', + 'scsi-reserved-13', + 'scsi-reserved-14', + 'scsi-reserved-15', + 'scsi-reserved-16', + 'scsi-reserved-17', + 'scsi-reserved-18', + 'scsi-reserved-19', + 'scsi-reserved-20', + 'scsi-reserved-21', + 'scsi-reserved-22', + 'scsi-reserved-23' + ] +} + +## +# @VirtioFeatureVirtfs: +# +# Virtio virtfs (9p) feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureVirtfs', + 'data': [ + 'mount-tag', + 'virtfs-reserved-1', + 'virtfs-reserved-2', + 'virtfs-reserved-3', + 'virtfs-reserved-4', + 'virtfs-reserved-5', + 'virtfs-reserved-6', + 'virtfs-reserved-7', + 'virtfs-reserved-8', + 'virtfs-reserved-9', + 'virtfs-reserved-10', + 'virtfs-reserved-11', + 'virtfs-reserved-12', + 'virtfs-reserved-13', + 'virtfs-reserved-14', + 'virtfs-reserved-15', + 'virtfs-reserved-16', + 'virtfs-reserved-17', + 'virtfs-reserved-18', + 'virtfs-reserved-19', + 'virtfs-reserved-20', + 'virtfs-reserved-21', + 'virtfs-reserved-22', + 'virtfs-reserved-23' + ] +} + +## +# @VirtioFeatureGpu: +# +# Virtio GPU device feature bits +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureGpu', + 'data': [ + 'virgl', + 'gpu-reserved-1', + 'gpu-reserved-2', + 'gpu-reserved-3', + 'gpu-reserved-4', + 'gpu-reserved-5', + 'gpu-reserved-6', + 'gpu-reserved-7', + 'gpu-reserved-8', + 'gpu-reserved-9', + 'gpu-reserved-10', + 'gpu-reserved-11', + 'gpu-reserved-12', + 'gpu-reserved-13', + 'gpu-reserved-14', + 'gpu-reserved-15', + 'gpu-reserved-16', + 'gpu-reserved-17', + 'gpu-reserved-18', + 'gpu-reserved-19', + 'gpu-reserved-20', + 'gpu-reserved-21', + 'gpu-reserved-22', + 'gpu-reserved-23' + ] +} + +## +# @VirtioFeatureOther: +# +# Feature bits of other virtio devices +# +# Since: 2.11 +# +## +{ + 'enum': 'VirtioFeatureOther', + 'data': [ + 'other-reserved-0', + 'other-reserved-1', + 'other-reserved-2', + 'other-reserved-3', + 'other-reserved-4', + 'other-reserved-5', + 'other-reserved-6', + 'other-reserved-7', + 'other-reserved-8', + 'other-reserved-9', + 'other-reserved-10', + 'other-reserved-11', + 'other-reserved-12', + 'other-reserved-13', + 'other-reserved-14', + 'other-reserved-15', + 'other-reserved-16', + 'other-reserved-17', + 'other-reserved-18', + 'other-reserved-19', + 'other-reserved-20', + 'other-reserved-21', + 'other-reserved-22', + 'other-reserved-23' + ] +} + +## +# @VirtioFeatureName: +# +# Since: 2.11 +# +## +{ + 'union': 'VirtioFeatureName', + 'data': { + 'common': 'VirtioFeatureCommon', + 'net': 'VirtioFeatureNet', + 'blk': 'VirtioFeatureBlk', + 'serial': 'VirtioFeatureSerial', + 'balloon': 'VirtioFeatureBalloon', + 'scsi': 'VirtioFeatureScsi', + 'virtfs': 'VirtioFeatureVirtfs', + 'gpu': 'VirtioFeatureGpu', + 'other': 'VirtioFeatureOther' + } +} + +## +# @VirtioFeature: +# +# Feature bit with negotiation status +# +# @host: true if host exposes the feature +# +# @guest: true if guest acknowledges the feature +# +# Since: 2.11 +# +## +{ + 'struct': 'VirtioFeature', + 'data': { + 'host': 'bool', + 'guest': 'bool', + 'name': 'VirtioFeatureName' + } +} + +## +# @VirtioInfo: +# +# Information about virtio device +# +# @qom-path: QOM path of the device +# +# @status: status bitmask +# +# @status-names: names of checked bits in status bitmask +# +# @host-features: bitmask of features, exposed by device +# +# @guest-features: bitmask of features, acknowledged by guest +# +# @features: negotiation status and names of device features +# +## +{ + 'struct': 'VirtioInfo', + 'data': { + 'qom-path': 'str', + 'status': 'uint8', + 'status-names': ['VirtioStatus'], + 'host-features': 'uint64', + 'guest-features': 'uint64', + 'features': ['VirtioFeature'] + } +} + +## +# @query-virtio: +# +# Returns virtio information such as device status and features +# +# Since: 2.11 +# +## +{ + 'command': 'query-virtio', + 'data': { '*path': 'str' }, + 'returns': ['VirtioInfo'] +} -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich @ 2017-11-02 18:52 ` Eric Blake 0 siblings, 0 replies; 8+ messages in thread From: Eric Blake @ 2017-11-02 18:52 UTC (permalink / raw) To: Jan Dakinevich, qemu-devel Cc: Denis V. Lunev, Dr. David Alan Gilbert, Michael S. Tsirkin, Markus Armbruster, Kevin Wolf, Cornelia Huck, Max Reitz, Stefan Hajnoczi, Paolo Bonzini [-- Attachment #1: Type: text/plain, Size: 6655 bytes --] On 10/24/2017 12:50 PM, Jan Dakinevich wrote: > The command is intended for gathering virtio information such as status, > feature bits, negotiation status. It is convenient and useful for debug > purpose. > > The commands returns generic virtio information for virtio such as > common feature names and status bits names and information for all > attached to current machine devices. > > Cc: Denis V. Lunev <den@virtuozzo.com> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > --- > +++ b/qapi-schema.json > @@ -3200,3 +3200,469 @@ > # Since: 2.11 > ## > { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} } > + > +## > +# @VirtioStatus: > +# > +# Virtio device status bits > +# > +# Since: 2.11 We've entered soft freeze, and this is a new feature, so it may be better to propose this for 2.12 rather than trying to rush it in now. > +# > +## > +{ > + 'enum': 'VirtioStatus', > + 'data': [ > + 'acknowledge', > + 'driver', > + 'driver-ok', > + 'features-ok', > + 'status-reserved-4', > + 'status-reserved-5', Are we ever going to report a '*-reserved-NNN' enum value to the end user? I hope not, in which case, we don't need to expose it in the QAPI enums. Yes, that means you have to do some mapping between named bits and enum strings (you would no longer have the property that 'status-reserved-4' is value 4, but would instead have to map that bit 6 translates to 'needs-reset' with enum value 4), but a clean public-facing interface may be worth the hassle. What's more, as newer qemu learns new bit names down the road, then expanding the enums to cover those new names is introspectible; whereas you can't rename an existing enum value while still retaining back-compat. > +## > +# @VirtioFeatureName: > +# > +# Since: 2.11 > +# > +## > +{ > + 'union': 'VirtioFeatureName', > + 'data': { > + 'common': 'VirtioFeatureCommon', > + 'net': 'VirtioFeatureNet', > + 'blk': 'VirtioFeatureBlk', Do we really need to abbreviate this one? > + 'serial': 'VirtioFeatureSerial', > + 'balloon': 'VirtioFeatureBalloon', > + 'scsi': 'VirtioFeatureScsi', > + 'virtfs': 'VirtioFeatureVirtfs', > + 'gpu': 'VirtioFeatureGpu', > + 'other': 'VirtioFeatureOther' > + } > +} This is a so-called "simple union" - but we do not want any more of these in our code. In fact, Markus and I are toying with the idea of renaming it to "legacy union", and/or making the syntax more painful, because we WANT to use "flat unions" in all new code (and make their syntax easier to type). So, a comparable flat union would be something like: { 'enum': 'VirtioFeatureCategory', 'data': [ 'common', 'net', 'block', 'serial', 'balloon', 'scsi', 'virtfs', 'gpu', 'other' ] } { 'union': 'VirtioFeatureName', 'base': { 'category': 'VirtioFeatureCategory' }, 'discriminator': 'category', 'data': { 'common': 'VirtioFeatureCommon', ... } } except that flat unions don't permit enum types (they require a struct type). Hmm. > + > +## > +# @VirtioFeature: > +# > +# Feature bit with negotiation status > +# > +# @host: true if host exposes the feature > +# > +# @guest: true if guest acknowledges the feature > +# > +# Since: 2.11 > +# > +## > +{ > + 'struct': 'VirtioFeature', > + 'data': { > + 'host': 'bool', > + 'guest': 'bool', > + 'name': 'VirtioFeatureName' > + } > +} With your proposal, an example on the wire might be: "feature": { "host": true, "guest": false, "name": { "type": "common", "data": "notify-on-empty" } } > + > +## > +# @VirtioInfo: > +# > +# Information about virtio device > +# > +# @qom-path: QOM path of the device > +# > +# @status: status bitmask > +# > +# @status-names: names of checked bits in status bitmask > +# > +# @host-features: bitmask of features, exposed by device > +# > +# @guest-features: bitmask of features, acknowledged by guest > +# > +# @features: negotiation status and names of device features > +# > +## > +{ > + 'struct': 'VirtioInfo', > + 'data': { > + 'qom-path': 'str', > + 'status': 'uint8', > + 'status-names': ['VirtioStatus'], Umm, isn't it better to just have 'status': 'VirtioStatus' (as we can only ever have a single status value, so we don't really need an array to tell us what all the other status values are named). > + 'host-features': 'uint64', > + 'guest-features': 'uint64', > + 'features': ['VirtioFeature'] Do we really want to expose both a 64-bit number and an array of feature names, where the array repeats the information on whether host and guest support the bit? I guess we first want to figure out WHAT we want to send over the wire. Maybe: "info": { "qom-path": "...", "status": "driver-ok", "host": [ { "common": "notify-on-empty" }, { "common": "any-layout" }, { "net": "csum" } ], "guest": [ { "common": "notify-on-empty" }, { "net": "csum" } ] } or "info": { "qom-path": "...", "status": "driver-ok", "host": [ { "type": "common", "value": "notify-on-empty" }, { "type": "common", "value": "any-layout" }, { "type": "net", "value": "csum" } ], "guest": [ { "type": "common", "value": "notify-on-empty" }, { "type": "net", "value": "csum" } ] } or "info": { "qom-path": "...", "status": "driver-ok", "host": { "common": [ "notify-on-empty", "any-layout" ], "net": [ "csum" ] }, "guest": { "common": [ "notify-on-empty" ] }, "net": [ "csum" ] } } (some of those forms may be easier than others to express in our current QAPI language; if we need to modify the qapi language to get our ideal form supported, then that may be worth the effort). > + > +## > +# @query-virtio: > +# > +# Returns virtio information such as device status and features > +# > +# Since: 2.11 > +# > +## > +{ > + 'command': 'query-virtio', > + 'data': { '*path': 'str' }, Do we need filterable queries, or is it better to just have the command return info on all virtio devices at once and let the client filter the results as desired? > + 'returns': ['VirtioInfo'] > +} > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC v5 2/2] virtio: add `info virtio' HMP command 2017-10-24 17:50 [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich @ 2017-10-24 17:50 ` Jan Dakinevich 2017-12-12 13:54 ` [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2 siblings, 0 replies; 8+ messages in thread From: Jan Dakinevich @ 2017-10-24 17:50 UTC (permalink / raw) To: qemu-devel Cc: Jan Dakinevich, Denis V. Lunev, Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Cornelia Huck, Max Reitz, Stefan Hajnoczi, Paolo Bonzini The command prints data from `query-virtio' QMP in human-readable format. Cc: Denis V. Lunev <den@virtuozzo.com> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> --- hmp-commands-info.hx | 14 ++++++ hmp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++ hmp.h | 1 + 3 files changed, 137 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 54c3e5e..292280a 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -868,6 +868,20 @@ enabled) memory in bytes. ETEXI STEXI +@item info virtio +@findex virtio +Display guest and host fetures for all virtio devices. +ETEXI + + { + .name = "virtio", + .args_type = "path:s?", + .params = "[path]", + .help = "show virtio info", + .cmd = hmp_info_virtio, + }, + +STEXI @end table ETEXI diff --git a/hmp.c b/hmp.c index ec61329..440e48c 100644 --- a/hmp.c +++ b/hmp.c @@ -43,6 +43,7 @@ #include "hw/intc/intc.h" #include "migration/snapshot.h" #include "migration/misc.h" +#include "hw/virtio/virtio.h" #ifdef CONFIG_SPICE #include <spice/enums.h> @@ -2901,3 +2902,124 @@ void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict) } hmp_handle_error(mon, &err); } + +#define HMP_INFO_VIRTIO_INDENT 2 +#define HMP_INFO_VIRTIO_ITEM 16 +#define HMP_INFO_VIRTIO_FIELD 24 + +static void hmp_info_virtio_print_status(Monitor *mon, VirtioInfo *info) +{ + VirtioStatusList *status; + const char *space = " "; + + monitor_printf(mon, "%*s%-*s0x%02"PRIx8"", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_ITEM, "status:", info->status); + + for (status = info->status_names; status; status = status->next) { + monitor_printf(mon, "%s%s", space, VirtioStatus_str(status->value)); + space = ","; + } + + monitor_printf(mon, "\n"); +} + +static void hmp_info_virtio_print_features(Monitor *mon, VirtioInfo *info) +{ + VirtioFeatureList *head; + + monitor_printf(mon, "%*s%-*s0x%016"PRIx64"\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_ITEM, "host features:", + info->host_features); + monitor_printf(mon, "%*s%-*s0x%016"PRIx64"\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_ITEM, "guest features:", + info->guest_features); + + if (info->features) { + monitor_printf(mon, "%*s%-*s\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_ITEM, "feature names:"); + monitor_printf(mon, "%*s%-*s%-*s%-*s%-*s\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_FIELD, "TYPE", + HMP_INFO_VIRTIO_FIELD, "NAME", + HMP_INFO_VIRTIO_FIELD, "HOST", + HMP_INFO_VIRTIO_FIELD, "GUEST"); + } + + for (head = info->features; head; head = head->next) { + VirtioFeature *feature = head->value; + const char *type = VirtioFeatureNameKind_str(feature->name->type); + const char *host = feature->host ? "true" : "false"; + const char *guest = feature->guest ? "true" : "false"; + const char *name; + + switch (feature->name->type) { + case VIRTIO_FEATURE_NAME_KIND_COMMON: + name = VirtioFeatureCommon_str(feature->name->u.common.data); + break; + case VIRTIO_FEATURE_NAME_KIND_NET: + name = VirtioFeatureNet_str(feature->name->u.net.data); + break; + case VIRTIO_FEATURE_NAME_KIND_BLK: + name = VirtioFeatureBlk_str(feature->name->u.blk.data); + break; + case VIRTIO_FEATURE_NAME_KIND_SERIAL: + name = VirtioFeatureSerial_str(feature->name->u.serial.data); + break; + case VIRTIO_FEATURE_NAME_KIND_BALLOON: + name = VirtioFeatureBalloon_str(feature->name->u.balloon.data); + break; + case VIRTIO_FEATURE_NAME_KIND_SCSI: + name = VirtioFeatureScsi_str(feature->name->u.scsi.data); + break; + case VIRTIO_FEATURE_NAME_KIND_VIRTFS: + name = VirtioFeatureVirtfs_str(feature->name->u.virtfs.data); + break; + case VIRTIO_FEATURE_NAME_KIND_GPU: + name = VirtioFeatureGpu_str(feature->name->u.gpu.data); + break; + default: + case VIRTIO_FEATURE_NAME_KIND_OTHER: + name = VirtioFeatureOther_str(feature->name->u.other.data); + break; + } + + monitor_printf(mon, "%*s%-*s%-*s%-*s%-*s\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_FIELD, type, + HMP_INFO_VIRTIO_FIELD, name, + HMP_INFO_VIRTIO_FIELD, host, + HMP_INFO_VIRTIO_FIELD, guest); + } +} + +static void hmp_info_virtio_print(Monitor *mon, VirtioInfo *info) +{ + Object *obj = object_resolve_path(info->qom_path, NULL); + char *path = qdev_get_dev_path(DEVICE(obj)); + + monitor_printf(mon, "%s at %s\n", object_get_typename(obj), path); + g_free(path); + + monitor_printf(mon, "%*s%-*s%s\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_ITEM, "QOM path:", info->qom_path); + + hmp_info_virtio_print_status(mon, info); + hmp_info_virtio_print_features(mon, info); +} + +void hmp_info_virtio(Monitor *mon, const QDict *qdict) +{ + const char *path = qdict_get_try_str(qdict, "path"); + Error *err = NULL; + VirtioInfoList *head, *info; + + + head = qmp_query_virtio(!!path, path, &err); + if (err) { + return; + } + + for (info = head; info; info = info->next) { + hmp_info_virtio_print(mon, info->value); + } + + qapi_free_VirtioInfoList(head); +} diff --git a/hmp.h b/hmp.h index 3605003..3e8f30a 100644 --- a/hmp.h +++ b/hmp.h @@ -146,5 +146,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict); void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict); void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict); void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict); +void hmp_info_virtio(Monitor *mon, const QDict *qdict); #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command 2017-10-24 17:50 [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 2/2] virtio: add `info virtio' HMP command Jan Dakinevich @ 2017-12-12 13:54 ` Jan Dakinevich 2017-12-14 11:05 ` Cornelia Huck 2 siblings, 1 reply; 8+ messages in thread From: Jan Dakinevich @ 2017-12-12 13:54 UTC (permalink / raw) To: qemu-devel Cc: Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Denis V. Lunev, Cornelia Huck, Max Reitz, Stefan Hajnoczi, Paolo Bonzini I am going to reanimate works under this QMP/HMP. First of all, it could be meaningful to settle what output would provide the QMP. I would like to suggest the following description: ## # @VirtioFeature: ## { 'struct': 'VirtioFeature', 'data': { 'name': 'str', 'acked': 'bool' } } ## # @VirtioInfo: ## { 'struct': 'VirtioInfo', 'data': { 'qom-path': 'str', 'status': 'uint8', 'host-features': 'uint64', 'guest-features': 'uint64', 'status-names': ['str'], 'common-features-names': ['VirtioFeature'], 'device-features-names': ['VirtioFeature'] } } ## # @query-virtio: ## { 'command': 'query-virtio', 'data': {'*path': 'str'}, 'returns': ['VirtioInfo'] } My final goal is to implement HMP which will print all exposed virtio features (both common and device-specific) with their acknowledgements, and virtio device configuration status. These are provided by last 3 fields in @VirtioInfo. For these who are going make own decision on features and status bitmask, respective fields with raw values are preserved. So, I expect following data on the wire in response to `query-virtio' command: { "return": [ { "qom-path": "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15, "host-features": 6325010438, "guest-features": 5100273670, "status-names": [ "acknowledge", "driver", "driver-ok", "features-ok" ], "common-features-names": [ {"name": "notify-on-empty", "acked": false}, {"name": "any-layout", "acked": false}, {"name": "indirect-desc", "acked": true}, {"name": "event-idx", "acked": true}, {"name": "bad-feature", "acked": false}, {"name": "version-1", "acked": true} ], "device-features-names": [ {"name": "hotplug", "acked": true}, {"name": "change", "acked": true} ] } ] } Eric Blake, returning to your question which would probably appear again after this mail: >> +## >> +# @query-virtio: >> ... >> +## >> +{ >> + 'command': 'query-virtio', >> + 'data': { '*path': 'str' }, > > Do we need filterable queries, or is it better to just have the > command return info on all virtio devices at once and let the client > filter the results as desired? I think it would be better to do here. I suppose, the client which uses HMP will not be happy on filtering monitor output. -- Best regards Jan Dakinevich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command 2017-12-12 13:54 ` [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich @ 2017-12-14 11:05 ` Cornelia Huck 2017-12-14 21:06 ` Jan Dakinevich 0 siblings, 1 reply; 8+ messages in thread From: Cornelia Huck @ 2017-12-14 11:05 UTC (permalink / raw) To: Jan Dakinevich Cc: qemu-devel, Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Denis V. Lunev, Max Reitz, Stefan Hajnoczi, Paolo Bonzini On Tue, 12 Dec 2017 16:54:50 +0300 Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote: > I am going to reanimate works under this QMP/HMP. First of all, it > could be meaningful to settle what output would provide the QMP. I would > like to suggest the following description: > > ## > # @VirtioFeature: > ## > { > 'struct': 'VirtioFeature', > 'data': { > 'name': 'str', > 'acked': 'bool' > } > } > > ## > # @VirtioInfo: > ## > { > 'struct': 'VirtioInfo', > 'data': { > 'qom-path': 'str', > > 'status': 'uint8', > 'host-features': 'uint64', > 'guest-features': 'uint64', > > 'status-names': ['str'], > 'common-features-names': ['VirtioFeature'], > 'device-features-names': ['VirtioFeature'] > } > } > > ## > # @query-virtio: > ## > { > 'command': 'query-virtio', > 'data': {'*path': 'str'}, > 'returns': ['VirtioInfo'] > } > > My final goal is to implement HMP which will print all exposed virtio > features (both common and device-specific) with their acknowledgements, > and virtio device configuration status. These are provided by last 3 > fields in @VirtioInfo. > > For these who are going make own decision on features and status > bitmask, respective fields with raw values are preserved. Looks sensible. What will you return an for the *-features-names fields if the status field indicates that negotiation is not yet done? (This has some fun interaction with the VERSION_1 feature bit...) > > So, I expect following data on the wire in response to `query-virtio' > command: > > { > "return": [ > { > "qom-path": "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15, > "host-features": 6325010438, > "guest-features": 5100273670, > "status-names": [ > "acknowledge", > "driver", > "driver-ok", > "features-ok" > ], > "common-features-names": [ > {"name": "notify-on-empty", "acked": false}, > {"name": "any-layout", "acked": false}, > {"name": "indirect-desc", "acked": true}, > {"name": "event-idx", "acked": true}, > {"name": "bad-feature", "acked": false}, > {"name": "version-1", "acked": true} > ], > "device-features-names": [ > {"name": "hotplug", "acked": true}, > {"name": "change", "acked": true} I suggest to use the #defines as names, especially as they are also used in the spec. Makes grepping easier. > ] > } > ] > } > > > Eric Blake, returning to your question which would probably appear again > after this mail: > > >> +## > >> +# @query-virtio: > >> ... > >> +## > >> +{ > >> + 'command': 'query-virtio', > >> + 'data': { '*path': 'str' }, > > > > Do we need filterable queries, or is it better to just have the > > command return info on all virtio devices at once and let the client > > filter the results as desired? > > I think it would be better to do here. I suppose, the client which uses > HMP will not be happy on filtering monitor output. I think being able to optionally specify a path is the most flexible solution. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command 2017-12-14 11:05 ` Cornelia Huck @ 2017-12-14 21:06 ` Jan Dakinevich 2017-12-15 14:41 ` Cornelia Huck 0 siblings, 1 reply; 8+ messages in thread From: Jan Dakinevich @ 2017-12-14 21:06 UTC (permalink / raw) To: Cornelia Huck Cc: qemu-devel, Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Denis V. Lunev, Max Reitz, Stefan Hajnoczi, Paolo Bonzini On Thu, 14 Dec 2017 12:05:19 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > On Tue, 12 Dec 2017 16:54:50 +0300 > Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote: > > > I am going to reanimate works under this QMP/HMP. First of all, it > > could be meaningful to settle what output would provide the QMP. I > > would like to suggest the following description: > > > > ## > > # @VirtioFeature: > > ## > > { > > 'struct': 'VirtioFeature', > > 'data': { > > 'name': 'str', > > 'acked': 'bool' > > } > > } > > > > ## > > # @VirtioInfo: > > ## > > { > > 'struct': 'VirtioInfo', > > 'data': { > > 'qom-path': 'str', > > > > 'status': 'uint8', > > 'host-features': 'uint64', > > 'guest-features': 'uint64', > > > > 'status-names': ['str'], > > 'common-features-names': ['VirtioFeature'], > > 'device-features-names': ['VirtioFeature'] > > } > > } > > > > ## > > # @query-virtio: > > ## > > { > > 'command': 'query-virtio', > > 'data': {'*path': 'str'}, > > 'returns': ['VirtioInfo'] > > } > > > > My final goal is to implement HMP which will print all exposed > > virtio features (both common and device-specific) with their > > acknowledgements, and virtio device configuration status. These are > > provided by last 3 fields in @VirtioInfo. > > > > For these who are going make own decision on features and status > > bitmask, respective fields with raw values are preserved. > > Looks sensible. What will you return an for the *-features-names > fields if the status field indicates that negotiation is not yet done? > (This has some fun interaction with the VERSION_1 feature bit...) > Hmm... I was going to return current features and theirs acknowledgments regardless if they were negotiated. Thus, *-features-names would contain all exposed host features with `false' in `acked' field. > > > > So, I expect following data on the wire in response to > > `query-virtio' command: > > > > { > > "return": [ > > { > > "qom-path": > > "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15, > > "host-features": 6325010438, "guest-features": 5100273670, > > "status-names": [ > > "acknowledge", > > "driver", > > "driver-ok", > > "features-ok" > > ], > > "common-features-names": [ > > {"name": "notify-on-empty", "acked": false}, > > {"name": "any-layout", "acked": false}, > > {"name": "indirect-desc", "acked": true}, > > {"name": "event-idx", "acked": true}, > > {"name": "bad-feature", "acked": false}, > > {"name": "version-1", "acked": true} > > ], > > "device-features-names": [ > > {"name": "hotplug", "acked": true}, > > {"name": "change", "acked": true} > > I suggest to use the #defines as names, especially as they are also > used in the spec. Makes grepping easier. > You mean, for example "VIRTIO_RING_F_EVENT_IDX" instead of "event-idx" should be used. Right? > > ] > > } > > ] > > } > > > > > > Eric Blake, returning to your question which would probably appear > > again after this mail: > > > > >> +## > > >> +# @query-virtio: > > >> ... > > >> +## > > >> +{ > > >> + 'command': 'query-virtio', > > >> + 'data': { '*path': 'str' }, > > > > > > Do we need filterable queries, or is it better to just have the > > > command return info on all virtio devices at once and let the > > > client filter the results as desired? > > > > I think it would be better to do here. I suppose, the client which > > uses HMP will not be happy on filtering monitor output. > > I think being able to optionally specify a path is the most flexible > solution. -- Best regards Jan Dakinevich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command 2017-12-14 21:06 ` Jan Dakinevich @ 2017-12-15 14:41 ` Cornelia Huck 0 siblings, 0 replies; 8+ messages in thread From: Cornelia Huck @ 2017-12-15 14:41 UTC (permalink / raw) To: Jan Dakinevich Cc: qemu-devel, Dr. David Alan Gilbert, Michael S. Tsirkin, Eric Blake, Markus Armbruster, Kevin Wolf, Denis V. Lunev, Max Reitz, Stefan Hajnoczi, Paolo Bonzini On Fri, 15 Dec 2017 00:06:06 +0300 Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote: > On Thu, 14 Dec 2017 12:05:19 +0100 > Cornelia Huck <cohuck@redhat.com> wrote: > > > On Tue, 12 Dec 2017 16:54:50 +0300 > > Jan Dakinevich <jan.dakinevich@virtuozzo.com> wrote: > > > > > I am going to reanimate works under this QMP/HMP. First of all, it > > > could be meaningful to settle what output would provide the QMP. I > > > would like to suggest the following description: > > > > > > ## > > > # @VirtioFeature: > > > ## > > > { > > > 'struct': 'VirtioFeature', > > > 'data': { > > > 'name': 'str', > > > 'acked': 'bool' > > > } > > > } > > > > > > ## > > > # @VirtioInfo: > > > ## > > > { > > > 'struct': 'VirtioInfo', > > > 'data': { > > > 'qom-path': 'str', > > > > > > 'status': 'uint8', > > > 'host-features': 'uint64', > > > 'guest-features': 'uint64', > > > > > > 'status-names': ['str'], > > > 'common-features-names': ['VirtioFeature'], > > > 'device-features-names': ['VirtioFeature'] > > > } > > > } > > > > > > ## > > > # @query-virtio: > > > ## > > > { > > > 'command': 'query-virtio', > > > 'data': {'*path': 'str'}, > > > 'returns': ['VirtioInfo'] > > > } > > > > > > My final goal is to implement HMP which will print all exposed > > > virtio features (both common and device-specific) with their > > > acknowledgements, and virtio device configuration status. These are > > > provided by last 3 fields in @VirtioInfo. > > > > > > For these who are going make own decision on features and status > > > bitmask, respective fields with raw values are preserved. > > > > Looks sensible. What will you return an for the *-features-names > > fields if the status field indicates that negotiation is not yet done? > > (This has some fun interaction with the VERSION_1 feature bit...) > > > > Hmm... I was going to return current features and theirs > acknowledgments regardless if they were negotiated. Thus, > *-features-names would contain all exposed host features with `false' > in `acked' field. acked=false is probably the most sensible approach. > > > > > > > So, I expect following data on the wire in response to > > > `query-virtio' command: > > > > > > { > > > "return": [ > > > { > > > "qom-path": > > > "/machine/peripheral-anon/device[0]/virtio-backend", "status": 15, > > > "host-features": 6325010438, "guest-features": 5100273670, > > > "status-names": [ > > > "acknowledge", > > > "driver", > > > "driver-ok", > > > "features-ok" > > > ], > > > "common-features-names": [ > > > {"name": "notify-on-empty", "acked": false}, > > > {"name": "any-layout", "acked": false}, > > > {"name": "indirect-desc", "acked": true}, > > > {"name": "event-idx", "acked": true}, > > > {"name": "bad-feature", "acked": false}, > > > {"name": "version-1", "acked": true} > > > ], > > > "device-features-names": [ > > > {"name": "hotplug", "acked": true}, > > > {"name": "change", "acked": true} > > > > I suggest to use the #defines as names, especially as they are also > > used in the spec. Makes grepping easier. > > > > You mean, for example "VIRTIO_RING_F_EVENT_IDX" instead of "event-idx" > should be used. Right? Right, that's what I meant. > > > > ] > > > } > > > ] > > > } > > > > > > > > > Eric Blake, returning to your question which would probably appear > > > again after this mail: > > > > > > >> +## > > > >> +# @query-virtio: > > > >> ... > > > >> +## > > > >> +{ > > > >> + 'command': 'query-virtio', > > > >> + 'data': { '*path': 'str' }, > > > > > > > > Do we need filterable queries, or is it better to just have the > > > > command return info on all virtio devices at once and let the > > > > client filter the results as desired? > > > > > > I think it would be better to do here. I suppose, the client which > > > uses HMP will not be happy on filtering monitor output. > > > > I think being able to optionally specify a path is the most flexible > > solution. > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-12-15 14:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-24 17:50 [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich 2017-11-02 18:52 ` Eric Blake 2017-10-24 17:50 ` [Qemu-devel] [RFC v5 2/2] virtio: add `info virtio' HMP command Jan Dakinevich 2017-12-12 13:54 ` [Qemu-devel] [RFC v5 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-12-14 11:05 ` Cornelia Huck 2017-12-14 21:06 ` Jan Dakinevich 2017-12-15 14:41 ` Cornelia Huck
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).