[Qemu-devel] (no subject)

[Qemu-devel] (no subject)

[Jun Li]

stefanha@redhat.com
Bcc: 
Subject: Re: [Qemu-devel] qcow2: Can create qcow2 image format on rbd server
Reply-To: 
In-Reply-To: <5481E4E7.9010402@redhat.com>

On Fri, 12/05 18:01, Max Reitz wrote:
> On 2014-12-05 at 16:32, Jun Li wrote:
> >Currently, qemu-img can not create qcow2 image format on rbd server. Analysis
> >the code as followings:
> >when create qcow2 format image:
> >qcow2_create2
> >   bdrv_create_file(filename, opts, &local_err);  --> Here will create a 0 size
> >   file(e.g: file1) on rbd server.
> >   ...
> >   ret = bdrv_pwrite(bs, 0, header, cluster_size); --> So here can not write
> >   qcow2 header into file1 due to the file1's length is 0. Seems
> >   qemu_rbd_aio_writev can not write beyond EOF.
> >   ...
> >
> >As above analysis, there are two methods to solve the above bz as followings:
> >1, When create file1, just create a fixed-size file1 on rbd server(not 0 size).
> 
> Should be possible by using -o preallocation=falloc or -o
> preallocation=full.

Sure. If bdrv_create_file(filename, opts, &local_err) create a fixed-size(not
0 size) just as using "preallocation=falloc or preallocation=full", it will
create a fixed-size file on rbd server. So it won't exist above issue.

> 
> I can't say a lot about making rbd growable because I know near to nothing
> about rbd; but there are protocols which really simply don't support writes
> beyond the end of file, and where that's intended (for instance, while nbd
> somehow does support it when using the qemu nbd server, normally (strictly
> according to the protocol) it does not); so for these protocols, you have to
> use a preallocated image file or an image format which does not grow on
> writes (such as raw).
> 

Here just want to use rbd_resize to realize rbd growable.

> Of course, while that may be a solution for nbd, it doesn't sound like a
> good solution for rbd, so writes beyond the EOF should probably be supported
> there (although once again, I don't know rbd well enough to judge that).
> 

Yes, you are right. Also talked with stefan. Here just want to ask Josh Durgin
whether it has other solutions or rbd can support asynchronous rbd_resize.

Regards,
Jun Li

> 
> >2, When write the qcow2 header into file1, just let qemu_rbd_aio_writev can
> >enlarge the file1. So should add qemu_rbd_truncate inside qemu_rbd_aio_writev.
> >qemu_rbd_truncate will call rbd_resize, but seems rbd_resize is
> >synchronous function. If so, when do bdrv_pwrite, guest will hang.This is not
> >our expected.
> >
> >For method 1, maybe it's not corresponding to the original principle of qcow2.
> >Yes, it's very easy to solve the above bz. Nevertheless, I just want to use
> >method 2 to solve above issue.
> >
> >For method 2, could anyone give some suggestions on howto realize a
> >asynchronous rbd_resize. Thanks.
> >
> >Above analysis also based on stefan's hints. Thanks.
> >
> >Best Regards,
> >Jun Li
> 

Re: [Qemu-devel] your mail

[Jun Li]

Please ignore it. Sorry...

On Sat, 12/06 18:54, Jun Li wrote:
> stefanha@redhat.com
> Bcc: 
> Subject: Re: [Qemu-devel] qcow2: Can create qcow2 image format on rbd server
> Reply-To: 
> In-Reply-To: <5481E4E7.9010402@redhat.com>
> 
> On Fri, 12/05 18:01, Max Reitz wrote:
> > On 2014-12-05 at 16:32, Jun Li wrote:
> > >Currently, qemu-img can not create qcow2 image format on rbd server. Analysis
> > >the code as followings:
> > >when create qcow2 format image:
> > >qcow2_create2
> > >   bdrv_create_file(filename, opts, &local_err);  --> Here will create a 0 size
> > >   file(e.g: file1) on rbd server.
> > >   ...
> > >   ret = bdrv_pwrite(bs, 0, header, cluster_size); --> So here can not write
> > >   qcow2 header into file1 due to the file1's length is 0. Seems
> > >   qemu_rbd_aio_writev can not write beyond EOF.
> > >   ...
> > >
> > >As above analysis, there are two methods to solve the above bz as followings:
> > >1, When create file1, just create a fixed-size file1 on rbd server(not 0 size).
> > 
> > Should be possible by using -o preallocation=falloc or -o
> > preallocation=full.
> 
> Sure. If bdrv_create_file(filename, opts, &local_err) create a fixed-size(not
> 0 size) just as using "preallocation=falloc or preallocation=full", it will
> create a fixed-size file on rbd server. So it won't exist above issue.
> 
> > 
> > I can't say a lot about making rbd growable because I know near to nothing
> > about rbd; but there are protocols which really simply don't support writes
> > beyond the end of file, and where that's intended (for instance, while nbd
> > somehow does support it when using the qemu nbd server, normally (strictly
> > according to the protocol) it does not); so for these protocols, you have to
> > use a preallocated image file or an image format which does not grow on
> > writes (such as raw).
> > 
> 
> Here just want to use rbd_resize to realize rbd growable.
> 
> > Of course, while that may be a solution for nbd, it doesn't sound like a
> > good solution for rbd, so writes beyond the EOF should probably be supported
> > there (although once again, I don't know rbd well enough to judge that).
> > 
> 
> Yes, you are right. Also talked with stefan. Here just want to ask Josh Durgin
> whether it has other solutions or rbd can support asynchronous rbd_resize.
> 
> Regards,
> Jun Li
> 
> > 
> > >2, When write the qcow2 header into file1, just let qemu_rbd_aio_writev can
> > >enlarge the file1. So should add qemu_rbd_truncate inside qemu_rbd_aio_writev.
> > >qemu_rbd_truncate will call rbd_resize, but seems rbd_resize is
> > >synchronous function. If so, when do bdrv_pwrite, guest will hang.This is not
> > >our expected.
> > >
> > >For method 1, maybe it's not corresponding to the original principle of qcow2.
> > >Yes, it's very easy to solve the above bz. Nevertheless, I just want to use
> > >method 2 to solve above issue.
> > >
> > >For method 2, could anyone give some suggestions on howto realize a
> > >asynchronous rbd_resize. Thanks.
> > >
> > >Above analysis also based on stefan's hints. Thanks.
> > >
> > >Best Regards,
> > >Jun Li
> > 

Re: [Qemu-devel] your mail

[Kevin Wolf]

Am 16.11.2016 um 20:41 hat Christopher Oliver geschrieben:
> This patch (hack?) works around the slowness in SEEK_HOLE for large dense files
> on Linux tmpfs.  It may improve life elsewhere as well, and the penalty of the checks
> should be vanishingly small where it is not needed.
> 
> If I'm subtly (or not so subtly) wrong, please fire back.

Can't quote the commit message easily because you put it into an
attachment, but I'm not sure if we can make the assumption you're
making. Accessing a raw image from multiple VMs sounds valid to me
as long as a cluster file system is used.

Kevin

Re: [Qemu-devel] your mail

[Peter Xu]

On Tue, Jun 26, 2018 at 07:21:49PM +0200, Markus Armbruster wrote:
> I fooled around a bit, and I think there are a few lose ends.
> 
> Lets update the examples in docs/interop/qmp-spec.txt to show the
> current greeting (section 3.1) and how to accept a capability (section
> 3.2).  The capability negotiation documentation could use some polish.
> I'll post a patch.

Thanks for doing that!

> 
> Talking to a QMP monitor that supports OOB:
> 
>     $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": ["oob"]}}
>     QMP> { "execute": "qmp_capabilities", "arguments": { "oob": true } }
>     {"error": {"class": "GenericError", "desc": "Parameter 'oob' is unexpected"}}
>     QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
>     {"return": {}}
>     QMP> { "execute": "query-qmp-schema" }
>     {"error": {"class": "GenericError", "desc": "Out-Of-Band capability requires that every command contains an 'id' field"}}
> 
> Why does every command require 'id'?

The COMMAND_DROPPED event is one reason, as you mentioned in the other
thread. Meanwhile as long as we have OOB command it means that we can
have more than one commands running in parallel, then it's a must that
we can mark that out in the response to mark out which command we are
replying to.

> 
> Talking to a QMP monitor that doesn't support OOB:
> 
>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": []}}
>     QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
>     {"error": {"class": "GenericError", "desc": "This monitor does not support Out-Of-Band (OOB)"}}
>     QMP> { "execute": "qmp_capabilities" }
>     {"return": {}}
>     QMP> { "execute": "query-kvm" }
>     {"return": {"enabled": true, "present": true}}
>     QMP> { "execute": "query-kvm", "control": { "run-oob": true } }
>     {"error": {"class": "GenericError", "desc": "Please enable Out-Of-Band first for the session during capabilities negotiation"}}
> 
> Telling people to enable OOB when that cannot be done is suboptimal.
> More so when it cannot be used here anyway.  I'll post a patch.

Thanks again!

-- 
Peter Xu

Re: [Qemu-devel] your mail

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Jun 26, 2018 at 07:21:49PM +0200, Markus Armbruster wrote:
>> I fooled around a bit, and I think there are a few lose ends.
[...]
>> Talking to a QMP monitor that supports OOB:
>> 
>>     $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": ["oob"]}}
>>     QMP> { "execute": "qmp_capabilities", "arguments": { "oob": true } }
>>     {"error": {"class": "GenericError", "desc": "Parameter 'oob' is unexpected"}}
>>     QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
>>     {"return": {}}
>>     QMP> { "execute": "query-qmp-schema" }
>>     {"error": {"class": "GenericError", "desc": "Out-Of-Band capability requires that every command contains an 'id' field"}}
>> 
>> Why does every command require 'id'?

Why am I asking?  Requiring 'id' is rather onerous when you play with
QMP by hand.

> The COMMAND_DROPPED event is one reason, as you mentioned in the other
> thread. Meanwhile as long as we have OOB command it means that we can
> have more than one commands running in parallel, then it's a must that
> we can mark that out in the response to mark out which command we are
> replying to.

Without OOB, the client can match response to command, because it'll
receive responses in command issue order.

Example 1: after sending

    { "execute": "cmd1", ... }
    { "execute": "cmd2", ... }
    { "execute": "cmd3", ... }

the client will receive three responses, first one is cmd1's, second one
is cmd2's, third one is cmd3's.

With OOB, we have to independent command streams, in-band and
out-of-band.  Each separate stream's responses arrive in-order, but the
two streams may be interleaved.

Example 2: after sending

    { "execute": "cmd1", ... }
    { "execute": "cmd2", ... }
    { "execute": "cmd3", ... }
    { "execute": "oob1", "control": { "run-oob": true }, ... }
    { "execute": "oob2", "control": { "run-oob": true }, ... }
    { "execute": "oob3", "control": { "run-oob": true }, ... }

the client will receive six responses: cmd1's before cmd2's before
cmd3's, and oob1's before oob2's before oob3's.

If the client sends "id" with each command, it can match responses to
commands by id.

But that's not the only way to match.  Imagine the client had a perfect
oracle to tell it whether a response is in-band or out-of-band.  Then
matching can work pretty much as in example 1: the first in-band
response is cmd1's, the second one is cmd2's, and the third one is
cmd3's; the first out-of-band response is oob1's, the second one is
oob2's and the third one is oob3's.

Possible solutions other than requiring "id":

1. Make out-of-band responses recognizable

   Just like we copy "id" to the response, we could copy "run-oob", or
   maybe "control".

   Solves the "match response to command" problem, doesn't solve the
   "match COMMAND_DROPPED event to command" problem.

   Permit me a digression.  "control": { "run-oob": true } is awfully
   verbose.  Back when we hashed out the OOB design, I proposed to use
   "exec-oob" instead of "execute" to run a command OOB.  I missed when
   that morphed into flag "run-oob" wrapped in object "control".  Was it
   in response to reviewer feedback?  Can you give me a pointer?

   The counterpart to "exec-oob" would be "return-oob" and "error-oob".
   Hmm.

2. Do nothing; it's the client's problem

   If the client needs to match responses and events to commands, it
   should use the feature that was expressly designed for helping with
   that: "id".

   Note that QMP's initial design assumed asynchronous commands,
   i.e. respones that may arrive in any order.  Nevertheless, it did not
   require "id".  Same reasoning: if the client needs to match, it can
   use "id".

As so often when "do nothing" is a viable solution, it looks mighty
attractive to me :)

[...]

Re: [Qemu-devel] your mail

[Eric Blake]

On 06/28/2018 03:31 AM, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
>> On Tue, Jun 26, 2018 at 07:21:49PM +0200, Markus Armbruster wrote:
>>> I fooled around a bit, and I think there are a few lose ends.
> [...]
>>> Talking to a QMP monitor that supports OOB:
>>>
>>>      $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>>      {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": ["oob"]}}
>>>      QMP> { "execute": "qmp_capabilities", "arguments": { "oob": true } }
>>>      {"error": {"class": "GenericError", "desc": "Parameter 'oob' is unexpected"}}
>>>      QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
>>>      {"return": {}}
>>>      QMP> { "execute": "query-qmp-schema" }
>>>      {"error": {"class": "GenericError", "desc": "Out-Of-Band capability requires that every command contains an 'id' field"}}
>>>
>>> Why does every command require 'id'?
> 
> Why am I asking?  Requiring 'id' is rather onerous when you play with
> QMP by hand.
> 

> Possible solutions other than requiring "id":
> 
> 1. Make out-of-band responses recognizable
> 
>     Just like we copy "id" to the response, we could copy "run-oob", or
>     maybe "control".
> 
>     Solves the "match response to command" problem, doesn't solve the
>     "match COMMAND_DROPPED event to command" problem.
> 
>     Permit me a digression.  "control": { "run-oob": true } is awfully
>     verbose.  Back when we hashed out the OOB design, I proposed to use
>     "exec-oob" instead of "execute" to run a command OOB.  I missed when
>     that morphed into flag "run-oob" wrapped in object "control".  Was it
>     in response to reviewer feedback?  Can you give me a pointer?

It's not too late to change back.  Since OOB is still new to 3.0, we 
could indeed go with a shorter invocation of 'exec-oob' (and I'd 
actually be in favor of such a change).

> 
>     The counterpart to "exec-oob" would be "return-oob" and "error-oob".
>     Hmm.

In other words, the change in the keyword of the response will let you 
know that it is replying to the most recent oob command.  This works 
well if we guarantee that we never have more than one unprocessed oob 
command in the pipeline at a time; but fails if oob commands can be 
threaded against one another and start replying in a different order 
than original submission.  Or, we could state that if you use 
'exec-oob', then 'id' is mandatory (and 'id' in the 'return' or 'error' 
is then sufficient to tie it back); but when using plain 'execute', 'id' 
remains optional.

> 
> 2. Do nothing; it's the client's problem
> 
>     If the client needs to match responses and events to commands, it
>     should use the feature that was expressly designed for helping with
>     that: "id".
> 
>     Note that QMP's initial design assumed asynchronous commands,
>     i.e. respones that may arrive in any order.  Nevertheless, it did not
>     require "id".  Same reasoning: if the client needs to match, it can
>     use "id".
> 
> As so often when "do nothing" is a viable solution, it looks mighty
> attractive to me :)

Indeed, although the documentation should still be explicit on 
recommending the use of 'id' when oob is enabled.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] your mail

[Daniel P. Berrangé]

On Thu, Jun 28, 2018 at 10:31:42AM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Jun 26, 2018 at 07:21:49PM +0200, Markus Armbruster wrote:
> >> I fooled around a bit, and I think there are a few lose ends.
> [...]
> >> Talking to a QMP monitor that supports OOB:
> >> 
> >>     $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> '
> >>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": ["oob"]}}
> >>     QMP> { "execute": "qmp_capabilities", "arguments": { "oob": true } }
> >>     {"error": {"class": "GenericError", "desc": "Parameter 'oob' is unexpected"}}
> >>     QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
> >>     {"return": {}}
> >>     QMP> { "execute": "query-qmp-schema" }
> >>     {"error": {"class": "GenericError", "desc": "Out-Of-Band capability requires that every command contains an 'id' field"}}
> >> 
> >> Why does every command require 'id'?
> 
> Why am I asking?  Requiring 'id' is rather onerous when you play with
> QMP by hand.
> 
> > The COMMAND_DROPPED event is one reason, as you mentioned in the other
> > thread. Meanwhile as long as we have OOB command it means that we can
> > have more than one commands running in parallel, then it's a must that
> > we can mark that out in the response to mark out which command we are
> > replying to.
> 
> Without OOB, the client can match response to command, because it'll
> receive responses in command issue order.
> 
> Example 1: after sending
> 
>     { "execute": "cmd1", ... }
>     { "execute": "cmd2", ... }
>     { "execute": "cmd3", ... }
> 
> the client will receive three responses, first one is cmd1's, second one
> is cmd2's, third one is cmd3's.
> 
> With OOB, we have to independent command streams, in-band and
> out-of-band.  Each separate stream's responses arrive in-order, but the
> two streams may be interleaved.
> 
> Example 2: after sending
> 
>     { "execute": "cmd1", ... }
>     { "execute": "cmd2", ... }
>     { "execute": "cmd3", ... }
>     { "execute": "oob1", "control": { "run-oob": true }, ... }
>     { "execute": "oob2", "control": { "run-oob": true }, ... }
>     { "execute": "oob3", "control": { "run-oob": true }, ... }
> 
> the client will receive six responses: cmd1's before cmd2's before
> cmd3's, and oob1's before oob2's before oob3's.

I thought the intention was that oob1, oob2, and oob3 are defined
to return in any order. It just happens that we only hve a single
oob processing stream right now, but we may have more in future.

> If the client sends "id" with each command, it can match responses to
> commands by id.
> 
> But that's not the only way to match.  Imagine the client had a perfect
> oracle to tell it whether a response is in-band or out-of-band.  Then
> matching can work pretty much as in example 1: the first in-band
> response is cmd1's, the second one is cmd2's, and the third one is
> cmd3's; the first out-of-band response is oob1's, the second one is
> oob2's and the third one is oob3's.

Not if oob1/2/3 can retunr in any order in future, which I think we
should be allowing fore.

IMHO 'id' should be mandatory for oob usage.

> Possible solutions other than requiring "id":
> 
> 1. Make out-of-band responses recognizable
> 
>    Just like we copy "id" to the response, we could copy "run-oob", or
>    maybe "control".
> 
>    Solves the "match response to command" problem, doesn't solve the
>    "match COMMAND_DROPPED event to command" problem.
> 
>    Permit me a digression.  "control": { "run-oob": true } is awfully
>    verbose.  Back when we hashed out the OOB design, I proposed to use
>    "exec-oob" instead of "execute" to run a command OOB.  I missed when
>    that morphed into flag "run-oob" wrapped in object "control".  Was it
>    in response to reviewer feedback?  Can you give me a pointer?
> 
>    The counterpart to "exec-oob" would be "return-oob" and "error-oob".
>    Hmm.

I do prefer the less verbose proposal too.

> 2. Do nothing; it's the client's problem
> 
>    If the client needs to match responses and events to commands, it
>    should use the feature that was expressly designed for helping with
>    that: "id".
> 
>    Note that QMP's initial design assumed asynchronous commands,
>    i.e. respones that may arrive in any order.  Nevertheless, it did not
>    require "id".  Same reasoning: if the client needs to match, it can
>    use "id".

IMHO we should just mandate 'id' when using "exec-oob", as it is
conceptually broken to use OOB without a way to match responses.
We don't want clients relying on all OOB replies coming in sequence
now, and then breaking when we allow multiple OOB processing threads.
That would require us to add a eec-oob-no-really-i-mean-it command
to allow overlapping OOB responses later.

> 
> As so often when "do nothing" is a viable solution, it looks mighty
> attractive to me :)
> 
> [...]
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] your mail

[Peter Xu]

On Thu, Jun 28, 2018 at 01:00:44PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 28, 2018 at 10:31:42AM +0200, Markus Armbruster wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > On Tue, Jun 26, 2018 at 07:21:49PM +0200, Markus Armbruster wrote:
> > >> I fooled around a bit, and I think there are a few lose ends.
> > [...]
> > >> Talking to a QMP monitor that supports OOB:
> > >> 
> > >>     $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> '
> > >>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, "package": "v2.12.0-1703-gb909799463"}, "capabilities": ["oob"]}}
> > >>     QMP> { "execute": "qmp_capabilities", "arguments": { "oob": true } }
> > >>     {"error": {"class": "GenericError", "desc": "Parameter 'oob' is unexpected"}}
> > >>     QMP> { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
> > >>     {"return": {}}
> > >>     QMP> { "execute": "query-qmp-schema" }
> > >>     {"error": {"class": "GenericError", "desc": "Out-Of-Band capability requires that every command contains an 'id' field"}}
> > >> 
> > >> Why does every command require 'id'?
> > 
> > Why am I asking?  Requiring 'id' is rather onerous when you play with
> > QMP by hand.
> > 
> > > The COMMAND_DROPPED event is one reason, as you mentioned in the other
> > > thread. Meanwhile as long as we have OOB command it means that we can
> > > have more than one commands running in parallel, then it's a must that
> > > we can mark that out in the response to mark out which command we are
> > > replying to.
> > 
> > Without OOB, the client can match response to command, because it'll
> > receive responses in command issue order.
> > 
> > Example 1: after sending
> > 
> >     { "execute": "cmd1", ... }
> >     { "execute": "cmd2", ... }
> >     { "execute": "cmd3", ... }
> > 
> > the client will receive three responses, first one is cmd1's, second one
> > is cmd2's, third one is cmd3's.
> > 
> > With OOB, we have to independent command streams, in-band and
> > out-of-band.  Each separate stream's responses arrive in-order, but the
> > two streams may be interleaved.
> > 
> > Example 2: after sending
> > 
> >     { "execute": "cmd1", ... }
> >     { "execute": "cmd2", ... }
> >     { "execute": "cmd3", ... }
> >     { "execute": "oob1", "control": { "run-oob": true }, ... }
> >     { "execute": "oob2", "control": { "run-oob": true }, ... }
> >     { "execute": "oob3", "control": { "run-oob": true }, ... }
> > 
> > the client will receive six responses: cmd1's before cmd2's before
> > cmd3's, and oob1's before oob2's before oob3's.
> 
> I thought the intention was that oob1, oob2, and oob3 are defined
> to return in any order. It just happens that we only hve a single
> oob processing stream right now, but we may have more in future.

Exactly.

> 
> > If the client sends "id" with each command, it can match responses to
> > commands by id.
> > 
> > But that's not the only way to match.  Imagine the client had a perfect
> > oracle to tell it whether a response is in-band or out-of-band.  Then
> > matching can work pretty much as in example 1: the first in-band
> > response is cmd1's, the second one is cmd2's, and the third one is
> > cmd3's; the first out-of-band response is oob1's, the second one is
> > oob2's and the third one is oob3's.
> 
> Not if oob1/2/3 can retunr in any order in future, which I think we
> should be allowing fore.
> 
> IMHO 'id' should be mandatory for oob usage.
> 
> > Possible solutions other than requiring "id":
> > 
> > 1. Make out-of-band responses recognizable
> > 
> >    Just like we copy "id" to the response, we could copy "run-oob", or
> >    maybe "control".
> > 
> >    Solves the "match response to command" problem, doesn't solve the
> >    "match COMMAND_DROPPED event to command" problem.
> > 
> >    Permit me a digression.  "control": { "run-oob": true } is awfully
> >    verbose.  Back when we hashed out the OOB design, I proposed to use
> >    "exec-oob" instead of "execute" to run a command OOB.  I missed when
> >    that morphed into flag "run-oob" wrapped in object "control".  Was it
> >    in response to reviewer feedback?  Can you give me a pointer?

It's me who proposed this, not from any reviewer's feedback.  It just
happened that no one was against it.

> > 
> >    The counterpart to "exec-oob" would be "return-oob" and "error-oob".
> >    Hmm.
> 
> I do prefer the less verbose proposal too.

But frankly speaking I still prefer current way.  QMP is majorly for
clients (computer programs) rather than users to use it.  Comparing to
verbosity, I would care more about coherency for how we define the
interface.  Say, OOB execution is still one way to execute a command,
so naturally it should still be using the same way that we execute a
QMP command, we just need some extra fields to tell the server that
"this message is more urgent than the other ones".

If we are discussing HMP interfaces, I'll have the same preference
with both of you to consider more about whether it's user-friendly or
not.  But now we are talking about QMP, then I'll prefer "control".

In the future, it's also possible to add some more sub-fields into the
"control" field.  When that happens, do we want to introduce another
standalone wording for that?  I would assume the answer is no.

> 
> > 2. Do nothing; it's the client's problem
> > 
> >    If the client needs to match responses and events to commands, it
> >    should use the feature that was expressly designed for helping with
> >    that: "id".
> > 
> >    Note that QMP's initial design assumed asynchronous commands,
> >    i.e. respones that may arrive in any order.  Nevertheless, it did not
> >    require "id".  Same reasoning: if the client needs to match, it can
> >    use "id".
> 
> IMHO we should just mandate 'id' when using "exec-oob", as it is
> conceptually broken to use OOB without a way to match responses.
> We don't want clients relying on all OOB replies coming in sequence
> now, and then breaking when we allow multiple OOB processing threads.
> That would require us to add a eec-oob-no-really-i-mean-it command
> to allow overlapping OOB responses later.

Agreed, again.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] your mail

[Eric Blake]

On 06/29/2018 04:57 AM, Peter Xu wrote:

>>>     Permit me a digression.  "control": { "run-oob": true } is awfully
>>>     verbose.  Back when we hashed out the OOB design, I proposed to use
>>>     "exec-oob" instead of "execute" to run a command OOB.  I missed when
>>>     that morphed into flag "run-oob" wrapped in object "control".  Was it
>>>     in response to reviewer feedback?  Can you give me a pointer?
> 
> It's me who proposed this, not from any reviewer's feedback.  It just
> happened that no one was against it.

Only because I didn't notice the difference, and was helping clear the 
QAPI backlog before the release.  You've now had both the maintainer and 
the backup express a desire for the shorter form.

> 
>>>
>>>     The counterpart to "exec-oob" would be "return-oob" and "error-oob".
>>>     Hmm.
>>
>> I do prefer the less verbose proposal too.
> 
> But frankly speaking I still prefer current way.  QMP is majorly for
> clients (computer programs) rather than users to use it.  Comparing to
> verbosity, I would care more about coherency for how we define the
> interface.  Say, OOB execution is still one way to execute a command,
> so naturally it should still be using the same way that we execute a
> QMP command, we just need some extra fields to tell the server that
> "this message is more urgent than the other ones".

Code-wise, it's actually simpler for libvirt to write:

if (oob) {
     virJSONValueObjectCreate(&cmd, "s:exec-oob", cmdname, ...);
} else {
     virJSONValueObjectCreate(&cmd, "s:execute", cmdname, ...);
}

that it is to write:

virJSONValueObjectCreate(&cmd, "s:execute", cmdname, ...);
if (oob) {
     virJSONValuePtr control;
     virJSONValueObjectCreate(&control, "b:run-oob", true, NULL);
     virJSONValueObjectAppend(&cmd, "control", control);
}

(plus error-checking that I omitted).

In short, adding extra fields is MORE work than just using the command 
name AS the additional bit of information.

> 
> If we are discussing HMP interfaces, I'll have the same preference
> with both of you to consider more about whether it's user-friendly or
> not.  But now we are talking about QMP, then I'll prefer "control".

If you don't want to write the patch, then probably Markus or I will.

> 
> In the future, it's also possible to add some more sub-fields into the
> "control" field.  When that happens, do we want to introduce another
> standalone wording for that?  I would assume the answer is no.

We may add a "control" field at that time, and may even offer 
convenience syntax to allow "exec-oob" to behave as if a "control" field 
were passed.  But the addition of new control features is rare, so I'd 
rather deal with that when we have something to actually add, rather 
than making us suffer with unneeded verbosity for potentially years 
waiting for that next control field to materialize.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org