From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLqpi-0006XJ-4j for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:35:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLqpc-0000xh-8I for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:35:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33213) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLqpb-0000xX-UT for qemu-devel@nongnu.org; Wed, 10 Oct 2012 03:35:52 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9A7ZptS005850 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 10 Oct 2012 03:35:51 -0400 Message-ID: <50752554.1010601@redhat.com> Date: Wed, 10 Oct 2012 09:35:48 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <633f30c7b1812692ca9cd4497f502d5a7366e7e3.1349848348.git.jcody@redhat.com> In-Reply-To: <633f30c7b1812692ca9cd4497f502d5a7366e7e3.1349848348.git.jcody@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] block: in commit, determine base image from the top image List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, qemu-devel@nongnu.org Il 10/10/2012 07:56, Jeff Cody ha scritto: > 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(-) > > diff --git a/block/commit.c b/block/commit.c > index 733c914..13d9e82 100644 > --- a/block/commit.c > +++ b/block/commit.c > @@ -211,15 +211,6 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, > return; > } > > - /* top and base may be valid, but let's make sure that base is reachable > - * from top */ > - if (bdrv_find_backing_image(top, base->filename) != base) { > - error_setg(errp, > - "Base (%s) is not reachable from top (%s)", > - base->filename, top->filename); > - return; > - } > - > overlay_bs = bdrv_find_overlay(bs, top); > > if (overlay_bs == NULL) { > diff --git a/blockdev.c b/blockdev.c > index 99828ad..7052287 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1157,16 +1157,6 @@ void qmp_block_commit(const char *device, > error_set(errp, QERR_DEVICE_NOT_FOUND, device); > return; > } > - if (base && has_base) { > - base_bs = bdrv_find_backing_image(bs, base); > - } else { > - base_bs = bdrv_find_base(bs); > - } > - > - if (base_bs == NULL) { > - error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL"); > - return; > - } > > /* default top_bs is the active layer */ > top_bs = bs; > @@ -1182,6 +1172,17 @@ void qmp_block_commit(const char *device, > return; > } > > + if (base && has_base) { > + base_bs = bdrv_find_backing_image(top_bs, base); > + } else { > + base_bs = bdrv_find_base(top_bs); > + } > + > + if (base_bs == NULL) { > + error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL"); > + return; > + } > + > commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs, > &local_err); > if (local_err != NULL) { > This one looks good, Reviewed-by: Paolo Bonzini Paolo