| Message ID | 20170522213636.11550-1-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > In the process of getting rid of docs/qmp-commands.txt, we > managed to regress on any text that changed after the point > where the move was first branched and when the move actually > occurred. For example, commit 3282eca for blockdev-snapshot > re-added the extra "options" layer which had been cleaned up > in commit 0153d2f. > > While I didn't audit for all such regressions, I did scrub > for all bogus uses of nested "options". I figure anything that changed in qmp-commands.txt between the first base of Marc-André's work and its merge into master is at risk. I don't know the exact first base. "[PATCH 00/30] Move qapi documentation to schema (part 1/5)" was posted on 2016-09-13. September 2016 looks like a fair guess. With a bit of extra margin: $ git-log --oneline --since 2016-08-01 02b351d -- docs/qmp-commands.txt e1ff3c6 monitor: fix qmp/hmp query-memdev not reporting IDs of memory backends 23dce38 block/curl: Drop TFTP "support" 312fe09 block: Add 'base-node' parameter to the 'block-stream' command d89e666 COLO: Add 'x-colo-lost-heartbeat' command to trigger failover 68b5359 COLO: Add checkpoint-delay parameter for migrate-set-parameters 35a6ed4 migration: Introduce capability 'x-colo' to migration 0153d2f block: Remove "options" indirection from blockdev-add 2ff3025 migrate: move max-bandwidth and downtime-limit to migrate_set_parameter 0f183e6 Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 77a6da2 docs: Belatedly update for move of QMP/* to docs/ 2d76e72 block: Add qdev ID to DEVICE_TRAY_MOVED 9ec8873 block: Remove BB interface from blockdev-add/del 7a9877a block: Accept device model name for block_set_io_throttle 70e2cb3 block: Accept device model name for blockdev-change-medium fbe2d81 block: Accept device model name for eject 00949ba block: Accept device model name for x-blockdev-remove-medium 716df21 block: Accept device model name for x-blockdev-insert-medium b33945c block: Accept device model name for blockdev-open/close-tray bd6092e Replace qmp-commands.hx by docs/qmp-commands.txt We can exclude the last one, because is Marc-André's. Let's review the diff: $ git-diff bd6092e 02b351d -- docs/qmp-commands.txt diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt index acebeb3..18db4cd 100644 --- a/docs/qmp-commands.txt +++ b/docs/qmp-commands.txt @@ -20,7 +20,7 @@ Also, the following notation is used to denote data flow: -> data issued by the Client <- Server data response -Please, refer to the QMP specification (QMP/qmp-spec.txt) for detailed +Please, refer to the QMP specification (docs/qmp-spec.txt) for detailed information on the Server command and response formats. NOTE: This document is temporary and will be replaced soon. Rebased correctly. @@ -72,12 +72,14 @@ Eject a removable medium. Arguments: -- force: force ejection (json-bool, optional) -- device: device name (json-string) +- "force": force ejection (json-bool, optional) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) This is eject. The changed part was deleted by Marc-André because it duplicates QAPI schema comments in block.json. These got the change. Okay. Example: --> { "execute": "eject", "arguments": { "device": "ide1-cd0" } } +-> { "execute": "eject", "arguments": { "id": "ide0-1-0" } } <- { "return": {} } Note: The "force" argument defaults to false. Regressed, not fixed in your patch. @@ -552,6 +554,16 @@ Example: -> { "execute": "migrate_set_downtime", "arguments": { "value": 0.1 } } <- { "return": {} } +x-colo-lost-heartbeat +-------------------- + +Tell COLO that heartbeat is lost, a failover or takeover is needed. + +Example: + +-> { "execute": "x-colo-lost-heartbeat" } +<- { "return": {} } + qapi-schema.json got the change. client_migrate_info ------------------- @@ -738,8 +750,11 @@ Arguments: - "job-id": Identifier for the newly-created block job. If omitted, the device name will be used. (json-string, optional) - "device": The device name or node-name of a root node (json-string) -- "base": The file name of the backing image above which copying starts - (json-string, optional) +- "base": The file name of the backing image above which copying starts. + It cannot be set if 'base-node' is also set (json-string, optional) +- "base-node": the node name of the backing image above which copying starts. + It cannot be set if 'base' is also set. + (json-string, optional) (Since 2.8) - "backing-file": The backing file string to write into the active layer. This filename is not validated. This is block-stream. block-core.json got the change. @@ -1088,11 +1103,11 @@ Arguments: Example: -> { "execute": "blockdev-add", - "arguments": { "options": { "driver": "qcow2", - "node-name": "node1534", - "file": { "driver": "file", - "filename": "hd1.qcow2" }, - "backing": "" } } } + "arguments": { "driver": "qcow2", + "node-name": "node1534", + "file": { "driver": "file", + "filename": "hd1.qcow2" }, + "backing": "" } } <- { "return": {} } This is blockdev-snapshot. Regressed, fixed in your patch. @@ -1457,7 +1472,9 @@ Change I/O throttle limits for a block drive. Arguments: -- "device": device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) - "bps": total throughput limit in bytes per second (json-int) - "bps_rd": read throughput limit in bytes per second (json-int) - "bps_wr": write throughput limit in bytes per second (json-int) This is block_set_io_throttle. block-core.json got the change, in documentation of BlockIOThrottle, and ... @@ -1481,7 +1498,7 @@ Arguments: Example: --> { "execute": "block_set_io_throttle", "arguments": { "device": "virtio0", +-> { "execute": "block_set_io_throttle", "arguments": { "id": "ide0-1-0", "bps": 1000000, "bps_rd": 0, "bps_wr": 0, ... block_set_io_throttle. @@ -1786,7 +1803,7 @@ Each json-object contain the following: "file", "file", "ftp", "ftps", "host_cdrom", "host_device", "http", "https", "nbd", "parallels", "qcow", "qcow2", "raw", - "tftp", "vdi", "vmdk", "vpc", "vvfat" + "vdi", "vmdk", "vpc", "vvfat" - "backing_file": backing file name (json-string, optional) - "backing_file_depth": number of files in the backing file chain (json-int) - "encrypted": true if encrypted, false otherwise (json-bool) This is query-block. block-core.json got the change, in documentation of BlockDeviceInfo. @@ -2857,6 +2874,7 @@ Enable/Disable migration capabilities - "compress": use multiple compression threads to accelerate live migration - "events": generate events for each migration state change - "postcopy-ram": postcopy mode for live migration +- "x-colo": COarse-Grain LOck Stepping (COLO) for Non-stop Service Arguments: This is migrate-set-capabilities. qapi-schema.json got the change, in documentation of MigrationCapability. @@ -2878,6 +2896,7 @@ Query current migration capabilities - "compress": Multiple compression threads state (json-bool) - "events": Migration state change event state (json-bool) - "postcopy-ram": postcopy ram state (json-bool) + - "x-colo": COarse-Grain LOck Stepping for Non-stop Service (json-bool) Arguments: This is query-migrate-capabilities. qapi-schema.json got the change, in MigrationCapability. @@ -2891,7 +2910,8 @@ Example: {"state": false, "capability": "zero-blocks"}, {"state": false, "capability": "compress"}, {"state": true, "capability": "events"}, - {"state": false, "capability": "postcopy-ram"} + {"state": false, "capability": "postcopy-ram"}, + {"state": false, "capability": "x-colo"} ]} Still query-migrate-capabilities. Likewise. migrate-set-parameters @@ -2906,6 +2926,10 @@ Set migration parameters throttled for auto-converge (json-int) - "cpu-throttle-increment": set throttle increasing percentage for auto-converge (json-int) +- "max-bandwidth": set maximum speed for migrations (in bytes/sec) (json-int) +- "downtime-limit": set maximum tolerated downtime (in milliseconds) for + migrations (json-int) +- "x-checkpoint-delay": set the delay time for periodic checkpoint (json-int) Arguments: This is migrate-set-parameters. qapi-schema.json got the change, in MigrationParameters. @@ -2927,7 +2951,10 @@ Query current migration parameters throttled (json-int) - "cpu-throttle-increment" : throttle increasing percentage for auto-converge (json-int) - + - "max-bandwidth" : maximium migration speed in bytes per second + (json-int) + - "downtime-limit" : maximum tolerated downtime of migration in + milliseconds (json-int) Arguments: This is query-migrate-parameters. Likewise. Example: @@ -2939,7 +2966,9 @@ Example: "cpu-throttle-increment": 10, "compress-threads": 8, "compress-level": 1, - "cpu-throttle-initial": 20 + "cpu-throttle-initial": 20, + "max-bandwidth": 33554432, + "downtime-limit": 300 } } Still query-migrate-parameters. qapi-schema.json got the change. @@ -3119,41 +3148,37 @@ This command is still a work in progress. It doesn't support all block drivers among other things. Stay away from it unless you want to help with its development. -Arguments: - -- "options": block driver options +For the arguments, see the QAPI schema documentation of BlockdevOptions. Example (1): -> { "execute": "blockdev-add", - "arguments": { "options" : { "driver": "qcow2", - "file": { "driver": "file", - "filename": "test.qcow2" } } } } + "arguments": { "driver": "qcow2", + "file": { "driver": "file", + "filename": "test.qcow2" } } } <- { "return": {} } Example (2): -> { "execute": "blockdev-add", "arguments": { - "options": { - "driver": "qcow2", - "id": "my_disk", - "discard": "unmap", - "cache": { - "direct": true, - "writeback": true - }, - "file": { - "driver": "file", - "filename": "/tmp/test.qcow2" - }, - "backing": { - "driver": "raw", - "file": { - "driver": "file", - "filename": "/dev/fdset/4" - } - } + "driver": "qcow2", + "node-name": "my_disk", + "discard": "unmap", + "cache": { + "direct": true, + "writeback": true + }, + "file": { + "driver": "file", + "filename": "/tmp/test.qcow2" + }, + "backing": { + "driver": "raw", + "file": { + "driver": "file", + "filename": "/dev/fdset/4" + } } } } This is blockdev-add. Regressed initially, but was fixed in commit b1660997. @@ -3164,18 +3189,9 @@ x-blockdev-del ------------ Since 2.5 -Deletes a block device thas has been added using blockdev-add. -The selected device can be either a block backend or a graph node. - -In the former case the backend will be destroyed, along with its -inserted medium if there's any. The command will fail if the backend -or its medium are in use. - -In the latter case the node will be destroyed. The command will fail -if the node is attached to a block backend or is otherwise being -used. - -One of "id" or "node-name" must be specified, but not both. +Deletes a block device that has been added using blockdev-add. +The command will fail if the node is attached to a device or is +otherwise being used. This command is still a work in progress and is considered experimental. Stay away from it unless you want to help with its This is blockdev-del. block-core.json got the change. @@ -3183,20 +3199,17 @@ development. Arguments: -- "id": Name of the block backend device to delete (json-string, optional) -- "node-name": Name of the graph node to delete (json-string, optional) +- "node-name": Name of the graph node to delete (json-string) Example: -> { "execute": "blockdev-add", "arguments": { - "options": { - "driver": "qcow2", - "id": "drive0", - "file": { - "driver": "file", - "filename": "test.qcow2" - } + "driver": "qcow2", + "node-name": "node0", + "file": { + "driver": "file", + "filename": "test.qcow2" } } } @@ -3204,7 +3217,7 @@ Example: <- { "return": {} } -> { "execute": "x-blockdev-del", - "arguments": { "id": "drive0" } + "arguments": { "node-name": "node0" } } <- { "return": {} } Likewise. @@ -3228,7 +3241,9 @@ which no such event will be generated, these include: Arguments: -- "device": block device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) - "force": if false (the default), an eject request will be sent to the guest if it has locked the tray (and the tray will not be opened immediately); if true, the tray will be opened regardless of whether it is locked This is blockdev-open-tray. block-core.json got the change. @@ -3237,12 +3252,13 @@ Arguments: Example: -> { "execute": "blockdev-open-tray", - "arguments": { "device": "ide1-cd0" } } + "arguments": { "id": "ide0-1-0" } } <- { "timestamp": { "seconds": 1418751016, "microseconds": 716996 }, "event": "DEVICE_TRAY_MOVED", "data": { "device": "ide1-cd0", + "id": "ide0-1-0", "tray-open": true } } <- { "return": {} } Likewise. @@ -3258,17 +3274,20 @@ If the tray was already closed before, this will be a no-op. Arguments: -- "device": block device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) Example: -> { "execute": "blockdev-close-tray", - "arguments": { "device": "ide1-cd0" } } + "arguments": { "id": "ide0-1-0" } } <- { "timestamp": { "seconds": 1418751345, "microseconds": 272147 }, "event": "DEVICE_TRAY_MOVED", "data": { "device": "ide1-cd0", + "id": "ide0-1-0", "tray-open": false } } <- { "return": {} } This is blockdev-close-tray. block-core.json got the change. @@ -3286,29 +3305,32 @@ Stay away from it unless you want to help with its development. Arguments: -- "device": block device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) Example: -> { "execute": "x-blockdev-remove-medium", - "arguments": { "device": "ide1-cd0" } } + "arguments": { "id": "ide0-1-0" } } <- { "error": { "class": "GenericError", - "desc": "Tray of device 'ide1-cd0' is not open" } } + "desc": "Tray of device 'ide0-1-0' is not open" } } -> { "execute": "blockdev-open-tray", - "arguments": { "device": "ide1-cd0" } } + "arguments": { "id": "ide0-1-0" } } <- { "timestamp": { "seconds": 1418751627, "microseconds": 549958 }, "event": "DEVICE_TRAY_MOVED", "data": { "device": "ide1-cd0", + "id": "ide0-1-0", "tray-open": true } } <- { "return": {} } -> { "execute": "x-blockdev-remove-medium", - "arguments": { "device": "ide1-cd0" } } + "arguments": { "device": "ide0-1-0" } } <- { "return": {} } This is x-blockdev-remove-medium. block-core.json got the change. However, the example still uses deprecated "device" instead of "id". @@ -3324,21 +3346,23 @@ Stay away from it unless you want to help with its development. Arguments: -- "device": block device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) - "node-name": root node of the BDS tree to insert into the block device This is x-blockdev-insert-medium. block-core.json got the change. Example: -> { "execute": "blockdev-add", - "arguments": { "options": { "node-name": "node0", - "driver": "raw", - "file": { "driver": "file", - "filename": "fedora.iso" } } } } + "arguments": { { "node-name": "node0", + "driver": "raw", + "file": { "driver": "file", + "filename": "fedora.iso" } } } <- { "return": {} } -> { "execute": "x-blockdev-insert-medium", - "arguments": { "device": "ide1-cd0", + "arguments": { "id": "ide0-1-0", "node-name": "node0" } } <- { "return": {} } Regressed, fixed in your patch. @@ -3371,10 +3395,10 @@ Example: Add a new node to a quorum -> { "execute": "blockdev-add", - "arguments": { "options": { "driver": "raw", - "node-name": "new_node", - "file": { "driver": "file", - "filename": "test.raw" } } } } + "arguments": { "driver": "raw", + "node-name": "new_node", + "file": { "driver": "file", + "filename": "test.raw" } } } <- { "return": {} } -> { "execute": "x-blockdev-change", "arguments": { "parent": "disk1", This is x-blockdev-change. Regressed, fixed in your patch. @@ -3448,7 +3472,9 @@ and loading a new image file which is inserted as the new medium. Arguments: -- "device": device name (json-string) +- "device": block device name (deprecated, use @id instead) + (json-string, optional) +- "id": the name or QOM path of the guest device (json-string, optional) - "filename": filename of the new image (json-string) - "format": format of the new image (json-string, optional) - "read-only-mode": new read-only mode (json-string, optional) This is blockdev-change-medium. block-core.json got the change. @@ -3459,7 +3485,7 @@ Examples: 1. Change a removable medium -> { "execute": "blockdev-change-medium", - "arguments": { "device": "ide1-cd0", + "arguments": { "id": "ide0-1-0", "filename": "/srv/images/Fedora-12-x86_64-DVD.iso", "format": "raw" } } <- { "return": {} } @@ -3467,7 +3493,7 @@ Examples: 2. Load a read-only medium into a writable drive -> { "execute": "blockdev-change-medium", - "arguments": { "device": "isa-fd0", + "arguments": { "id": "floppyA", "filename": "/srv/images/ro.img", "format": "raw", "read-only-mode": "retain" } } @@ -3477,7 +3503,7 @@ Examples: "desc": "Could not open '/srv/images/ro.img': Permission denied" } } -> { "execute": "blockdev-change-medium", - "arguments": { "device": "isa-fd0", + "arguments": { "id": "floppyA", "filename": "/srv/images/ro.img", "format": "raw", "read-only-mode": "read-only" } } Likewise. @@ -3503,6 +3529,7 @@ Example (1): "policy": "bind" }, { + "id": "mem1", "size": 536870912, "merge": false, "dump": true, This is query-memdev. qapi-schema.json got the change, but in the *other* element of the returned list. I guess that's okay. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > qapi/block-core.json | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 6b974b9..3bd7fa2 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1206,11 +1206,11 @@ > # Example: > # > # -> { "execute": "blockdev-add", > -# "arguments": { "options": { "driver": "qcow2", > -# "node-name": "node1534", > -# "file": { "driver": "file", > -# "filename": "hd1.qcow2" }, > -# "backing": "" } } } > +# "arguments": { "driver": "qcow2", > +# "node-name": "node1534", > +# "file": { "driver": "file", > +# "filename": "hd1.qcow2" }, > +# "backing": "" } } > # > # <- { "return": {} } > # > @@ -3237,10 +3237,10 @@ > # > # -> { "execute": "blockdev-add", > # "arguments": { > -# "options": { "node-name": "node0", > -# "driver": "raw", > -# "file": { "driver": "file", > -# "filename": "fedora.iso" } } } } > +# "node-name": "node0", > +# "driver": "raw", > +# "file": { "driver": "file", > +# "filename": "fedora.iso" } } } > # <- { "return": {} } > # > # -> { "execute": "x-blockdev-insert-medium", > @@ -3693,10 +3693,10 @@ > # 1. Add a new node to a quorum > # -> { "execute": "blockdev-add", > # "arguments": { > -# "options": { "driver": "raw", > -# "node-name": "new_node", > -# "file": { "driver": "file", > -# "filename": "test.raw" } } } } > +# "driver": "raw", > +# "node-name": "new_node", > +# "file": { "driver": "file", > +# "filename": "test.raw" } } } > # <- { "return": {} } > # -> { "execute": "x-blockdev-change", > # "arguments": { "parent": "disk1", Please throw in a fix for the remaining regression of 'eject'. Whether you squash it into this one or keep it separate is up to you. For this part: Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 05/23/2017 03:39 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> In the process of getting rid of docs/qmp-commands.txt, we >> managed to regress on any text that changed after the point >> where the move was first branched and when the move actually >> occurred. For example, commit 3282eca for blockdev-snapshot >> re-added the extra "options" layer which had been cleaned up >> in commit 0153d2f. >> >> While I didn't audit for all such regressions, I did scrub >> for all bogus uses of nested "options". > > I figure anything that changed in qmp-commands.txt between the first > base of Marc-André's work and its merge into master is at risk. > > I don't know the exact first base. "[PATCH 00/30] Move qapi > documentation to schema (part 1/5)" was posted on 2016-09-13. September > 2016 looks like a fair guess. With a bit of extra margin: Thanks for the audit! > > Please throw in a fix for the remaining regression of 'eject'. Whether > you squash it into this one or keep it separate is up to you. For this > part: > > Reviewed-by: Markus Armbruster <armbru@redhat.com> I'll post a v2, since I have another pending doc patch that hasn't been reviewed yet, and since your audit means I need to update my commit message anyway.
On 05/23/2017 09:32 AM, Eric Blake wrote: > On 05/23/2017 03:39 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> In the process of getting rid of docs/qmp-commands.txt, we >>> managed to regress on any text that changed after the point >>> where the move was first branched and when the move actually >>> occurred. For example, commit 3282eca for blockdev-snapshot >>> re-added the extra "options" layer which had been cleaned up >>> in commit 0153d2f. >>> > > I'll post a v2, since I have another pending doc patch that hasn't been > reviewed yet, and since your audit means I need to update my commit > message anyway. Actually, it just got queued as trivial: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05323.html although git handles things just fine if it goes in earlier through any other tree.
diff --git a/qapi/block-core.json b/qapi/block-core.json index 6b974b9..3bd7fa2 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1206,11 +1206,11 @@ # Example: # # -> { "execute": "blockdev-add", -# "arguments": { "options": { "driver": "qcow2", -# "node-name": "node1534", -# "file": { "driver": "file", -# "filename": "hd1.qcow2" }, -# "backing": "" } } } +# "arguments": { "driver": "qcow2", +# "node-name": "node1534", +# "file": { "driver": "file", +# "filename": "hd1.qcow2" }, +# "backing": "" } } # # <- { "return": {} } # @@ -3237,10 +3237,10 @@ # # -> { "execute": "blockdev-add", # "arguments": { -# "options": { "node-name": "node0", -# "driver": "raw", -# "file": { "driver": "file", -# "filename": "fedora.iso" } } } } +# "node-name": "node0", +# "driver": "raw", +# "file": { "driver": "file", +# "filename": "fedora.iso" } } } # <- { "return": {} } # # -> { "execute": "x-blockdev-insert-medium", @@ -3693,10 +3693,10 @@ # 1. Add a new node to a quorum # -> { "execute": "blockdev-add", # "arguments": { -# "options": { "driver": "raw", -# "node-name": "new_node", -# "file": { "driver": "file", -# "filename": "test.raw" } } } } +# "driver": "raw", +# "node-name": "new_node", +# "file": { "driver": "file", +# "filename": "test.raw" } } } # <- { "return": {} } # -> { "execute": "x-blockdev-change", # "arguments": { "parent": "disk1",
In the process of getting rid of docs/qmp-commands.txt, we managed to regress on any text that changed after the point where the move was first branched and when the move actually occurred. For example, commit 3282eca for blockdev-snapshot re-added the extra "options" layer which had been cleaned up in commit 0153d2f. While I didn't audit for all such regressions, I did scrub for all bogus uses of nested "options". Signed-off-by: Eric Blake <eblake@redhat.com> --- qapi/block-core.json | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)