Comparing branches: left: image-fleecing.v15 right: image-fleecing.v16 commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Add BlockOpType enum block: Add BlockOpType enum This adds the enum of all the operations that can be take This adds the enum of all the operations that can be take device. device. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet Reviewed-by: Benoit Canet diff --git a/include/block/block.h b/include/block/block.h diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block.h --- a/include/block/block.h +++ b/include/block/block.h +++ b/include/block/block.h @@ -XXX,X +XXX,X @@ typedef struct BDRVReopenState { @@ -XXX,X +XXX,X @@ typedef struct BDRVReopenState { void *opaque; void *opaque; } BDRVReopenState; } BDRVReopenState; +/* +/* + * Block operation types + * Block operation types + */ + */ +typedef enum BlockOpType { +typedef enum BlockOpType { + BLOCK_OP_TYPE_BACKUP_SOURCE, + BLOCK_OP_TYPE_BACKUP_SOURCE, + BLOCK_OP_TYPE_BACKUP_TARGET, + BLOCK_OP_TYPE_BACKUP_TARGET, + BLOCK_OP_TYPE_CHANGE, + BLOCK_OP_TYPE_CHANGE, + BLOCK_OP_TYPE_COMMIT, + BLOCK_OP_TYPE_COMMIT, + BLOCK_OP_TYPE_DATAPLANE, + BLOCK_OP_TYPE_DATAPLANE, + BLOCK_OP_TYPE_DRIVE_DEL, + BLOCK_OP_TYPE_DRIVE_DEL, + BLOCK_OP_TYPE_EJECT, + BLOCK_OP_TYPE_EJECT, + BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, + BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, + BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, + BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, + BLOCK_OP_TYPE_MIRROR, + BLOCK_OP_TYPE_MIRROR, + BLOCK_OP_TYPE_RESIZE, + BLOCK_OP_TYPE_RESIZE, + BLOCK_OP_TYPE_STREAM, + BLOCK_OP_TYPE_STREAM, + BLOCK_OP_TYPE_MAX, + BLOCK_OP_TYPE_MAX, +} BlockOpType; +} BlockOpType; void bdrv_iostatus_enable(BlockDriverState *bs); void bdrv_iostatus_enable(BlockDriverState *bs); void bdrv_iostatus_reset(BlockDriverState *bs); void bdrv_iostatus_reset(BlockDriverState *bs); commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Introduce op_blockers to BlockDriverState block: Introduce op_blockers to BlockDriverState BlockDriverState.op_blockers is an array of lists with BL BlockDriverState.op_blockers is an array of lists with BL elements. Each list is a list of blockers of an operation elements. Each list is a list of blockers of an operation (BlockOpType), that marks this BDS as currently blocked f (BlockOpType), that marks this BDS as currently blocked f type of operation with reason errors stored in the list. type of operation with reason errors stored in the list. usage is: usage is: * BDS user who wants to take an operation should check i * BDS user who wants to take an operation should check i blocker of the type with bdrv_op_is_blocked(). blocker of the type with bdrv_op_is_blocked(). * BDS user who wants to block certain types of operation * BDS user who wants to block certain types of operation bdrv_op_block (or bdrv_op_block_all to block all types bdrv_op_block (or bdrv_op_block_all to block all types which is similar to the existing bdrv_set_in_use()). which is similar to the existing bdrv_set_in_use()). * A blocker is only referenced by op_blockers, so the li * A blocker is only referenced by op_blockers, so the li managed by caller, and shouldn't be lost until unblock managed by caller, and shouldn't be lost until unblock a caller does these: a caller does these: - Allocate a blocker with error_setg or similar, call - Allocate a blocker with error_setg or similar, call to block some operations. to block some operations. - Hold the blocker, do his job. - Hold the blocker, do his job. - Unblock operations that it blocked, with the same re - Unblock operations that it blocked, with the same re passed to bdrv_op_unblock(). passed to bdrv_op_unblock(). - Release the blocker with error_free(). - Release the blocker with error_free(). Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet Reviewed-by: Benoit Canet diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ void bdrv_register(BlockDriver *bdrv) @@ -XXX,X +XXX,X @@ void bdrv_register(BlockDriver *bdrv) BlockDriverState *bdrv_new(const char *device_name) BlockDriverState *bdrv_new(const char *device_name) { { BlockDriverState *bs; BlockDriverState *bs; + int i; + int i; bs = g_malloc0(sizeof(BlockDriverState)); bs = g_malloc0(sizeof(BlockDriverState)); QLIST_INIT(&bs->dirty_bitmaps); QLIST_INIT(&bs->dirty_bitmaps); @@ -XXX,X +XXX,X @@ BlockDriverState *bdrv_new(const char *de @@ -XXX,X +XXX,X @@ BlockDriverState *bdrv_new(const char *de if (device_name[0] != '\0') { if (device_name[0] != '\0') { QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); } } + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + QLIST_INIT(&bs->op_blockers[i]); + QLIST_INIT(&bs->op_blockers[i]); + } + } bdrv_iostatus_disable(bs); bdrv_iostatus_disable(bs); notifier_list_init(&bs->close_notifiers); notifier_list_init(&bs->close_notifiers); notifier_with_return_list_init(&bs->before_write_notifie notifier_with_return_list_init(&bs->before_write_notifie @@ -XXX,X +XXX,X @@ static void bdrv_move_feature_fields(Bloc @@ -XXX,X +XXX,X @@ static void bdrv_move_feature_fields(Bloc * We do want to swap name but don't want to swap linked * We do want to swap name but don't want to swap linked */ */ bs_dest->node_list = bs_src->node_list; bs_dest->node_list = bs_src->node_list; + memcpy(bs_dest->op_blockers, bs_src->op_blockers, + memcpy(bs_dest->op_blockers, bs_src->op_blockers, + sizeof(bs_dest->op_blockers)); + sizeof(bs_dest->op_blockers)); } } /* /* @@ -XXX,X +XXX,X @@ void bdrv_unref(BlockDriverState *bs) @@ -XXX,X +XXX,X @@ void bdrv_unref(BlockDriverState *bs) } } } } +struct BdrvOpBlocker { +struct BdrvOpBlocker { + Error *reason; + Error *reason; + QLIST_ENTRY(BdrvOpBlocker) list; + QLIST_ENTRY(BdrvOpBlocker) list; +}; +}; + + +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op +{ +{ + BdrvOpBlocker *blocker; + BdrvOpBlocker *blocker; + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + if (!QLIST_EMPTY(&bs->op_blockers[op])) { + if (!QLIST_EMPTY(&bs->op_blockers[op])) { + blocker = QLIST_FIRST(&bs->op_blockers[op]); + blocker = QLIST_FIRST(&bs->op_blockers[op]); + if (errp) { + if (errp) { + *errp = error_copy(blocker->reason); + *errp = error_copy(blocker->reason); + } + } + return true; + return true; + } + } + return false; + return false; +} +} + + +void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err +void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err +{ +{ + BdrvOpBlocker *blocker; + BdrvOpBlocker *blocker; + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + + + blocker = g_malloc0(sizeof(BdrvOpBlocker)); + blocker = g_malloc0(sizeof(BdrvOpBlocker)); + blocker->reason = reason; + blocker->reason = reason; + QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list); + QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list); +} +} + + +void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, E +void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, E +{ +{ + BdrvOpBlocker *blocker, *next; + BdrvOpBlocker *blocker, *next; + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); + QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, + QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, + if (blocker->reason == reason) { + if (blocker->reason == reason) { + QLIST_REMOVE(blocker, list); + QLIST_REMOVE(blocker, list); + g_free(blocker); + g_free(blocker); + } + } + } + } +} +} + + +void bdrv_op_block_all(BlockDriverState *bs, Error *reason) +void bdrv_op_block_all(BlockDriverState *bs, Error *reason) +{ +{ + int i; + int i; + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + bdrv_op_block(bs, i, reason); + bdrv_op_block(bs, i, reason); + } + } +} +} + + +void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason +void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason +{ +{ + int i; + int i; + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + bdrv_op_unblock(bs, i, reason); + bdrv_op_unblock(bs, i, reason); + } + } +} +} + + +bool bdrv_op_blocker_is_empty(BlockDriverState *bs) +bool bdrv_op_blocker_is_empty(BlockDriverState *bs) +{ +{ + int i; + int i; + + + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { + if (!QLIST_EMPTY(&bs->op_blockers[i])) { + if (!QLIST_EMPTY(&bs->op_blockers[i])) { + return false; + return false; + } + } + } + } + return true; + return true; +} +} + + void bdrv_set_in_use(BlockDriverState *bs, int in_use) void bdrv_set_in_use(BlockDriverState *bs, int in_use) { { assert(bs->in_use != in_use); assert(bs->in_use != in_use); diff --git a/include/block/block.h b/include/block/block.h diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block.h --- a/include/block/block.h +++ b/include/block/block.h +++ b/include/block/block.h @@ -XXX,X +XXX,X @@ void bdrv_unref(BlockDriverState *bs); @@ -XXX,X +XXX,X @@ void bdrv_unref(BlockDriverState *bs); void bdrv_set_in_use(BlockDriverState *bs, int in_use); void bdrv_set_in_use(BlockDriverState *bs, int in_use); int bdrv_in_use(BlockDriverState *bs); int bdrv_in_use(BlockDriverState *bs); +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op +void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err +void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err +void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, E +void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, E +void bdrv_op_block_all(BlockDriverState *bs, Error *reason); +void bdrv_op_block_all(BlockDriverState *bs, Error *reason); +void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason +void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason +bool bdrv_op_blocker_is_empty(BlockDriverState *bs); +bool bdrv_op_blocker_is_empty(BlockDriverState *bs); + + #ifdef CONFIG_LINUX_AIO #ifdef CONFIG_LINUX_AIO int raw_get_aio_fd(BlockDriverState *bs); int raw_get_aio_fd(BlockDriverState *bs); #else #else diff --git a/include/block/block_int.h b/include/block/block_ diff --git a/include/block/block_int.h b/include/block/block_ index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block_int.h --- a/include/block/block_int.h +++ b/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,X +XXX,X @@ typedef struct BlockLimits { @@ -XXX,X +XXX,X @@ typedef struct BlockLimits { size_t opt_mem_alignment; size_t opt_mem_alignment; } BlockLimits; } BlockLimits; +typedef struct BdrvOpBlocker BdrvOpBlocker; +typedef struct BdrvOpBlocker BdrvOpBlocker; + + /* /* * Note: the function bdrv_append() copies and swaps content * Note: the function bdrv_append() copies and swaps content * BlockDriverStates, so if you add new fields to this struc * BlockDriverStates, so if you add new fields to this struc @@ -XXX,X +XXX,X @@ struct BlockDriverState { @@ -XXX,X +XXX,X @@ struct BlockDriverState { QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; + /* operation blockers */ + /* operation blockers */ + QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MA + QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MA + + /* long-running background operation */ /* long-running background operation */ BlockJob *job; BlockJob *job; commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Replace in_use with operation blocker block: Replace in_use with operation blocker This drops BlockDriverState.in_use with op_blockers: This drops BlockDriverState.in_use with op_blockers: - Call bdrv_op_block_all in place of bdrv_set_in_use(bs - Call bdrv_op_block_all in place of bdrv_set_in_use(bs - Call bdrv_op_unblock_all in place of bdrv_set_in_use( - Call bdrv_op_unblock_all in place of bdrv_set_in_use( - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs The specific types are used, e.g. in place of startin The specific types are used, e.g. in place of startin bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...). bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...). - Check bdrv_op_blocker_is_empty() in place of assert(! - Check bdrv_op_blocker_is_empty() in place of assert(! Note: there is only bdrv_op_block_all and bdrv_op_unblock Note: there is only bdrv_op_block_all and bdrv_op_unblock this moment. So although the checks are specific to op ty this moment. So although the checks are specific to op ty changes can still be seen as identical logic with previou changes can still be seen as identical logic with previou in_use. The difference is error message are improved beca in_use. The difference is error message are improved beca error info. error info. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet | Signed-off-by: Markus Armbruster diff --git a/block-migration.c b/block-migration.c diff --git a/block-migration.c b/block-migration.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block-migration.c --- a/block-migration.c +++ b/block-migration.c +++ b/block-migration.c @@ -XXX,X +XXX,X @@ typedef struct BlkMigDevState { @@ -XXX,X +XXX,X @@ typedef struct BlkMigDevState { unsigned long *aio_bitmap; unsigned long *aio_bitmap; int64_t completed_sectors; int64_t completed_sectors; BdrvDirtyBitmap *dirty_bitmap; BdrvDirtyBitmap *dirty_bitmap; + Error *blocker; + Error *blocker; } BlkMigDevState; } BlkMigDevState; typedef struct BlkMigBlock { typedef struct BlkMigBlock { @@ -XXX,X +XXX,X @@ static void init_blk_migration_it(void *o @@ -XXX,X +XXX,X @@ static void init_blk_migration_it(void *o bmds->completed_sectors = 0; bmds->completed_sectors = 0; bmds->shared_base = block_mig_state.shared_base; bmds->shared_base = block_mig_state.shared_base; alloc_aio_bitmap(bmds); alloc_aio_bitmap(bmds); - bdrv_set_in_use(bs, 1); - bdrv_set_in_use(bs, 1); + error_setg(&bmds->blocker, "block device is in use b + error_setg(&bmds->blocker, "block device is in use b + bdrv_op_block_all(bs, bmds->blocker); + bdrv_op_block_all(bs, bmds->blocker); bdrv_ref(bs); bdrv_ref(bs); block_mig_state.total_sector_sum += sectors; block_mig_state.total_sector_sum += sectors; @@ -XXX,X +XXX,X @@ static void blk_mig_cleanup(void) @@ -XXX,X +XXX,X @@ static void blk_mig_cleanup(void) blk_mig_lock(); blk_mig_lock(); while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, ent QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, ent - bdrv_set_in_use(bmds->bs, 0); - bdrv_set_in_use(bmds->bs, 0); + bdrv_op_unblock_all(bmds->bs, bmds->blocker); + bdrv_op_unblock_all(bmds->bs, bmds->blocker); + error_free(bmds->blocker); + error_free(bmds->blocker); bdrv_unref(bmds->bs); bdrv_unref(bmds->bs); g_free(bmds->aio_bitmap); g_free(bmds->aio_bitmap); g_free(bmds); g_free(bmds); diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ static void bdrv_move_feature_fields(Bloc @@ -XXX,X +XXX,X @@ static void bdrv_move_feature_fields(Bloc bs_dest->refcnt = bs_src->refcnt; bs_dest->refcnt = bs_src->refcnt; /* job */ /* job */ - bs_dest->in_use = bs_src->in_use; - bs_dest->in_use = bs_src->in_use; bs_dest->job = bs_src->job; bs_dest->job = bs_src->job; /* keep the same entry in bdrv_states */ /* keep the same entry in bdrv_states */ @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); assert(bs_new->job == NULL); assert(bs_new->dev == NULL); assert(bs_new->dev == NULL); - assert(bs_new->in_use == 0); - assert(bs_new->in_use == 0); + assert(bdrv_op_blocker_is_empty(bs_new)); + assert(bdrv_op_blocker_is_empty(bs_new)); assert(bs_new->io_limits_enabled == false); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); assert(!throttle_have_timer(&bs_new->throttle_state)); @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, /* Check a few fields that should remain attached to the /* Check a few fields that should remain attached to the assert(bs_new->dev == NULL); assert(bs_new->dev == NULL); assert(bs_new->job == NULL); assert(bs_new->job == NULL); - assert(bs_new->in_use == 0); - assert(bs_new->in_use == 0); + assert(bdrv_op_blocker_is_empty(bs_new)); + assert(bdrv_op_blocker_is_empty(bs_new)); assert(bs_new->io_limits_enabled == false); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); assert(!throttle_have_timer(&bs_new->throttle_state)); @@ -XXX,X +XXX,X @@ static void bdrv_delete(BlockDriverState @@ -XXX,X +XXX,X @@ static void bdrv_delete(BlockDriverState { { assert(!bs->dev); assert(!bs->dev); assert(!bs->job); assert(!bs->job); - assert(!bs->in_use); - assert(!bs->in_use); + assert(bdrv_op_blocker_is_empty(bs)); + assert(bdrv_op_blocker_is_empty(bs)); assert(!bs->refcnt); assert(!bs->refcnt); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); @@ -XXX,X +XXX,X @@ int bdrv_commit(BlockDriverState *bs) @@ -XXX,X +XXX,X @@ int bdrv_commit(BlockDriverState *bs) return -ENOTSUP; return -ENOTSUP; } } - if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) { - if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) { + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, NULL) | + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, NULL) | + bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COM + bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COM return -EBUSY; return -EBUSY; } } @@ -XXX,X +XXX,X @@ int bdrv_truncate(BlockDriverState *bs, i @@ -XXX,X +XXX,X @@ int bdrv_truncate(BlockDriverState *bs, i return -ENOTSUP; return -ENOTSUP; if (bs->read_only) if (bs->read_only) return -EACCES; return -EACCES; - if (bdrv_in_use(bs)) - if (bdrv_in_use(bs)) + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) return -EBUSY; return -EBUSY; + } + } ret = drv->bdrv_truncate(bs, offset); ret = drv->bdrv_truncate(bs, offset); if (ret == 0) { if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTO ret = refresh_total_sectors(bs, offset >> BDRV_SECTO @@ -XXX,X +XXX,X @@ bool bdrv_op_blocker_is_empty(BlockDriver @@ -XXX,X +XXX,X @@ bool bdrv_op_blocker_is_empty(BlockDriver return true; return true; } } -void bdrv_set_in_use(BlockDriverState *bs, int in_use) -void bdrv_set_in_use(BlockDriverState *bs, int in_use) -{ -{ - assert(bs->in_use != in_use); - assert(bs->in_use != in_use); - bs->in_use = in_use; - bs->in_use = in_use; -} -} - - -int bdrv_in_use(BlockDriverState *bs) -int bdrv_in_use(BlockDriverState *bs) -{ -{ - return bs->in_use; - return bs->in_use; -} -} - - void bdrv_iostatus_enable(BlockDriverState *bs) void bdrv_iostatus_enable(BlockDriverState *bs) { { bs->iostatus_enabled = true; bs->iostatus_enabled = true; diff --git a/blockdev.c b/blockdev.c diff --git a/blockdev.c b/blockdev.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockdev.c --- a/blockdev.c +++ b/blockdev.c +++ b/blockdev.c @@ -XXX,X +XXX,X @@ static void external_snapshot_prepare(Blk @@ -XXX,X +XXX,X @@ static void external_snapshot_prepare(Blk return; return; } } - if (bdrv_in_use(state->old_bs)) { - if (bdrv_in_use(state->old_bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(state->old_bs, + if (bdrv_op_is_blocked(state->old_bs, + BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, return; return; } } @@ -XXX,X +XXX,X @@ exit: @@ -XXX,X +XXX,X @@ exit: static void eject_device(BlockDriverState *bs, int force, Er static void eject_device(BlockDriverState *bs, int force, Er { { - if (bdrv_in_use(bs)) { - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ - error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { return; return; } } if (!bdrv_dev_has_removable_media(bs)) { if (!bdrv_dev_has_removable_media(bs)) { @@ -XXX,X +XXX,X @@ int do_drive_del(Monitor *mon, const QDic @@ -XXX,X +XXX,X @@ int do_drive_del(Monitor *mon, const QDic > { > const char *id = qdict_get_str(qdict, "id"); > BlockDriverState *bs; > + Error *local_err = NULL; > > bs = bdrv_find(id); > if (!bs) { qerror_report(QERR_DEVICE_NOT_FOUND, id); qerror_report(QERR_DEVICE_NOT_FOUND, id); return -1; return -1; } } - if (bdrv_in_use(bs)) { - if (bdrv_in_use(bs)) { + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL | - qerror_report(QERR_DEVICE_IN_USE, id); qerror_report(QERR_DEVICE_IN_USE, id); | + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &loc > + error_report("%s", error_get_pretty(local_err)); > + error_free(local_err); return -1; return -1; } } > @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, } } } } - if (bdrv_in_use(bs)) { - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, return; return; } } @@ -XXX,X +XXX,X @@ void qmp_drive_mirror(const char *device, @@ -XXX,X +XXX,X @@ void qmp_drive_mirror(const char *device, } } } } - if (bdrv_in_use(bs)) { - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) return; return; } } diff --git a/blockjob.c b/blockjob.c diff --git a/blockjob.c b/blockjob.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockjob.c --- a/blockjob.c +++ b/blockjob.c +++ b/blockjob.c @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv { { BlockJob *job; BlockJob *job; - if (bs->job || bdrv_in_use(bs)) { - if (bs->job || bdrv_in_use(bs)) { + if (bs->job || !bdrv_op_blocker_is_empty(bs)) { + if (bs->job || !bdrv_op_blocker_is_empty(bs)) { error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ return NULL; return NULL; } } bdrv_ref(bs); bdrv_ref(bs); - bdrv_set_in_use(bs, 1); - bdrv_set_in_use(bs, 1); - - job = g_malloc0(driver->instance_size); job = g_malloc0(driver->instance_size); + error_setg(&job->blocker, "block device is in use by blo + error_setg(&job->blocker, "block device is in use by blo + BlockJobType_lookup[driver->job_type]); + BlockJobType_lookup[driver->job_type]); + bdrv_op_block_all(bs, job->blocker); + bdrv_op_block_all(bs, job->blocker); + + job->driver = driver; job->driver = driver; job->bs = bs; job->bs = bs; job->cb = cb; job->cb = cb; @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv block_job_set_speed(job, speed, &local_err); block_job_set_speed(job, speed, &local_err); if (local_err) { if (local_err) { bs->job = NULL; bs->job = NULL; + bdrv_op_unblock_all(bs, job->blocker); + bdrv_op_unblock_all(bs, job->blocker); + error_free(job->blocker); + error_free(job->blocker); g_free(job); g_free(job); - bdrv_set_in_use(bs, 0); - bdrv_set_in_use(bs, 0); error_propagate(errp, local_err); error_propagate(errp, local_err); return NULL; return NULL; } } @@ -XXX,X +XXX,X @@ void block_job_completed(BlockJob *job, i @@ -XXX,X +XXX,X @@ void block_job_completed(BlockJob *job, i assert(bs->job == job); assert(bs->job == job); job->cb(job->opaque, ret); job->cb(job->opaque, ret); bs->job = NULL; bs->job = NULL; + bdrv_op_unblock_all(bs, job->blocker); + bdrv_op_unblock_all(bs, job->blocker); + error_free(job->blocker); + error_free(job->blocker); g_free(job); g_free(job); - bdrv_set_in_use(bs, 0); - bdrv_set_in_use(bs, 0); } } void block_job_set_speed(BlockJob *job, int64_t speed, Error void block_job_set_speed(BlockJob *job, int64_t speed, Error diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/datap diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/datap index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/hw/block/dataplane/virtio-blk.c --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -XXX,X +XXX,X @@ struct VirtIOBlockDataPlane { @@ -XXX,X +XXX,X @@ struct VirtIOBlockDataPlane { queue */ queue */ unsigned int num_reqs; unsigned int num_reqs; + + + /* Operation blocker on BDS */ + /* Operation blocker on BDS */ + Error *blocker; + Error *blocker; }; }; /* Raise an interrupt to signal guest, if necessary */ /* Raise an interrupt to signal guest, if necessary */ @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD { { VirtIOBlockDataPlane *s; VirtIOBlockDataPlane *s; int fd; int fd; + Error *local_err = NULL; + Error *local_err = NULL; *dataplane = NULL; *dataplane = NULL; @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD /* If dataplane is (re-)enabled while the guest is runni /* If dataplane is (re-)enabled while the guest is runni * block jobs that can conflict. * block jobs that can conflict. */ */ - if (bdrv_in_use(blk->conf.bs)) { - if (bdrv_in_use(blk->conf.bs)) { - error_setg(errp, - error_setg(errp, - "cannot start dataplane thread while devi - "cannot start dataplane thread while devi + if (bdrv_op_is_blocked(blk->conf.bs, BLOCK_OP_TYPE_DATAP + if (bdrv_op_is_blocked(blk->conf.bs, BLOCK_OP_TYPE_DATAP + error_report("cannot start dataplane thread: %s", + error_report("cannot start dataplane thread: %s", + error_get_pretty(local_err)); + error_get_pretty(local_err)); + error_free(local_err); + error_free(local_err); return; return; } } @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_create(VirtIOD s->vdev = vdev; s->vdev = vdev; s->fd = fd; s->fd = fd; s->blk = blk; s->blk = blk; - - - /* Prevent block operations that conflict with data plan - /* Prevent block operations that conflict with data plan - bdrv_set_in_use(blk->conf.bs, 1); - bdrv_set_in_use(blk->conf.bs, 1); + error_setg(&s->blocker, "block device is in use by data + error_setg(&s->blocker, "block device is in use by data + bdrv_op_block_all(blk->conf.bs, s->blocker); + bdrv_op_block_all(blk->conf.bs, s->blocker); *dataplane = s; *dataplane = s; } } @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_destroy(VirtIO @@ -XXX,X +XXX,X @@ void virtio_blk_data_plane_destroy(VirtIO } } virtio_blk_data_plane_stop(s); virtio_blk_data_plane_stop(s); - bdrv_set_in_use(s->blk->conf.bs, 0); - bdrv_set_in_use(s->blk->conf.bs, 0); + bdrv_op_unblock_all(s->blk->conf.bs, s->blocker); + bdrv_op_unblock_all(s->blk->conf.bs, s->blocker); + error_free(s->blocker); + error_free(s->blocker); g_free(s); g_free(s); } } diff --git a/include/block/block.h b/include/block/block.h diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block.h --- a/include/block/block.h +++ b/include/block/block.h +++ b/include/block/block.h @@ -XXX,X +XXX,X @@ void bdrv_disable_copy_on_read(BlockDrive @@ -XXX,X +XXX,X @@ void bdrv_disable_copy_on_read(BlockDrive void bdrv_ref(BlockDriverState *bs); void bdrv_ref(BlockDriverState *bs); void bdrv_unref(BlockDriverState *bs); void bdrv_unref(BlockDriverState *bs); -void bdrv_set_in_use(BlockDriverState *bs, int in_use); -void bdrv_set_in_use(BlockDriverState *bs, int in_use); -int bdrv_in_use(BlockDriverState *bs); -int bdrv_in_use(BlockDriverState *bs); bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Err diff --git a/include/block/block_int.h b/include/block/block_ diff --git a/include/block/block_int.h b/include/block/block_ index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block_int.h --- a/include/block/block_int.h +++ b/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,X +XXX,X @@ struct BlockDriverState { @@ -XXX,X +XXX,X @@ struct BlockDriverState { QTAILQ_ENTRY(BlockDriverState) device_list; QTAILQ_ENTRY(BlockDriverState) device_list; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; int refcnt; int refcnt; - int in_use; /* users other than guest access, eg. block - int in_use; /* users other than guest access, eg. block QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; diff --git a/include/block/blockjob.h b/include/block/blockjo diff --git a/include/block/blockjob.h b/include/block/blockjo index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/blockjob.h --- a/include/block/blockjob.h +++ b/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -XXX,X +XXX,X @@ struct BlockJob { @@ -XXX,X +XXX,X @@ struct BlockJob { /** The completion function that will be called when the /** The completion function that will be called when the BlockDriverCompletionFunc *cb; BlockDriverCompletionFunc *cb; + /** Block other operations when block job is running */ + /** Block other operations when block job is running */ + Error *blocker; + Error *blocker; + + /** The opaque value that is passed to the completion fu /** The opaque value that is passed to the completion fu void *opaque; void *opaque; }; }; commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Move op_blocker check from block_job_create to its block: Move op_blocker check from block_job_create to its It makes no sense to check for "any" blocker on bs, we ar It makes no sense to check for "any" blocker on bs, we ar because of the mechanical conversion from in_use to op_bl because of the mechanical conversion from in_use to op_bl it now, and let the callers check specific operation type it now, and let the callers check specific operation type mirror already have it, add checker to stream and commit. mirror already have it, add checker to stream and commit. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet Reviewed-by: Benoit Canet diff --git a/blockdev.c b/blockdev.c diff --git a/blockdev.c b/blockdev.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockdev.c --- a/blockdev.c +++ b/blockdev.c +++ b/blockdev.c @@ -XXX,X +XXX,X @@ void qmp_block_stream(const char *device, @@ -XXX,X +XXX,X @@ void qmp_block_stream(const char *device, return; return; } } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) + return; + return; + } + } + + if (base) { if (base) { base_bs = bdrv_find_backing_image(bs, base); base_bs = bdrv_find_backing_image(bs, base); if (base_bs == NULL) { if (base_bs == NULL) { @@ -XXX,X +XXX,X @@ void qmp_block_commit(const char *device, @@ -XXX,X +XXX,X @@ void qmp_block_commit(const char *device, return; return; } } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) + return; + return; + } + } + + /* default top_bs is the active layer */ /* default top_bs is the active layer */ top_bs = bs; top_bs = bs; diff --git a/blockjob.c b/blockjob.c diff --git a/blockjob.c b/blockjob.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockjob.c --- a/blockjob.c +++ b/blockjob.c +++ b/blockjob.c @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv @@ -XXX,X +XXX,X @@ void *block_job_create(const BlockJobDriv { { BlockJob *job; BlockJob *job; - if (bs->job || !bdrv_op_blocker_is_empty(bs)) { - if (bs->job || !bdrv_op_blocker_is_empty(bs)) { + if (bs->job) { + if (bs->job) { error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_ return NULL; return NULL; } } commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Add bdrv_set_backing_hd() block: Add bdrv_set_backing_hd() This is the common but non-trivial steps to assign or cha This is the common but non-trivial steps to assign or cha backing_hd of BDS. backing_hd of BDS. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ fail: @@ -XXX,X +XXX,X @@ fail: return ret; return ret; } } +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverSt +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverSt +{ +{ + if (backing_hd) { < + /* Grab the reference before unref original backing_ < + * when rebasing in the backing chain. < + */ < + bdrv_ref(backing_hd); < + } < + < + if (bs->backing_hd) { < + bdrv_unref(bs->backing_hd); < + } < + + + bs->backing_hd = backing_hd; + bs->backing_hd = backing_hd; + if (!backing_hd) { + if (!backing_hd) { + bs->backing_file[0] = '\0'; + bs->backing_file[0] = '\0'; + bs->backing_format[0] = '\0'; + bs->backing_format[0] = '\0'; + goto out; + goto out; + } + } > + bs->open_flags &= ~BDRV_O_NO_BACKING; + pstrcpy(bs->backing_file, sizeof(bs->backing_file), back + pstrcpy(bs->backing_file, sizeof(bs->backing_file), back + pstrcpy(bs->backing_format, sizeof(bs->backing_format), + pstrcpy(bs->backing_format, sizeof(bs->backing_format), + backing_hd->drv ? backing_hd->drv->format_name : + backing_hd->drv ? backing_hd->drv->format_name : +out: +out: + bdrv_refresh_limits(bs); + bdrv_refresh_limits(bs); +} +} + + /* /* * Opens the backing file for a BlockDriverState if not yet * Opens the backing file for a BlockDriverState if not yet * * @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta char backing_filename[PATH_MAX]; char backing_filename[PATH_MAX]; int back_flags, ret; int back_flags, ret; BlockDriver *back_drv = NULL; BlockDriver *back_drv = NULL; + BlockDriverState *backing_hd; + BlockDriverState *backing_hd; Error *local_err = NULL; Error *local_err = NULL; if (bs->backing_hd != NULL) { if (bs->backing_hd != NULL) { @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta sizeof(backing_filena sizeof(backing_filena } } + backing_hd = bdrv_new(""); + backing_hd = bdrv_new(""); + + if (bs->backing_format[0] != '\0') { if (bs->backing_format[0] != '\0') { back_drv = bdrv_find_format(bs->backing_format); back_drv = bdrv_find_format(bs->backing_format); } } @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta @@ -XXX,X +XXX,X @@ int bdrv_open_backing_file(BlockDriverSta back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNA back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNA BDRV_O_COPY_ON_READ); BDRV_O_COPY_ON_READ); - assert(bs->backing_hd == NULL); - assert(bs->backing_hd == NULL); - ret = bdrv_open(&bs->backing_hd, - ret = bdrv_open(&bs->backing_hd, + ret = bdrv_open(&backing_hd, + ret = bdrv_open(&backing_hd, *backing_filename ? backing_filename : N *backing_filename ? backing_filename : N back_flags, back_drv, &local_err); back_flags, back_drv, &local_err); if (ret < 0) { if (ret < 0) { bs->backing_hd = NULL; | - bs->backing_hd = NULL; + bdrv_unref(backing_hd); + bdrv_unref(backing_hd); + backing_hd = NULL; + backing_hd = NULL; bs->open_flags |= BDRV_O_NO_BACKING; bs->open_flags |= BDRV_O_NO_BACKING; error_setg(errp, "Could not open backing file: %s", error_setg(errp, "Could not open backing file: %s", error_get_pretty(local_err)); error_get_pretty(local_err)); error_free(local_err); error_free(local_err); return ret; return ret; } } - - - if (bs->backing_hd->file) { - if (bs->backing_hd->file) { - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - bs->backing_hd->file->filename); - bs->backing_hd->file->filename); - } - } + bdrv_set_backing_hd(bs, backing_hd); + bdrv_set_backing_hd(bs, backing_hd); + /* Now we have refcnt = 2 on backing_hd by bdrv_new and < + * bdrv_set_backing_hd, while we only need 1 */ < + assert(backing_hd->refcnt == 2); < + bdrv_unref(backing_hd); < /* Recalculate the BlockLimits with the backing file */ /* Recalculate the BlockLimits with the backing file */ bdrv_refresh_limits(bs); bdrv_refresh_limits(bs); @@ -XXX,X +XXX,X @@ void bdrv_append(BlockDriverState *bs_new @@ -XXX,X +XXX,X @@ void bdrv_append(BlockDriverState *bs_new /* The contents of 'tmp' will become bs_top, as we are /* The contents of 'tmp' will become bs_top, as we are * swapping bs_new and bs_top contents. */ * swapping bs_new and bs_top contents. */ - bs_top->backing_hd = bs_new; - bs_top->backing_hd = bs_new; > - bs_top->open_flags &= ~BDRV_O_NO_BACKING; > - pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_fil > - bs_new->filename); > - pstrcpy(bs_top->backing_format, sizeof(bs_top->backing_f > - bs_new->drv ? bs_new->drv->format_name : ""); + bdrv_set_backing_hd(bs_top, bs_new); + bdrv_set_backing_hd(bs_top, bs_new); bs_top->open_flags &= ~BDRV_O_NO_BACKING; | } pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_fil | bs_new->filename); | static void bdrv_delete(BlockDriverState *bs) diff --git a/include/block/block.h b/include/block/block.h diff --git a/include/block/block.h b/include/block/block.h index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block.h --- a/include/block/block.h +++ b/include/block/block.h +++ b/include/block/block.h @@ -XXX,X +XXX,X @@ int bdrv_parse_discard_flags(const char * @@ -XXX,X +XXX,X @@ int bdrv_parse_discard_flags(const char * int bdrv_open_image(BlockDriverState **pbs, const char *file int bdrv_open_image(BlockDriverState **pbs, const char *file QDict *options, const char *bdref_key, i QDict *options, const char *bdref_key, i bool allow_none, Error **errp); bool allow_none, Error **errp); +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverSt +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverSt int bdrv_open_backing_file(BlockDriverState *bs, QDict *opti int bdrv_open_backing_file(BlockDriverState *bs, QDict *opti int bdrv_open(BlockDriverState **pbs, const char *filename, int bdrv_open(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int fla const char *reference, QDict *options, int fla commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Add backing_blocker in BlockDriverState block: Add backing_blocker in BlockDriverState This makes use of op_blocker and blocks all the operation This makes use of op_blocker and blocks all the operation commit target, on each BlockDriverState->backing_hd. commit target, on each BlockDriverState->backing_hd. The asserts for op_blocker in bdrv_swap are removed becau The asserts for op_blocker in bdrv_swap are removed becau change, the target of block commit has at least the backi change, the target of block commit has at least the backi its child, so the assertion is not true. Callers should d its child, so the assertion is not true. Callers should d Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ void bdrv_set_backing_hd(BlockDriverState | @@ -XXX,X +XXX,X @@ fail: } | void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverSt > { if (bs->backing_hd) { | + if (bs->backing_hd) { + assert(error_is_set(&bs->backing_blocker)); + assert(error_is_set(&bs->backing_blocker)); + bdrv_op_unblock_all(bs->backing_hd, bs->backing_bloc + bdrv_op_unblock_all(bs->backing_hd, bs->backing_bloc bdrv_unref(bs->backing_hd); < + } else if (backing_hd) { + } else if (backing_hd) { + error_setg(&bs->backing_blocker, + error_setg(&bs->backing_blocker, + "device is used as backing hd of '%s'", + "device is used as backing hd of '%s'", + bs->device_name); + bs->device_name); } | + } | + bs->backing_hd = backing_hd; bs->backing_hd = backing_hd; if (!backing_hd) { if (!backing_hd) { bs->backing_file[0] = '\0'; bs->backing_file[0] = '\0'; bs->backing_format[0] = '\0'; bs->backing_format[0] = '\0'; + if (error_is_set(&bs->backing_blocker)) { + if (error_is_set(&bs->backing_blocker)) { + error_free(bs->backing_blocker); + error_free(bs->backing_blocker); + } + } goto out; goto out; } } > bs->open_flags &= ~BDRV_O_NO_BACKING; pstrcpy(bs->backing_file, sizeof(bs->backing_file), back pstrcpy(bs->backing_file, sizeof(bs->backing_file), back pstrcpy(bs->backing_format, sizeof(bs->backing_format), pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_hd->drv ? backing_hd->drv->format_name : backing_hd->drv ? backing_hd->drv->format_name : + + + bdrv_op_block_all(bs->backing_hd, bs->backing_blocker); + bdrv_op_block_all(bs->backing_hd, bs->backing_blocker); + /* Otherwise we won't be able to commit due to check in + /* Otherwise we won't be able to commit due to check in + bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, + bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, + bs->backing_blocker); + bs->backing_blocker); out: out: bdrv_refresh_limits(bs); bdrv_refresh_limits(bs); } } @@ -XXX,X +XXX,X @@ void bdrv_close(BlockDriverState *bs) @@ -XXX,X +XXX,X @@ void bdrv_close(BlockDriverState *bs) if (bs->drv) { if (bs->drv) { if (bs->backing_hd) { if (bs->backing_hd) { - bdrv_unref(bs->backing_hd); - bdrv_unref(bs->backing_hd); - bs->backing_hd = NULL; - bs->backing_hd = NULL; > + BlockDriverState *backing_hd = bs->backing_hd; + bdrv_set_backing_hd(bs, NULL); + bdrv_set_backing_hd(bs, NULL); > + bdrv_unref(backing_hd); } } bs->drv->bdrv_close(bs); bs->drv->bdrv_close(bs); g_free(bs->opaque); g_free(bs->opaque); @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); assert(bs_new->job == NULL); assert(bs_new->dev == NULL); assert(bs_new->dev == NULL); - assert(bdrv_op_blocker_is_empty(bs_new)); - assert(bdrv_op_blocker_is_empty(bs_new)); assert(bs_new->io_limits_enabled == false); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); assert(!throttle_have_timer(&bs_new->throttle_state)); @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, @@ -XXX,X +XXX,X @@ void bdrv_swap(BlockDriverState *bs_new, /* Check a few fields that should remain attached to the /* Check a few fields that should remain attached to the assert(bs_new->dev == NULL); assert(bs_new->dev == NULL); assert(bs_new->job == NULL); assert(bs_new->job == NULL); - assert(bdrv_op_blocker_is_empty(bs_new)); - assert(bdrv_op_blocker_is_empty(bs_new)); assert(bs_new->io_limits_enabled == false); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); assert(!throttle_have_timer(&bs_new->throttle_state)); diff --git a/block/mirror.c b/block/mirror.c diff --git a/block/mirror.c b/block/mirror.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block/mirror.c --- a/block/mirror.c +++ b/block/mirror.c +++ b/block/mirror.c @@ -XXX,X +XXX,X @@ immediate_exit: @@ -XXX,X +XXX,X @@ immediate_exit: * trigger the unref from the top one */ * trigger the unref from the top one */ BlockDriverState *p = s->base->backing_hd; BlockDriverState *p = s->base->backing_hd; s->base->backing_hd = NULL; s->base->backing_hd = NULL; + bdrv_op_unblock_all(p, s->base->backing_blocker) + bdrv_op_unblock_all(p, s->base->backing_blocker) bdrv_unref(p); bdrv_unref(p); } } } } diff --git a/include/block/block_int.h b/include/block/block_ diff --git a/include/block/block_int.h b/include/block/block_ index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/include/block/block_int.h --- a/include/block/block_int.h +++ b/include/block/block_int.h +++ b/include/block/block_int.h @@ -XXX,X +XXX,X @@ struct BlockDriverState { @@ -XXX,X +XXX,X @@ struct BlockDriverState { BlockJob *job; BlockJob *job; QDict *options; QDict *options; + + + /* The error object in use for blocking operations on ba + /* The error object in use for blocking operations on ba + Error *backing_blocker; + Error *backing_blocker; }; }; int get_tmp_filename(char *filename, int size); int get_tmp_filename(char *filename, int size); commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Parse "backing" option to reference existing BDS block: Parse "backing" option to reference existing BDS Now it's safe to allow reference for backing_hd in the in Now it's safe to allow reference for backing_hd in the in Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ int bdrv_open(BlockDriverState **pbs, con @@ -XXX,X +XXX,X @@ int bdrv_open(BlockDriverState **pbs, con /* If there is a backing file, use it */ /* If there is a backing file, use it */ if ((flags & BDRV_O_NO_BACKING) == 0) { if ((flags & BDRV_O_NO_BACKING) == 0) { QDict *backing_options; QDict *backing_options; + const char *backing_name; + const char *backing_name; + BlockDriverState *backing_hd; + BlockDriverState *backing_hd; + backing_name = qdict_get_try_str(options, "backing") + backing_name = qdict_get_try_str(options, "backing") qdict_extract_subqdict(options, &backing_options, "b qdict_extract_subqdict(options, &backing_options, "b - ret = bdrv_open_backing_file(bs, backing_options, &l - ret = bdrv_open_backing_file(bs, backing_options, &l - if (ret < 0) { - if (ret < 0) { + + + if (backing_name && qdict_size(backing_options)) { + if (backing_name && qdict_size(backing_options)) { + error_setg(&local_err, + error_setg(&local_err, + "Option \"backing\" and \"backing.*\" + "Option \"backing\" and \"backing.*\" + "used together"); + "used together"); + ret = -EINVAL; + ret = -EINVAL; goto close_and_fail; goto close_and_fail; } } + if (backing_name) { + if (backing_name) { + backing_hd = bdrv_find(backing_name); + backing_hd = bdrv_find(backing_name); + if (!backing_hd) { + if (!backing_hd) { + error_set(&local_err, QERR_DEVICE_NOT_FOUND, + error_set(&local_err, QERR_DEVICE_NOT_FOUND, + ret = -ENOENT; + ret = -ENOENT; + goto close_and_fail; + goto close_and_fail; + } + } + qdict_del(options, "backing"); + qdict_del(options, "backing"); + bdrv_set_backing_hd(bs, backing_hd); + bdrv_set_backing_hd(bs, backing_hd); > + bdrv_ref(backing_hd); + } else { + } else { + ret = bdrv_open_backing_file(bs, backing_options + ret = bdrv_open_backing_file(bs, backing_options + if (ret < 0) { + if (ret < 0) { + goto close_and_fail; + goto close_and_fail; + } + } + } + } } } done: done: commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Support dropping active in bdrv_drop_intermediate block: Support dropping active in bdrv_drop_intermediate Dropping intermediate could be useful both for commit and Dropping intermediate could be useful both for commit and BDS refcnt plus bdrv_swap could do most of the job nicely BDS refcnt plus bdrv_swap could do most of the job nicely to work with op blockers. to work with op blockers. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ BlockDriverState *bdrv_find_overlay(Block @@ -XXX,X +XXX,X @@ BlockDriverState *bdrv_find_overlay(Block return overlay; return overlay; } } -typedef struct BlkIntermediateStates { -typedef struct BlkIntermediateStates { - BlockDriverState *bs; - BlockDriverState *bs; - QSIMPLEQ_ENTRY(BlkIntermediateStates) entry; - QSIMPLEQ_ENTRY(BlkIntermediateStates) entry; -} BlkIntermediateStates; -} BlkIntermediateStates; - - - - /* /* - * Drops images above 'base' up to and including 'top', and - * Drops images above 'base' up to and including 'top', and - * above 'top' to have base as its backing file. - * above 'top' to have base as its backing file. + * Drops images above 'base' up to and including 'top', and + * Drops images above 'base' up to and including 'top', and + * backing_hd of top's overlay (the image orignally has 'top + * backing_hd of top's overlay (the image orignally has 'top + * top's overlay may be NULL if 'top' is active, no such upd + * top's overlay may be NULL if 'top' is active, no such upd + * Requires that the top's overlay to 'top' is opened r/w. + * Requires that the top's overlay to 'top' is opened r/w. + * + * + * 1) This will convert the following chain: + * 1) This will convert the following chain: + * + * + * ... <- base <- ... <- top <- overlay <-... <- active + * ... <- base <- ... <- top <- overlay <-... <- active + * < + * to < + * < + * ... <- base <- overlay <- active < * * - * Requires that the overlay to 'top' is opened r/w, so that - * Requires that the overlay to 'top' is opened r/w, so that - * information in 'bs' can be properly updated. - * information in 'bs' can be properly updated. > + * to > + * > + * ... <- base <- overlay <- active > + * + * 2) It is allowed for bottom==base, in which case it conve + * 2) It is allowed for bottom==base, in which case it conve * * - * E.g., this will convert the following chain: - * E.g., this will convert the following chain: - * bottom <- base <- intermediate <- top <- active - * bottom <- base <- intermediate <- top <- active + * base <- ... <- top <- overlay <- ... <- active + * base <- ... <- top <- overlay <- ... <- active * * * to * to * * - * bottom <- base <- active - * bottom <- base <- active + * base <- overlay <- active + * base <- overlay <- active * * - * It is allowed for bottom==base, in which case it converts - * It is allowed for bottom==base, in which case it converts + * 2) It also allows active==top, in which case it converts: + * 2) It also allows active==top, in which case it converts: * * - * base <- intermediate <- top <- active - * base <- intermediate <- top <- active + * ... <- base <- ... <- top (active) + * ... <- base <- ... <- top (active) * * * to * to * * - * base <- active - * base <- active + * ... <- base == active == top + * ... <- base == active == top + * + * + * i.e. only base and lower remains: *top == *base when retu + * i.e. only base and lower remains: *top == *base when retu + * + * + * 3) If base==NULL, it will drop all the BDS below overlay + * 3) If base==NULL, it will drop all the BDS below overlay + * backing_hd to NULL. I.e.: + * backing_hd to NULL. I.e.: + * < + * base(NULL) <- ... <- overlay <- ... <- active < + * < + * to < * * - * Error conditions: - * Error conditions: - * if active == top, that is considered an error - * if active == top, that is considered an error > + * base(NULL) <- ... <- overlay <- ... <- active > + * > + * to > + * + * overlay <- ... <- active + * overlay <- ... <- active * * */ */ int bdrv_drop_intermediate(BlockDriverState *active, BlockDr int bdrv_drop_intermediate(BlockDriverState *active, BlockDr BlockDriverState *base) BlockDriverState *base) { { - BlockDriverState *intermediate; - BlockDriverState *intermediate; - BlockDriverState *base_bs = NULL; - BlockDriverState *base_bs = NULL; - BlockDriverState *new_top_bs = NULL; - BlockDriverState *new_top_bs = NULL; - BlkIntermediateStates *intermediate_state, *next; - BlkIntermediateStates *intermediate_state, *next; - int ret = -EIO; - int ret = -EIO; - - - QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) s - QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) s - QSIMPLEQ_INIT(&states_to_delete); - QSIMPLEQ_INIT(&states_to_delete); - | + BlockDriverState *drop_start, *overlay, *bs; - if (!top->drv || !base->drv) { < - goto exit; < - } < + BlockDriverState *drop_start, *overlay; < + int ret = -EINVAL; + int ret = -EINVAL; > - if (!top->drv || !base->drv) { > + assert(active); > + assert(top); > + /* Verify that top is in backing chain of active */ > + bs = active; > + while (bs && bs != top) { > + bs = bs->backing_hd; > + } > + if (!bs) { > goto exit; > } > + /* Verify that base is in backing chain of top */ > + if (base) { > + while (bs && bs != base) { > + bs = bs->backing_hd; > + } > + if (bs != base) { > + goto exit; > + } > + } > - new_top_bs = bdrv_find_overlay(active, top); - new_top_bs = bdrv_find_overlay(active, top); - - - if (new_top_bs == NULL) { - if (new_top_bs == NULL) { - /* we could not find the image above 'top', this is - /* we could not find the image above 'top', this is + if (!top->drv || (base && !base->drv)) { + if (!top->drv || (base && !base->drv)) { goto exit; goto exit; } } - - - /* special case of new_top_bs->backing_hd already pointi - /* special case of new_top_bs->backing_hd already pointi - * to do, no intermediate images */ - * to do, no intermediate images */ - if (new_top_bs->backing_hd == base) { - if (new_top_bs->backing_hd == base) { + if (top == base) { + if (top == base) { > + ret = 0; > + goto exit; > + } 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; ret = 0; - goto exit; | goto exit; - } | } - | - intermediate = top; - intermediate = top; - - - /* now we will go down through the list, and add each BD - /* now we will go down through the list, and add each BD - * into our deletion queue, until we hit the 'base' - * into our deletion queue, until we hit the 'base' - */ - */ - while (intermediate) { - while (intermediate) { - intermediate_state = g_malloc0(sizeof(BlkIntermediat - intermediate_state = g_malloc0(sizeof(BlkIntermediat - intermediate_state->bs = intermediate; - intermediate_state->bs = intermediate; - QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate - QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate - - - if (intermediate->backing_hd == base) { - if (intermediate->backing_hd == base) { - base_bs = intermediate->backing_hd; - base_bs = intermediate->backing_hd; - break; - 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 (base) { < + overlay = bdrv_find_overlay(active, top); < + bdrv_set_backing_hd(overlay, base); < + ret = 0; < + } else { < + /* If there's an overlay, its backing_hd points to t < + * the top image is dropped but this BDS structure i < + * with base, this way we keep the pointers valid af < + overlay = bdrv_find_overlay(active, top); < + if (!overlay) { < + goto exit; < } < - intermediate = intermediate->backing_hd; - intermediate = intermediate->backing_hd; - } - } - if (base_bs == NULL) { - if (base_bs == NULL) { - /* something went wrong, we did not end at the base. - /* something went wrong, we did not end at the base. - * unravel everything, and exit with error */ - * unravel everything, and exit with error */ - goto exit; | + overlay = bdrv_find_overlay(active, top); - } | + if (!overlay) { > goto exit; > } - - - /* success - we can delete the intermediate states, and - /* success - we can delete the intermediate states, and - ret = bdrv_change_backing_file(new_top_bs, base_bs->file - ret = bdrv_change_backing_file(new_top_bs, base_bs->file - base_bs->drv ? base_bs->d - base_bs->drv ? base_bs->d - if (ret) { | + ret = bdrv_change_backing_file(overlay, - goto exit; | + base ? base->filename : N - } | + base ? base->drv->format_ > if (ret) { > goto exit; > } - new_top_bs->backing_hd = base_bs; - new_top_bs->backing_hd = base_bs; - | - bdrv_refresh_limits(new_top_bs); - bdrv_refresh_limits(new_top_bs); - - - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_del - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_del - /* so that bdrv_close() does not recursively close t - /* so that bdrv_close() does not recursively close t - intermediate_state->bs->backing_hd = NULL; - intermediate_state->bs->backing_hd = NULL; - bdrv_unref(intermediate_state->bs); - bdrv_unref(intermediate_state->bs); + ret = bdrv_change_backing_file(overlay, NULL, NULL); | + bs = overlay->backing_hd; + if (ret) { | + bdrv_set_backing_hd(overlay, base); + goto exit; | + if (base) { + } | + bdrv_ref(base); + bdrv_set_backing_hd(overlay, NULL); | + } > + if (bs) { > + bdrv_unref(bs); } } - ret = 0; | ret = 0; - - exit: exit: - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_del - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_del - g_free(intermediate_state); - g_free(intermediate_state); - } - } return ret; return ret; } } - - static int bdrv_check_byte_request(BlockDriverState *bs, int static int bdrv_check_byte_request(BlockDriverState *bs, int size_t size) size_t size) { { diff --git a/block/commit.c b/block/commit.c diff --git a/block/commit.c b/block/commit.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block/commit.c --- a/block/commit.c +++ b/block/commit.c +++ b/block/commit.c @@ -XXX,X +XXX,X @@ static void coroutine_fn commit_run(void @@ -XXX,X +XXX,X @@ static void coroutine_fn commit_run(void int bytes_written = 0; int bytes_written = 0; int64_t base_len; int64_t base_len; + overlay_bs = bdrv_find_overlay(active, top); + overlay_bs = bdrv_find_overlay(active, top); ret = s->common.len = bdrv_getlength(top); ret = s->common.len = bdrv_getlength(top); @@ -XXX,X +XXX,X @@ exit_restore_reopen: @@ -XXX,X +XXX,X @@ exit_restore_reopen: if (s->base_flags != bdrv_get_flags(base)) { if (s->base_flags != bdrv_get_flags(base)) { bdrv_reopen(base, s->base_flags, NULL); bdrv_reopen(base, s->base_flags, NULL); } } - overlay_bs = bdrv_find_overlay(active, top); - overlay_bs = bdrv_find_overlay(active, top); if (overlay_bs && s->orig_overlay_flags != bdrv_get_flag if (overlay_bs && s->orig_overlay_flags != bdrv_get_flag bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL) bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL) } } commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... stream: Use bdrv_drop_intermediate and drop close_unused_ stream: Use bdrv_drop_intermediate and drop close_unused_ This reuses the new bdrv_drop_intermediate. This reuses the new bdrv_drop_intermediate. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block/stream.c b/block/stream.c diff --git a/block/stream.c b/block/stream.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block/stream.c --- a/block/stream.c +++ b/block/stream.c +++ b/block/stream.c > @@ -XXX,X +XXX,X @@ typedef struct StreamBlockJob { > RateLimit limit; > BlockDriverState *base; > BlockdevOnError on_error; > - char backing_file_id[1024]; > } StreamBlockJob; > > static int coroutine_fn stream_populate(BlockDriverState *bs @@ -XXX,X +XXX,X @@ static int coroutine_fn stream_populate(B @@ -XXX,X +XXX,X @@ static int coroutine_fn stream_populate(B return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, } } -static void close_unused_images(BlockDriverState *top, Block -static void close_unused_images(BlockDriverState *top, Block - const char *base_id) - const char *base_id) -{ -{ - BlockDriverState *intermediate; - BlockDriverState *intermediate; - intermediate = top->backing_hd; - intermediate = top->backing_hd; - - - /* Must assign before bdrv_delete() to prevent traversin - /* Must assign before bdrv_delete() to prevent traversin - * while we delete backing image instances. - * while we delete backing image instances. - */ - */ - top->backing_hd = base; - top->backing_hd = base; - - - while (intermediate) { - while (intermediate) { - BlockDriverState *unused; - BlockDriverState *unused; - - - /* reached base */ - /* reached base */ - if (intermediate == base) { - if (intermediate == base) { - break; - break; - } - } - - - unused = intermediate; - unused = intermediate; - intermediate = intermediate->backing_hd; - intermediate = intermediate->backing_hd; - unused->backing_hd = NULL; - unused->backing_hd = NULL; - bdrv_unref(unused); - bdrv_unref(unused); - } - } - - - bdrv_refresh_limits(top); - bdrv_refresh_limits(top); -} -} - - static void coroutine_fn stream_run(void *opaque) static void coroutine_fn stream_run(void *opaque) { { StreamBlockJob *s = opaque; StreamBlockJob *s = opaque; @@ -XXX,X +XXX,X @@ wait: @@ -XXX,X +XXX,X @@ wait: } | ret = error; } | ret = bdrv_change_backing_file(bs, base_id, base_fmt | if (!block_job_is_cancelled(&s->common) && sector_num == > - const char *base_id = NULL, *base_fmt = NULL; > - if (base) { > - base_id = s->backing_file_id; > - if (base->drv) { > - base_fmt = base->drv->format_name; > - } > - } > - ret = bdrv_change_backing_file(bs, base_id, base_fmt - close_unused_images(bs, base, base_id); - close_unused_images(bs, base, base_id); + bdrv_drop_intermediate(bs, bs->backing_hd, base); | + ret = bdrv_drop_intermediate(bs, bs->backing_hd, bas } } qemu_vfree(buf); qemu_vfree(buf); > @@ -XXX,X +XXX,X @@ void stream_start(BlockDriverState *bs, B > } > > s->base = base; > - if (base_id) { > - pstrcpy(s->backing_file_id, sizeof(s->backing_file_i > - } > > s->on_error = on_error; > s->common.co = qemu_coroutine_create(stream_run); commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... qmp: Add command 'blockdev-backup' qmp: Add command 'blockdev-backup' Similar to drive-backup, but this command uses a device i Similar to drive-backup, but this command uses a device i instead of creating/opening an image file. instead of creating/opening an image file. Also add blocker on target bs, since the target is also a Also add blocker on target bs, since the target is also a now. now. Add check and report error for bs == target which became Add check and report error for bs == target which became an illegal case with introduction of blockdev-backup. an illegal case with introduction of blockdev-backup. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block/backup.c b/block/backup.c diff --git a/block/backup.c b/block/backup.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block/backup.c --- a/block/backup.c +++ b/block/backup.c +++ b/block/backup.c @@ -XXX,X +XXX,X @@ static void coroutine_fn backup_run(void @@ -XXX,X +XXX,X @@ static void coroutine_fn backup_run(void hbitmap_free(job->bitmap); hbitmap_free(job->bitmap); bdrv_iostatus_disable(target); bdrv_iostatus_disable(target); + bdrv_op_unblock_all(target, job->common.blocker); + bdrv_op_unblock_all(target, job->common.blocker); bdrv_unref(target); bdrv_unref(target); block_job_completed(&job->common, ret); block_job_completed(&job->common, ret); @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B assert(target); assert(target); assert(cb); assert(cb); + if (bs == target) { + if (bs == target) { + error_setg(errp, "Source and target cannot be the sa + error_setg(errp, "Source and target cannot be the sa + return; + return; + } + } + + if ((on_source_error == BLOCKDEV_ON_ERROR_STOP || if ((on_source_error == BLOCKDEV_ON_ERROR_STOP || on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) && on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) && !bdrv_iostatus_is_enabled(bs)) { !bdrv_iostatus_is_enabled(bs)) { @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B return; return; } } + if (!bdrv_is_inserted(bs)) { + if (!bdrv_is_inserted(bs)) { + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bs->devic + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bs->devic + return; + return; + } + } + + + if (!bdrv_is_inserted(target)) { + if (!bdrv_is_inserted(target)) { + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, target->d + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, target->d + return; + return; + } + } + + + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, + return; + return; + } + } + + + if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARG + if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARG + return; + return; + } + } + + len = bdrv_getlength(bs); len = bdrv_getlength(bs); if (len < 0) { if (len < 0) { error_setg_errno(errp, -len, "unable to get length f error_setg_errno(errp, -len, "unable to get length f @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B @@ -XXX,X +XXX,X @@ void backup_start(BlockDriverState *bs, B return; return; } } + bdrv_op_block_all(target, job->common.blocker); + bdrv_op_block_all(target, job->common.blocker); + + job->on_source_error = on_source_error; job->on_source_error = on_source_error; job->on_target_error = on_target_error; job->on_target_error = on_target_error; job->target = target; job->target = target; diff --git a/blockdev.c b/blockdev.c diff --git a/blockdev.c b/blockdev.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockdev.c --- a/blockdev.c +++ b/blockdev.c +++ b/blockdev.c @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, return; return; } } + /* Although backup_run has this check too, we need to us + /* Although backup_run has this check too, we need to us + * do an early check redundantly. */ + * do an early check redundantly. */ if (!bdrv_is_inserted(bs)) { if (!bdrv_is_inserted(bs)) { error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); return; return; @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, @@ -XXX,X +XXX,X @@ void qmp_drive_backup(const char *device, } } } } + /* Early check to avoid creating target */ + /* Early check to avoid creating target */ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, return; return; } } @@ -XXX,X +XXX,X @@ BlockDeviceInfoList *qmp_query_named_bloc @@ -XXX,X +XXX,X @@ BlockDeviceInfoList *qmp_query_named_bloc return bdrv_named_nodes_list(); return bdrv_named_nodes_list(); } } +void qmp_blockdev_backup(const char *device, const char *tar +void qmp_blockdev_backup(const char *device, const char *tar + enum MirrorSyncMode sync, + enum MirrorSyncMode sync, + bool has_speed, int64_t speed, + bool has_speed, int64_t speed, + bool has_on_source_error, + bool has_on_source_error, + BlockdevOnError on_source_error, + BlockdevOnError on_source_error, + bool has_on_target_error, + bool has_on_target_error, + BlockdevOnError on_target_error, + BlockdevOnError on_target_error, + Error **errp) + Error **errp) +{ +{ + BlockDriverState *bs; + BlockDriverState *bs; + BlockDriverState *target_bs; + BlockDriverState *target_bs; + Error *local_err = NULL; + Error *local_err = NULL; + + + if (!has_speed) { + if (!has_speed) { + speed = 0; + speed = 0; + } + } + if (!has_on_source_error) { + if (!has_on_source_error) { + on_source_error = BLOCKDEV_ON_ERROR_REPORT; + on_source_error = BLOCKDEV_ON_ERROR_REPORT; + } + } + if (!has_on_target_error) { + if (!has_on_target_error) { + on_target_error = BLOCKDEV_ON_ERROR_REPORT; + on_target_error = BLOCKDEV_ON_ERROR_REPORT; + } + } + + + bs = bdrv_find(device); + bs = bdrv_find(device); + if (!bs) { + if (!bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, device); + error_set(errp, QERR_DEVICE_NOT_FOUND, device); + return; + return; + } + } + + + target_bs = bdrv_find(target); + target_bs = bdrv_find(target); + if (!target_bs) { + if (!target_bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, target); + error_set(errp, QERR_DEVICE_NOT_FOUND, target); + return; + return; + } + } + + + bdrv_ref(target_bs); + bdrv_ref(target_bs); + backup_start(bs, target_bs, speed, sync, on_source_error + backup_start(bs, target_bs, speed, sync, on_source_error + block_job_cb, bs, &local_err); + block_job_cb, bs, &local_err); + if (local_err != NULL) { + if (local_err != NULL) { + bdrv_unref(target_bs); + bdrv_unref(target_bs); + error_propagate(errp, local_err); + error_propagate(errp, local_err); + } + } +} +} + + #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) void qmp_drive_mirror(const char *device, const char *target void qmp_drive_mirror(const char *device, const char *target diff --git a/qapi-schema.json b/qapi-schema.json diff --git a/qapi-schema.json b/qapi-schema.json index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/qapi-schema.json --- a/qapi-schema.json +++ b/qapi-schema.json +++ b/qapi-schema.json @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ '*on-target-error': 'BlockdevOnError' } } '*on-target-error': 'BlockdevOnError' } } ## ## +# @BlockdevBackup +# @BlockdevBackup +# +# +# @device: the name of the device which should be copied. +# @device: the name of the device which should be copied. +# +# +# @target: the name of the backup target device. +# @target: the name of the backup target device. +# +# +# @sync: what parts of the disk image should be copied to th +# @sync: what parts of the disk image should be copied to th +# (all the disk, only the sectors allocated in the to +# (all the disk, only the sectors allocated in the to +# only new I/O). +# only new I/O). +# +# +# @speed: #optional the maximum speed, in bytes per second. +# @speed: #optional the maximum speed, in bytes per second. +# +# +# @on-source-error: #optional the action to take on an error +# @on-source-error: #optional the action to take on an error +# default 'report'. 'stop' and 'enospc' c +# default 'report'. 'stop' and 'enospc' c +# if the block device supports io-status ( +# if the block device supports io-status ( +# +# +# @on-target-error: #optional the action to take on an error +# @on-target-error: #optional the action to take on an error +# default 'report' (no limitations, since +# default 'report' (no limitations, since +# a different block device than @device). +# a different block device than @device). +# +# +# Note that @on-source-error and @on-target-error only affec +# Note that @on-source-error and @on-target-error only affec +# If an error occurs during a guest write request, the devic +# If an error occurs during a guest write request, the devic +# actions will be used. +# actions will be used. +# +# +# Since: 2.0 +# Since: 2.0 +## +## +{ 'type': 'BlockdevBackup', +{ 'type': 'BlockdevBackup', + 'data': { 'device': 'str', 'target': 'str', + 'data': { 'device': 'str', 'target': 'str', + 'sync': 'MirrorSyncMode', + 'sync': 'MirrorSyncMode', + '*speed': 'int', + '*speed': 'int', + '*on-source-error': 'BlockdevOnError', + '*on-source-error': 'BlockdevOnError', + '*on-target-error': 'BlockdevOnError' } } + '*on-target-error': 'BlockdevOnError' } } + + +## +## # @Abort # @Abort # # # This action can be used to test transaction failure. # This action can be used to test transaction failure. @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ { 'command': 'query-named-block-nodes', 'returns': [ 'BlockD { 'command': 'query-named-block-nodes', 'returns': [ 'BlockD ## ## +# @blockdev-backup +# @blockdev-backup +# +# +# Block device version for drive-backup command. Use existin +# Block device version for drive-backup command. Use existin +# of backup. +# of backup. +# +# +# For the arguments, see the documentation of BlockdevBackup +# For the arguments, see the documentation of BlockdevBackup +# +# +# Returns: Nothing on success. +# Returns: Nothing on success. +# If @device or @target is not a valid block device +# If @device or @target is not a valid block device +# +# +# Since 2.0 +# Since 2.0 +## +## +{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' } +{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' } + + +## +## # @drive-mirror # @drive-mirror # # # Start mirroring a block device's writes to a new destinati # Start mirroring a block device's writes to a new destinati diff --git a/qmp-commands.hx b/qmp-commands.hx diff --git a/qmp-commands.hx b/qmp-commands.hx index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/qmp-commands.hx --- a/qmp-commands.hx +++ b/qmp-commands.hx +++ b/qmp-commands.hx @@ -XXX,X +XXX,X @@ Example: @@ -XXX,X +XXX,X @@ Example: "sync": "full "sync": "full "target": "ba "target": "ba <- { "return": {} } <- { "return": {} } + + +EQMP +EQMP + + + { + { + .name = "blockdev-backup", + .name = "blockdev-backup", + .args_type = "sync:s,device:B,target:B,speed:i?," + .args_type = "sync:s,device:B,target:B,speed:i?," + "on-source-error:s?,on-target-error:s? + "on-source-error:s?,on-target-error:s? + .mhandler.cmd_new = qmp_marshal_input_blockdev_backu + .mhandler.cmd_new = qmp_marshal_input_blockdev_backu + }, + }, + + +SQMP +SQMP +blockdev-backup +blockdev-backup +------------ +------------ + + +The device version of drive-backup: this command takes a exi +The device version of drive-backup: this command takes a exi +as backup target. +as backup target. + + +Arguments: +Arguments: + + +- "device": the name of the device which should be copied. +- "device": the name of the device which should be copied. + (json-string) + (json-string) +- "target": the target of the new image. If the file exists, +- "target": the target of the new image. If the file exists, + device, the existing file/device will be used as + device, the existing file/device will be used as + destination. If it does not exist, a new file w + destination. If it does not exist, a new file w + (json-string) + (json-string) +- "sync": what parts of the disk image should be copied to t +- "sync": what parts of the disk image should be copied to t + possibilities include "full" for all the disk, "to + possibilities include "full" for all the disk, "to + sectors allocated in the topmost image, or "none" + sectors allocated in the topmost image, or "none" + new I/O (MirrorSyncMode). + new I/O (MirrorSyncMode). +- "speed": the maximum speed, in bytes per second (json-int, +- "speed": the maximum speed, in bytes per second (json-int, +- "on-source-error": the action to take on an error on the s +- "on-source-error": the action to take on an error on the s + 'report'. 'stop' and 'enospc' can only + 'report'. 'stop' and 'enospc' can only + if the block device supports io-status. + if the block device supports io-status. + (BlockdevOnError, optional) + (BlockdevOnError, optional) +- "on-target-error": the action to take on an error on the t +- "on-target-error": the action to take on an error on the t + 'report' (no limitations, since this ap + 'report' (no limitations, since this ap + a different block device than device). + a different block device than device). + (BlockdevOnError, optional) + (BlockdevOnError, optional) + + +Example: +Example: +-> { "execute": "blockdev-backup", "arguments": { "device": +-> { "execute": "blockdev-backup", "arguments": { "device": + "target": + "target": +<- { "return": {} } +<- { "return": {} } + + EQMP EQMP { { commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Allow backup on referenced named BlockDriverState block: Allow backup on referenced named BlockDriverState Drive backup is a read only operation on source bs. We wa Drive backup is a read only operation on source bs. We wa this specific case to enable image-fleecing. Note that wh this specific case to enable image-fleecing. Note that wh image-fleecing job starts, the job still add its blocker image-fleecing job starts, the job still add its blocker and any other operation on it will be blocked by that. and any other operation on it will be blocked by that. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/block.c b/block.c diff --git a/block.c b/block.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/block.c --- a/block.c +++ b/block.c +++ b/block.c @@ -XXX,X +XXX,X @@ void bdrv_set_backing_hd(BlockDriverState @@ -XXX,X +XXX,X @@ void bdrv_set_backing_hd(BlockDriverState /* Otherwise we won't be able to commit due to check in /* Otherwise we won't be able to commit due to check in bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, bs->backing_blocker); bs->backing_blocker); + bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_BACKUP_SOU + bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_BACKUP_SOU + bs->backing_blocker); + bs->backing_blocker); out: out: bdrv_refresh_limits(bs); bdrv_refresh_limits(bs); } } commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... block: Add blockdev-backup to transaction block: Add blockdev-backup to transaction Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/blockdev.c b/blockdev.c diff --git a/blockdev.c b/blockdev.c index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/blockdev.c --- a/blockdev.c +++ b/blockdev.c +++ b/blockdev.c @@ -XXX,X +XXX,X @@ static void drive_backup_abort(BlkTransac @@ -XXX,X +XXX,X @@ static void drive_backup_abort(BlkTransac } } } } +typedef struct BlockdevBackupState { +typedef struct BlockdevBackupState { + BlkTransactionState common; + BlkTransactionState common; + BlockDriverState *bs; + BlockDriverState *bs; + BlockJob *job; + BlockJob *job; +} BlockdevBackupState; +} BlockdevBackupState; + + +static void blockdev_backup_prepare(BlkTransactionState *com +static void blockdev_backup_prepare(BlkTransactionState *com +{ +{ + BlockdevBackupState *state = DO_UPCAST(BlockdevBackupSta + BlockdevBackupState *state = DO_UPCAST(BlockdevBackupSta + BlockdevBackup *backup; + BlockdevBackup *backup; + Error *local_err = NULL; + Error *local_err = NULL; + + + assert(common->action->kind == TRANSACTION_ACTION_KIND_B + assert(common->action->kind == TRANSACTION_ACTION_KIND_B + backup = common->action->blockdev_backup; + backup = common->action->blockdev_backup; + + + qmp_blockdev_backup(backup->device, backup->target, + qmp_blockdev_backup(backup->device, backup->target, + backup->sync, + backup->sync, + backup->has_speed, backup->speed, + backup->has_speed, backup->speed, + backup->has_on_source_error, backup- + backup->has_on_source_error, backup- + backup->has_on_target_error, backup- + backup->has_on_target_error, backup- + &local_err); + &local_err); + if (error_is_set(&local_err)) { + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + error_propagate(errp, local_err); + state->bs = NULL; + state->bs = NULL; + state->job = NULL; + state->job = NULL; + return; + return; + } + } + + + state->bs = bdrv_find(backup->device); + state->bs = bdrv_find(backup->device); + state->job = state->bs->job; + state->job = state->bs->job; +} +} + + +static void blockdev_backup_abort(BlkTransactionState *commo +static void blockdev_backup_abort(BlkTransactionState *commo +{ +{ + BlockdevBackupState *state = DO_UPCAST(BlockdevBackupSta + BlockdevBackupState *state = DO_UPCAST(BlockdevBackupSta + BlockDriverState *bs = state->bs; + BlockDriverState *bs = state->bs; + + + /* Only cancel if it's the job we started */ + /* Only cancel if it's the job we started */ + if (bs && bs->job && bs->job == state->job) { + if (bs && bs->job && bs->job == state->job) { + block_job_cancel_sync(bs->job); + block_job_cancel_sync(bs->job); + } + } +} +} + + static void abort_prepare(BlkTransactionState *common, Error static void abort_prepare(BlkTransactionState *common, Error { { error_setg(errp, "Transaction aborted using Abort action error_setg(errp, "Transaction aborted using Abort action @@ -XXX,X +XXX,X @@ static const BdrvActionOps actions[] = { @@ -XXX,X +XXX,X @@ static const BdrvActionOps actions[] = { .prepare = drive_backup_prepare, .prepare = drive_backup_prepare, .abort = drive_backup_abort, .abort = drive_backup_abort, }, }, + [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = { + [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = { + .instance_size = sizeof(BlockdevBackupState), + .instance_size = sizeof(BlockdevBackupState), + .prepare = blockdev_backup_prepare, + .prepare = blockdev_backup_prepare, + .abort = blockdev_backup_abort, + .abort = blockdev_backup_abort, + }, + }, [TRANSACTION_ACTION_KIND_ABORT] = { [TRANSACTION_ACTION_KIND_ABORT] = { .instance_size = sizeof(BlkTransactionState), .instance_size = sizeof(BlkTransactionState), .prepare = abort_prepare, .prepare = abort_prepare, diff --git a/qapi-schema.json b/qapi-schema.json diff --git a/qapi-schema.json b/qapi-schema.json index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/qapi-schema.json --- a/qapi-schema.json +++ b/qapi-schema.json +++ b/qapi-schema.json @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ 'data': { 'data': { 'blockdev-snapshot-sync': 'BlockdevSnapshot', 'blockdev-snapshot-sync': 'BlockdevSnapshot', 'drive-backup': 'DriveBackup', 'drive-backup': 'DriveBackup', + 'blockdev-backup': 'BlockdevBackup', + 'blockdev-backup': 'BlockdevBackup', 'abort': 'Abort', 'abort': 'Abort', 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotI 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotI } } } } commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... qemu-iotests: Test blockdev-backup in 055 qemu-iotests: Test blockdev-backup in 055 This applies cases on drive-backup on blockdev-backup, ex This applies cases on drive-backup on blockdev-backup, ex target format and mode. target format and mode. Also add a case to check source == target. Also add a case to check source == target. Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/tests/qemu-iotests/055 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ #!/usr/bin/env python #!/usr/bin/env python # # -# Tests for drive-backup -# Tests for drive-backup +# Tests for drive-backup and blockdev-backup +# Tests for drive-backup and blockdev-backup # # # Copyright (C) 2013 Red Hat, Inc. # Copyright (C) 2013 Red Hat, Inc. # # @@ -XXX,X +XXX,X @@ from iotests import qemu_img, qemu_io @@ -XXX,X +XXX,X @@ from iotests import qemu_img, qemu_io test_img = os.path.join(iotests.test_dir, 'test.img') test_img = os.path.join(iotests.test_dir, 'test.img') target_img = os.path.join(iotests.test_dir, 'target.img') target_img = os.path.join(iotests.test_dir, 'target.img') +blockdev_target_img = os.path.join(iotests.test_dir, 'blockd +blockdev_target_img = os.path.join(iotests.test_dir, 'blockd class TestSingleDrive(iotests.QMPTestCase): class TestSingleDrive(iotests.QMPTestCase): image_len = 64 * 1024 * 1024 # MB image_len = 64 * 1024 * 1024 # MB @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase qemu_io('-c', 'write -P0xd5 1M 32k', test_img) qemu_io('-c', 'write -P0xd5 1M 32k', test_img) qemu_io('-c', 'write -P0xdc 32M 124k', test_img) qemu_io('-c', 'write -P0xdc 32M 124k', test_img) qemu_io('-c', 'write -P0xdc 67043328 64k', test_img) qemu_io('-c', 'write -P0xdc 67043328 64k', test_img) + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta - self.vm = iotests.VM().add_drive(test_img) - self.vm = iotests.VM().add_drive(test_img) + self.vm = iotests.VM().add_drive(test_img).add_drive + self.vm = iotests.VM().add_drive(test_img).add_drive self.vm.launch() self.vm.launch() def tearDown(self): def tearDown(self): self.vm.shutdown() self.vm.shutdown() os.remove(test_img) os.remove(test_img) + os.remove(blockdev_target_img) + os.remove(blockdev_target_img) try: try: os.remove(target_img) os.remove(target_img) except OSError: except OSError: pass pass - def test_cancel(self): - def test_cancel(self): + def do_test_cancel(self, test_drive_backup): + def do_test_cancel(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full') - target=target_img, sync='full') + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) event = self.cancel_and_wait() event = self.cancel_and_wait() self.assert_qmp(event, 'data/type', 'backup') self.assert_qmp(event, 'data/type', 'backup') - def test_pause(self): - def test_pause(self): + def test_cancel(self): + def test_cancel(self): + self.do_test_cancel(True) + self.do_test_cancel(True) + self.do_test_cancel(False) + self.do_test_cancel(False) + + + def do_test_pause(self, test_drive_backup): + def do_test_pause(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() self.vm.pause_drive('drive0') self.vm.pause_drive('drive0') - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full') - target=target_img, sync='full') + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-pause', device='driv result = self.vm.qmp('block-job-pause', device='driv @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase self.wait_until_completed() self.wait_until_completed() self.vm.shutdown() self.vm.shutdown() - self.assertTrue(iotests.compare_images(test_img, tar - self.assertTrue(iotests.compare_images(test_img, tar - 'target image does not match source - 'target image does not match source + if test_drive_backup: + if test_drive_backup: + self.assertTrue(iotests.compare_images(test_img, + self.assertTrue(iotests.compare_images(test_img, + 'target image does not match sou + 'target image does not match sou + else: + else: + self.assertTrue(iotests.compare_images(test_img, + self.assertTrue(iotests.compare_images(test_img, + 'target image does not match sou + 'target image does not match sou + + + def test_pause_drive_backup(self): + def test_pause_drive_backup(self): + self.do_test_pause(True) + self.do_test_pause(True) + + + def test_pause_blockdev_backup(self): + def test_pause_blockdev_backup(self): + self.do_test_pause(False) + self.do_test_pause(False) def test_medium_not_found(self): def test_medium_not_found(self): result = self.vm.qmp('drive-backup', device='ide1-cd result = self.vm.qmp('drive-backup', device='ide1-cd target=target_img, sync='full') target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'GenericError self.assert_qmp(result, 'error/class', 'GenericError + result = self.vm.qmp('blockdev-backup', device='ide1 + result = self.vm.qmp('blockdev-backup', device='ide1 + target='drive1', sync='full') + target='drive1', sync='full') + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError + + def test_image_not_found(self): def test_image_not_found(self): result = self.vm.qmp('drive-backup', device='drive0' result = self.vm.qmp('drive-backup', device='drive0' target=target_img, sync='full', target=target_img, sync='full', @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase @@ -XXX,X +XXX,X @@ class TestSingleDrive(iotests.QMPTestCase target=target_img, sync='full') target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'DeviceNotFou self.assert_qmp(result, 'error/class', 'DeviceNotFou + result = self.vm.qmp('blockdev-backup', device='none + result = self.vm.qmp('blockdev-backup', device='none + target='drive0', sync='full') + target='drive0', sync='full') + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + result = self.vm.qmp('blockdev-backup', device='driv + result = self.vm.qmp('blockdev-backup', device='driv + target='nonexistent', sync='ful + target='nonexistent', sync='ful + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + result = self.vm.qmp('blockdev-backup', device='none + result = self.vm.qmp('blockdev-backup', device='none + target='nonexistent', sync='ful + target='nonexistent', sync='ful + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + def test_target_is_source(self): + def test_target_is_source(self): + result = self.vm.qmp('blockdev-backup', device='driv + result = self.vm.qmp('blockdev-backup', device='driv + target='drive0', sync='full') + target='drive0', sync='full') + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError + + class TestSetSpeed(iotests.QMPTestCase): class TestSetSpeed(iotests.QMPTestCase): image_len = 80 * 1024 * 1024 # MB image_len = 80 * 1024 * 1024 # MB def setUp(self): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, test_img, s qemu_img('create', '-f', iotests.imgfmt, test_img, s qemu_io('-c', 'write -P1 0 512', test_img) qemu_io('-c', 'write -P1 0 512', test_img) - self.vm = iotests.VM().add_drive(test_img) - self.vm = iotests.VM().add_drive(test_img) + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta + + + self.vm = iotests.VM().add_drive(test_img).add_drive + self.vm = iotests.VM().add_drive(test_img).add_drive self.vm.launch() self.vm.launch() def tearDown(self): def tearDown(self): self.vm.shutdown() self.vm.shutdown() os.remove(test_img) os.remove(test_img) - os.remove(target_img) - os.remove(target_img) + try: + try: + os.remove(blockdev_target_img) + os.remove(blockdev_target_img) + except OSError: + except OSError: + pass + pass + try: + try: + os.remove(target_img) + os.remove(target_img) + except OSError: + except OSError: + pass + pass - def test_set_speed(self): - def test_set_speed(self): + def do_test_set_speed(self, test_drive_backup): + def do_test_set_speed(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() self.vm.pause_drive('drive0') self.vm.pause_drive('drive0') - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full') - target=target_img, sync='full') + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) # Default speed is 0 # Default speed is 0 @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): event = self.cancel_and_wait(resume=True) event = self.cancel_and_wait(resume=True) self.assert_qmp(event, 'data/type', 'backup') self.assert_qmp(event, 'data/type', 'backup') - # Check setting speed in drive-backup works - # Check setting speed in drive-backup works + # Check setting speed option works + # Check setting speed option works self.vm.pause_drive('drive0') self.vm.pause_drive('drive0') - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full', - target=target_img, sync='full', + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block-jobs') result = self.vm.qmp('query-block-jobs') @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): event = self.cancel_and_wait(resume=True) event = self.cancel_and_wait(resume=True) self.assert_qmp(event, 'data/type', 'backup') self.assert_qmp(event, 'data/type', 'backup') - def test_set_speed_invalid(self): - def test_set_speed_invalid(self): + def test_set_speed_drive_backup(self): + def test_set_speed_drive_backup(self): + self.do_test_set_speed(True) + self.do_test_set_speed(True) + + + def test_set_speed_blockdev_backup(self): + def test_set_speed_blockdev_backup(self): + self.do_test_set_speed(False) + self.do_test_set_speed(False) + + + def do_test_set_speed_invalid(self, test_drive_backup): + def do_test_set_speed_invalid(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full', - target=target_img, sync='full', + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'error/class', 'GenericError self.assert_qmp(result, 'error/class', 'GenericError self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() self.vm.pause_drive('drive0') self.vm.pause_drive('drive0') - result = self.vm.qmp('drive-backup', device='drive0' - result = self.vm.qmp('drive-backup', device='drive0' - target=target_img, sync='full') - target=target_img, sync='full') + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('drive-backup', device='dri + result = self.vm.qmp('drive-backup', device='dri + target=target_img, sync='fu + target=target_img, sync='fu + else: + else: + result = self.vm.qmp('blockdev-backup', device=' + result = self.vm.qmp('blockdev-backup', device=' + target='drive1', sync='full + target='drive1', sync='full self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-set-speed', device=' result = self.vm.qmp('block-job-set-speed', device=' @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): @@ -XXX,X +XXX,X @@ class TestSetSpeed(iotests.QMPTestCase): event = self.cancel_and_wait(resume=True) event = self.cancel_and_wait(resume=True) self.assert_qmp(event, 'data/type', 'backup') self.assert_qmp(event, 'data/type', 'backup') + def test_set_speed_invalid_drive_backup(self): + def test_set_speed_invalid_drive_backup(self): + self.do_test_set_speed_invalid(True) + self.do_test_set_speed_invalid(True) + + + def test_set_speed_invalid_blockdev_backup(self): + def test_set_speed_invalid_blockdev_backup(self): + self.do_test_set_speed_invalid(False) + self.do_test_set_speed_invalid(False) + + class TestSingleTransaction(iotests.QMPTestCase): class TestSingleTransaction(iotests.QMPTestCase): image_len = 64 * 1024 * 1024 # MB image_len = 64 * 1024 * 1024 # MB @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe qemu_io('-c', 'write -P0xd5 1M 32k', test_img) qemu_io('-c', 'write -P0xd5 1M 32k', test_img) qemu_io('-c', 'write -P0xdc 32M 124k', test_img) qemu_io('-c', 'write -P0xdc 32M 124k', test_img) qemu_io('-c', 'write -P0xdc 67043328 64k', test_img) qemu_io('-c', 'write -P0xdc 67043328 64k', test_img) + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta + qemu_img('create', '-f', iotests.imgfmt, blockdev_ta - self.vm = iotests.VM().add_drive(test_img) - self.vm = iotests.VM().add_drive(test_img) + self.vm = iotests.VM().add_drive(test_img).add_drive + self.vm = iotests.VM().add_drive(test_img).add_drive self.vm.launch() self.vm.launch() def tearDown(self): def tearDown(self): self.vm.shutdown() self.vm.shutdown() os.remove(test_img) os.remove(test_img) + os.remove(blockdev_target_img) + os.remove(blockdev_target_img) try: try: os.remove(target_img) os.remove(target_img) except OSError: except OSError: pass pass - def test_cancel(self): - def test_cancel(self): + def do_test_cancel(self, test_drive_backup): + def do_test_cancel(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() - result = self.vm.qmp('transaction', actions=[{ - result = self.vm.qmp('transaction', actions=[{ - 'type': 'drive-backup', - 'type': 'drive-backup', - 'data': { 'device': 'drive0', - 'data': { 'device': 'drive0', - 'target': target_img, - 'target': target_img, - 'sync': 'full' }, - 'sync': 'full' }, - } - } - ]) - ]) + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'drive-backup', + 'type': 'drive-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': target_img, + 'target': target_img, + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + else: + else: + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': 'drive1', + 'target': 'drive1', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + + self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) event = self.cancel_and_wait() event = self.cancel_and_wait() self.assert_qmp(event, 'data/type', 'backup') self.assert_qmp(event, 'data/type', 'backup') - def test_pause(self): - def test_pause(self): + def test_cancel_drive_backup(self): + def test_cancel_drive_backup(self): + self.do_test_cancel(True) + self.do_test_cancel(True) + + + def test_cancel_blockdev_backup(self): + def test_cancel_blockdev_backup(self): + self.do_test_cancel(False) + self.do_test_cancel(False) + + + def do_test_pause(self, test_drive_backup): + def do_test_pause(self, test_drive_backup): self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() self.vm.pause_drive('drive0') self.vm.pause_drive('drive0') - result = self.vm.qmp('transaction', actions=[{ - result = self.vm.qmp('transaction', actions=[{ - 'type': 'drive-backup', - 'type': 'drive-backup', - 'data': { 'device': 'drive0', - 'data': { 'device': 'drive0', - 'target': target_img, - 'target': target_img, - 'sync': 'full' }, - 'sync': 'full' }, - } - } - ]) - ]) + if test_drive_backup: + if test_drive_backup: + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'drive-backup', + 'type': 'drive-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': target_img, + 'target': target_img, + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + else: + else: + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': 'drive1', + 'target': 'drive1', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) self.assert_qmp(result, 'return', {}) self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-pause', device='driv result = self.vm.qmp('block-job-pause', device='driv @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe self.wait_until_completed() self.wait_until_completed() self.vm.shutdown() self.vm.shutdown() - self.assertTrue(iotests.compare_images(test_img, tar - self.assertTrue(iotests.compare_images(test_img, tar - 'target image does not match source - 'target image does not match source + if test_drive_backup: + if test_drive_backup: + self.assertTrue(iotests.compare_images(test_img, + self.assertTrue(iotests.compare_images(test_img, + 'target image does not match sou + 'target image does not match sou + else: + else: + self.assertTrue(iotests.compare_images(test_img, + self.assertTrue(iotests.compare_images(test_img, + 'target image does not match sou + 'target image does not match sou + + + def test_pause_drive_backup(self): + def test_pause_drive_backup(self): + self.do_test_pause(True) + self.do_test_pause(True) + + + def test_pause_blockdev_backup(self): + def test_pause_blockdev_backup(self): + self.do_test_pause(False) + self.do_test_pause(False) def test_medium_not_found(self): def test_medium_not_found(self): result = self.vm.qmp('transaction', actions=[{ result = self.vm.qmp('transaction', actions=[{ @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe } } ]) ]) self.assert_qmp(result, 'error/class', 'GenericError self.assert_qmp(result, 'error/class', 'GenericError + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'ide1-cd0', + 'data': { 'device': 'ide1-cd0', + 'target': 'drive1', + 'target': 'drive1', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError def test_image_not_found(self): def test_image_not_found(self): result = self.vm.qmp('transaction', actions=[{ result = self.vm.qmp('transaction', actions=[{ @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe ]) ]) self.assert_qmp(result, 'error/class', 'DeviceNotFou self.assert_qmp(result, 'error/class', 'DeviceNotFou + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'nonexistent', + 'data': { 'device': 'nonexistent', + 'target': 'drive1', + 'target': 'drive1', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': 'nonexistent', + 'target': 'nonexistent', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'nonexistent', + 'data': { 'device': 'nonexistent', + 'target': 'nonexistent', + 'target': 'nonexistent', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'DeviceNotFou + self.assert_qmp(result, 'error/class', 'DeviceNotFou + + + def test_target_is_source(self): + def test_target_is_source(self): + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': 'drive0', + 'target': 'drive0', + 'sync': 'full' }, + 'sync': 'full' }, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError + + def test_abort(self): def test_abort(self): result = self.vm.qmp('transaction', actions=[{ result = self.vm.qmp('transaction', actions=[{ 'type': 'drive-backup', 'type': 'drive-backup', @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe @@ -XXX,X +XXX,X @@ class TestSingleTransaction(iotests.QMPTe self.assert_qmp(result, 'error/class', 'GenericError self.assert_qmp(result, 'error/class', 'GenericError self.assert_no_active_block_jobs() self.assert_no_active_block_jobs() + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'nonexistent', + 'data': { 'device': 'nonexistent', + 'target': 'drive1', + 'target': 'drive1', + 'sync': 'full' }, + 'sync': 'full' }, + }, { + }, { + 'type': 'Abort', + 'type': 'Abort', + 'data': {}, + 'data': {}, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_no_active_block_jobs() + self.assert_no_active_block_jobs() + + + result = self.vm.qmp('transaction', actions=[{ + result = self.vm.qmp('transaction', actions=[{ + 'type': 'blockdev-backup', + 'type': 'blockdev-backup', + 'data': { 'device': 'drive0', + 'data': { 'device': 'drive0', + 'target': 'nonexistent', + 'target': 'nonexistent', + 'sync': 'full' }, + 'sync': 'full' }, + }, { + }, { + 'type': 'Abort', + 'type': 'Abort', + 'data': {}, + 'data': {}, + } + } + ]) + ]) + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_qmp(result, 'error/class', 'GenericError + self.assert_no_active_block_jobs() + self.assert_no_active_block_jobs() + + if __name__ == '__main__': if __name__ == '__main__': iotests.main(supported_fmts=['raw', 'qcow2']) iotests.main(supported_fmts=['raw', 'qcow2']) diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/ diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/ index XXXXXXX..XXXXXXX XXXXXX index XXXXXXX..XXXXXXX XXXXXX --- a/tests/qemu-iotests/055.out --- a/tests/qemu-iotests/055.out +++ b/tests/qemu-iotests/055.out +++ b/tests/qemu-iotests/055.out @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ -.............. -.............. +..................... +..................... ------------------------------------------------------------ ------------------------------------------------------------ -Ran 14 tests -Ran 14 tests +Ran 21 tests +Ran 21 tests OK OK commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX commit XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX Author: Fam Zheng Author: Fam Zheng Date: ... Date: ... qemu-iotests: Image fleecing test case 083 qemu-iotests: Image fleecing test case 083 This tests the workflow of creating a lightweight point-i This tests the workflow of creating a lightweight point-i with blockdev-backup command and export it with built-in with blockdev-backup command and export it with built-in It's tested that after the snapshot it created, writing t It's tested that after the snapshot it created, writing t device doesn't change data that can be read from target w device doesn't change data that can be read from target w Signed-off-by: Fam Zheng Signed-off-by: Fam Zheng diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 new file mode 100755 new file mode 100755 index XXXXXXX..XXXXXXX index XXXXXXX..XXXXXXX --- /dev/null --- /dev/null +++ b/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -XXX,X +XXX,X @@ @@ -XXX,X +XXX,X @@ +#!/usr/bin/env python +#!/usr/bin/env python +# +# +# Tests for image fleecing (point in time snapshot export to +# Tests for image fleecing (point in time snapshot export to +# +# +# Copyright (C) 2014 Red Hat, Inc. +# Copyright (C) 2014 Red Hat, Inc. +# +# +# Based on 055. +# Based on 055. +# +# +# This program is free software; you can redistribute it and +# This program is free software; you can redistribute it and +# it under the terms of the GNU General Public License as pu +# it under the terms of the GNU General Public License as pu +# the Free Software Foundation; either version 2 of the Lice +# the Free Software Foundation; either version 2 of the Lice +# (at your option) any later version. +# (at your option) any later version. +# +# +# This program is distributed in the hope that it will be us +# This program is distributed in the hope that it will be us +# but WITHOUT ANY WARRANTY; without even the implied warrant +# but WITHOUT ANY WARRANTY; without even the implied warrant +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +# GNU General Public License for more details. +# GNU General Public License for more details. +# +# +# You should have received a copy of the GNU General Public +# You should have received a copy of the GNU General Public +# along with this program. If not, see