On 9/4/20 7:33 AM, Lukas Straub wrote:
+## +# @YankInstances: +# +# @instances: List of yank instances. +# +# Yank instances are named after the following schema: +# "blockdev:<node-name>", "chardev:<chardev-name>" and "migration" +# +# Since: 5.1 +## +{ 'struct': 'YankInstances', 'data': {'instances': ['str'] } }I'm afraid this is a problematic QMP interface. By making YankInstances a struct, you keep the door open to adding more members, which is good. But by making its 'instances' member a ['str'], you close the door to using anything but a single string for the individual instances. Not so good. The single string encodes information which QMP client will need to parse from the string. We frown on that in QMP. Use QAPI complex types capabilities for structured data. Could you use something like this instead? { 'enum': 'YankInstanceType', 'data': { 'block-node', 'chardev', 'migration' } } { 'struct': 'YankInstanceBlockNode', 'data': { 'node-name': 'str' } } { 'struct': 'YankInstanceChardev', 'data' { 'label': 'str' } } { 'union': 'YankInstance', 'base': { 'type': 'YankInstanceType' }, 'discriminator': 'type', 'data': { 'block-node': 'YankInstanceBlockNode', 'chardev': 'YankInstanceChardev' } } { 'command': 'yank', 'data': { 'instances': ['YankInstance'] }, 'allow-oob': true }This proposal looks good to me. Does everyone agree?
Yes; this is also more introspectible, so that if we add more yank instances down the road, or even more optional features to existing yank instances, it becomes easier to detect whether a particular qemu has those additions.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
