From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UEcaZ-0005k7-MS for qemu-devel@nongnu.org; Sun, 10 Mar 2013 05:30:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UEcaW-0005n8-Ce for qemu-devel@nongnu.org; Sun, 10 Mar 2013 05:30:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52680) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UEcaW-0005n2-3o for qemu-devel@nongnu.org; Sun, 10 Mar 2013 05:30:40 -0400 Date: Sun, 10 Mar 2013 11:30:58 +0200 From: "Michael S. Tsirkin" Message-ID: <20130310093058.GA9148@redhat.com> References: <20130307184647.GA31012@redhat.com> <87d2vbx9rz.fsf@blackfin.pond.sub.org> <20130307202309.GA13088@redhat.com> <87a9qel3v0.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a9qel3v0.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v4] qdev: DEVICE_DELETED event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , Anthony Liguori , Eduardo Habkost , libvir-list@redhat.com, Stefan Hajnoczi , qemu-devel@nongnu.org, Luiz Capitulino , Gerd Hoffmann , Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= On Fri, Mar 08, 2013 at 08:58:43AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > > > On Thu, Mar 07, 2013 at 08:57:52PM +0100, Markus Armbruster wrote: > >> "Michael S. Tsirkin" writes: > >> > >> > libvirt has a long-standing bug: when removing the device, > >> > it can request removal but does not know when the > >> > removal completes. Add an event so we can fix this in a robust way. > >> > > >> > Signed-off-by: Michael S. Tsirkin > >> > >> Speaking as the acting QMP maintainer, just to avoid misunderstandings: > >> there's disagreement on the event's design, namely when it should fire, > >> and how it should name the device. I don't want the discussion > >> preempted by a commit. > > > > Yes, you are asking for more functionality, but can I add this in a > > follow-up commit please? I prefer this patch as is, as it can be > > backported to stable branches and downstreams. Upstream a follow up > > patch can add fields and more triggers which won't apply to any > > downstreams. > > If you want to address my review comments in a separate patch, go right > ahead. Please post both together as a series, for coherent review and > to simplify patch tracking. Sure. > I'm asking for two things: > > 1. Event member path. Fair to call this "more functionality". I agree > that backporting it to pre-QOM versions isn't practical. > > 2. Sane event trigger condition: on any device deletion, not just when > the device happens to have a qdev ID. This isn't "more", it's > "different". > > I'd definitely backport this part, because: > > * I abhor subtle semantic differences to upstream like a different > event trigger. > > * Backporting it reduces the difference to event member path missing. > Syntactic and in-your-face. > > * Without member path, the event triggered by deleting a device > without a qdev ID can't tell us which device went away. But you > can find out using the polling code you need anyway. Thus, the > event trigger is not only simpler and consistent with upstream, it > can also be more useful. > > [...] Will do, thanks for the comments. -- MST