Eric Blake <ebl...@redhat.com> writes: > On 07/02/2018 11:21 AM, Markus Armbruster wrote: >> Commit cf869d53172 "qmp: support out-of-band (oob) execution" made >> "id" mandatory for all commands when the client accepted capability >> "oob". This is rather onerous when you play with QMP by hand, and >> unnecessarily so: only out-of-band commands need an ID for reliable >> matching of response to command. >> >> Revert that part of commit cf869d53172 for now. We may still make >> "id" mandatory for out-of-band commands. > > Fair enough for now (I still think mandatory "id" for oob commands is > worthwhile, but I also think "exec-oob" is worthwhile, and it's > probably easier to write the logic for mandating "id" after that > tweak). > >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> docs/interop/qmp-spec.txt | 9 +++------ >> monitor.c | 7 ------- >> 2 files changed, 3 insertions(+), 13 deletions(-) >> > >> +++ b/docs/interop/qmp-spec.txt >> @@ -103,16 +103,13 @@ The format for command execution is: >> required. Each command documents what contents will be considered >> valid when handling the json-argument >> - The "id" member is a transaction identification associated with the >> - command execution. It is required for all commands if the OOB - >> - capability was enabled at startup, and optional otherwise. The same >> - "id" field will be part of the response if provided. The "id" >> - member can be any json-value. A json-number incremented for each >> - successive command works fine. >> + command execution, it is optional and will be part of the response >> + if provided. The "id" member can be any json-value. A json-number >> + incremented for each successive command works fine. >> - The optional "control" member further specifies how the command is >> to be executed. Currently, its only member is optional "run-oob". >> See section "2.3.1 Out-of-band execution" for details. > > We should STILL recommend that a client should use "id" when using > oob, whether or not we mandate it later.
Yes. I'll take care of it in v2. > >> - >> 2.3.1 Out-of-band execution >> --------------------------- >> diff --git a/monitor.c b/monitor.c >> index 96e87d8664..b7d74b01b4 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -4291,13 +4291,6 @@ static void handle_qmp_command(JSONMessageParser >> *parser, GQueue *tokens) >> id = qdict_get(qdict, "id"); >> - /* When OOB is enabled, the "id" field is mandatory. */ >> - if (qmp_oob_enabled(mon) && !id) { >> - error_setg(&err, "Out-of-band capability requires that " >> - "every command contains an 'id' field"); >> - goto err; >> - } > > I can live with this hunk right away, but the documentation feels weak > enough that I'm reluctant to give R-b to the patch as-is