From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGs3U-0002xZ-7W for qemu-devel@nongnu.org; Wed, 26 Sep 2012 09:53:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGs3P-0001M5-VF for qemu-devel@nongnu.org; Wed, 26 Sep 2012 09:53:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45129) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGs3P-0001Lh-Gy for qemu-devel@nongnu.org; Wed, 26 Sep 2012 09:53:31 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8QDrU6j005304 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 26 Sep 2012 09:53:30 -0400 Message-ID: <506308D7.6080209@redhat.com> Date: Wed, 26 Sep 2012 15:53:27 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <75d3d050da8a4b3cee36d8c3c6dbf67014b7db6c.1348589526.git.jcody@redhat.com> In-Reply-To: <75d3d050da8a4b3cee36d8c3c6dbf67014b7db6c.1348589526.git.jcody@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/7] block: add support functions for live commit, to find and delete images. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: pbonzini@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org Am 25.09.2012 18:29, schrieb Jeff Cody: > Add bdrv_find_overlay(), and bdrv_drop_intermediate(). > > bdrv_find_overlay(): given 'bs' and the active (topmost) BDS of an image chain, > find the image that is the immediate top of 'bs' > > bdrv_drop_intermediate(): > Given 3 BDS (active, top, base), drop images above > base up to and including top, and set base to be the > backing file of top's overlay node. > > E.g., this converts: > > bottom <- base <- intermediate <- top <- active > > to > > bottom <- base <- active > > Signed-off-by: Jeff Cody > +/* > + * Drops images above 'base' up to and including 'top', and sets the image > + * above 'top' to have base as its backing file. > + * > + * Requires that the overlay to 'top' is opened r/w, so that the backing file > + * information in 'bs' can be properly updated. > + * > + * E.g., this will convert the following chain: > + * bottom <- base <- intermediate <- top <- active > + * > + * to > + * > + * bottom <- base <- active > + * > + * It is allowed for bottom==base, in which case it converts: > + * > + * base <- intermediate <- top <- active > + * > + * to > + * > + * base <- active Compared to RFC v2 you deleted this part of the comment: + * + * It is also allowed for top==active, except in that case active is not + * deleted: You describe the condition now as an error (which I think is what our conclusion on IRC was), but now I think the following example must be removed as well: > + * > + * base <- intermediate <- top > + * > + * becomes > + * > + * base <- top > + * > + * Error conditions: > + * if active == top, that is considered an error > + * > + */ > +int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, > + BlockDriverState *base) > +{ > + BlockDriverState *intermediate; > + BlockDriverState *base_bs = NULL; > + BlockDriverState *new_top_bs = NULL; > + BlkIntermediateStates *intermediate_state, *next; > + int ret = -EIO; > + > + QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete; > + QSIMPLEQ_INIT(&states_to_delete); > + > + if (!top->drv || !base->drv) { > + goto exit; > + } > + > + new_top_bs = bdrv_find_overlay(active, top); > + > + if (new_top_bs == NULL) { > + /* we could not find the image above 'top', this is an error */ > + goto exit; > + } > + > + /* special case of new_top_bs->backing_hd already pointing to base - nothing > + * to do, no intermediate images */ > + if (new_top_bs->backing_hd == base) { > + ret = 0; > + goto exit; > + } > + > + intermediate = new_top_bs->backing_hd; /* this should be the same > + as 'top' */ Should? Is it the same or not? If yes, you can write it as an assert(); it no, the comment is only confusing. (In fact it's obvious enough that I'd just go with intermediate = top; without any comment) Kevin