* [Qemu-devel] [PATCH v10 01/11] block: Add BlockOpType enum
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 02/11] block: Introduce op_blockers to BlockDriverState Fam Zheng
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
This adds the enum of all the operations that can be taken on a block
device.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
include/block/block.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index 36efaea..2bc39fe 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -151,6 +151,25 @@ typedef struct BDRVReopenState {
void *opaque;
} BDRVReopenState;
+/*
+ * Block operation types
+ */
+typedef enum BlockOpType {
+ BLOCK_OP_TYPE_BACKUP_SOURCE,
+ BLOCK_OP_TYPE_BACKUP_TARGET,
+ BLOCK_OP_TYPE_CHANGE,
+ BLOCK_OP_TYPE_COMMIT,
+ BLOCK_OP_TYPE_DATAPLANE,
+ BLOCK_OP_TYPE_DRIVE_DEL,
+ BLOCK_OP_TYPE_EJECT,
+ BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
+ BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
+ BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
+ BLOCK_OP_TYPE_MIRROR,
+ BLOCK_OP_TYPE_RESIZE,
+ BLOCK_OP_TYPE_STREAM,
+ BLOCK_OP_TYPE_MAX,
+} BlockOpType;
void bdrv_iostatus_enable(BlockDriverState *bs);
void bdrv_iostatus_reset(BlockDriverState *bs);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 02/11] block: Introduce op_blockers to BlockDriverState
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 01/11] block: Add BlockOpType enum Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 03/11] block: Replace in_use with operation blocker Fam Zheng
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX
elements. Each list is a list of blockers of an operation type
(BlockOpType), that marks this BDS as currently blocked for a certain
type of operation with reason errors stored in the list. The rule of
usage is:
* BDS user who wants to take an operation should check if there's any
blocker of the type with bdrv_op_is_blocked().
* BDS user who wants to block certain types of operation, should call
bdrv_op_block (or bdrv_op_block_all to block all types of operations,
which is similar to the existing bdrv_set_in_use()).
* A blocker is only referenced by op_blockers, so the lifecycle is
managed by caller, and shouldn't be lost until unblock, so typically
a caller does these:
- Allocate a blocker with error_setg or similar, call bdrv_op_block()
to block some operations.
- Hold the blocker, do his job.
- Unblock operations that it blocked, with the same reason pointer
passed to bdrv_op_unblock().
- Release the blocker with error_free().
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 7 +++++
include/block/block_int.h | 5 ++++
3 files changed, 83 insertions(+)
diff --git a/block.c b/block.c
index 64e7d22..91cda9c 100644
--- a/block.c
+++ b/block.c
@@ -1627,6 +1627,8 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
/* keep the same entry in bdrv_states */
pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
bs_src->device_name);
+ memcpy(bs_dest->op_blockers, bs_src->op_blockers,
+ sizeof(bs_dest->op_blockers));
bs_dest->list = bs_src->list;
}
@@ -4634,6 +4636,75 @@ void bdrv_unref(BlockDriverState *bs)
}
}
+struct BdrvOpBlocker {
+ Error *reason;
+ QLIST_ENTRY(BdrvOpBlocker) list;
+};
+
+bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
+{
+ BdrvOpBlocker *blocker;
+ assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+ if (!QLIST_EMPTY(&bs->op_blockers[op])) {
+ blocker = QLIST_FIRST(&bs->op_blockers[op]);
+ if (errp) {
+ *errp = error_copy(blocker->reason);
+ }
+ return true;
+ }
+ return false;
+}
+
+void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason)
+{
+ BdrvOpBlocker *blocker;
+ assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+
+ blocker = g_malloc0(sizeof(BdrvOpBlocker));
+ blocker->reason = reason;
+ QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list);
+}
+
+void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason)
+{
+ BdrvOpBlocker *blocker, *next;
+ assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+ QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) {
+ if (blocker->reason == reason) {
+ QLIST_REMOVE(blocker, list);
+ g_free(blocker);
+ }
+ }
+}
+
+void bdrv_op_block_all(BlockDriverState *bs, Error *reason)
+{
+ int i;
+ for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+ bdrv_op_block(bs, i, reason);
+ }
+}
+
+void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason)
+{
+ int i;
+ for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+ bdrv_op_unblock(bs, i, reason);
+ }
+}
+
+bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
+{
+ int i;
+
+ for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+ if (!QLIST_EMPTY(&bs->op_blockers[i])) {
+ return false;
+ }
+ }
+ return true;
+}
+
void bdrv_set_in_use(BlockDriverState *bs, int in_use)
{
assert(bs->in_use != in_use);
diff --git a/include/block/block.h b/include/block/block.h
index 2bc39fe..e6073e7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -461,6 +461,13 @@ void bdrv_unref(BlockDriverState *bs);
void bdrv_set_in_use(BlockDriverState *bs, int in_use);
int bdrv_in_use(BlockDriverState *bs);
+bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
+void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
+void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
+void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason);
+bool bdrv_op_blocker_is_empty(BlockDriverState *bs);
+
#ifdef CONFIG_LINUX_AIO
int raw_get_aio_fd(BlockDriverState *bs);
#else
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8b132d7..458acd6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -252,6 +252,8 @@ typedef struct BlockLimits {
int opt_transfer_length;
} BlockLimits;
+typedef struct BdrvOpBlocker BdrvOpBlocker;
+
/*
* Note: the function bdrv_append() copies and swaps contents of
* BlockDriverStates, so if you add new fields to this struct, please
@@ -333,6 +335,9 @@ struct BlockDriverState {
QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
+ /* operation blockers */
+ QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX];
+
/* long-running background operation */
BlockJob *job;
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 03/11] block: Replace in_use with operation blocker
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 01/11] block: Add BlockOpType enum Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 02/11] block: Introduce op_blockers to BlockDriverState Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 04/11] block: Move op_blocker check from block_job_create to its caller Fam Zheng
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
This drops BlockDriverState.in_use with op_blockers:
- Call bdrv_op_block_all in place of bdrv_set_in_use(bs, 1).
- Call bdrv_op_unblock_all in place of bdrv_set_in_use(bs, 0).
- Check bdrv_op_is_blocked() in place of bdrv_in_use(bs).
The specific types are used, e.g. in place of starting block backup,
bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...).
- Check bdrv_op_blocker_is_empty() in place of assert(!bs->in_use).
Note: there is only bdrv_op_block_all and bdrv_op_unblock_all callers at
this moment. So although the checks are specific to op types, this
changes can still be seen as identical logic with previously with
in_use. The difference is error message are improved because of blocker
error info.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block-migration.c | 7 +++++--
block.c | 24 +++++++-----------------
blockdev.c | 15 ++++++---------
blockjob.c | 14 +++++++++-----
hw/block/dataplane/virtio-blk.c | 19 ++++++++++++-------
include/block/block.h | 2 --
include/block/block_int.h | 1 -
include/block/blockjob.h | 3 +++
8 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 897fdba..bf9a25f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -59,6 +59,7 @@ typedef struct BlkMigDevState {
unsigned long *aio_bitmap;
int64_t completed_sectors;
BdrvDirtyBitmap *dirty_bitmap;
+ Error *blocker;
} BlkMigDevState;
typedef struct BlkMigBlock {
@@ -346,7 +347,8 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs)
bmds->completed_sectors = 0;
bmds->shared_base = block_mig_state.shared_base;
alloc_aio_bitmap(bmds);
- bdrv_set_in_use(bs, 1);
+ error_setg(&bmds->blocker, "block device is in use by migration");
+ bdrv_op_block_all(bs, bmds->blocker);
bdrv_ref(bs);
block_mig_state.total_sector_sum += sectors;
@@ -584,7 +586,8 @@ static void blk_mig_cleanup(void)
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
- bdrv_set_in_use(bmds->bs, 0);
+ bdrv_op_unblock_all(bmds->bs, bmds->blocker);
+ error_free(bmds->blocker);
bdrv_unref(bmds->bs);
g_free(bmds->aio_bitmap);
g_free(bmds);
diff --git a/block.c b/block.c
index 91cda9c..b122154 100644
--- a/block.c
+++ b/block.c
@@ -1621,7 +1621,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
bs_dest->refcnt = bs_src->refcnt;
/* job */
- bs_dest->in_use = bs_src->in_use;
bs_dest->job = bs_src->job;
/* keep the same entry in bdrv_states */
@@ -1653,7 +1652,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
assert(bs_new->job == NULL);
assert(bs_new->dev == NULL);
- assert(bs_new->in_use == 0);
+ assert(bdrv_op_blocker_is_empty(bs_new));
assert(bs_new->io_limits_enabled == false);
assert(!throttle_have_timer(&bs_new->throttle_state));
@@ -1672,7 +1671,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
/* Check a few fields that should remain attached to the device */
assert(bs_new->dev == NULL);
assert(bs_new->job == NULL);
- assert(bs_new->in_use == 0);
+ assert(bdrv_op_blocker_is_empty(bs_new));
assert(bs_new->io_limits_enabled == false);
assert(!throttle_have_timer(&bs_new->throttle_state));
@@ -1709,7 +1708,7 @@ static void bdrv_delete(BlockDriverState *bs)
{
assert(!bs->dev);
assert(!bs->job);
- assert(!bs->in_use);
+ assert(bdrv_op_blocker_is_empty(bs));
assert(!bs->refcnt);
assert(QLIST_EMPTY(&bs->dirty_bitmaps));
@@ -1891,7 +1890,8 @@ int bdrv_commit(BlockDriverState *bs)
return -ENOTSUP;
}
- if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) {
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, NULL) ||
+ bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, NULL)) {
return -EBUSY;
}
@@ -2924,8 +2924,9 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
return -ENOTSUP;
if (bs->read_only)
return -EACCES;
- if (bdrv_in_use(bs))
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
return -EBUSY;
+ }
ret = drv->bdrv_truncate(bs, offset);
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
@@ -4705,17 +4706,6 @@ bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
return true;
}
-void bdrv_set_in_use(BlockDriverState *bs, int in_use)
-{
- assert(bs->in_use != in_use);
- bs->in_use = in_use;
-}
-
-int bdrv_in_use(BlockDriverState *bs)
-{
- return bs->in_use;
-}
-
void bdrv_iostatus_enable(BlockDriverState *bs)
{
bs->iostatus_enabled = true;
diff --git a/blockdev.c b/blockdev.c
index 6a85961..9d36775 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1224,8 +1224,8 @@ static void external_snapshot_prepare(BlkTransactionState *common,
return;
}
- if (bdrv_in_use(state->old_bs)) {
- error_set(errp, QERR_DEVICE_IN_USE, device);
+ if (bdrv_op_is_blocked(state->old_bs,
+ BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {
return;
}
@@ -1441,8 +1441,7 @@ exit:
static void eject_device(BlockDriverState *bs, int force, Error **errp)
{
- if (bdrv_in_use(bs)) {
- error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
return;
}
if (!bdrv_dev_has_removable_media(bs)) {
@@ -1643,7 +1642,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
qerror_report(QERR_DEVICE_NOT_FOUND, id);
return -1;
}
- if (bdrv_in_use(bs)) {
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) {
qerror_report(QERR_DEVICE_IN_USE, id);
return -1;
}
@@ -1881,8 +1880,7 @@ void qmp_drive_backup(const char *device, const char *target,
}
}
- if (bdrv_in_use(bs)) {
- error_set(errp, QERR_DEVICE_IN_USE, device);
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
return;
}
@@ -2011,8 +2009,7 @@ void qmp_drive_mirror(const char *device, const char *target,
}
}
- if (bdrv_in_use(bs)) {
- error_set(errp, QERR_DEVICE_IN_USE, device);
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
return;
}
diff --git a/blockjob.c b/blockjob.c
index 9e5fd5c..f1ff036 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -41,14 +41,16 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
{
BlockJob *job;
- if (bs->job || bdrv_in_use(bs)) {
+ if (bs->job || !bdrv_op_blocker_is_empty(bs)) {
error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
return NULL;
}
bdrv_ref(bs);
- bdrv_set_in_use(bs, 1);
-
job = g_malloc0(driver->instance_size);
+ error_setg(&job->blocker, "block device is in use by block job: %s",
+ BlockJobType_lookup[driver->job_type]);
+ bdrv_op_block_all(bs, job->blocker);
+
job->driver = driver;
job->bs = bs;
job->cb = cb;
@@ -63,8 +65,9 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
block_job_set_speed(job, speed, &local_err);
if (error_is_set(&local_err)) {
bs->job = NULL;
+ bdrv_op_unblock_all(bs, job->blocker);
+ error_free(job->blocker);
g_free(job);
- bdrv_set_in_use(bs, 0);
error_propagate(errp, local_err);
return NULL;
}
@@ -79,8 +82,9 @@ void block_job_completed(BlockJob *job, int ret)
assert(bs->job == job);
job->cb(job->opaque, ret);
bs->job = NULL;
+ bdrv_op_unblock_all(bs, job->blocker);
+ error_free(job->blocker);
g_free(job);
- bdrv_set_in_use(bs, 0);
}
void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 1e57f3a..3fb7f82 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -69,6 +69,9 @@ struct VirtIOBlockDataPlane {
queue */
unsigned int num_reqs;
+
+ /* Operation blocker on BDS */
+ Error *blocker;
};
/* Raise an interrupt to signal guest, if necessary */
@@ -386,6 +389,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
{
VirtIOBlockDataPlane *s;
int fd;
+ Error *local_err = NULL;
*dataplane = NULL;
@@ -408,9 +412,10 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
/* If dataplane is (re-)enabled while the guest is running there could be
* block jobs that can conflict.
*/
- if (bdrv_in_use(blk->conf.bs)) {
- error_setg(errp,
- "cannot start dataplane thread while device is in use");
+ if (bdrv_op_is_blocked(blk->conf.bs, BLOCK_OP_TYPE_DATAPLANE, &local_err)) {
+ error_report("cannot start dataplane thread: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
return;
}
@@ -425,9 +430,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
s->vdev = vdev;
s->fd = fd;
s->blk = blk;
-
- /* Prevent block operations that conflict with data plane thread */
- bdrv_set_in_use(blk->conf.bs, 1);
+ error_setg(&s->blocker, "block device is in use by data plane");
+ bdrv_op_block_all(blk->conf.bs, s->blocker);
*dataplane = s;
}
@@ -439,7 +443,8 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
}
virtio_blk_data_plane_stop(s);
- bdrv_set_in_use(s->blk->conf.bs, 0);
+ bdrv_op_unblock_all(s->blk->conf.bs, s->blocker);
+ error_free(s->blocker);
g_free(s);
}
diff --git a/include/block/block.h b/include/block/block.h
index e6073e7..5339f92 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -458,8 +458,6 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
void bdrv_ref(BlockDriverState *bs);
void bdrv_unref(BlockDriverState *bs);
-void bdrv_set_in_use(BlockDriverState *bs, int in_use);
-int bdrv_in_use(BlockDriverState *bs);
bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 458acd6..2f6556d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -330,7 +330,6 @@ struct BlockDriverState {
char device_name[32];
QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
int refcnt;
- int in_use; /* users other than guest access, eg. block migration */
QTAILQ_ENTRY(BlockDriverState) list;
QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d76de62..c0a7875 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -106,6 +106,9 @@ struct BlockJob {
/** The completion function that will be called when the job completes. */
BlockDriverCompletionFunc *cb;
+ /** Block other operations when block job is running */
+ Error *blocker;
+
/** The opaque value that is passed to the completion function. */
void *opaque;
};
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 04/11] block: Move op_blocker check from block_job_create to its caller
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (2 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 03/11] block: Replace in_use with operation blocker Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 05/11] block: Add bdrv_set_backing_hd() Fam Zheng
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
It makes no sense to check for "any" blocker on bs, we are here only
because of the mechanical conversion from in_use to op_blockers. Remove
it now, and let the callers check specific operation types. Backup and
mirror already have it, add checker to stream and commit.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 8 ++++++++
blockjob.c | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 9d36775..7f305d8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1754,6 +1754,10 @@ void qmp_block_stream(const char *device, bool has_base,
return;
}
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
+ return;
+ }
+
if (base) {
base_bs = bdrv_find_backing_image(bs, base);
if (base_bs == NULL) {
@@ -1794,6 +1798,10 @@ void qmp_block_commit(const char *device,
return;
}
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) {
+ return;
+ }
+
/* default top_bs is the active layer */
top_bs = bs;
diff --git a/blockjob.c b/blockjob.c
index f1ff036..21e21c0 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -41,7 +41,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
{
BlockJob *job;
- if (bs->job || !bdrv_op_blocker_is_empty(bs)) {
+ if (bs->job) {
error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
return NULL;
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 05/11] block: Add bdrv_set_backing_hd()
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (3 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 04/11] block: Move op_blocker check from block_job_create to its caller Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 06/11] block: Add backing_blocker in BlockDriverState Fam Zheng
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
This is the common but non-trivial steps to assign or change the
backing_hd of BDS.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 34 ++++++++++++++++++++++++++++------
include/block/block.h | 1 +
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index b122154..ff25749 100644
--- a/block.c
+++ b/block.c
@@ -958,6 +958,29 @@ fail:
return ret;
}
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
+{
+ if (bs->backing_hd) {
+ bdrv_unref(bs->backing_hd);
+ }
+
+ bs->backing_hd = backing_hd;
+ if (!backing_hd) {
+ bs->backing_file[0] = '\0';
+ bs->backing_format[0] = '\0';
+ return;
+ }
+ pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
+ pstrcpy(bs->backing_format, sizeof(bs->backing_file),
+ backing_hd->drv ? backing_hd->drv->format_name : "");
+ bdrv_ref(bs->backing_hd);
+
+ pstrcpy(bs->backing_file, sizeof(bs->backing_file),
+ bs->backing_hd->file->filename);
+ pstrcpy(bs->backing_format, sizeof(bs->backing_format),
+ bs->backing_hd->drv->format_name);
+}
+
/*
* Opens the backing file for a BlockDriverState if not yet open
*
@@ -971,6 +994,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
char backing_filename[PATH_MAX];
int back_flags, ret;
BlockDriver *back_drv = NULL;
+ BlockDriverState *backing_hd;
Error *local_err = NULL;
if (bs->backing_hd != NULL) {
@@ -994,7 +1018,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
sizeof(backing_filename));
}
- bs->backing_hd = bdrv_new("");
+ backing_hd = bdrv_new("");
if (bs->backing_format[0] != '\0') {
back_drv = bdrv_find_format(bs->backing_format);
@@ -1004,20 +1028,18 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
BDRV_O_COPY_ON_READ);
- ret = bdrv_open(bs->backing_hd,
+ ret = bdrv_open(backing_hd,
*backing_filename ? backing_filename : NULL, options,
back_flags, back_drv, &local_err);
if (ret < 0) {
- bdrv_unref(bs->backing_hd);
- bs->backing_hd = NULL;
+ bdrv_unref(backing_hd);
bs->open_flags |= BDRV_O_NO_BACKING;
error_setg(errp, "Could not open backing file: %s",
error_get_pretty(local_err));
error_free(local_err);
return ret;
}
- pstrcpy(bs->backing_file, sizeof(bs->backing_file),
- bs->backing_hd->file->filename);
+ bdrv_set_backing_hd(bs, backing_hd);
return 0;
}
diff --git a/include/block/block.h b/include/block/block.h
index 5339f92..4ec0069 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -204,6 +204,7 @@ int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_discard_flags(const char *mode, int *flags);
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
QDict *options, int flags, Error **errp);
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
int flags, BlockDriver *drv, Error **errp);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 06/11] block: Add backing_blocker in BlockDriverState
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (4 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 05/11] block: Add bdrv_set_backing_hd() Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 07/11] block: Parse "backing" option to reference existing BDS Fam Zheng
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
This makes use of op_blocker and blocks all the operations except for
commit target, on each BlockDriverState->backing_hd.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 16 ++++++++++++++--
include/block/block_int.h | 3 +++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index ff25749..907c483 100644
--- a/block.c
+++ b/block.c
@@ -961,13 +961,22 @@ fail:
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
{
if (bs->backing_hd) {
+ assert(error_is_set(&bs->backing_blocker));
+ bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
bdrv_unref(bs->backing_hd);
+ } else if (backing_hd) {
+ error_setg(&bs->backing_blocker,
+ "device is used as backing hd of '%s'",
+ bs->device_name);
}
bs->backing_hd = backing_hd;
if (!backing_hd) {
bs->backing_file[0] = '\0';
bs->backing_format[0] = '\0';
+ if (error_is_set(&bs->backing_blocker)) {
+ error_free(bs->backing_blocker);
+ }
return;
}
pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
@@ -975,6 +984,10 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
backing_hd->drv ? backing_hd->drv->format_name : "");
bdrv_ref(bs->backing_hd);
+ bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
+ /* Otherwise we won't be able to commit due to check in bdrv_commit */
+ bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
+ bs->backing_blocker);
pstrcpy(bs->backing_file, sizeof(bs->backing_file),
bs->backing_hd->file->filename);
pstrcpy(bs->backing_format, sizeof(bs->backing_format),
@@ -1481,8 +1494,7 @@ void bdrv_close(BlockDriverState *bs)
if (bs->drv) {
if (bs->backing_hd) {
- bdrv_unref(bs->backing_hd);
- bs->backing_hd = NULL;
+ bdrv_set_backing_hd(bs, NULL);
}
bs->drv->bdrv_close(bs);
g_free(bs->opaque);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2f6556d..1ac17d5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -341,6 +341,9 @@ struct BlockDriverState {
BlockJob *job;
QDict *options;
+
+ /* The error object in use for blocking operations on backing_hd */
+ Error *backing_blocker;
};
int get_tmp_filename(char *filename, int size);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 07/11] block: Parse "backing" option to reference existing BDS
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (5 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 06/11] block: Add backing_blocker in BlockDriverState Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 08/11] block: Support dropping active in bdrv_drop_intermediate Fam Zheng
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
Now it's safe to allow reference for backing_hd in the interface.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 907c483..618b8c3 100644
--- a/block.c
+++ b/block.c
@@ -1199,12 +1199,34 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
/* If there is a backing file, use it */
if ((flags & BDRV_O_NO_BACKING) == 0) {
QDict *backing_options;
+ const char *backing_name;
+ BlockDriverState *backing_hd;
+ backing_name = qdict_get_try_str(options, "backing");
qdict_extract_subqdict(options, &backing_options, "backing.");
- ret = bdrv_open_backing_file(bs, backing_options, &local_err);
- if (ret < 0) {
+
+ if (backing_name && qdict_size(backing_options)) {
+ error_setg(&local_err,
+ "Option \"backing\" and \"backing.*\" cannot be "
+ "used together");
+ ret = -EINVAL;
goto close_and_fail;
}
+ if (backing_name) {
+ backing_hd = bdrv_find(backing_name);
+ if (!backing_hd) {
+ error_set(&local_err, QERR_DEVICE_NOT_FOUND, backing_name);
+ ret = -ENOENT;
+ goto close_and_fail;
+ }
+ qdict_del(options, "backing");
+ bdrv_set_backing_hd(bs, backing_hd);
+ } else {
+ ret = bdrv_open_backing_file(bs, backing_options, &local_err);
+ if (ret < 0) {
+ goto close_and_fail;
+ }
+ }
}
/* Check if any unknown options were used */
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 08/11] block: Support dropping active in bdrv_drop_intermediate
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (6 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 07/11] block: Parse "backing" option to reference existing BDS Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images Fam Zheng
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
Dropping intermediate could be useful both for commit and stream, and
BDS refcnt plus bdrv_swap could do most of the job nicely. It also needs
to work with op blockers.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 145 ++++++++++++++++++++++++++-------------------------------
block/commit.c | 1 +
2 files changed, 66 insertions(+), 80 deletions(-)
diff --git a/block.c b/block.c
index 618b8c3..b28fb93 100644
--- a/block.c
+++ b/block.c
@@ -2190,114 +2190,99 @@ BlockDriverState *bdrv_find_overlay(BlockDriverState *active,
return overlay;
}
-typedef struct BlkIntermediateStates {
- BlockDriverState *bs;
- QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
-
/*
- * Drops images above 'base' up to and including 'top', and sets the image
- * above 'top' to have base as its backing file.
+ * Drops images above 'base' up to and including 'top', and sets new 'base'
+ * as backing_hd of top_overlay (the image orignally has 'top' as backing
+ * file). top_overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top_overlay to 'top' is opened r/w.
*
- * Requires that the overlay to 'top' is opened r/w, so that the backing file
- * information in 'bs' can be properly updated.
+ * 1) This will convert the following chain:
*
- * E.g., this will convert the following chain:
- * bottom <- base <- intermediate <- top <- active
+ * ... <- base <- ... <- top <- overlay <-... <- active
*
* to
*
- * bottom <- base <- active
+ * ... <- base <- overlay <- active
*
- * It is allowed for bottom==base, in which case it converts:
+ * 2) It is allowed for bottom==base, in which case it converts:
*
- * base <- intermediate <- top <- active
+ * base <- ... <- top <- overlay <- ... <- active
*
* to
*
- * base <- active
+ * base <- overlay <- active
+ *
+ * 2) It also allows active==top, in which case it converts:
+ *
+ * ... <- base <- ... <- top (active)
+ *
+ * to
+ *
+ * ... <- base == active == top
+ *
+ * i.e. only base and lower remains: *top == *base when return.
+ *
+ * 3) If base==NULL, it will drop all the BDS below overlay and set its
+ * backing_hd to NULL. I.e.:
+ *
+ * base(NULL) <- ... <- overlay <- ... <- active
+ *
+ * to
*
- * Error conditions:
- * if active == top, that is considered an error
+ * overlay <- ... <- active
*
*/
int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
BlockDriverState *base)
{
- BlockDriverState *intermediate;
- BlockDriverState *base_bs = NULL;
- BlockDriverState *new_top_bs = NULL;
- BlkIntermediateStates *intermediate_state, *next;
- int ret = -EIO;
-
- QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete;
- QSIMPLEQ_INIT(&states_to_delete);
-
- if (!top->drv || !base->drv) {
- goto exit;
- }
-
- new_top_bs = bdrv_find_overlay(active, top);
+ BlockDriverState *drop_start, *overlay;
+ int ret = -EINVAL;
- if (new_top_bs == NULL) {
- /* we could not find the image above 'top', this is an error */
+ if (!top->drv || (base && !base->drv)) {
goto exit;
}
-
- /* special case of new_top_bs->backing_hd already pointing to base - nothing
- * to do, no intermediate images */
- if (new_top_bs->backing_hd == base) {
+ if (top == base) {
ret = 0;
- goto exit;
- }
-
- intermediate = top;
-
- /* now we will go down through the list, and add each BDS we find
- * into our deletion queue, until we hit the 'base'
- */
- while (intermediate) {
- intermediate_state = g_malloc0(sizeof(BlkIntermediateStates));
- intermediate_state->bs = intermediate;
- QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry);
-
- if (intermediate->backing_hd == base) {
- base_bs = intermediate->backing_hd;
- break;
+ } else if (top == active) {
+ assert(base);
+ drop_start = active->backing_hd;
+ bdrv_swap(active, base);
+ base->backing_hd = NULL;
+ bdrv_unref(drop_start);
+ ret = 0;
+ } else {
+ /* If there's an overlay, its backing_hd points to top's BDS now,
+ * the top image is dropped but this BDS structure is kept and swapped
+ * with base, this way we keep the pointers valid after dropping top */
+ overlay = bdrv_find_overlay(active, top);
+ if (!overlay) {
+ goto exit;
+ }
+ if (base) {
+ ret = bdrv_change_backing_file(overlay, base->filename,
+ base->drv->format_name);
+ } else {
+ ret = bdrv_change_backing_file(overlay, NULL, NULL);
+ }
+ if (ret) {
+ goto exit;
+ }
+ if (base) {
+ drop_start = top->backing_hd;
+ bdrv_swap(top, base);
+ /* Break the loop formed by bdrv_swap */
+ bdrv_set_backing_hd(base, NULL);
+ } else {
+ bdrv_set_backing_hd(overlay, NULL);
+ drop_start = top;
}
- intermediate = intermediate->backing_hd;
- }
- if (base_bs == NULL) {
- /* something went wrong, we did not end at the base. safely
- * unravel everything, and exit with error */
- goto exit;
- }
-
- /* success - we can delete the intermediate states, and link top->base */
- ret = bdrv_change_backing_file(new_top_bs, base_bs->filename,
- base_bs->drv ? base_bs->drv->format_name : "");
- if (ret) {
- goto exit;
- }
- new_top_bs->backing_hd = base_bs;
-
- QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
- /* so that bdrv_close() does not recursively close the chain */
- intermediate_state->bs->backing_hd = NULL;
- bdrv_unref(intermediate_state->bs);
+ bdrv_unref(drop_start);
}
- ret = 0;
-
exit:
- QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
- g_free(intermediate_state);
- }
return ret;
}
-
static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
size_t size)
{
diff --git a/block/commit.c b/block/commit.c
index d4090cb..4d8cd05 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -142,6 +142,7 @@ wait:
if (!block_job_is_cancelled(&s->common) && sector_num == end) {
/* success */
ret = bdrv_drop_intermediate(active, top, base);
+ base = top;
}
exit_free_buf:
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (7 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 08/11] block: Support dropping active in bdrv_drop_intermediate Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 10/11] qmp: Add command 'blockdev-backup' Fam Zheng
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
This reuses the new bdrv_drop_intermediate.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/stream.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index 46bec7d..9cdcf0e 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -51,32 +51,6 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,
return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, &qiov);
}
-static void close_unused_images(BlockDriverState *top, BlockDriverState *base,
- const char *base_id)
-{
- BlockDriverState *intermediate;
- intermediate = top->backing_hd;
-
- /* Must assign before bdrv_delete() to prevent traversing dangling pointer
- * while we delete backing image instances.
- */
- top->backing_hd = base;
-
- while (intermediate) {
- BlockDriverState *unused;
-
- /* reached base */
- if (intermediate == base) {
- break;
- }
-
- unused = intermediate;
- intermediate = intermediate->backing_hd;
- unused->backing_hd = NULL;
- bdrv_unref(unused);
- }
-}
-
static void coroutine_fn stream_run(void *opaque)
{
StreamBlockJob *s = opaque;
@@ -190,7 +164,7 @@ wait:
}
}
ret = bdrv_change_backing_file(bs, base_id, base_fmt);
- close_unused_images(bs, base, base_id);
+ bdrv_drop_intermediate(bs, bs->backing_hd, base);
}
qemu_vfree(buf);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 10/11] qmp: Add command 'blockdev-backup'
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (8 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 11/11] block: Allow backup on referenced named BlockDriverState Fam Zheng
2014-01-08 6:20 ` [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
Similar to drive-backup, but this command uses a device id as target
instead of creating/opening an image file.
Also add blocker on target bs, since the target is also a named device
now.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/backup.c | 21 +++++++++++++++++++++
blockdev.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 44 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 161 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index 0198514..c8fe1a9 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -339,6 +339,7 @@ static void coroutine_fn backup_run(void *opaque)
hbitmap_free(job->bitmap);
bdrv_iostatus_disable(target);
+ bdrv_op_unblock_all(target, job->common.blocker);
bdrv_unref(target);
block_job_completed(&job->common, ret);
@@ -364,6 +365,24 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ if (!bdrv_is_inserted(bs)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bs->device_name);
+ return;
+ }
+
+ if (!bdrv_is_inserted(target)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, target->device_name);
+ return;
+ }
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+ return;
+ }
+
+ if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
+ return;
+ }
+
len = bdrv_getlength(bs);
if (len < 0) {
error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -377,6 +396,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ bdrv_op_block_all(target, job->common.blocker);
+
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
job->target = target;
diff --git a/blockdev.c b/blockdev.c
index 7f305d8..5627e5d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1872,6 +1872,8 @@ void qmp_drive_backup(const char *device, const char *target,
return;
}
+ /* Although backup_run has this check too, we need to use bs->drv below, so
+ * do an early check redundantly. */
if (!bdrv_is_inserted(bs)) {
error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
return;
@@ -1888,6 +1890,7 @@ void qmp_drive_backup(const char *device, const char *target,
}
}
+ /* Early check to avoid creating target */
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
return;
}
@@ -1946,6 +1949,50 @@ void qmp_drive_backup(const char *device, const char *target,
}
}
+void qmp_blockdev_backup(const char *device, const char *target,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BlockDriverState *target_bs;
+ Error *local_err = NULL;
+
+ if (!has_speed) {
+ speed = 0;
+ }
+ if (!has_on_source_error) {
+ on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_on_target_error) {
+ on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+
+ bs = bdrv_find(device);
+ if (!bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ target_bs = bdrv_find(target);
+ if (!target_bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, target);
+ return;
+ }
+
+ bdrv_ref(target_bs);
+ backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+ if (local_err != NULL) {
+ bdrv_unref(target_bs);
+ error_propagate(errp, local_err);
+ }
+}
+
#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
void qmp_drive_mirror(const char *device, const char *target,
diff --git a/qapi-schema.json b/qapi-schema.json
index c3c939c..2b5b9af 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1819,6 +1819,40 @@
'*on-target-error': 'BlockdevOnError' } }
##
+# @BlockdevBackup
+#
+# @device: the name of the device which should be copied.
+#
+# @target: the name of the backup target device.
+#
+# @sync: what parts of the disk image should be copied to the destination
+# (all the disk, only the sectors allocated in the topmost image, or
+# only new I/O).
+#
+# @speed: #optional the maximum speed, in bytes per second.
+#
+# @on-source-error: #optional the action to take on an error on the source,
+# default 'report'. 'stop' and 'enospc' can only be used
+# if the block device supports io-status (see BlockInfo).
+#
+# @on-target-error: #optional the action to take on an error on the target,
+# default 'report' (no limitations, since this applies to
+# a different block device than @device).
+#
+# Note that @on-source-error and @on-target-error only affect background I/O.
+# If an error occurs during a guest write request, the device's rerror/werror
+# actions will be used.
+#
+# Since: 2.0
+##
+{ 'type': 'BlockdevBackup',
+ 'data': { 'device': 'str', 'target': 'str',
+ 'sync': 'MirrorSyncMode',
+ '*speed': 'int',
+ '*on-source-error': 'BlockdevOnError',
+ '*on-target-error': 'BlockdevOnError' } }
+
+##
# @Abort
#
# This action can be used to test transaction failure.
@@ -2008,6 +2042,21 @@
{ 'command': 'drive-backup', 'data': 'DriveBackup' }
##
+# @blockdev-backup
+#
+# Block device version for drive-backup command. Use existing device as target
+# of backup.
+#
+# For the arguments, see the documentation of BlockdevBackup.
+#
+# Returns: Nothing on success.
+# If @device or @target is not a valid block device, DeviceNotFound.
+#
+# Since 2.0
+##
+{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' }
+
+##
# @drive-mirror
#
# Start mirroring a block device's writes to a new destination.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..32e2b10 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -963,6 +963,50 @@ Example:
"sync": "full",
"target": "backup.img" } }
<- { "return": {} }
+
+EQMP
+
+ {
+ .name = "blockdev-backup",
+ .args_type = "sync:s,device:B,target:B,speed:i?,"
+ "on-source-error:s?,on-target-error:s?",
+ .mhandler.cmd_new = qmp_marshal_input_blockdev_backup,
+ },
+
+SQMP
+blockdev-backup
+------------
+
+The device version of drive-backup: this command takes a existing named device
+as backup target.
+
+Arguments:
+
+- "device": the name of the device which should be copied.
+ (json-string)
+- "target": the target of the new image. If the file exists, or if it is a
+ device, the existing file/device will be used as the new
+ destination. If it does not exist, a new file will be created.
+ (json-string)
+- "sync": what parts of the disk image should be copied to the destination;
+ possibilities include "full" for all the disk, "top" for only the
+ sectors allocated in the topmost image, or "none" to only replicate
+ new I/O (MirrorSyncMode).
+- "speed": the maximum speed, in bytes per second (json-int, optional)
+- "on-source-error": the action to take on an error on the source, default
+ 'report'. 'stop' and 'enospc' can only be used
+ if the block device supports io-status.
+ (BlockdevOnError, optional)
+- "on-target-error": the action to take on an error on the target, default
+ 'report' (no limitations, since this applies to
+ a different block device than device).
+ (BlockdevOnError, optional)
+
+Example:
+-> { "execute": "blockdev-backup", "arguments": { "device": "src-id",
+ "target": "tgt-id" } }
+<- { "return": {} }
+
EQMP
{
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH v10 11/11] block: Allow backup on referenced named BlockDriverState
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (9 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 10/11] qmp: Add command 'blockdev-backup' Fam Zheng
@ 2014-01-08 3:42 ` Fam Zheng
2014-01-08 6:20 ` [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
Drive backup is a read only operation on source bs. We want to allow
this specific case to enable image-fleecing. Note that when
image-fleecing job starts, the job still add its blocker to source bs,
and any other operation on it will be blocked by that.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block.c b/block.c
index b28fb93..1bc00ef 100644
--- a/block.c
+++ b/block.c
@@ -988,6 +988,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
/* Otherwise we won't be able to commit due to check in bdrv_commit */
bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
bs->backing_blocker);
+ bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE,
+ bs->backing_blocker);
pstrcpy(bs->backing_file, sizeof(bs->backing_file),
bs->backing_hd->file->filename);
pstrcpy(bs->backing_format, sizeof(bs->backing_format),
--
1.8.5.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
2014-01-08 3:42 [Qemu-devel] [PATCH v10 00/11] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD Fam Zheng
` (10 preceding siblings ...)
2014-01-08 3:42 ` [Qemu-devel] [PATCH v10 11/11] block: Allow backup on referenced named BlockDriverState Fam Zheng
@ 2014-01-08 6:20 ` Fam Zheng
11 siblings, 0 replies; 13+ messages in thread
From: Fam Zheng @ 2014-01-08 6:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, rjones, armbru, imain, stefanha, pbonzini
On 2014年01月08日 11:42, Fam Zheng wrote:
> This series adds for point-in-time snapshot NBD exporting based on
> blockdev-backup (variant of drive-backup with existing device as target).
>
> We get a thin point-in-time snapshot by COW mechanism of drive-backup, and
> export it through built in NBD server. The steps are as below:
>
> 1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 <source size here>
>
> (Alternatively we can use -o backing_file=RUNNING-VM.img to omit explicitly
> providing the size by ourselves, but it's risky because RUNNING-VM.qcow2 is
> used r/w by guest. Whether or not setting backing file in the image file
> doesn't matter, as we are going to override the backing hd in the next
> step)
>
> 2. (QMP) blockdev-add backing=source-drive file.driver=file file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2
>
> (where source-drive is the running BlockDriverState name for
> RUNNING-VM.img. This patch implements "backing=" option to override
> backing_hd for added drive)
>
> 3. (QMP) blockdev-backup device=source-drive sync=none target=target0
>
> (this is the QMP command introduced by this series, which use a named
> device as target of drive-backup)
>
> 4. (QMP) nbd-server-add device=target0
>
> When image fleecing done:
>
> 1. (QMP) block-job-cancel device=source-drive
>
> 2. (HMP) drive_del target0
>
> 3. (SHELL) rm BACKUP.qcow2
>
> v8 -> v10: Rebased to qemu.git. Address Stefan's comments:
>
> [01/11] block: Add BlockOpType enum
> Change enum definition as internal.
>
> [05/11] block: Add bdrv_set_backing_hd()
> Set bs->backing_file and bs->backing_format.
>
> [06/11] block: Add backing_blocker in BlockDriverState
> Reuse bdrv_set_backing_hd().
>
> [07/11] block: Parse "backing" option to reference existing BDS
> Fix use-after-free.
> Check for "backing=" and "backing.file=" conflict.
> Remove unintended bdrv_swap hunks.
Actually it is not unintended, this breaks qemu-iotests 040. The asserts
have to be removed to allow block commit.
Will respin, sorry for posting three revisions in a row.
Fam
>
> [08/11] block: Support dropping active in bdrv_drop_intermediate
> Fix function comment.
>
> [09/11] stream: Use bdrv_drop_intermediate and drop close_unused_images
>
>
> Fam Zheng (11):
> block: Add BlockOpType enum
> block: Introduce op_blockers to BlockDriverState
> block: Replace in_use with operation blocker
> block: Move op_blocker check from block_job_create to its caller
> block: Add bdrv_set_backing_hd()
> block: Add backing_blocker in BlockDriverState
> block: Parse "backing" option to reference existing BDS
> block: Support dropping active in bdrv_drop_intermediate
> stream: Use bdrv_drop_intermediate and drop close_unused_images
> qmp: Add command 'blockdev-backup'
> block: Allow backup on referenced named BlockDriverState
>
> block-migration.c | 7 +-
> block.c | 306 +++++++++++++++++++++++++++-------------
> block/backup.c | 21 +++
> block/commit.c | 1 +
> block/stream.c | 28 +---
> blockdev.c | 70 +++++++--
> blockjob.c | 14 +-
> hw/block/dataplane/virtio-blk.c | 19 ++-
> include/block/block.h | 29 +++-
> include/block/block_int.h | 9 +-
> include/block/blockjob.h | 3 +
> qapi-schema.json | 49 +++++++
> qmp-commands.hx | 44 ++++++
> 13 files changed, 446 insertions(+), 154 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread