[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatc
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher |
Date: |
Mon, 18 Dec 2017 09:10:53 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Mon, Dec 18, 2017 at 01:26:49PM +0800, Peter Xu wrote:
> On Sat, Dec 16, 2017 at 09:23:22AM +0000, Stefan Hajnoczi wrote:
> > On Sat, Dec 16, 2017 at 02:37:03PM +0800, Peter Xu wrote:
> > > On Wed, Dec 13, 2017 at 08:09:38PM +0000, Stefan Hajnoczi wrote:
> > > > On Tue, Dec 05, 2017 at 01:51:50PM +0800, Peter Xu wrote:
> > > > > @@ -3956,12 +3968,122 @@ static void
> > > > > handle_qmp_command(JSONMessageParser *parser, GQueue *tokens,
> > > > > }
> > > > > }
> > > > >
> > > > > -err_out:
> > > > > - monitor_qmp_respond(mon, rsp, err, id);
> > > > > + /* Respond if necessary */
> > > > > + monitor_qmp_respond(mon, rsp, NULL, id);
> > > > > +
> > > > > + /* This pairs with the monitor_suspend() in
> > > > > handle_qmp_command(). */
> > > > > + if (!qmp_oob_enabled(mon)) {
> > > > > + monitor_resume(mon);
> > > >
> > > > monitor_resume() does not work between threads: if the event loop is
> > > > currently blocked in poll() it won't notice that the monitor fd should
> > > > be watched again.
> > > >
> > > > Please add aio_notify() to monitor_resume() and monitor_suspend(). That
> > > > way the event loop is forced to check can_read() again.
> > >
> > > Ah, yes. I think monitor_suspend() does not need the notify? Since
> > > if it's sleeping it won't miss the next check in can_read() after all?
> >
> > No, that would be a bug. Imagine the IOThread is blocked in poll
> > monitoring the chardev file descriptor when the main loop calls
> > monitor_suspend(). If the file descriptors becomes readable then the
> > handler function executes even though the monitor is supposed to be
> > suspended!
>
> When you say "the handler function executes", do you mean the handler
> that has already added to the qmp request queue, or the one that
> hasn't yet parsed by the parser?
The chardev file descriptor handler function (the QMP parser).
> For the previous case (the handler that has queued already): IMHO
> that's what we expect it to behave, say, when we call
> monitor_suspend(), we only stop accepting and parsing new inputs from
> the user, but the requests on the queue should still be processed.
>
> For the latter (the handler of a newly typed command):
> monitor_suspend() should suspend the parser already, so
> monitor_can_read() check should fail, then that command should never
> be queued until we call another monitor_resume().
You are assuming that monitor_can_read() is called *after* poll()
returns. This is what I tried to explain in the previous reply.
The the monitor_can_read() function is called *before* the blocking
poll() syscall. If something changes the monitor_can_read() return
value, you must* kick the event loop to ensure that the event loop
reflects this change.
If you want to check how this works, see chardev/char-io.c for how
fd_can_read() is used.
* Currently monitor.c doesn't need to kick the event loop explicitly
because it runs within the main loop thread. Therefore the event loop
always calls monitor_can_read() again before entering poll().
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [RFC v5 15/26] monitor: let suspend_cnt be thread safe, (continued)
[Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/12/05
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Stefan Hajnoczi, 2017/12/13
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/12/16
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/12/16
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Stefan Hajnoczi, 2017/12/16
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/12/18
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher,
Stefan Hajnoczi <=
- Re: [Qemu-devel] [RFC v5 16/26] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/12/18
[Qemu-devel] [RFC v5 17/26] qmp: add new event "request-dropped", Peter Xu, 2017/12/05
[Qemu-devel] [RFC v5 18/26] monitor: send event when request queue full, Peter Xu, 2017/12/05