From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpfVp-0003IG-4O for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:06:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UpfVn-000358-SV for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:06:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpfVn-00034w-M3 for qemu-devel@nongnu.org; Thu, 20 Jun 2013 10:06:55 -0400 Message-ID: <51C30C7C.3020701@redhat.com> Date: Thu, 20 Jun 2013 16:06:52 +0200 From: Pavel Hrdina MIME-Version: 1.0 References: <6c33b1e54320663db2ae9c606bde4cb13ca42928.1371474572.git.phrdina@redhat.com> <20130619101658.GA31475@stefanha-thinkpad.muc.redhat.com> In-Reply-To: <20130619101658.GA31475@stefanha-thinkpad.muc.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] block: move the bdrv_dev_change_media_cb() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com On 19.6.2013 12:16, Stefan Hajnoczi wrote: > On Mon, Jun 17, 2013 at 03:21:41PM +0200, Pavel Hrdina wrote: >> The bdrv_dev_change_media_cb() should be called only for eject and change >> commands. We should call that function only if that command is successful. >> >> What this function does is that it calls the change_media_cb() and also emit >> the QEVENT_DEVICE_TRAY_MOVED event. >> >> If a password is not required, but user provides some, the error is used as >> warning. >> >> Signed-off-by: Pavel Hrdina >> --- >> block.c | 8 -------- >> blockdev.c | 7 +++++++ >> 2 files changed, 7 insertions(+), 8 deletions(-) > > This commit description explains what the code changes do but it doesn't > explain why. The cover letter mentions a regression without going into > detail, and that will not be commited to git. Please add information > about the regression that this patch fixes so the git history has enough > information to justify this patch. Thanks, I explain this change directly in the commit message. > > Markus posted a list of places that are affected by this change. Have > you worked through them to show this patch is safe? Today I've checked hopefully all possible ways how to get into the 'bdrv_dev_change_media_cb()' regarding Markus' comment. The only relevant callers are the qmp_change and qmp_eject because the purpose of the 'bdrv_dev_change_media_cb()' is to call the devices' handlers for change media event, update the tray status and also emit the DEVICE_TRAY_MOVED event. Pavel > > Stefan >