On Thu, Oct 12, 2017 at 01:56:20PM +0100, Stefan Hajnoczi wrote: > On Fri, Sep 29, 2017 at 11:38:37AM +0800, Peter Xu wrote: > > Set maximum QMP request queue length to 8. If queue full, instead of > > queue the command, we directly return a "request-dropped" event, telling > > client that specific command is dropped. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > --- > > monitor.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/monitor.c b/monitor.c > > index 1e9a6cb6a5..d9bed31248 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -3971,6 +3971,8 @@ static void monitor_qmp_bh_dispatcher(void *data) > > } > > } > > > > +#define QMP_ASYNC_QUEUE_LEN_MAX (8) > > Why 8?
I proposed this in previous discussion and no one objected, so I just used it. It's here: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03989.html (please don't go over the thread; I'll copy the related paragraphs) """ ... Regarding to queue size: I am afraid max_size=1 may not suffice? Otherwise a simple batch of: {"execute": "query-status"} {"execute": "query-status"} Will trigger the failure. But I definitely agree it should not be something very large. The total memory will be this: json limit * queue length limit * monitor count limit (X) (Y) (Z) Now we have (X) already (in form of a few tunables for JSON token counts, etc.), we don't have (Z), and we definitely need (Y). How about we add limits on Y=16 and Z=8? We can do some math if we want some more exact number though. ... """ Oops, I proposed "16", but I used "8"; I hope 8 is good enough, but I am definitely not sure whether "1" is good. > > My understanding is that this patch series is not about asynchronous QMP > commands. Instead it's about executing certain commands immediately in > the parser thread. Indeed, but IMHO the series does something further than that - we do have async queues for QMP requests/responses now. IMHO that's real async, though totally different from the idea of "async QMP commands" for sure. > > Therefore, I suggest hardcoding length 1 for now and not calling it > "async". You may also be able to simplify the code since a queue isn't > actually needed. For the queue length: discussed above, I'm not sure whether queue=1 is really what we want. Again, I may be wrong. For the naming: how about QMP_REQ_QUEUE_LEN_MAX? Thanks, -- Peter Xu
