[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues |
Date: |
Wed, 13 Jun 2018 15:30:05 +0100 |
User-agent: |
Mutt/1.9.5 (2018-04-13) |
On Tue, Jun 12, 2018 at 01:47:00PM +0800, Peter Xu wrote:
> On Mon, Jun 11, 2018 at 05:45:49PM +0100, Stefan Hajnoczi wrote:
> > On Fri, Jun 08, 2018 at 10:18:25AM +0200, Markus Armbruster wrote:
> > > Stefan Hajnoczi <address@hidden> writes:
> > > > On Fri, Jun 08, 2018 at 12:42:35PM +0800, Peter Xu wrote:
> > > >> On Thu, Jun 07, 2018 at 01:53:01PM +0200, Markus Armbruster wrote:
> > > >> > Peter Xu <address@hidden> writes:
> > > >> >
> > > >> > > Previously we cleanup the queues when we got CLOSED event. It was
> > > >> > > used
> > > >> >
> > > >> > we clean up
> > > >> >
> > > >> > > to make sure we won't leftover replies/events of a old client to a
> > > >> > > new
> > > >> >
> > > >> > we won't send leftover replies/events of an old client
> > > >> >
> > > >> > > client. Now this patch postpones that until OPENED.
> > > >> >
> > > >> > What if OPENED never comes?
> > > >>
> > > >> Then we clean that up until destruction of the monitor. IMHO it's
> > > >> fine, but I'm not sure whether that's an overall acceptable approach.
> > > >
> > > > I think this patch fixes the problem at the wrong level. Marc-André's
> > > > fix seemed like a cleaner solution.
> > >
> > > Is it the right solution?
> > >
> > > I proposed another one:
> >
> > Sorry, I won't be able to participate in this because I'm behind on
> > other patches and tasks. Therefore, feel free to disregard but I'll
> > give my 2 cents:
> >
> > This seems like a chardev bug. The solution should probably be in the
> > chardev layer (Marc-André's patch or something else), not in the monitor
> > (this patch).
>
> Yes that's why I said I like Marc-Andre's patch. :) I just don't know
> an reliable way to achieve what we want there.
>
> The thing is that we don't really monitor the ioc_out for fd-typed
> chardevs. We only do that when we call qemu_chr_fe_add_watch() (e.g.,
> in monitor_flush_locked()) when the writting buffer is full. But
> normally we can't detect any event from the output side, hence no way
> to deliever a CLOSED event when the output fd is the last fd that is
> closed.
>
> Maybe we can keep that output watch for the whole lifecycle of the
> chardev? I'm not sure yet.
It's not possible to keep a POLLOUT fd watch since poll(2) would return
instantly and the event loop would spin. However, POLLHUP (G_IO_HUP)
might work.
Stefan
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, (continued)
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Markus Armbruster, 2018/06/07
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Peter Xu, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Markus Armbruster, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Stefan Hajnoczi, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Markus Armbruster, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Peter Xu, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Peter Xu, 2018/06/08
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Markus Armbruster, 2018/06/11
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Stefan Hajnoczi, 2018/06/11
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Peter Xu, 2018/06/12
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues,
Stefan Hajnoczi <=
- Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues, Peter Xu, 2018/06/13