| Message ID | 1513542355-28203-2-git-send-email-jan.dakinevich@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio: introduce `info virtio' hmp command | expand |
QAPI/QMP interface review only. You neglected to cc: the maintainers of qapi-schema.json, so I'm doing that for you. Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > 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 `tell_feature_name' > callback in VirtioDeviceClass also was introduced. > > Cc: Denis V. Lunev <den@virtuozzo.com> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> [...] > diff --git a/qapi-schema.json b/qapi-schema.json > index 1845795..51cd0f3 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3200,3 +3200,73 @@ > # Since: 2.11 > ## > { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} } > + > +## > +# @VirtioFeature: > +# > +# Virtio feature bit with negotiation status > +# > +# @name: name of feature bit > +# > +# @acked: negotiation status, `true' if guest acknowledged the feature > +# > +# Since: 2.12 > +## > +{ > + 'struct': 'VirtioFeature', > + 'data': { > + 'name': 'str', > + 'acked': 'bool' > + } > +} > + > +## > +# @VirtioInfo: > +# > +# Information about virtio device > +# > +# @qom-path: QOM path of the device > +# > +# @status: status bitmask > +# > +# @host-features: bitmask of features, exposed by device > +# > +# @guest-features: bitmask of features, acknowledged by guest Quoting my review of v4: "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"? As far as I can see, I didn't get a reply back then. You'll have to give one to get the patch accepted. In case symbolic names may not be known at QEMU compile time (me having to wonder at v6 is a sign of rather ineffective patch review, sorry about that): you have to explain this in the doc comment, with a reference to where the bits are specified. > +# > +# @status-names: names of checked bits in status bitmask How are the strings in @status-names connected to the bits in @status? Spell it out in this doc string, please. > +# > +# @common-features-names: names exposed features and negotiation status, > +# that are common to all devices > +# > +# @device-features-names: names exposed features and negotiation status, > +# that are specific to this device Likewise. > +# > +# Since: 2.12 > +## > +{ > + '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: > +# > +# Returns virtio information such as device status and features > +# > +# Since: 2.12 > +## > +{ > + 'command': 'query-virtio', > + 'data': {'*path': 'str'}, > + 'returns': ['VirtioInfo'] > +}
On Tue, 19 Dec 2017 15:57:18 +0100 Markus Armbruster <armbru@redhat.com> wrote: > QAPI/QMP interface review only. > > You neglected to cc: the maintainers of qapi-schema.json, so I'm doing > that for you. > Thank you so much. I screwed up with this patchset at least twice: with list of maintainers and with my own e-mail. > Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > > > 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 `tell_feature_name' > > callback in VirtioDeviceClass also was introduced. > > > > Cc: Denis V. Lunev <den@virtuozzo.com> > > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > [...] > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 1845795..51cd0f3 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -3200,3 +3200,73 @@ > > # Since: 2.11 > > ## > > { 'command': 'watchdog-set-action', 'data' : {'action': > > 'WatchdogAction'} } + > > +## > > +# @VirtioFeature: > > +# > > +# Virtio feature bit with negotiation status > > +# > > +# @name: name of feature bit > > +# > > +# @acked: negotiation status, `true' if guest acknowledged the > > feature +# > > +# Since: 2.12 > > +## > > +{ > > + 'struct': 'VirtioFeature', > > + 'data': { > > + 'name': 'str', > > + 'acked': 'bool' > > + } > > +} > > + > > +## > > +# @VirtioInfo: > > +# > > +# Information about virtio device > > +# > > +# @qom-path: QOM path of the device > > +# > > +# @status: status bitmask > > +# > > +# @host-features: bitmask of features, exposed by device > > +# > > +# @guest-features: bitmask of features, acknowledged by guest > > Quoting my review of v4: "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"? As far as I can see, I > didn't get a reply back then. You'll have to give one to get the > patch accepted. > Most probably, I didn't understand you last time. Although, I'm still far from correct understanding. About 'array of enum', I could make the following description: { 'enum': 'VirtioFeatureScsiBit', 'data': ['inout', 'hotplug', 'change', 'ta10_pi'] } { 'struct': 'VirtioFeatureScsi', 'data' : { 'bit': 'VirtioFeatureScsiBit', 'acked': 'bool' } } { 'enum': 'VirtioFeatureSerialBit', 'data': ['size', 'multiport', 'emerg-write'] } { 'struct': 'VirtioFeatureSerial', 'data' : { 'bit': 'VirtioFeatureSerialBit', 'acked': 'bool' } } { 'enum': 'VirtioFeatureCategory', 'data': ['scsi', 'serial'] } { 'union': 'VirtioFeature', 'base': { 'category': 'VirtioFeatureCategory' }, 'discriminator': 'categoty', 'data': { 'scsi': 'VirtioFeatureScsi', 'serial': 'VirtioFeatureSerial' } } { 'union': 'VirtioInfo', 'data': { 'qom-path': 'str', 'device-features': ['VirtioFeature'] ... } } But IMO, it only increases complexity without any benefit. To use this description, at first it should be listed all features names in enums. Then, these enums should be mapped into bit numbers and differently for each virtio device. I expect something like this: VirtioFeatureScsiBit bit = ... switch (bit) { case VIRTIO_FEATURE_SCSI_BIT_INOUT: return VIRTIO_SCSI_F_INOUT; case VIRTIO_FEATURE_SERIAL_BIT_HOTPLUG: return VIRTIO_SCSI_F_HOTPLUG; case VIRTIO_FEATURE_SERIAL_BIT_CHANGE: return VIRTIO_SCSI_F_CHANGE; case VIRTIO_FEATURE_SERIAL_BIT_T10_PI: return VIRTIO_SCSI_F_T10_PI; } So, boilerplate for mapping between bit numbers and and theirs names will not go away. Furthermore, HMP command should produce readable output from QMP data, features's enum values should be converted to strings from json description. For that, for each possible VirtioFeatureCategary should be called respective VirtioFeature*_str macro to resolve enum value. So, all my attempts to declare bit names within QAPI brings me to familiar solutions. > In case symbolic names may not be known at QEMU compile time (me > having to wonder at v6 is a sign of rather ineffective patch review, > sorry about that): you have to explain this in the doc comment, with a > reference to where the bits are specified. > ok > > +# > > +# @status-names: names of checked bits in status bitmask > > How are the strings in @status-names connected to the bits in @status? > Spell it out in this doc string, please. > ok ...but, strictly saying, I was not going to show in QAPI how these names are mapped into bits, QMP answer would not contain how these names are connected to bits. Otherwise, It would be reinvention of v4. > > +# > > +# @common-features-names: names exposed features and negotiation > > status, +# that are common to all devices > > +# > > +# @device-features-names: names exposed features and negotiation > > status, +# that are specific to this device > > Likewise. > ok > > +# > > +# Since: 2.12 > > +## > > +{ > > + '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: > > +# > > +# Returns virtio information such as device status and features > > +# > > +# Since: 2.12 > > +## > > +{ > > + 'command': 'query-virtio', > > + 'data': {'*path': 'str'}, > > + 'returns': ['VirtioInfo'] > > +}
Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > On Tue, 19 Dec 2017 15:57:18 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> QAPI/QMP interface review only. >> >> You neglected to cc: the maintainers of qapi-schema.json, so I'm doing >> that for you. >> > > Thank you so much. I screwed up with this patchset at least twice: with > list of maintainers and with my own e-mail. > >> Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: >> >> > 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 `tell_feature_name' >> > callback in VirtioDeviceClass also was introduced. >> > >> > Cc: Denis V. Lunev <den@virtuozzo.com> >> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> >> [...] >> > diff --git a/qapi-schema.json b/qapi-schema.json >> > index 1845795..51cd0f3 100644 >> > --- a/qapi-schema.json >> > +++ b/qapi-schema.json >> > @@ -3200,3 +3200,73 @@ >> > # Since: 2.11 >> > ## >> > { 'command': 'watchdog-set-action', 'data' : {'action': >> > 'WatchdogAction'} } + >> > +## >> > +# @VirtioFeature: >> > +# >> > +# Virtio feature bit with negotiation status >> > +# >> > +# @name: name of feature bit >> > +# >> > +# @acked: negotiation status, `true' if guest acknowledged the >> > feature +# >> > +# Since: 2.12 >> > +## >> > +{ >> > + 'struct': 'VirtioFeature', >> > + 'data': { >> > + 'name': 'str', >> > + 'acked': 'bool' >> > + } >> > +} >> > + >> > +## >> > +# @VirtioInfo: >> > +# >> > +# Information about virtio device >> > +# >> > +# @qom-path: QOM path of the device >> > +# >> > +# @status: status bitmask >> > +# >> > +# @host-features: bitmask of features, exposed by device >> > +# >> > +# @guest-features: bitmask of features, acknowledged by guest >> >> Quoting my review of v4: "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"? As far as I can see, I >> didn't get a reply back then. You'll have to give one to get the >> patch accepted. >> > > Most probably, I didn't understand you last time. Although, I'm still > far from correct understanding. > > About 'array of enum', I could make the following description: > > { > 'enum': 'VirtioFeatureScsiBit', > 'data': ['inout', 'hotplug', 'change', 'ta10_pi'] > } > > { > 'struct': 'VirtioFeatureScsi', > 'data' : { > 'bit': 'VirtioFeatureScsiBit', > 'acked': 'bool' > } > } > > { > 'enum': 'VirtioFeatureSerialBit', > 'data': ['size', 'multiport', 'emerg-write'] > } > > { > 'struct': 'VirtioFeatureSerial', > 'data' : { > 'bit': 'VirtioFeatureSerialBit', > 'acked': 'bool' > } > } > > { > 'enum': 'VirtioFeatureCategory', > 'data': ['scsi', 'serial'] > } > > { > 'union': 'VirtioFeature', > 'base': { > 'category': 'VirtioFeatureCategory' > }, > 'discriminator': 'categoty', > 'data': { > 'scsi': 'VirtioFeatureScsi', > 'serial': 'VirtioFeatureSerial' > } > } > > { > 'union': 'VirtioInfo', > 'data': { > 'qom-path': 'str', > 'device-features': ['VirtioFeature'] > ... > } > } > > But IMO, it only increases complexity without any benefit. To use this > description, at first it should be listed all features names in enums. > Then, these enums should be mapped into bit numbers and differently for > each virtio device. I expect something like this: > > VirtioFeatureScsiBit bit = ... > > switch (bit) { > case VIRTIO_FEATURE_SCSI_BIT_INOUT: > return VIRTIO_SCSI_F_INOUT; > case VIRTIO_FEATURE_SERIAL_BIT_HOTPLUG: > return VIRTIO_SCSI_F_HOTPLUG; > case VIRTIO_FEATURE_SERIAL_BIT_CHANGE: > return VIRTIO_SCSI_F_CHANGE; > case VIRTIO_FEATURE_SERIAL_BIT_T10_PI: > return VIRTIO_SCSI_F_T10_PI; > } > > So, boilerplate for mapping between bit numbers and and theirs names > will not go away. No argument. It's not about simplifying the implementation, it's about finding the appropriate QMP interface. > Furthermore, HMP command should produce readable output from QMP data, > features's enum values should be converted to strings from json > description. For that, for each possible VirtioFeatureCategary should > be called respective VirtioFeature*_str macro to resolve enum value. > > So, all my attempts to declare bit names within QAPI brings me to > familiar solutions. QMP prefers symbols (strings on the wire, preferably enums in the schema) over magic numbers. That said, numbers may be okay in certain cases, say when they're sufficiently well-known and QEMU only passes them along. Drawback: defeats introspection; you can't see what bits this QEMU supports. Why is that not interesting? Taking a step back: what's the intended purpose of query-virtio? The cover letter doesn't really say: After some discussion, I am going to suggest reworked QMP/HMP for gathering virtio info. It would provide the following monitor output. (qemu) info virtio virtio-blk-device at 0000:00:02.0 QOM path: /machine/peripheral-anon/device[0]/virtio-backend status: 0x0f VIRTIO_CONFIG_S_ACKNOWLEDGE VIRTIO_CONFIG_S_DRIVER VIRTIO_CONFIG_S_DRIVER_OK VIRTIO_CONFIG_S_FEATURES_OK host features: 0x0000000179000e54 guest features: 0x0000000130000e54 common features: VIRTIO_F_NOTIFY_ON_EMPTY VIRTIO_F_ANY_LAYOUT VIRTIO_RING_F_INDIRECT_DESC acked VIRTIO_RING_F_EVENT_IDX acked VIRTIO_F_BAD_FEATURE VIRTIO_F_VERSION_1 acked device features: VIRTIO_BLK_F_SEG_MAX acked VIRTIO_BLK_F_BLK_SIZE acked VIRTIO_BLK_F_FLUSH acked VIRTIO_BLK_F_TOPOLOGY acked The first sentence carries no useful information; suggest to scratch it. The rest is an HMP example. Examples are great, but we really need to understand *why* you need this command. >> In case symbolic names may not be known at QEMU compile time (me >> having to wonder at v6 is a sign of rather ineffective patch review, >> sorry about that): you have to explain this in the doc comment, with a >> reference to where the bits are specified. >> > > ok Can you think of a scenario where status or feature bits occur that are not known to QEMU at compile time? >> > +# >> > +# @status-names: names of checked bits in status bitmask >> >> How are the strings in @status-names connected to the bits in @status? >> Spell it out in this doc string, please. >> > > ok > > ...but, strictly saying, I was not going to show in QAPI how these names > are mapped into bits, QMP answer would not contain how these names are > connected to bits. Otherwise, It would be reinvention of v4. To avoid going in circles some more, let's take a step back and examine what exactly you're trying to accomplish. Give us use cases: this is my need, and this is how you use the proposed command to satisfy my need. Also give us design limitations: what the command is *not* trying to do. >> > +# >> > +# @common-features-names: names exposed features and negotiation >> > status, +# that are common to all devices >> > +# >> > +# @device-features-names: names exposed features and negotiation >> > status, +# that are specific to this device >> >> Likewise. >> > > ok > >> > +# >> > +# Since: 2.12 >> > +## >> > +{ >> > + '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: >> > +# >> > +# Returns virtio information such as device status and features >> > +# >> > +# Since: 2.12 >> > +## >> > +{ >> > + 'command': 'query-virtio', >> > + 'data': {'*path': 'str'}, >> > + 'returns': ['VirtioInfo'] >> > +}
On Wed, 20 Dec 2017 11:16:46 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > > > On Tue, 19 Dec 2017 15:57:18 +0100 > > Markus Armbruster <armbru@redhat.com> wrote: > >> Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > Taking a step back: what's the intended purpose of query-virtio? The > cover letter doesn't really say: > > After some discussion, I am going to suggest reworked QMP/HMP for > gathering virtio info. It would provide the following monitor output. > > (qemu) info virtio > virtio-blk-device at 0000:00:02.0 > QOM path: /machine/peripheral-anon/device[0]/virtio-backend > status: 0x0f > VIRTIO_CONFIG_S_ACKNOWLEDGE > VIRTIO_CONFIG_S_DRIVER > VIRTIO_CONFIG_S_DRIVER_OK > VIRTIO_CONFIG_S_FEATURES_OK > host features: 0x0000000179000e54 > guest features: 0x0000000130000e54 > common features: > VIRTIO_F_NOTIFY_ON_EMPTY > VIRTIO_F_ANY_LAYOUT > VIRTIO_RING_F_INDIRECT_DESC acked > VIRTIO_RING_F_EVENT_IDX acked > VIRTIO_F_BAD_FEATURE > VIRTIO_F_VERSION_1 acked > device features: > VIRTIO_BLK_F_SEG_MAX acked > VIRTIO_BLK_F_BLK_SIZE acked > VIRTIO_BLK_F_FLUSH acked > VIRTIO_BLK_F_TOPOLOGY acked > > The first sentence carries no useful information; suggest to scratch it. > The rest is an HMP example. Examples are great, but we really need to > understand *why* you need this command. From my side, I see at least two use cases: - Monitoring the state of virtio devices. If I can see from outside that VIRTIO_CONFIG_S_FAILED or VIRTIO_CONFIG_S_NEEDS_RESET has been set for a device, I know that something went wrong for that device. - Debugging. This command gives me a nice way to find out why something is not working as it is supposed to, or to verify that something indeed does work as intended (I would have found such a command quite useful when implementing virtio-1). > > >> In case symbolic names may not be known at QEMU compile time (me > >> having to wonder at v6 is a sign of rather ineffective patch review, > >> sorry about that): you have to explain this in the doc comment, with a > >> reference to where the bits are specified. > >> > > > > ok > > Can you think of a scenario where status or feature bits occur that are > not known to QEMU at compile time? Could happen for status bits (although very unlikely). For feature bits, the guest is supposed to ack a subset of the offered bits, so this would mean a fundamentally broken guest (also unlikely, but not impossible). > > >> > +# > >> > +# @status-names: names of checked bits in status bitmask > >> > >> How are the strings in @status-names connected to the bits in @status? > >> Spell it out in this doc string, please. > >> > > > > ok > > > > ...but, strictly saying, I was not going to show in QAPI how these names > > are mapped into bits, QMP answer would not contain how these names are > > connected to bits. Otherwise, It would be reinvention of v4. > > To avoid going in circles some more, let's take a step back and examine > what exactly you're trying to accomplish. Give us use cases: this is my > need, and this is how you use the proposed command to satisfy my need. > Also give us design limitations: what the command is *not* trying to do.
* Markus Armbruster (armbru@redhat.com) wrote: > Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > > > On Tue, 19 Dec 2017 15:57:18 +0100 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> QAPI/QMP interface review only. > >> > >> You neglected to cc: the maintainers of qapi-schema.json, so I'm doing > >> that for you. > >> > > > > Thank you so much. I screwed up with this patchset at least twice: with > > list of maintainers and with my own e-mail. > > > >> Jan Dakinevich <jan.dakinevich@virtuozzo.com> writes: > >> > >> > 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 `tell_feature_name' > >> > callback in VirtioDeviceClass also was introduced. > >> > > >> > Cc: Denis V. Lunev <den@virtuozzo.com> > >> > Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com> > >> [...] > >> > diff --git a/qapi-schema.json b/qapi-schema.json > >> > index 1845795..51cd0f3 100644 > >> > --- a/qapi-schema.json > >> > +++ b/qapi-schema.json > >> > @@ -3200,3 +3200,73 @@ > >> > # Since: 2.11 > >> > ## > >> > { 'command': 'watchdog-set-action', 'data' : {'action': > >> > 'WatchdogAction'} } + > >> > +## > >> > +# @VirtioFeature: > >> > +# > >> > +# Virtio feature bit with negotiation status > >> > +# > >> > +# @name: name of feature bit > >> > +# > >> > +# @acked: negotiation status, `true' if guest acknowledged the > >> > feature +# > >> > +# Since: 2.12 > >> > +## > >> > +{ > >> > + 'struct': 'VirtioFeature', > >> > + 'data': { > >> > + 'name': 'str', > >> > + 'acked': 'bool' > >> > + } > >> > +} > >> > + > >> > +## > >> > +# @VirtioInfo: > >> > +# > >> > +# Information about virtio device > >> > +# > >> > +# @qom-path: QOM path of the device > >> > +# > >> > +# @status: status bitmask > >> > +# > >> > +# @host-features: bitmask of features, exposed by device > >> > +# > >> > +# @guest-features: bitmask of features, acknowledged by guest > >> > >> Quoting my review of v4: "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"? As far as I can see, I > >> didn't get a reply back then. You'll have to give one to get the > >> patch accepted. > >> > > > > Most probably, I didn't understand you last time. Although, I'm still > > far from correct understanding. > > > > About 'array of enum', I could make the following description: > > > > { > > 'enum': 'VirtioFeatureScsiBit', > > 'data': ['inout', 'hotplug', 'change', 'ta10_pi'] > > } > > > > { > > 'struct': 'VirtioFeatureScsi', > > 'data' : { > > 'bit': 'VirtioFeatureScsiBit', > > 'acked': 'bool' > > } > > } > > > > { > > 'enum': 'VirtioFeatureSerialBit', > > 'data': ['size', 'multiport', 'emerg-write'] > > } > > > > { > > 'struct': 'VirtioFeatureSerial', > > 'data' : { > > 'bit': 'VirtioFeatureSerialBit', > > 'acked': 'bool' > > } > > } > > > > { > > 'enum': 'VirtioFeatureCategory', > > 'data': ['scsi', 'serial'] > > } > > > > { > > 'union': 'VirtioFeature', > > 'base': { > > 'category': 'VirtioFeatureCategory' > > }, > > 'discriminator': 'categoty', > > 'data': { > > 'scsi': 'VirtioFeatureScsi', > > 'serial': 'VirtioFeatureSerial' > > } > > } > > > > { > > 'union': 'VirtioInfo', > > 'data': { > > 'qom-path': 'str', > > 'device-features': ['VirtioFeature'] > > ... > > } > > } > > > > But IMO, it only increases complexity without any benefit. To use this > > description, at first it should be listed all features names in enums. > > Then, these enums should be mapped into bit numbers and differently for > > each virtio device. I expect something like this: > > > > VirtioFeatureScsiBit bit = ... > > > > switch (bit) { > > case VIRTIO_FEATURE_SCSI_BIT_INOUT: > > return VIRTIO_SCSI_F_INOUT; > > case VIRTIO_FEATURE_SERIAL_BIT_HOTPLUG: > > return VIRTIO_SCSI_F_HOTPLUG; > > case VIRTIO_FEATURE_SERIAL_BIT_CHANGE: > > return VIRTIO_SCSI_F_CHANGE; > > case VIRTIO_FEATURE_SERIAL_BIT_T10_PI: > > return VIRTIO_SCSI_F_T10_PI; > > } > > > > So, boilerplate for mapping between bit numbers and and theirs names > > will not go away. > > No argument. It's not about simplifying the implementation, it's about > finding the appropriate QMP interface. > > > Furthermore, HMP command should produce readable output from QMP data, > > features's enum values should be converted to strings from json > > description. For that, for each possible VirtioFeatureCategary should > > be called respective VirtioFeature*_str macro to resolve enum value. > > > > So, all my attempts to declare bit names within QAPI brings me to > > familiar solutions. > > QMP prefers symbols (strings on the wire, preferably enums in the > schema) over magic numbers. > > That said, numbers may be okay in certain cases, say when they're > sufficiently well-known and QEMU only passes them along. Drawback: > defeats introspection; you can't see what bits this QEMU supports. Why > is that not interesting? The point of having the numbers though was to allow presentation of state even if the qemu doesn't know about it. > Taking a step back: what's the intended purpose of query-virtio? The > cover letter doesn't really say: > > After some discussion, I am going to suggest reworked QMP/HMP for > gathering virtio info. It would provide the following monitor output. > > (qemu) info virtio > virtio-blk-device at 0000:00:02.0 > QOM path: /machine/peripheral-anon/device[0]/virtio-backend > status: 0x0f > VIRTIO_CONFIG_S_ACKNOWLEDGE > VIRTIO_CONFIG_S_DRIVER > VIRTIO_CONFIG_S_DRIVER_OK > VIRTIO_CONFIG_S_FEATURES_OK > host features: 0x0000000179000e54 > guest features: 0x0000000130000e54 > common features: > VIRTIO_F_NOTIFY_ON_EMPTY > VIRTIO_F_ANY_LAYOUT > VIRTIO_RING_F_INDIRECT_DESC acked > VIRTIO_RING_F_EVENT_IDX acked > VIRTIO_F_BAD_FEATURE > VIRTIO_F_VERSION_1 acked > device features: > VIRTIO_BLK_F_SEG_MAX acked > VIRTIO_BLK_F_BLK_SIZE acked > VIRTIO_BLK_F_FLUSH acked > VIRTIO_BLK_F_TOPOLOGY acked > > The first sentence carries no useful information; suggest to scratch it. > The rest is an HMP example. Examples are great, but we really need to > understand *why* you need this command. Lets go all the way back to v1; this was just for debug; http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html the qmp interface is only there because people said that that having QMP is the norm, and now it's dragged on for ages now trying to come up with a way to get a stable interface for something that was only intended as a useful bit of debug! > >> In case symbolic names may not be known at QEMU compile time (me > >> having to wonder at v6 is a sign of rather ineffective patch review, > >> sorry about that): you have to explain this in the doc comment, with a > >> reference to where the bits are specified. > >> > > > > ok > > Can you think of a scenario where status or feature bits occur that are > not known to QEMU at compile time? Can't that happen when a newer guest driver asks for features the host doesn't have? Or do those bits get masked off before we get to see them? IMHO it would be wrong to ignore bits you didn't know about; you want to flag them up at least for someone to go 'that's odd...' - and what's the best way to represent bits you didn't know about other than a good old integer? Dave > >> > +# > >> > +# @status-names: names of checked bits in status bitmask > >> > >> How are the strings in @status-names connected to the bits in @status? > >> Spell it out in this doc string, please. > >> > > > > ok > > > > ...but, strictly saying, I was not going to show in QAPI how these names > > are mapped into bits, QMP answer would not contain how these names are > > connected to bits. Otherwise, It would be reinvention of v4. > > To avoid going in circles some more, let's take a step back and examine > what exactly you're trying to accomplish. Give us use cases: this is my > need, and this is how you use the proposed command to satisfy my need. > Also give us design limitations: what the command is *not* trying to do. > > >> > +# > >> > +# @common-features-names: names exposed features and negotiation > >> > status, +# that are common to all devices > >> > +# > >> > +# @device-features-names: names exposed features and negotiation > >> > status, +# that are specific to this device > >> > >> Likewise. > >> > > > > ok > > > >> > +# > >> > +# Since: 2.12 > >> > +## > >> > +{ > >> > + '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: > >> > +# > >> > +# Returns virtio information such as device status and features > >> > +# > >> > +# Since: 2.12 > >> > +## > >> > +{ > >> > + 'command': 'query-virtio', > >> > + 'data': {'*path': 'str'}, > >> > + 'returns': ['VirtioInfo'] > >> > +} -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 05d1440..2ffd949 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1017,6 +1017,25 @@ static Property virtio_blk_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_blk_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_BLK_F_BARRIER); + FBIT(VIRTIO_BLK_F_SIZE_MAX); + FBIT(VIRTIO_BLK_F_SEG_MAX); + FBIT(VIRTIO_BLK_F_RO); + FBIT(VIRTIO_BLK_F_BLK_SIZE); + FBIT(VIRTIO_BLK_F_SCSI); + FBIT(VIRTIO_BLK_F_TOPOLOGY); + FBIT(VIRTIO_BLK_F_FLUSH); + FBIT(VIRTIO_BLK_F_MQ); + } +#undef FBIT + return NULL; +} + static void virtio_blk_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1030,6 +1049,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->tell_feature_name = virtio_blk_tell_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..1d4678b 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -1156,6 +1156,19 @@ static Property virtio_serial_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_serial_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_CONSOLE_F_SIZE); + FBIT(VIRTIO_CONSOLE_F_MULTIPORT); + FBIT(VIRTIO_CONSOLE_F_EMERG_WRITE); + }; +#undef FBIT + return NULL; +} + static void virtio_serial_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1170,6 +1183,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->tell_feature_name = virtio_serial_tell_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 274e365..1906b68 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1317,6 +1317,17 @@ static Property virtio_gpu_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_gpu_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_GPU_F_VIRGL); + }; +#undef FBIT + return NULL; +} + static void virtio_gpu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1327,6 +1338,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->tell_feature_name = virtio_gpu_tell_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 38674b0..0fbd055 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -2163,6 +2163,39 @@ static Property virtio_net_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_net_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_NET_F_CSUM); + FBIT(VIRTIO_NET_F_GUEST_CSUM); + FBIT(VIRTIO_NET_F_CTRL_GUEST_OFFLOADS); + FBIT(VIRTIO_NET_F_MTU); + FBIT(VIRTIO_NET_F_MAC); + FBIT(VIRTIO_NET_F_GSO); + FBIT(VIRTIO_NET_F_GUEST_TSO4); + FBIT(VIRTIO_NET_F_GUEST_TSO6); + FBIT(VIRTIO_NET_F_GUEST_ECN); + FBIT(VIRTIO_NET_F_GUEST_UFO); + FBIT(VIRTIO_NET_F_HOST_TSO4); + FBIT(VIRTIO_NET_F_HOST_TSO6); + FBIT(VIRTIO_NET_F_HOST_ECN); + FBIT(VIRTIO_NET_F_HOST_UFO); + FBIT(VIRTIO_NET_F_MRG_RXBUF); + FBIT(VIRTIO_NET_F_STATUS); + FBIT(VIRTIO_NET_F_CTRL_VQ); + FBIT(VIRTIO_NET_F_CTRL_RX); + FBIT(VIRTIO_NET_F_CTRL_VLAN); + FBIT(VIRTIO_NET_F_CTRL_RX_EXTRA); + FBIT(VIRTIO_NET_F_GUEST_ANNOUNCE); + FBIT(VIRTIO_NET_F_MQ); + FBIT(VIRTIO_NET_F_CTRL_MAC_ADDR); + }; +#undef FBIT + return NULL; +} + static void virtio_net_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -2178,6 +2211,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->tell_feature_name = virtio_net_tell_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..4be5df8 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -942,6 +942,20 @@ static const VMStateDescription vmstate_virtio_scsi = { }, }; +static const char *virtio_scsi_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_SCSI_F_INOUT); + FBIT(VIRTIO_SCSI_F_HOTPLUG); + FBIT(VIRTIO_SCSI_F_CHANGE); + FBIT(VIRTIO_SCSI_F_T10_PI); + }; +#undef FBIT + return NULL; +} + static void virtio_scsi_common_class_init(ObjectClass *klass, void *data) { VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); @@ -964,6 +978,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->tell_feature_name = virtio_scsi_tell_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..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-balloon.c b/hw/virtio/virtio-balloon.c index 37cde38..d97c8ce 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -509,6 +509,19 @@ static Property virtio_balloon_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const char *virtio_balloon_tell_feature_name(VirtIODevice *vdev, + unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_BALLOON_F_MUST_TELL_HOST); + FBIT(VIRTIO_BALLOON_F_STATS_VQ); + FBIT(VIRTIO_BALLOON_F_DEFLATE_ON_OOM); + }; +#undef FBIT + return NULL; +} + static void virtio_balloon_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -523,6 +536,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->tell_feature_name = virtio_balloon_tell_feature_name; vdc->set_status = virtio_balloon_set_status; vdc->vmsd = &vmstate_virtio_balloon_device; } diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c new file mode 100644 index 0000000..6eaef9a --- /dev/null +++ b/hw/virtio/virtio-qmp.c @@ -0,0 +1,134 @@ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qmp-commands.h" + +#include "hw/virtio/virtio.h" + +static VirtioInfoList *qmp_query_virtio_one(VirtIODevice *vdev) +{ + VirtioInfoList *info_list; + VirtioInfo *info; + unsigned int idx; + + info_list = g_new0(VirtioInfoList, 1); + info_list->value = g_new0(VirtioInfo, 1); + + info = 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; + + for (idx = 8; idx--; ) { + const char *name = virtio_tell_status_name(vdev, idx); + strList *status; + + if (!name) { + continue; + } + if (!(vdev->status & (1 << idx))) { + continue; + } + + status = g_new(strList, 1); + status->value = g_strdup(name); + + status->next = info->status_names; + info->status_names = status; + } + + for (idx = 64; idx--; ) { + const char *name = virtio_tell_device_feature_name(vdev, idx); + VirtioFeatureList **head = &info->device_features_names; + VirtioFeatureList *feature; + + if (!name) { + name = virtio_tell_common_feature_name(vdev, idx); + head = &info->common_features_names; + } + if (!name) { + continue; + } + if (!virtio_host_has_feature(vdev, idx)) { + continue; + } + + feature = g_new0(VirtioFeatureList, 1); + feature->value = g_new0(VirtioFeature, 1); + + feature->value->name = g_strdup(name); + feature->value->acked = virtio_vdev_has_feature(vdev, idx); + + feature->next = *head; + *head = feature; + } + + return info_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/hw/virtio/virtio.c b/hw/virtio/virtio.c index ad564b0..23b33db 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2198,6 +2198,47 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) return 0; } +const char *virtio_tell_status_name(VirtIODevice *vdev, unsigned sbit) +{ +#define SBIT(sbit) case sbit: return #sbit + switch (1 << sbit) { + SBIT(VIRTIO_CONFIG_S_ACKNOWLEDGE); + SBIT(VIRTIO_CONFIG_S_DRIVER); + SBIT(VIRTIO_CONFIG_S_DRIVER_OK); + SBIT(VIRTIO_CONFIG_S_FEATURES_OK); + SBIT(VIRTIO_CONFIG_S_NEEDS_RESET); + SBIT(VIRTIO_CONFIG_S_FAILED); + } +#undef SBIT + return NULL; +} + +const char *virtio_tell_common_feature_name(VirtIODevice *vdev, unsigned fbit) +{ +#define FBIT(fbit) case fbit: return #fbit + switch (fbit) { + FBIT(VIRTIO_F_NOTIFY_ON_EMPTY); + FBIT(VIRTIO_F_ANY_LAYOUT); + FBIT(VIRTIO_RING_F_INDIRECT_DESC); + FBIT(VIRTIO_RING_F_EVENT_IDX); + FBIT(VIRTIO_F_BAD_FEATURE); + FBIT(VIRTIO_F_VERSION_1); + FBIT(VIRTIO_F_IOMMU_PLATFORM); + } +#undef FBIT + return NULL; +} + +const char *virtio_tell_device_feature_name(VirtIODevice *vdev, unsigned fbit) +{ + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); + + if (vdc->tell_feature_name) { + return vdc->tell_feature_name(vdev, fbit); + } + return NULL; +} + void virtio_cleanup(VirtIODevice *vdev) { qemu_del_vm_change_state_handler(vdev->vmstate); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 098bdaa..25f47da 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; + /* Tells the name of device specific feature */ + const char *(*tell_feature_name)(VirtIODevice *vdev, unsigned fbit); } VirtioDeviceClass; void virtio_instance_init_common(Object *proxy_obj, void *data, @@ -292,6 +294,10 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx, VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector); VirtQueue *virtio_vector_next_queue(VirtQueue *vq); +const char *virtio_tell_status_name(VirtIODevice *vdev, unsigned sbit); +const char *virtio_tell_common_feature_name(VirtIODevice *vdev, unsigned fbit); +const char *virtio_tell_device_feature_name(VirtIODevice *vdev, unsigned fbit); + static inline void virtio_add_feature(uint64_t *features, unsigned int fbit) { assert(fbit < 64); diff --git a/qapi-schema.json b/qapi-schema.json index 1845795..51cd0f3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3200,3 +3200,73 @@ # Since: 2.11 ## { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} } + +## +# @VirtioFeature: +# +# Virtio feature bit with negotiation status +# +# @name: name of feature bit +# +# @acked: negotiation status, `true' if guest acknowledged the feature +# +# Since: 2.12 +## +{ + 'struct': 'VirtioFeature', + 'data': { + 'name': 'str', + 'acked': 'bool' + } +} + +## +# @VirtioInfo: +# +# Information about virtio device +# +# @qom-path: QOM path of the device +# +# @status: status bitmask +# +# @host-features: bitmask of features, exposed by device +# +# @guest-features: bitmask of features, acknowledged by guest +# +# @status-names: names of checked bits in status bitmask +# +# @common-features-names: names exposed features and negotiation status, +# that are common to all devices +# +# @device-features-names: names exposed features and negotiation status, +# that are specific to this device +# +# Since: 2.12 +## +{ + '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: +# +# Returns virtio information such as device status and features +# +# Since: 2.12 +## +{ + 'command': 'query-virtio', + 'data': {'*path': 'str'}, + 'returns': ['VirtioInfo'] +}
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 `tell_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 | 20 +++++++ hw/char/virtio-serial-bus.c | 14 +++++ hw/display/virtio-gpu.c | 12 ++++ hw/net/virtio-net.c | 34 +++++++++++ hw/scsi/virtio-scsi.c | 15 +++++ hw/virtio/Makefile.objs | 3 + hw/virtio/virtio-balloon.c | 14 +++++ hw/virtio/virtio-qmp.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ hw/virtio/virtio-stub.c | 9 +++ hw/virtio/virtio.c | 41 ++++++++++++++ include/hw/virtio/virtio.h | 6 ++ qapi-schema.json | 70 +++++++++++++++++++++++ 12 files changed, 372 insertions(+) create mode 100644 hw/virtio/virtio-qmp.c create mode 100644 hw/virtio/virtio-stub.c