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 <f...@redhat.com>
---
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:
--
1.8.3.1
