* [Qemu-devel] [PATCH v4 0/2] virtio: introduce `info virtio' hmp command @ 2017-10-03 12:47 Jan Dakinevich 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 2/2] virtio: add `info virtio' HMP command Jan Dakinevich 0 siblings, 2 replies; 11+ messages in thread From: Jan Dakinevich @ 2017-10-03 12:47 UTC (permalink / raw) To: qemu-devel Cc: Cornelia Huck, Kevin Wolf, Dr. David Alan Gilbert, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Jan Dakinevich v4: * Implement `query-virtio' QMP * Split the work into QMP and HMP parts 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 | 79 ++++++++++++++++++++++++++++++++++ hmp.h | 1 + hw/block/virtio-blk.c | 21 +++++++++ hw/char/virtio-serial-bus.c | 15 +++++++ hw/display/virtio-gpu.c | 13 ++++++ hw/net/virtio-net.c | 35 +++++++++++++++ hw/scsi/virtio-scsi.c | 16 +++++++ hw/virtio/Makefile.objs | 2 + hw/virtio/virtio-balloon.c | 15 +++++++ hw/virtio/virtio-stub.c | 9 ++++ hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 + qapi-schema.json | 1 + qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ 15 files changed, 418 insertions(+) create mode 100644 hw/virtio/virtio-stub.c create mode 100644 qapi/virtio.json -- 2.1.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 12:47 [Qemu-devel] [PATCH v4 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich @ 2017-10-03 12:47 ` Jan Dakinevich 2017-10-03 14:02 ` Eric Blake 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 2/2] virtio: add `info virtio' HMP command Jan Dakinevich 1 sibling, 1 reply; 11+ messages in thread From: Jan Dakinevich @ 2017-10-03 12:47 UTC (permalink / raw) To: qemu-devel Cc: Cornelia Huck, Kevin Wolf, Dr. David Alan Gilbert, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Jan Dakinevich, Michael S. Tsirkin, Eric Blake, qemu-block 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. To retrieve names of device-specific features `get_feature_name' callback in VirtioDeviceClass also was introduced. Cc: Denis V. Lunev <den@virtuozzo.com> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> --- hw/block/virtio-blk.c | 21 +++++++++ hw/char/virtio-serial-bus.c | 15 +++++++ hw/display/virtio-gpu.c | 13 ++++++ hw/net/virtio-net.c | 35 +++++++++++++++ hw/scsi/virtio-scsi.c | 16 +++++++ hw/virtio/Makefile.objs | 2 + hw/virtio/virtio-balloon.c | 15 +++++++ hw/virtio/virtio-stub.c | 9 ++++ hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 + qapi-schema.json | 1 + qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ 12 files changed, 324 insertions(+) create mode 100644 hw/virtio/virtio-stub.c create mode 100644 qapi/virtio.json diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 05d1440..c77f651 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1017,6 +1017,26 @@ static Property virtio_blk_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_blk_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_BLK_F_BARRIER] = "barrier", + [VIRTIO_BLK_F_SIZE_MAX] = "size_max", + [VIRTIO_BLK_F_SEG_MAX] = "seg_max", + [VIRTIO_BLK_F_RO] = "ro", + [VIRTIO_BLK_F_BLK_SIZE] = "blk_size", + [VIRTIO_BLK_F_SCSI] = "scsi", + [VIRTIO_BLK_F_TOPOLOGY] = "topology", + [VIRTIO_BLK_F_FLUSH] = "flush", + [VIRTIO_BLK_F_MQ] = "mq", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_blk_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1030,6 +1050,7 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_blk_update_config; vdc->set_config = virtio_blk_set_config; vdc->get_features = virtio_blk_get_features; + vdc->get_feature_name = virtio_blk_get_feature_name; vdc->set_status = virtio_blk_set_status; vdc->reset = virtio_blk_reset; vdc->save = virtio_blk_save_device; diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 9470bd7..41f4466 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -1156,6 +1156,20 @@ static Property virtio_serial_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_serial_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_CONSOLE_F_SIZE] = "size", + [VIRTIO_CONSOLE_F_MULTIPORT] = "multiport", + [VIRTIO_CONSOLE_F_EMERG_WRITE] = "emerg_write", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_serial_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1170,6 +1184,7 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data) vdc->realize = virtio_serial_device_realize; vdc->unrealize = virtio_serial_device_unrealize; vdc->get_features = get_features; + vdc->get_feature_name = virtio_serial_get_feature_name; vdc->get_config = get_config; vdc->set_config = set_config; vdc->set_status = set_status; diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 3a8f1e1..49aa214 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1307,6 +1307,18 @@ static Property virtio_gpu_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_gpu_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_GPU_F_VIRGL] = "virgl", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_gpu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1317,6 +1329,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_gpu_get_config; vdc->set_config = virtio_gpu_set_config; vdc->get_features = virtio_gpu_get_features; + vdc->get_feature_name = virtio_gpu_get_feature_name; vdc->set_features = virtio_gpu_set_features; vdc->reset = virtio_gpu_reset; diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 148071a..f4d20a85 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -2154,6 +2154,40 @@ static Property virtio_net_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_net_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_NET_F_CSUM] = "csum", + [VIRTIO_NET_F_GUEST_CSUM] = "guest_csum", + [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "ctrl_guest_offloads", + [VIRTIO_NET_F_MTU] = "mtu", + [VIRTIO_NET_F_MAC] = "mac", + [VIRTIO_NET_F_GSO] = "gso", + [VIRTIO_NET_F_GUEST_TSO4] = "guest_tso4", + [VIRTIO_NET_F_GUEST_TSO6] = "guest_tso6", + [VIRTIO_NET_F_GUEST_ECN] = "guest_ecn", + [VIRTIO_NET_F_GUEST_UFO] = "guest_ufo", + [VIRTIO_NET_F_HOST_TSO4] = "host_tso4", + [VIRTIO_NET_F_HOST_TSO6] = "host_tso6", + [VIRTIO_NET_F_HOST_ECN] = "host_ecn", + [VIRTIO_NET_F_HOST_UFO] = "host_ufo", + [VIRTIO_NET_F_MRG_RXBUF] = "mrg_rxbuf", + [VIRTIO_NET_F_STATUS] = "status", + [VIRTIO_NET_F_CTRL_VQ] = "ctrl_vq", + [VIRTIO_NET_F_CTRL_RX] = "ctrl_rx", + [VIRTIO_NET_F_CTRL_VLAN] = "ctrl_vlan", + [VIRTIO_NET_F_CTRL_RX_EXTRA] = "ctrl_rx_extra", + [VIRTIO_NET_F_GUEST_ANNOUNCE] = "guest_announce", + [VIRTIO_NET_F_MQ] = "mq", + [VIRTIO_NET_F_CTRL_MAC_ADDR] = "ctrl_mac_addr", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_net_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -2169,6 +2203,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data) vdc->get_features = virtio_net_get_features; vdc->set_features = virtio_net_set_features; vdc->bad_features = virtio_net_bad_features; + vdc->get_feature_name = virtio_net_get_feature_name; vdc->reset = virtio_net_reset; vdc->set_status = virtio_net_set_status; vdc->guest_notifier_mask = virtio_net_guest_notifier_mask; diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3aa9971..b53cf8b 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -942,6 +942,21 @@ static const VMStateDescription vmstate_virtio_scsi = { }, }; +static const char *virtio_scsi_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_SCSI_F_INOUT] = "inout", + [VIRTIO_SCSI_F_HOTPLUG] = "hotplug", + [VIRTIO_SCSI_F_CHANGE] = "change", + [VIRTIO_SCSI_F_T10_PI] = "t10_pi", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_scsi_common_class_init(ObjectClass *klass, void *data) { VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); @@ -964,6 +979,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data) vdc->unrealize = virtio_scsi_device_unrealize; vdc->set_config = virtio_scsi_set_config; vdc->get_features = virtio_scsi_get_features; + vdc->get_feature_name = virtio_scsi_get_feature_name; vdc->reset = virtio_scsi_reset; vdc->start_ioeventfd = virtio_scsi_dataplane_start; vdc->stop_ioeventfd = virtio_scsi_dataplane_stop; diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index 765d363..73f3527 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -9,6 +9,8 @@ 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-balloon.c b/hw/virtio/virtio-balloon.c index 37cde38..b396eb1 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -509,6 +509,20 @@ static Property virtio_balloon_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_balloon_get_feature_name(VirtIODevice *vdev, + unsigned feature) +{ + static const char *names[] = { + [VIRTIO_BALLOON_F_MUST_TELL_HOST] = "must_tell_host", + [VIRTIO_BALLOON_F_STATS_VQ] = "stats_vq", + [VIRTIO_BALLOON_F_DEFLATE_ON_OOM] = "deflate_on_oom", + }; + if (feature >= ARRAY_SIZE(names)) { + return NULL; + } + return names[feature]; +} + static void virtio_balloon_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -523,6 +537,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_balloon_get_config; vdc->set_config = virtio_balloon_set_config; vdc->get_features = virtio_balloon_get_features; + vdc->get_feature_name = virtio_balloon_get_feature_name; vdc->set_status = virtio_balloon_set_status; vdc->vmsd = &vmstate_virtio_balloon_device; } diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c new file mode 100644 index 0000000..51a99de --- /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" + +VirtioInfo *qmp_query_virtio(Error **errp) +{ + error_setg(errp, QERR_UNSUPPORTED); + return NULL; +} diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 3129d25..f182aa7 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qmp-commands.h" #include "qemu-common.h" #include "cpu.h" #include "trace.h" @@ -2665,6 +2666,106 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev) return virtio_bus_ioeventfd_enabled(vbus); } +static void virtio_info_bit_list_add(VirtioInfoBitList **list, unsigned bit, + const char *name) +{ + VirtioInfoBitList *info = g_new0(VirtioInfoBitList, 1); + + info->next = *list; + *list = info; + + info->value = g_new0(VirtioInfoBit, 1); + info->value->bit = bit; + info->value->name = g_strdup(name); +} + +static void virtio_info_device_list_add(VirtioInfoDeviceList **list, + VirtIODevice *vdev) +{ + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); + VirtioInfoDeviceList *device; + unsigned idx; + + device = g_new0(VirtioInfoDeviceList, 1); + device->value = g_new0(VirtioInfoDevice, 1); + device->next = *list; + + *list = device; + + device->value->qom_path = object_get_canonical_path(OBJECT(vdev)); + device->value->host_features = vdev->host_features; + device->value->guest_features = vdev->guest_features; + device->value->status = vdev->status; + + if (!vdc->get_feature_name) { + return; + } + + for (idx = 64; idx--; ) { + const char *name = vdc->get_feature_name(vdev, idx); + if (!name) { + continue; + } + virtio_info_bit_list_add(&device->value->feature_names, idx, name); + } +} + +static void qmp_query_virtio_recursive(VirtioInfo *info, 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)) { + virtio_info_device_list_add(&info->devices, VIRTIO_DEVICE(dev)); + } + QLIST_FOREACH(child, &dev->child_bus, sibling) { + qmp_query_virtio_recursive(info, child); + } + } +} + +VirtioInfo *qmp_query_virtio(Error **errp) +{ + BusState *bus = sysbus_get_default(); + VirtioInfo *info = g_new0(VirtioInfo, 1); + + /* Common features */ + virtio_info_bit_list_add(&info->feature_names, VIRTIO_F_IOMMU_PLATFORM, + "iommu_platform"); + virtio_info_bit_list_add(&info->feature_names, VIRTIO_F_VERSION_1, + "version_1"); + virtio_info_bit_list_add(&info->feature_names, VIRTIO_RING_F_EVENT_IDX, + "event_idx"); + virtio_info_bit_list_add(&info->feature_names, VIRTIO_RING_F_INDIRECT_DESC, + "indirect_desc"); + virtio_info_bit_list_add(&info->feature_names, VIRTIO_F_ANY_LAYOUT, + "any_layout"); + virtio_info_bit_list_add(&info->feature_names, VIRTIO_F_NOTIFY_ON_EMPTY, + "notify_on_empty"); + + /* Status bits */ + virtio_info_bit_list_add(&info->status_names, 7 /* VIRTIO_CONFIG_S_FAILED */, + "failed"); + virtio_info_bit_list_add(&info->status_names, 6 /* VIRTIO_CONFIG_S_NEEDS_RESET */, + "needs_reset"); + virtio_info_bit_list_add(&info->status_names, 3 /* VIRTIO_CONFIG_S_FEATURES_OK */, + "features_ok"); + virtio_info_bit_list_add(&info->status_names, 2 /* VIRTIO_CONFIG_S_DRIVER_OK */, + "driver_ok"); + virtio_info_bit_list_add(&info->status_names, 1 /* VIRTIO_CONFIG_S_DRIVER */, + "driver"); + virtio_info_bit_list_add(&info->status_names, 0 /* VIRTIO_CONFIG_S_ACKNOWLEDGE */, + "acknowledge"); + + if (bus) { + qmp_query_virtio_recursive(info, bus); + } + return info; +} + static const TypeInfo virtio_device_info = { .name = TYPE_VIRTIO_DEVICE, .parent = TYPE_DEVICE, diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 80c45c3..3ea2cd0 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -141,6 +141,8 @@ typedef struct VirtioDeviceClass { void (*save)(VirtIODevice *vdev, QEMUFile *f); int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id); const VMStateDescription *vmsd; + /* Get the name of device specific feature */ + const char *(*get_feature_name)(VirtIODevice *vdev, unsigned feature); } VirtioDeviceClass; void virtio_instance_init_common(Object *proxy_obj, void *data, diff --git a/qapi-schema.json b/qapi-schema.json index a3ba1c9..78762b1 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -92,6 +92,7 @@ { 'include': 'qapi/transaction.json' } { 'include': 'qapi/trace.json' } { 'include': 'qapi/introspect.json' } +{ 'include': 'qapi/virtio.json' } ## # = Miscellanea diff --git a/qapi/virtio.json b/qapi/virtio.json new file mode 100644 index 0000000..b8b52ac --- /dev/null +++ b/qapi/virtio.json @@ -0,0 +1,94 @@ +# -*- Mode: Python -*- +# + +## +# = Virtio devices +## + +{ 'include': 'common.json' } + +## +# @VirtioInfoBit: +# +# Named virtio bit +# +# @bit: bit number +# +# @name: bit name +# +# Since: 2.11.0 +# +## +{ + 'struct': 'VirtioInfoBit', + 'data': { + 'bit': 'uint64', + 'name': 'str' + } +} + +## +# @VirtioInfoDevice: +# +# Information about specific virtio device +# +# @qom_path: QOM path of the device +# +# @feature-names: names of device-specific features +# +# @host-features: bitmask of features, provided by devices +# +# @guest-features: bitmask of features, acknowledged by guest +# +# @status: virtio device status bitmask +# +# Since: 2.11.0 +# +## +{ + 'struct': 'VirtioInfoDevice', + 'data': { + 'qom_path': 'str', + 'feature-names': ['VirtioInfoBit'], + 'host-features': 'uint64', + 'guest-features': 'uint64', + 'status': 'uint64' + } +} + +## +# @VirtioInfo: +# +# Information about virtio devices +# +# @feature-names: names of common virtio features +# +# @status-names: names of bits which represents virtio device status +# +# @devices: list of per-device virtio information +# +# Since: 2.11.0 +# +## +{ + 'struct': 'VirtioInfo', + 'data': { + 'feature-names': ['VirtioInfoBit'], + 'status-names': ['VirtioInfoBit'], + 'devices': ['VirtioInfoDevice'] + } +} + + +## +# @query-virtio: +# +# Returns generic and per-device virtio information +# +# Since: 2.11.0 +# +## +{ + 'command': 'query-virtio', + 'returns': 'VirtioInfo' +} -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich @ 2017-10-03 14:02 ` Eric Blake 2017-10-03 14:32 ` Jan Dakinevich 0 siblings, 1 reply; 11+ messages in thread From: Eric Blake @ 2017-10-03 14:02 UTC (permalink / raw) To: Jan Dakinevich, qemu-devel Cc: Cornelia Huck, Kevin Wolf, Dr. David Alan Gilbert, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block [-- Attachment #1: Type: text/plain, Size: 4283 bytes --] On 10/03/2017 07:47 AM, 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. > > To retrieve names of device-specific features `get_feature_name' > callback in VirtioDeviceClass also was introduced. > > Cc: Denis V. Lunev <den@virtuozzo.com> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > --- > hw/block/virtio-blk.c | 21 +++++++++ > hw/char/virtio-serial-bus.c | 15 +++++++ > hw/display/virtio-gpu.c | 13 ++++++ > hw/net/virtio-net.c | 35 +++++++++++++++ > hw/scsi/virtio-scsi.c | 16 +++++++ > hw/virtio/Makefile.objs | 2 + > hw/virtio/virtio-balloon.c | 15 +++++++ > hw/virtio/virtio-stub.c | 9 ++++ > hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ > include/hw/virtio/virtio.h | 2 + > qapi-schema.json | 1 + > qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ > 12 files changed, 324 insertions(+) > create mode 100644 hw/virtio/virtio-stub.c > create mode 100644 qapi/virtio.json This creates a new .json file, but does not touch MAINTAINERS. Our idea in splitting the .json files was to make it easier for each sub-file that needs a specific maintainer in addition to the overall *.json line for QAPI maintainers, so this may deserve a MAINTAINERS entry. > +++ b/qapi/virtio.json > @@ -0,0 +1,94 @@ > +# -*- Mode: Python -*- > +# > + > +## > +# = Virtio devices > +## > + > +{ 'include': 'common.json' } > + > +## > +# @VirtioInfoBit: > +# > +# Named virtio bit > +# > +# @bit: bit number > +# > +# @name: bit name > +# > +# Since: 2.11.0 > +# > +## > +{ > + 'struct': 'VirtioInfoBit', > + 'data': { > + 'bit': 'uint64', Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, ...? The documentation on 'bit number' is rather sparse. > + 'name': 'str' Wouldn't an enum type be better than an open-ended string? > + } > +} > + > +## > +# @VirtioInfoDevice: > +# > +# Information about specific virtio device > +# > +# @qom_path: QOM path of the device Please make this 'qom-path' - new interfaces should prefer '-' over '_'. > +# > +# @feature-names: names of device-specific features > +# > +# @host-features: bitmask of features, provided by devices > +# > +# @guest-features: bitmask of features, acknowledged by guest > +# > +# @status: virtio device status bitmask > +# > +# Since: 2.11.0 > +# > +## > +{ > + 'struct': 'VirtioInfoDevice', > + 'data': { > + 'qom_path': 'str', > + 'feature-names': ['VirtioInfoBit'], > + 'host-features': 'uint64', > + 'guest-features': 'uint64', > + 'status': 'uint64' I'm wondering if this is the best representation (where the caller has to parse the integer and then lookup in feature-names what each bit of the integer represents). But I'm not sure I have anything better off the top of my head. > + } > +} > + > +## > +# @VirtioInfo: > +# > +# Information about virtio devices > +# > +# @feature-names: names of common virtio features > +# > +# @status-names: names of bits which represents virtio device status > +# > +# @devices: list of per-device virtio information > +# > +# Since: 2.11.0 > +# > +## > +{ > + 'struct': 'VirtioInfo', > + 'data': { > + 'feature-names': ['VirtioInfoBit'], Why is feature-names listed at two different nestings of the return value? > + 'status-names': ['VirtioInfoBit'], > + 'devices': ['VirtioInfoDevice'] > + } > +} > + > + > +## > +# @query-virtio: > +# > +# Returns generic and per-device virtio information > +# > +# Since: 2.11.0 > +# > +## > +{ > + 'command': 'query-virtio', > + '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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 14:02 ` Eric Blake @ 2017-10-03 14:32 ` Jan Dakinevich 2017-10-03 16:29 ` Dr. David Alan Gilbert 2017-10-06 5:36 ` Markus Armbruster 0 siblings, 2 replies; 11+ messages in thread From: Jan Dakinevich @ 2017-10-03 14:32 UTC (permalink / raw) To: Eric Blake, qemu-devel Cc: Cornelia Huck, Kevin Wolf, Dr. David Alan Gilbert, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block On 10/03/2017 05:02 PM, Eric Blake wrote: > On 10/03/2017 07:47 AM, 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. >> >> To retrieve names of device-specific features `get_feature_name' >> callback in VirtioDeviceClass also was introduced. >> >> Cc: Denis V. Lunev <den@virtuozzo.com> >> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> >> --- >> hw/block/virtio-blk.c | 21 +++++++++ >> hw/char/virtio-serial-bus.c | 15 +++++++ >> hw/display/virtio-gpu.c | 13 ++++++ >> hw/net/virtio-net.c | 35 +++++++++++++++ >> hw/scsi/virtio-scsi.c | 16 +++++++ >> hw/virtio/Makefile.objs | 2 + >> hw/virtio/virtio-balloon.c | 15 +++++++ >> hw/virtio/virtio-stub.c | 9 ++++ >> hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/virtio/virtio.h | 2 + >> qapi-schema.json | 1 + >> qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ >> 12 files changed, 324 insertions(+) >> create mode 100644 hw/virtio/virtio-stub.c >> create mode 100644 qapi/virtio.json > > This creates a new .json file, but does not touch MAINTAINERS. Our idea > in splitting the .json files was to make it easier for each sub-file > that needs a specific maintainer in addition to the overall *.json line > for QAPI maintainers, so this may deserve a MAINTAINERS entry. > Ok. >> +++ b/qapi/virtio.json >> @@ -0,0 +1,94 @@ >> +# -*- Mode: Python -*- >> +# >> + >> +## >> +# = Virtio devices >> +## >> + >> +{ 'include': 'common.json' } >> + >> +## >> +# @VirtioInfoBit: >> +# >> +# Named virtio bit >> +# >> +# @bit: bit number >> +# >> +# @name: bit name >> +# >> +# Since: 2.11.0 >> +# >> +## >> +{ >> + 'struct': 'VirtioInfoBit', >> + 'data': { >> + 'bit': 'uint64', > > Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, > ...? The documentation on 'bit number' is rather sparse. I would prefer `uint' here, but I don't see generic unsigned type (may be, I am mistaken). I could use uint8 here, though. > >> + 'name': 'str' > > Wouldn't an enum type be better than an open-ended string? > Bit names are not known here, they are obtained from virtio device implementations. >> + } >> +} >> + >> +## >> +# @VirtioInfoDevice: >> +# >> +# Information about specific virtio device >> +# >> +# @qom_path: QOM path of the device > > Please make this 'qom-path' - new interfaces should prefer '-' over '_'. Ok. >> +# >> +# @feature-names: names of device-specific features >> +# >> +# @host-features: bitmask of features, provided by devices >> +# >> +# @guest-features: bitmask of features, acknowledged by guest >> +# >> +# @status: virtio device status bitmask >> +# >> +# Since: 2.11.0 >> +# >> +## >> +{ >> + 'struct': 'VirtioInfoDevice', >> + 'data': { >> + 'qom_path': 'str', >> + 'feature-names': ['VirtioInfoBit'], >> + 'host-features': 'uint64', >> + 'guest-features': 'uint64', >> + 'status': 'uint64' > > I'm wondering if this is the best representation (where the caller has > to parse the integer and then lookup in feature-names what each bit of > the integer represents). But I'm not sure I have anything better off > the top of my head. > Consider it as way to tell caller about names of supported features. >> + } >> +} >> + >> +## >> +# @VirtioInfo: >> +# >> +# Information about virtio devices >> +# >> +# @feature-names: names of common virtio features >> +# >> +# @status-names: names of bits which represents virtio device status >> +# >> +# @devices: list of per-device virtio information >> +# >> +# Since: 2.11.0 >> +# >> +## >> +{ >> + 'struct': 'VirtioInfo', >> + 'data': { >> + 'feature-names': ['VirtioInfoBit'], > > Why is feature-names listed at two different nestings of the return value? > These are different feature names. First names are common and predefined for all devices. Second names are device-specific. >> + 'status-names': ['VirtioInfoBit'], >> + 'devices': ['VirtioInfoDevice'] >> + } >> +} >> + >> + >> +## >> +# @query-virtio: >> +# >> +# Returns generic and per-device virtio information >> +# >> +# Since: 2.11.0 >> +# >> +## >> +{ >> + 'command': 'query-virtio', >> + 'returns': 'VirtioInfo' >> +} >> > -- Best regards Jan Dakinevich ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 14:32 ` Jan Dakinevich @ 2017-10-03 16:29 ` Dr. David Alan Gilbert 2017-10-04 14:26 ` Jan Dakinevich 2017-10-06 5:36 ` Markus Armbruster 1 sibling, 1 reply; 11+ messages in thread From: Dr. David Alan Gilbert @ 2017-10-03 16:29 UTC (permalink / raw) To: Jan Dakinevich Cc: Eric Blake, qemu-devel, Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block * Jan Dakinevich (jan.dakinevich@virtuozzo.com) wrote: > > > On 10/03/2017 05:02 PM, Eric Blake wrote: > > On 10/03/2017 07:47 AM, 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. > >> > >> To retrieve names of device-specific features `get_feature_name' > >> callback in VirtioDeviceClass also was introduced. > >> > >> Cc: Denis V. Lunev <den@virtuozzo.com> > >> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > >> --- > >> hw/block/virtio-blk.c | 21 +++++++++ > >> hw/char/virtio-serial-bus.c | 15 +++++++ > >> hw/display/virtio-gpu.c | 13 ++++++ > >> hw/net/virtio-net.c | 35 +++++++++++++++ > >> hw/scsi/virtio-scsi.c | 16 +++++++ > >> hw/virtio/Makefile.objs | 2 + > >> hw/virtio/virtio-balloon.c | 15 +++++++ > >> hw/virtio/virtio-stub.c | 9 ++++ > >> hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ > >> include/hw/virtio/virtio.h | 2 + > >> qapi-schema.json | 1 + > >> qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ > >> 12 files changed, 324 insertions(+) > >> create mode 100644 hw/virtio/virtio-stub.c > >> create mode 100644 qapi/virtio.json > > > > This creates a new .json file, but does not touch MAINTAINERS. Our idea > > in splitting the .json files was to make it easier for each sub-file > > that needs a specific maintainer in addition to the overall *.json line > > for QAPI maintainers, so this may deserve a MAINTAINERS entry. > > > > Ok. > > >> +++ b/qapi/virtio.json > >> @@ -0,0 +1,94 @@ > >> +# -*- Mode: Python -*- > >> +# > >> + > >> +## > >> +# = Virtio devices > >> +## > >> + > >> +{ 'include': 'common.json' } > >> + > >> +## > >> +# @VirtioInfoBit: > >> +# > >> +# Named virtio bit > >> +# > >> +# @bit: bit number > >> +# > >> +# @name: bit name > >> +# > >> +# Since: 2.11.0 > >> +# > >> +## > >> +{ > >> + 'struct': 'VirtioInfoBit', > >> + 'data': { > >> + 'bit': 'uint64', > > > > Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, > > ...? The documentation on 'bit number' is rather sparse. > > I would prefer `uint' here, but I don't see generic unsigned type (may > be, I am mistaken). I could use uint8 here, though. > > > > >> + 'name': 'str' > > > > Wouldn't an enum type be better than an open-ended string? > > > > Bit names are not known here, they are obtained from virtio device > implementations. > > >> + } > >> +} > >> + > >> +## > >> +# @VirtioInfoDevice: > >> +# > >> +# Information about specific virtio device > >> +# > >> +# @qom_path: QOM path of the device > > > > Please make this 'qom-path' - new interfaces should prefer '-' over '_'. > > Ok. > > >> +# > >> +# @feature-names: names of device-specific features > >> +# > >> +# @host-features: bitmask of features, provided by devices > >> +# > >> +# @guest-features: bitmask of features, acknowledged by guest > >> +# > >> +# @status: virtio device status bitmask > >> +# > >> +# Since: 2.11.0 > >> +# > >> +## > >> +{ > >> + 'struct': 'VirtioInfoDevice', > >> + 'data': { > >> + 'qom_path': 'str', > >> + 'feature-names': ['VirtioInfoBit'], > >> + 'host-features': 'uint64', > >> + 'guest-features': 'uint64', > >> + 'status': 'uint64' > > > > I'm wondering if this is the best representation (where the caller has > > to parse the integer and then lookup in feature-names what each bit of > > the integer represents). But I'm not sure I have anything better off > > the top of my head. > > > > Consider it as way to tell caller about names of supported features. > > >> + } > >> +} > >> + > >> +## > >> +# @VirtioInfo: > >> +# > >> +# Information about virtio devices > >> +# > >> +# @feature-names: names of common virtio features > >> +# > >> +# @status-names: names of bits which represents virtio device status > >> +# > >> +# @devices: list of per-device virtio information > >> +# > >> +# Since: 2.11.0 > >> +# > >> +## > >> +{ > >> + 'struct': 'VirtioInfo', > >> + 'data': { > >> + 'feature-names': ['VirtioInfoBit'], > > > > Why is feature-names listed at two different nestings of the return value? > > > > These are different feature names. First names are common and predefined > for all devices. Second names are device-specific. If you can turn these into enums (union'd enums?) then you might be able to get rid of a lot of your array filling/naming conversion boilerplate. (Not sure if it's worth it, but it's worth looking). Dave > >> + 'status-names': ['VirtioInfoBit'], > >> + 'devices': ['VirtioInfoDevice'] > >> + } > >> +} > >> + > >> + > >> +## > >> +# @query-virtio: > >> +# > >> +# Returns generic and per-device virtio information > >> +# > >> +# Since: 2.11.0 > >> +# > >> +## > >> +{ > >> + 'command': 'query-virtio', > >> + 'returns': 'VirtioInfo' > >> +} > >> > > > > -- > Best regards > Jan Dakinevich -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 16:29 ` Dr. David Alan Gilbert @ 2017-10-04 14:26 ` Jan Dakinevich 2017-10-04 16:00 ` Eric Blake 0 siblings, 1 reply; 11+ messages in thread From: Jan Dakinevich @ 2017-10-04 14:26 UTC (permalink / raw) To: Dr. David Alan Gilbert Cc: Eric Blake, qemu-devel, Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block On 10/03/2017 07:29 PM, Dr. David Alan Gilbert wrote: > * Jan Dakinevich (jan.dakinevich@virtuozzo.com) wrote: >> >> >> On 10/03/2017 05:02 PM, Eric Blake wrote: >>> On 10/03/2017 07:47 AM, 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. >>>> >>>> To retrieve names of device-specific features `get_feature_name' >>>> callback in VirtioDeviceClass also was introduced. >>>> >>>> Cc: Denis V. Lunev <den@virtuozzo.com> >>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> >>>> --- >>>> hw/block/virtio-blk.c | 21 +++++++++ >>>> hw/char/virtio-serial-bus.c | 15 +++++++ >>>> hw/display/virtio-gpu.c | 13 ++++++ >>>> hw/net/virtio-net.c | 35 +++++++++++++++ >>>> hw/scsi/virtio-scsi.c | 16 +++++++ >>>> hw/virtio/Makefile.objs | 2 + >>>> hw/virtio/virtio-balloon.c | 15 +++++++ >>>> hw/virtio/virtio-stub.c | 9 ++++ >>>> hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ >>>> include/hw/virtio/virtio.h | 2 + >>>> qapi-schema.json | 1 + >>>> qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ >>>> 12 files changed, 324 insertions(+) >>>> create mode 100644 hw/virtio/virtio-stub.c >>>> create mode 100644 qapi/virtio.json >>> >>> This creates a new .json file, but does not touch MAINTAINERS. Our idea >>> in splitting the .json files was to make it easier for each sub-file >>> that needs a specific maintainer in addition to the overall *.json line >>> for QAPI maintainers, so this may deserve a MAINTAINERS entry. >>> >> >> Ok. >> >>>> +++ b/qapi/virtio.json >>>> @@ -0,0 +1,94 @@ >>>> +# -*- Mode: Python -*- >>>> +# >>>> + >>>> +## >>>> +# = Virtio devices >>>> +## >>>> + >>>> +{ 'include': 'common.json' } >>>> + >>>> +## >>>> +# @VirtioInfoBit: >>>> +# >>>> +# Named virtio bit >>>> +# >>>> +# @bit: bit number >>>> +# >>>> +# @name: bit name >>>> +# >>>> +# Since: 2.11.0 >>>> +# >>>> +## >>>> +{ >>>> + 'struct': 'VirtioInfoBit', >>>> + 'data': { >>>> + 'bit': 'uint64', >>> >>> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, >>> ...? The documentation on 'bit number' is rather sparse. >> >> I would prefer `uint' here, but I don't see generic unsigned type (may >> be, I am mistaken). I could use uint8 here, though. >> >>> >>>> + 'name': 'str' >>> >>> Wouldn't an enum type be better than an open-ended string? >>> >> >> Bit names are not known here, they are obtained from virtio device >> implementations. >> >>>> + } >>>> +} >>>> + >>>> +## >>>> +# @VirtioInfoDevice: >>>> +# >>>> +# Information about specific virtio device >>>> +# >>>> +# @qom_path: QOM path of the device >>> >>> Please make this 'qom-path' - new interfaces should prefer '-' over '_'. >> >> Ok. >> >>>> +# >>>> +# @feature-names: names of device-specific features >>>> +# >>>> +# @host-features: bitmask of features, provided by devices >>>> +# >>>> +# @guest-features: bitmask of features, acknowledged by guest >>>> +# >>>> +# @status: virtio device status bitmask >>>> +# >>>> +# Since: 2.11.0 >>>> +# >>>> +## >>>> +{ >>>> + 'struct': 'VirtioInfoDevice', >>>> + 'data': { >>>> + 'qom_path': 'str', >>>> + 'feature-names': ['VirtioInfoBit'], >>>> + 'host-features': 'uint64', >>>> + 'guest-features': 'uint64', >>>> + 'status': 'uint64' >>> >>> I'm wondering if this is the best representation (where the caller has >>> to parse the integer and then lookup in feature-names what each bit of >>> the integer represents). But I'm not sure I have anything better off >>> the top of my head. >>> >> >> Consider it as way to tell caller about names of supported features. >> >>>> + } >>>> +} >>>> + >>>> +## >>>> +# @VirtioInfo: >>>> +# >>>> +# Information about virtio devices >>>> +# >>>> +# @feature-names: names of common virtio features >>>> +# >>>> +# @status-names: names of bits which represents virtio device status >>>> +# >>>> +# @devices: list of per-device virtio information >>>> +# >>>> +# Since: 2.11.0 >>>> +# >>>> +## >>>> +{ >>>> + 'struct': 'VirtioInfo', >>>> + 'data': { >>>> + 'feature-names': ['VirtioInfoBit'], >>> >>> Why is feature-names listed at two different nestings of the return value? >>> >> >> These are different feature names. First names are common and predefined >> for all devices. Second names are device-specific. > > If you can turn these into enums (union'd enums?) then you might > be able to get rid of a lot of your array filling/naming conversion > boilerplate. (Not sure if it's worth it, but it's worth looking). > I would be happy to drop this boilerplate, but how enum could help here? To respond my requirement it should be something like set, not enum. Even so, having set, I would have been needed to declare mapping between names in set type and bit numbers within feature bitmask. > Dave > >>>> + 'status-names': ['VirtioInfoBit'], >>>> + 'devices': ['VirtioInfoDevice'] >>>> + } >>>> +} >>>> + >>>> + >>>> +## >>>> +# @query-virtio: >>>> +# >>>> +# Returns generic and per-device virtio information >>>> +# >>>> +# Since: 2.11.0 >>>> +# >>>> +## >>>> +{ >>>> + 'command': 'query-virtio', >>>> + 'returns': 'VirtioInfo' >>>> +} >>>> >>> >> >> -- >> Best regards >> Jan Dakinevich > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > -- Best regards Jan Dakinevich ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-04 14:26 ` Jan Dakinevich @ 2017-10-04 16:00 ` Eric Blake 2017-10-05 16:55 ` Jan Dakinevich 0 siblings, 1 reply; 11+ messages in thread From: Eric Blake @ 2017-10-04 16:00 UTC (permalink / raw) To: Jan Dakinevich, Dr. David Alan Gilbert Cc: qemu-devel, Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block [-- Attachment #1: Type: text/plain, Size: 1616 bytes --] On 10/04/2017 09:26 AM, Jan Dakinevich wrote: >>>>> +{ >>>>> + 'struct': 'VirtioInfo', >>>>> + 'data': { >>>>> + 'feature-names': ['VirtioInfoBit'], >>>> >>>> Why is feature-names listed at two different nestings of the return value? >>>> >>> >>> These are different feature names. First names are common and predefined >>> for all devices. Second names are device-specific. >> >> If you can turn these into enums (union'd enums?) then you might >> be able to get rid of a lot of your array filling/naming conversion >> boilerplate. (Not sure if it's worth it, but it's worth looking). >> > > I would be happy to drop this boilerplate, but how enum could help here? > To respond my requirement it should be something like set, not enum. > Even so, having set, I would have been needed to declare mapping between > names in set type and bit numbers within feature bitmask. Instead of returning a bitmask ("mask":123) as well as an array naming those bits ([{"bit":1,"name":"bit1"},{"bit":2","name":"bit2"},{"bit":4,"name":"bit4},...]), you could omit the bit numbers and just return an array of named bits (["bit1", "bit2", "bit4"]). An enum lets you declare up front what named bits are supported (and code can introspect when new named bits are supported in newer qemu). Perhaps it's easier to first take a step back, and show what the desired output might be like, and then we can figure out how to represent that output in QAPI. -- 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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-04 16:00 ` Eric Blake @ 2017-10-05 16:55 ` Jan Dakinevich 0 siblings, 0 replies; 11+ messages in thread From: Jan Dakinevich @ 2017-10-05 16:55 UTC (permalink / raw) To: Eric Blake, Dr. David Alan Gilbert Cc: qemu-devel, Cornelia Huck, Kevin Wolf, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Michael S. Tsirkin, qemu-block On 10/04/2017 07:00 PM, Eric Blake wrote: > On 10/04/2017 09:26 AM, Jan Dakinevich wrote: > >>>>>> +{ >>>>>> + 'struct': 'VirtioInfo', >>>>>> + 'data': { >>>>>> + 'feature-names': ['VirtioInfoBit'], >>>>> >>>>> Why is feature-names listed at two different nestings of the return value? >>>>> >>>> >>>> These are different feature names. First names are common and predefined >>>> for all devices. Second names are device-specific. >>> >>> If you can turn these into enums (union'd enums?) then you might >>> be able to get rid of a lot of your array filling/naming conversion >>> boilerplate. (Not sure if it's worth it, but it's worth looking). >>> >> >> I would be happy to drop this boilerplate, but how enum could help here? >> To respond my requirement it should be something like set, not enum. >> Even so, having set, I would have been needed to declare mapping between >> names in set type and bit numbers within feature bitmask. > > Instead of returning a bitmask ("mask":123) as well as an array naming > those bits > ([{"bit":1,"name":"bit1"},{"bit":2","name":"bit2"},{"bit":4,"name":"bit4},...]), > you could omit the bit numbers and just return an array of named bits > (["bit1", "bit2", "bit4"]). An enum lets you declare up front what > named bits are supported (and code can introspect when new named bits > are supported in newer qemu). > But how can I declare in this notation that name "bit1" is owned by bit #1, name "bit2" is owned by bit #2, name "bit4" is owned by bit #4, and all other bits has no names. > Perhaps it's easier to first take a step back, and show what the desired > output might be like, and then we can figure out how to represent that > output in QAPI. > Yeah... I expect the following HMP output: (qmue) info virtio virtio-blk-device at 0000:00:07.0 status: 0x07 acknowledge,driver,driver_ok host features: 0x0000000179000e54 guest features: 0x0000000030000e54 common features: notify_on_empty any_layout indirect_desc acked event_idx acked version_1 specific features: seg_max acked blk_size acked flush acked topology acked virtio-serial-device at 0000:00:08.0 status: 0x07 acknowledge,driver,driver_ok host features: 0x0000000179000006 guest features: 0x0000000030000002 common features: notify_on_empty any_layout indirect_desc acked event_idx acked version_1 specific features: multiport acked emerg_write -- Best regards Jan Dakinevich ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-03 14:32 ` Jan Dakinevich 2017-10-03 16:29 ` Dr. David Alan Gilbert @ 2017-10-06 5:36 ` Markus Armbruster 2017-10-06 8:27 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 11+ messages in thread From: Markus Armbruster @ 2017-10-06 5:36 UTC (permalink / raw) To: Jan Dakinevich Cc: Eric Blake, qemu-devel, Kevin Wolf, Denis V. Lunev, qemu-block, Amit Shah, Jason Wang, Cornelia Huck, Dr. David Alan Gilbert, Max Reitz, Michael S. Tsirkin, Stefan Hajnoczi, Paolo Bonzini Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > On 10/03/2017 05:02 PM, Eric Blake wrote: >> On 10/03/2017 07:47 AM, 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. >>> >>> To retrieve names of device-specific features `get_feature_name' >>> callback in VirtioDeviceClass also was introduced. >>> >>> Cc: Denis V. Lunev <den@virtuozzo.com> >>> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> >>> --- >>> hw/block/virtio-blk.c | 21 +++++++++ >>> hw/char/virtio-serial-bus.c | 15 +++++++ >>> hw/display/virtio-gpu.c | 13 ++++++ >>> hw/net/virtio-net.c | 35 +++++++++++++++ >>> hw/scsi/virtio-scsi.c | 16 +++++++ >>> hw/virtio/Makefile.objs | 2 + >>> hw/virtio/virtio-balloon.c | 15 +++++++ >>> hw/virtio/virtio-stub.c | 9 ++++ >>> hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/virtio/virtio.h | 2 + >>> qapi-schema.json | 1 + >>> qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ >>> 12 files changed, 324 insertions(+) >>> create mode 100644 hw/virtio/virtio-stub.c >>> create mode 100644 qapi/virtio.json >> >> This creates a new .json file, but does not touch MAINTAINERS. Our idea >> in splitting the .json files was to make it easier for each sub-file >> that needs a specific maintainer in addition to the overall *.json line >> for QAPI maintainers, so this may deserve a MAINTAINERS entry. >> > > Ok. > >>> +++ b/qapi/virtio.json >>> @@ -0,0 +1,94 @@ >>> +# -*- Mode: Python -*- >>> +# >>> + >>> +## >>> +# = Virtio devices >>> +## >>> + >>> +{ 'include': 'common.json' } >>> + >>> +## >>> +# @VirtioInfoBit: >>> +# >>> +# Named virtio bit >>> +# >>> +# @bit: bit number >>> +# >>> +# @name: bit name >>> +# >>> +# Since: 2.11.0 >>> +# >>> +## >>> +{ >>> + 'struct': 'VirtioInfoBit', >>> + 'data': { >>> + 'bit': 'uint64', >> >> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, >> ...? The documentation on 'bit number' is rather sparse. > > I would prefer `uint' here, but I don't see generic unsigned type (may > be, I am mistaken). I could use uint8 here, though. > >> >>> + 'name': 'str' >> >> Wouldn't an enum type be better than an open-ended string? >> > > Bit names are not known here, they are obtained from virtio device > implementations. What exactly uses these bits? Why do these uses justify pass-through? By pass-through, I mean the messenger (QEMU) merely passes them along, without understanding them. Defeats introspection. >>> + } >>> +} >>> + >>> +## >>> +# @VirtioInfoDevice: >>> +# >>> +# Information about specific virtio device >>> +# >>> +# @qom_path: QOM path of the device >> >> Please make this 'qom-path' - new interfaces should prefer '-' over '_'. > > Ok. > >>> +# >>> +# @feature-names: names of device-specific features >>> +# >>> +# @host-features: bitmask of features, provided by devices >>> +# >>> +# @guest-features: bitmask of features, acknowledged by guest >>> +# >>> +# @status: virtio device status bitmask >>> +# >>> +# Since: 2.11.0 >>> +# >>> +## >>> +{ >>> + 'struct': 'VirtioInfoDevice', >>> + 'data': { >>> + 'qom_path': 'str', >>> + 'feature-names': ['VirtioInfoBit'], >>> + 'host-features': 'uint64', >>> + 'guest-features': 'uint64', >>> + 'status': 'uint64' >> >> I'm wondering if this is the best representation (where the caller has >> to parse the integer and then lookup in feature-names what each bit of >> the integer represents). But I'm not sure I have anything better off >> the top of my head. >> > > Consider it as way to tell caller about names of supported features. "Unsigned integer interpreted as combination of well-known bit-valued symbols" is a fine C interface, but a pretty horrid QMP interface. What's wrong with doing a set the straightforward way as "array of enum"? >>> + } >>> +} >>> + >>> +## >>> +# @VirtioInfo: >>> +# >>> +# Information about virtio devices >>> +# >>> +# @feature-names: names of common virtio features >>> +# >>> +# @status-names: names of bits which represents virtio device status >>> +# >>> +# @devices: list of per-device virtio information >>> +# >>> +# Since: 2.11.0 >>> +# >>> +## >>> +{ >>> + 'struct': 'VirtioInfo', >>> + 'data': { >>> + 'feature-names': ['VirtioInfoBit'], >> >> Why is feature-names listed at two different nestings of the return value? >> > > These are different feature names. First names are common and predefined > for all devices. Second names are device-specific. > >>> + 'status-names': ['VirtioInfoBit'], >>> + 'devices': ['VirtioInfoDevice'] >>> + } >>> +} >>> + >>> + >>> +## >>> +# @query-virtio: >>> +# >>> +# Returns generic and per-device virtio information >>> +# >>> +# Since: 2.11.0 >>> +# >>> +## >>> +{ >>> + 'command': 'query-virtio', >>> + 'returns': 'VirtioInfo' >>> +} >>> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command 2017-10-06 5:36 ` Markus Armbruster @ 2017-10-06 8:27 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 11+ messages in thread From: Dr. David Alan Gilbert @ 2017-10-06 8:27 UTC (permalink / raw) To: Markus Armbruster Cc: Jan Dakinevich, Eric Blake, qemu-devel, Kevin Wolf, Denis V. Lunev, qemu-block, Amit Shah, Jason Wang, Cornelia Huck, Max Reitz, Michael S. Tsirkin, Stefan Hajnoczi, Paolo Bonzini * Markus Armbruster (armbru@redhat.com) wrote: > Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > > > On 10/03/2017 05:02 PM, Eric Blake wrote: > >> On 10/03/2017 07:47 AM, 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. > >>> > >>> To retrieve names of device-specific features `get_feature_name' > >>> callback in VirtioDeviceClass also was introduced. > >>> > >>> Cc: Denis V. Lunev <den@virtuozzo.com> > >>> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > >>> --- > >>> hw/block/virtio-blk.c | 21 +++++++++ > >>> hw/char/virtio-serial-bus.c | 15 +++++++ > >>> hw/display/virtio-gpu.c | 13 ++++++ > >>> hw/net/virtio-net.c | 35 +++++++++++++++ > >>> hw/scsi/virtio-scsi.c | 16 +++++++ > >>> hw/virtio/Makefile.objs | 2 + > >>> hw/virtio/virtio-balloon.c | 15 +++++++ > >>> hw/virtio/virtio-stub.c | 9 ++++ > >>> hw/virtio/virtio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ > >>> include/hw/virtio/virtio.h | 2 + > >>> qapi-schema.json | 1 + > >>> qapi/virtio.json | 94 +++++++++++++++++++++++++++++++++++++++++ > >>> 12 files changed, 324 insertions(+) > >>> create mode 100644 hw/virtio/virtio-stub.c > >>> create mode 100644 qapi/virtio.json > >> > >> This creates a new .json file, but does not touch MAINTAINERS. Our idea > >> in splitting the .json files was to make it easier for each sub-file > >> that needs a specific maintainer in addition to the overall *.json line > >> for QAPI maintainers, so this may deserve a MAINTAINERS entry. > >> > > > > Ok. > > > >>> +++ b/qapi/virtio.json > >>> @@ -0,0 +1,94 @@ > >>> +# -*- Mode: Python -*- > >>> +# > >>> + > >>> +## > >>> +# = Virtio devices > >>> +## > >>> + > >>> +{ 'include': 'common.json' } > >>> + > >>> +## > >>> +# @VirtioInfoBit: > >>> +# > >>> +# Named virtio bit > >>> +# > >>> +# @bit: bit number > >>> +# > >>> +# @name: bit name > >>> +# > >>> +# Since: 2.11.0 > >>> +# > >>> +## > >>> +{ > >>> + 'struct': 'VirtioInfoBit', > >>> + 'data': { > >>> + 'bit': 'uint64', > >> > >> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8, > >> ...? The documentation on 'bit number' is rather sparse. > > > > I would prefer `uint' here, but I don't see generic unsigned type (may > > be, I am mistaken). I could use uint8 here, though. > > > >> > >>> + 'name': 'str' > >> > >> Wouldn't an enum type be better than an open-ended string? > >> > > > > Bit names are not known here, they are obtained from virtio device > > implementations. > > What exactly uses these bits? > > Why do these uses justify pass-through? By pass-through, I mean the > messenger (QEMU) merely passes them along, without understanding them. > Defeats introspection. It should be noted originally it was HMP - this was just a debug command and it's only getting the need to be introspectable because people insisted it had a QMP version. I think the intent is to print all flags, even ones we dont yet understand. Dave > >>> + } > >>> +} > >>> + > >>> +## > >>> +# @VirtioInfoDevice: > >>> +# > >>> +# Information about specific virtio device > >>> +# > >>> +# @qom_path: QOM path of the device > >> > >> Please make this 'qom-path' - new interfaces should prefer '-' over '_'. > > > > Ok. > > > >>> +# > >>> +# @feature-names: names of device-specific features > >>> +# > >>> +# @host-features: bitmask of features, provided by devices > >>> +# > >>> +# @guest-features: bitmask of features, acknowledged by guest > >>> +# > >>> +# @status: virtio device status bitmask > >>> +# > >>> +# Since: 2.11.0 > >>> +# > >>> +## > >>> +{ > >>> + 'struct': 'VirtioInfoDevice', > >>> + 'data': { > >>> + 'qom_path': 'str', > >>> + 'feature-names': ['VirtioInfoBit'], > >>> + 'host-features': 'uint64', > >>> + 'guest-features': 'uint64', > >>> + 'status': 'uint64' > >> > >> I'm wondering if this is the best representation (where the caller has > >> to parse the integer and then lookup in feature-names what each bit of > >> the integer represents). But I'm not sure I have anything better off > >> the top of my head. > >> > > > > Consider it as way to tell caller about names of supported features. > > "Unsigned integer interpreted as combination of well-known bit-valued > symbols" is a fine C interface, but a pretty horrid QMP interface. > What's wrong with doing a set the straightforward way as "array of > enum"? > > >>> + } > >>> +} > >>> + > >>> +## > >>> +# @VirtioInfo: > >>> +# > >>> +# Information about virtio devices > >>> +# > >>> +# @feature-names: names of common virtio features > >>> +# > >>> +# @status-names: names of bits which represents virtio device status > >>> +# > >>> +# @devices: list of per-device virtio information > >>> +# > >>> +# Since: 2.11.0 > >>> +# > >>> +## > >>> +{ > >>> + 'struct': 'VirtioInfo', > >>> + 'data': { > >>> + 'feature-names': ['VirtioInfoBit'], > >> > >> Why is feature-names listed at two different nestings of the return value? > >> > > > > These are different feature names. First names are common and predefined > > for all devices. Second names are device-specific. > > > >>> + 'status-names': ['VirtioInfoBit'], > >>> + 'devices': ['VirtioInfoDevice'] > >>> + } > >>> +} > >>> + > >>> + > >>> +## > >>> +# @query-virtio: > >>> +# > >>> +# Returns generic and per-device virtio information > >>> +# > >>> +# Since: 2.11.0 > >>> +# > >>> +## > >>> +{ > >>> + 'command': 'query-virtio', > >>> + 'returns': 'VirtioInfo' > >>> +} > >>> > >> -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 2/2] virtio: add `info virtio' HMP command 2017-10-03 12:47 [Qemu-devel] [PATCH v4 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich @ 2017-10-03 12:47 ` Jan Dakinevich 1 sibling, 0 replies; 11+ messages in thread From: Jan Dakinevich @ 2017-10-03 12:47 UTC (permalink / raw) To: qemu-devel Cc: Cornelia Huck, Kevin Wolf, Dr. David Alan Gilbert, Stefan Hajnoczi, Max Reitz, Amit Shah, Paolo Bonzini, Jason Wang, Markus Armbruster, Denis V. Lunev, Jan Dakinevich 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 | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ hmp.h | 1 + 3 files changed, 94 insertions(+) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 4f1ece9..2550027 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -868,6 +868,20 @@ ETEXI }, STEXI +@item info virtio +@findex virtio +Display guest and host fetures for all virtio devices. +ETEXI + + { + .name = "virtio", + .args_type = "", + .params = "", + .help = "show virtio info", + .cmd = hmp_info_virtio, + }, + +STEXI @end table ETEXI diff --git a/hmp.c b/hmp.c index ace729d..c4dd280 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> @@ -2894,3 +2895,81 @@ 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_FIELD 32 + +static void hmp_info_virtio_print_status(Monitor *mon, VirtioInfo *info, + VirtioInfoDevice *device) +{ + VirtioInfoBitList *lbit; + const char *comma = ""; + + for (lbit = info->status_names; lbit; lbit = lbit->next) { + if (!(device->status & (1ull << lbit->value->bit))) { + continue; + } + monitor_printf(mon, "%s%s", comma, lbit->value->name); + comma = ","; + } + monitor_printf(mon, "\n"); +} + +static void hmp_info_virtio_print_features(Monitor *mon, + VirtioInfoBitList *list, + VirtioInfoDevice *device) +{ + VirtioInfoBitList *lbit; + + for (lbit = list; lbit; lbit = lbit->next) { + const char *ack = virtio_has_feature(device->guest_features, + lbit->value->bit) ? "acked" : ""; + if (!virtio_has_feature(device->host_features, lbit->value->bit)) { + continue; + } + monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "", + HMP_INFO_VIRTIO_FIELD, lbit->value->name, + HMP_INFO_VIRTIO_FIELD, ack); + } +} + +static void hmp_info_virtio_print(Monitor *mon, VirtioInfo *info, + VirtioInfoDevice *device) +{ + Object *obj = object_resolve_path(device->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, "%*sstatus: 0x%02"PRIx64" ", + HMP_INFO_VIRTIO_INDENT, "", device->status); + hmp_info_virtio_print_status(mon, info, device); + + monitor_printf(mon, "%*shost features: 0x%016"PRIx64"\n", + HMP_INFO_VIRTIO_INDENT, "", device->host_features); + monitor_printf(mon, "%*sguest features: 0x%016"PRIx64"\n", + HMP_INFO_VIRTIO_INDENT, "", device->guest_features); + + monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, ""); + hmp_info_virtio_print_features(mon, info->feature_names, device); + + monitor_printf(mon, "%*sspecific features:\n", HMP_INFO_VIRTIO_INDENT, ""); + hmp_info_virtio_print_features(mon, device->feature_names, device); +} + +void hmp_info_virtio(Monitor *mon, const QDict *qdict) +{ + Error *err = NULL; + VirtioInfo *info; + VirtioInfoDeviceList *ldevice; + + info = qmp_query_virtio(&err); + if (err) { + return; + } + + for (ldevice = info->devices; ldevice; ldevice = ldevice->next) { + hmp_info_virtio_print(mon, info, ldevice->value); + } +} 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] 11+ messages in thread
end of thread, other threads:[~2017-10-06 8:28 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-03 12:47 [Qemu-devel] [PATCH v4 0/2] virtio: introduce `info virtio' hmp command Jan Dakinevich 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command Jan Dakinevich 2017-10-03 14:02 ` Eric Blake 2017-10-03 14:32 ` Jan Dakinevich 2017-10-03 16:29 ` Dr. David Alan Gilbert 2017-10-04 14:26 ` Jan Dakinevich 2017-10-04 16:00 ` Eric Blake 2017-10-05 16:55 ` Jan Dakinevich 2017-10-06 5:36 ` Markus Armbruster 2017-10-06 8:27 ` Dr. David Alan Gilbert 2017-10-03 12:47 ` [Qemu-devel] [PATCH v4 2/2] virtio: add `info virtio' HMP command Jan Dakinevich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.