On 5/7/20 6:49 AM, Laurent Vivier wrote:
Display feature names instead of a features bitmap for host, guest
and backend.
Decode features according device type, transport features are
on the first line. Undecoded bits (if any) are stored in a separate
field.
Signed-off-by: Laurent Vivier
---
I didn't closely review the code, but for the QAPI parts:
+++ b/qapi/virtio.json
+##
+# @VirtioBlkFeature:
+#
+# An enumeration of Virtio block features
+#
+# Since: 5.1
+##
+
+{ 'enum': 'VirtioBlkFeature',
+ 'data': [ 'size-max', 'seg-max', 'geometry', 'ro', 'blk-size', 'topology',
'mq', 'discard', 'write-zeroes', 'barrier', 'scsi', 'flush',
Missing newline.
+##
+# @VirtioDeviceFeatures:
+#
+# An union to define the list of features for a virtio device
s/An/A/ (in English, 'an' goes with soft u, 'a' goes with pronounced u.
You can remember with "a unicorn gets an umbrella")
+#
+# Since: 5.1
+##
+
+{ 'union': 'VirtioDeviceFeatures',
+ 'data': {
+'virtio-serial': [ 'VirtioSerialFeature' ],
+'virtio-blk': [ 'VirtioBlkFeature' ],
+'virtio-gpu': [ 'VirtioGpuFeature' ],
+'virtio-net': [ 'VirtioNetFeature' ],
+'virtio-scsi': [ 'VirtioScsiFeature' ],
+'virtio-balloon': [ 'VirtioBalloonFeature' ],
+'virtio-iommu': [ 'VirtioIommuFeature' ]
+ }
+}
This is a legacy union rather than a flat union, which results in more
{} in the QMP wire format. Is it worth trying to make this a flat
union, by labeling an appropriate member as 'discriminator'?
+
+##
+# @VirtioStatusFeatures:
+#
+# @transport: the list of transport features of the virtio device
+#
+# @device: the list of the virtio device specific features
+#
+# @unknown: virtio bitmap of the undecoded features
+#
+# Since: 5.1
+##
+
+{ 'struct': 'VirtioStatusFeatures',
+ 'data': { 'transport': [ 'VirtioTransportFeature' ],
+'device': 'VirtioDeviceFeatures',
+'unknown': 'uint64' }
+}
+
##
# @VirtioStatus:
#
@@ -101,9 +245,9 @@
'data': {
'device-id': 'int',
'device-endian': 'VirtioStatusEndianness',
-'guest-features': 'uint64',
-'host-features': 'uint64',
-'backend-features': 'uint64',
+'guest-features': 'VirtioStatusFeatures',
+'host-features': 'VirtioStatusFeatures',
+'backend-features': 'VirtioStatusFeatures',
This is intra-series churn. Should we be trying to get the right types
in place from the get-go? Or at least clarify in the commit message of
the earlier patch that the format will be incrementally improved later?
'num-vqs': 'uint16'
}
}
@@ -123,18 +267,40 @@
#
# -> { "execute": "x-debug-virtio-status",
# "arguments": {
-# "path": "/machine/peripheral-anon/device[3]/virtio-backend"
+# "path": "/machine/peripheral-anon/device[1]/virtio-backend"
# }
# }
# <- { "return": {
-# "backend-features": 0,
-# "guest-features": 5111807911,
-# "num-vqs": 3,
-# "host-features": 6337593319,
# "device-endian": "little",
-# "device-id": 1
+# "device-id": 3,
+# "backend-features": {
+# "device": {
+# "type": "virtio-serial",
+# "data": []
+# },
+# "unknown": 0,
+# "transport": []
+# },
If we use a flat union, this could look like:
"backend-feature": {
"type": "virtio-serial",
"features": [],
"unknown": 0,
"transport": []
},
Should 'unknown' be optional so it can be omitted when zero? Should it
be named 'unknown-features'?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
