On Mon, Dec 25, 2017 at 03:22:24PM +0800, Peter Xu wrote: > On Mon, Dec 25, 2017 at 03:13:49PM +0800, Fam Zheng wrote: > > On Mon, 12/25 14:18, Peter Xu wrote: > > > On Mon, Dec 25, 2017 at 01:55:56PM +0800, Fam Zheng wrote: > > > > On Mon, 12/25 13:18, Peter Xu wrote: > > > > > On Thu, Dec 21, 2017 at 07:42:46PM +0800, Fam Zheng wrote: > > > > > > On Tue, 12/19 16:45, Peter Xu wrote: > > > > > > > Set maximum QMP command queue length to 8. If queue full, > > > > > > > instead of > > > > > > > queue the command, we directly return a "command-dropped" event, > > > > > > > telling > > > > > > > client that specific command is dropped. > > > > > > > > > > > > > > Note that this flow control mechanism is only valid if OOB is > > > > > > > enabled. > > > > > > > If it's not, the effective queue length will always be 1, which > > > > > > > strictly > > > > > > > follows original behavior of QMP command handling (which never > > > > > > > drop > > > > > > > messages). > > > > > > > > > > > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > > > > > > --- > > > > > > > monitor.c | 17 ++++++++++++++++- > > > > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/monitor.c b/monitor.c > > > > > > > index ed9a741d06..b571866659 100644 > > > > > > > --- a/monitor.c > > > > > > > +++ b/monitor.c > > > > > > > @@ -4038,6 +4038,8 @@ static void monitor_qmp_bh_dispatcher(void > > > > > > > *data) > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > +#define QMP_REQ_QUEUE_LEN_MAX (8) > > > > > > > > > > > > Is this limit introspectable on QMP? > > > > > > > > > > Not yet. IMHO it's really QMP internal stuff, and I see no benefit so > > > > > far for a client to know about this... > > > > > > > > A client may need this number to batch (non-oob) commands without > > > > worrying about > > > > getting command-dropped event. > > > > > > IMHO QMP batching will only be useful when performance matters, but > > > for now IMHO we don't need to worry much about QMP performance? When > > > we do, I suspect we need to do more things to make sure of it, and > > > exposing this single parameter may not really help much, say, for now > > > even the client batches the stuff, they still need to wait for the > > > completion of commands. > > > > I think when we declare that commands can be dropped when the queue is > > full, we > > should be clear about in what condition a queue is full, and don't make > > users > > guess. If we want is this incompleteness, and I'm the only one who doesn't > > like > > it, that may be fine. It's just that, like you said, this seems a bit > > useless. > > Just to clarify that this maximum queue length is still useful to us > (not the clients) in the sense that we'll have limited memory usage > for QMP command queues. Otherwise we have risk on using up all the > memory of the host or reach the limitation of existing QEMU process, > either of which is bad.
I think batching doesn't make much sense when "oob" is enabled. There are only very narrow use cases where it's reasonable to batch: Batched commands must be independent of each other because any batched command can be dropped: A B C A can be dropped if there are already one or more commands pending. If this happens then B or C might run or be dropped too, depending on timing. Possible combinations are: <none> A A B A C A B C B B C C The client has very little control and needs to implement retry logic if commands are dropped. I bet this is going to be very error prone for client developers... One reasonable use case is batching query-* commands. I can't think of many other use cases where it makes sense. I'd be perfectly happy if QEMU did no batching at all when "oob" is enabled (i.e. hardcoded 1 command lookahead). Stefan
signature.asc
Description: PGP signature
