From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46310) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeleA-0005lc-QJ for qemu-devel@nongnu.org; Tue, 21 May 2013 08:26:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uele6-0004oO-1m for qemu-devel@nongnu.org; Tue, 21 May 2013 08:26:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1777) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uele5-0004oE-QE for qemu-devel@nongnu.org; Tue, 21 May 2013 08:26:25 -0400 Date: Tue, 21 May 2013 08:26:22 -0400 From: Luiz Capitulino Message-ID: <20130521082622.0fcb2061@redhat.com> In-Reply-To: <51963D77.4090107@redhat.com> References: <1366393639-20651-1-git-send-email-lcapitulino@redhat.com> <20130422135343.GF21317@stefanha-thinkpad.redhat.com> <20130425095147.75b90841@redhat.com> <20130425142945.GA11388@stefanha-thinkpad.redhat.com> <20130425103159.6bcf026d@redhat.com> <51963D77.4090107@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/2] block: fix spurious DEVICE_TRAY_MOVED events on shutdown List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: kwolf@redhat.com, Stefan Hajnoczi , qemu-devel@nongnu.org, armbru@redhat.com On Fri, 17 May 2013 16:23:51 +0200 Pavel Hrdina wrote: > On 25.4.2013 16:31, Luiz Capitulino wrote: > > On Thu, 25 Apr 2013 16:29:45 +0200 > > Stefan Hajnoczi wrote: > > > >> On Thu, Apr 25, 2013 at 09:51:47AM -0400, Luiz Capitulino wrote: > >>> On Mon, 22 Apr 2013 15:53:43 +0200 > >>> Stefan Hajnoczi wrote: > >>> > >>>> On Fri, Apr 19, 2013 at 01:47:17PM -0400, Luiz Capitulino wrote: > >>>>> Hi, > >>>>> > >>>>> This fixes a regression introduced by commit 9ca111544, as detailed in > >>>>> patch 2/2, by moving bdrv_dev_change_media_cb() calls to callers of > >>>>> bdrv_close() that need it, as suggested by Kevin. > >>>>> > >>>>> Luiz Capitulino (2): > >>>>> block: make bdrv_dev_change_media_cb() public > >>>>> block: move bdrv_dev_change_media_cb() to callers that really need it > >>>>> > >>>>> block.c | 5 +---- > >>>>> blockdev.c | 2 ++ > >>>>> include/block/block.h | 1 + > >>>>> 3 files changed, 4 insertions(+), 4 deletions(-) > >>>> > >>>> Looks okay but I'll wait for Markus or Kevin to review too. The media > >>>> change code is subtle, we've had a long history of fixes :). > >>> > >>> I wouldn't say this is hugely important, but I'm targeting 1.5. > >>> > >>> So, maybe lack of review means you could apply it? :) > >> > >> Nice try :) > > > > Hehe. > > > >> We've never gotten media change right. I really would appreciate a > >> second pair of eyes. There are still a couple of days until hard > >> freeze. > >> > >> Holding off until then. > > > > Ok, no problem. > > > > Hi all, > > I've just tested the "side effect" of my original commit and the > DEVICE_TRAY_MOVED event is emitted only if the CD-ROM is opened. If you > shutdown/reboot the guest with closed CD-ROM tray there is no > DEVICE_TRAY_MOVED event emitted. I think that this behavior is correct. That's not what I'm seeing here, unless the tray is opened right before shutdown, but even then the events are wrong as they notify the transition closed -> opened twice. On HMP: (qemu) info block ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok [not inserted] (qemu) system_powerdown on QMP: { "timestamp": { "seconds": 1369139052, "microseconds": 766612 }, "event": "DEVICE_TRAY_MOVED", "data": { "device": "ide1-cd0", "tray-open": true } } ` { "timestamp": { "seconds": 1369139052, "microseconds": 766798 }, "event": "DEVICE_TRAY_MOVED", "data": { "device": "floppy0", "tray-open": true } }