From patchwork Sat Oct 12 06:05:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 282979 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9E3DB2C0349 for ; Sat, 12 Oct 2013 17:06:22 +1100 (EST) Received: from localhost ([::1]:57093 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUsLE-0007xK-3I for incoming@patchwork.ozlabs.org; Sat, 12 Oct 2013 02:06:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUsKt-0007xC-Gz for qemu-devel@nongnu.org; Sat, 12 Oct 2013 02:06:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VUsKn-0000RS-HN for qemu-devel@nongnu.org; Sat, 12 Oct 2013 02:05:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4763) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUsKn-0000PL-8P for qemu-devel@nongnu.org; Sat, 12 Oct 2013 02:05:53 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9C65q0a029002 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 12 Oct 2013 02:05:52 -0400 Received: from T430s.nay.redhat.com ([10.66.6.118]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r9C65kbw002865; Sat, 12 Oct 2013 02:05:48 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Sat, 12 Oct 2013 14:05:45 +0800 Message-Id: <1381557945-1484-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, jcody@redhat.com, stefanha@redhat.com Subject: [Qemu-devel] [PATCH] block: simplify bdrv_drop_intermediate X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org There is only one failure point: bdrv_change_backing_file in this function, so we can drop the qlist and try to change the backing file before deleting anything. This way bdrv_drop_intermediate is simplified while keeping the operation transactional. A bonus is dropping an active BDS is supported too by swapping the base and top. Although no caller uses this yet, the comment is updated to reflect the change. Signed-off-by: Fam Zheng --- block.c | 100 ++++++++++++++++++--------------------------------------- block/commit.c | 1 + 2 files changed, 32 insertions(+), 69 deletions(-) diff --git a/block.c b/block.c index fd05a80..b9e073f 100644 --- a/block.c +++ b/block.c @@ -2130,18 +2130,11 @@ BlockDriverState *bdrv_find_overlay(BlockDriverState *active, return overlay; } -typedef struct BlkIntermediateStates { - BlockDriverState *bs; - QSIMPLEQ_ENTRY(BlkIntermediateStates) entry; -} BlkIntermediateStates; - - /* - * 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. + * Drops images above 'base' up to and including 'top', and sets new 'base' + * as backing_hd of top_overlay (the image orignally has 'top' as backing + * file). top_overlay may be NULL if 'top' is active, no such update needed. + * Requires that the top_overlay to 'top' is opened r/w. * * E.g., this will convert the following chain: * bottom <- base <- intermediate <- top <- active @@ -2158,86 +2151,55 @@ typedef struct BlkIntermediateStates { * * base <- active * - * Error conditions: - * if active == top, that is considered an error + * It also allows active==top, in which case it converts: + * + * base <- intermediate <- active (also top) + * + * to + * + * base == active == top, i.e. only base remains: *top == *base when return. * */ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, BlockDriverState *base) { - BlockDriverState *intermediate; + BlockDriverState *pbs; + BlockDriverState *overlay = NULL; 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 = top; - - /* now we will go down through the list, and add each BDS we find - * into our deletion queue, until we hit the 'base' - */ - while (intermediate) { - intermediate_state = g_malloc0(sizeof(BlkIntermediateStates)); - intermediate_state->bs = intermediate; - QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry); - - if (intermediate->backing_hd == base) { - base_bs = intermediate->backing_hd; - break; + if (active != top) { + /* If there's an overlay, its backing_hd points to top's BDS now, + * the top image is dropped but this BDS structure is kept and swapped + * with base, this way we keep the pointers valid after dropping top */ + overlay = bdrv_find_overlay(active, top); + ret = bdrv_change_backing_file(overlay, base->filename, + base->drv ? + base->drv->format_name : ""); + if (ret) { + goto exit; } - intermediate = intermediate->backing_hd; - } - if (base_bs == NULL) { - /* something went wrong, we did not end at the base. safely - * unravel everything, and exit with error */ - goto exit; } - /* success - we can delete the intermediate states, and link top->base */ - ret = bdrv_change_backing_file(new_top_bs, base_bs->filename, - base_bs->drv ? base_bs->drv->format_name : ""); - if (ret) { - goto exit; + for (pbs = top->backing_hd; pbs != base; pbs = base_bs) { + assert(pbs); + base_bs = pbs->backing_hd; + pbs->backing_hd = NULL; + bdrv_unref(pbs); } - new_top_bs->backing_hd = base_bs; - - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { - /* so that bdrv_close() does not recursively close the chain */ - intermediate_state->bs->backing_hd = NULL; - bdrv_unref(intermediate_state->bs); - } - ret = 0; + bdrv_swap(base, top); + base->backing_hd = NULL; + bdrv_unref(base); exit: - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) { - g_free(intermediate_state); - } return ret; } - static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset, size_t size) { diff --git a/block/commit.c b/block/commit.c index d4090cb..4d8cd05 100644 --- a/block/commit.c +++ b/block/commit.c @@ -142,6 +142,7 @@ wait: if (!block_job_is_cancelled(&s->common) && sector_num == end) { /* success */ ret = bdrv_drop_intermediate(active, top, base); + base = top; } exit_free_buf: