Am 12.12.2013 um 09:23 hat Fam Zheng geschrieben: > Dropping intermediate could be useful both for commit and stream, and > BDS refcnt plus bdrv_swap could do most of the job nicely. It also need > some improvements in preparation for op blockers. > > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > block.c | 152 > +++++++++++++++++++++++++++------------------------------ > block/commit.c | 1 + > 2 files changed, 74 insertions(+), 79 deletions(-) > > diff --git a/block.c b/block.c > index 41562fd..681d3be 100644 > --- a/block.c > +++ b/block.c > @@ -2163,114 +2163,108 @@ BlockDriverState > *bdrv_find_overlay(BlockDriverState *active, > return overlay; > } > > -typedef struct BlkIntermediateStates { > - BlockDriverState *bs; > - QSIMPLEQ_ENTRY(BlkIntermediateStates) entry; > -} BlkIntermediateStates; > - > +static void bdrv_set_backing_hd(BlockDriverState *bs, > + BlockDriverState *new_backing) > +{ > + if (bs->backing_hd) { > + bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker); > + } > + bs->backing_hd = new_backing; > + if (new_backing) { > + bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
What about unblocking commit, like you did in patch 3? Should bdrv_open_backing_file() be using this function? > + } > +} > > /* > - * Drops images above 'base' up to and including 'top', and sets the image > - * above 'top' to have base as its backing file. > + * 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. > * > - * Requires that the overlay to 'top' is opened r/w, so that the backing file > - * information in 'bs' can be properly updated. > + * 1) This will convert the following chain: > + * ... <- base <- ... <- top <- overlay <-... <- active > * > - * E.g., this will convert the following chain: > - * bottom <- base <- intermediate <- top <- active > + * to > + * > + * ... <- base <- overlay <- active > + * > + * 2) It is allowed for bottom==base, in which case it converts: > + * > + * ... <- base <- ... <- top <- overlay <- ... <- active > * > * to > * > - * bottom <- base <- active > + * base <- overlay <- active > * > - * It is allowed for bottom==base, in which case it converts: > + * 2) It also allows active==top, in which case it converts: > * > - * base <- intermediate <- top <- active > + * ... <- base <- ... <- top (active) > * > * to > * > - * base <- active > + * base == active == top, i.e. only base and lower remains: *top == *base > when > + * return. > + * > + * 3) If base==NULL, it will drop all the BDS below overlay and set its > + * backing_hd to NULL. I.e.: > * > - * Error conditions: > - * if active == top, that is considered an error > + * base(NULL) <- ... <- overlay <- ... <- active > * > + * to > + * > + * overlay <- ... <- active > */ > 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; > - } > + BlockDriverState *drop_start, *overlay; > + int ret = -EINVAL; > > - 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 */ > + if (!top->drv || (base && !base->drv)) { > 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) { > + if (top == 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; > + } else if (top == active) { > + assert(base); > + drop_start = active->backing_hd; > + bdrv_swap(active, base); > + base->backing_hd = NULL; > + bdrv_unref(drop_start); > + ret = 0; > + } else { > + /* 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); > + if (!overlay) { > + goto exit; > + } > + if (base) { > + ret = bdrv_change_backing_file(overlay, base->filename, > + base->drv->format_name); > + } else { > + ret = bdrv_change_backing_file(overlay, "", ""); This should be NULL, NULL instead of empty strings. Kevin