From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TO97V-0007VG-Aq for qemu-devel@nongnu.org; Tue, 16 Oct 2012 11:31:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TO97R-0001PH-6C for qemu-devel@nongnu.org; Tue, 16 Oct 2012 11:31:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42205) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TO97Q-0001PB-SS for qemu-devel@nongnu.org; Tue, 16 Oct 2012 11:31:45 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9GFVhZ1005487 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 16 Oct 2012 11:31:44 -0400 Message-ID: <507D7DDE.5010802@redhat.com> Date: Tue, 16 Oct 2012 11:31:42 -0400 From: Jeff Cody MIME-Version: 1.0 References: <421ff837d7c7dc9f661d71b2606c25b5a2b8ac3c.1350398238.git.jcody@redhat.com> <507D7BB3.7090505@redhat.com> In-Reply-To: <507D7BB3.7090505@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/4] block: in commit, determine base image from the top image Reply-To: jcody@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, qemu-devel@nongnu.org On 10/16/2012 11:22 AM, Eric Blake wrote: > On 10/16/2012 08:44 AM, Jeff Cody wrote: >> This simplifies some code and error checking, and also fixes a bug. >> >> bdrv_find_backing_image() should only be passed absolute filenames, >> or filenames relative to the chain. In the QMP message handler for >> block commit, when looking up the base do so from the determined top >> image, so we know it is reachable from top. >> >> Signed-off-by: Jeff Cody >> --- >> block/commit.c | 9 --------- >> blockdev.c | 21 +++++++++++---------- >> 2 files changed, 11 insertions(+), 19 deletions(-) > > As long as you are touching this code: > >> @@ -1182,6 +1172,17 @@ void qmp_block_commit(const char *device, >> return; >> } >> >> + if (base && has_base) { > > please swap this to 'has_base && base' to avoid any potential of > valgrind warnings about conditional jump based on uninitialized memory. > OK. > Also, I raised another bug[1] about a bad error message regarding > top_bs, if the user passes a different spelling than the canonical name > of the active image. Is that worth fixing in this series, or is it okay > to leave it until you actually add support for committing the top image? > > [1] https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02067.html > Since this doesn't pose an actual issue now, I was planning on just addressing that with the stage-2 patches, since that will likely touch other related areas of the code anyway. If you have major heartburn over this, I'll change it now.