[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 15/22] monitor: send event when request queue f
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC v2 15/22] monitor: send event when request queue full |
Date: |
Wed, 18 Oct 2017 17:28:04 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Mon, Oct 16, 2017 at 04:11:58PM +0800, Peter Xu wrote:
> 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 <address@hidden>
> > > ---
> > > 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.
I understand the concern about breaking existing clients but choosing an
arbitrary magic number isn't a correct solution to that problem because
existing clients may exceed the magic number!
Instead I think QMP should only look ahead if the out-of-band feature
has been negotatiated. This way existing clients continue to work. New
clients will have to avoid sending a batch of requests or they must
handle the queue size limit error.
> For the naming: how about QMP_REQ_QUEUE_LEN_MAX?
Yes.
Stefan