Re: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues

[Stefan Hajnoczi]

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).

Even if there is a monitor change, it's probably necessary to at least
clarify the meaning of the CLOSE event to reduce the chance of similar
bugs in future chardev users.

Stefan

signature.asc
Description: PGP signature