[Qemu-devel] [PATCH] block: Split BLOCK_OP_TYPE_COMMIT to BLOCK_OP_TYPE_COMMIT_{SOURCE, TARGET}

[Fam Zheng]

Like BLOCK_OP_TYPE_BACKUP_SOURCE and BLOCK_OP_TYPE_BACKUP_TARGET,
block-commit involves two asymmetric devices.

This change is not user-visible (yet), because commit only works with
device names.

But once we enable backing reference in blockdev-add, or specifying
node-name in block-commit command, we don't want the user to start two
commit jobs on the same backing chain, which will corrupt things because
of the final bdrv_swap.

Before we have per category blockers, splitting this type is still
better.

Signed-off-by: Fam Zheng 
---
 block.c   | 6 +++---
 blockdev.c| 6 +-
 include/block/block.h | 3 ++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index d06dd51..f47e6a0 100644
--- a/block.c
+++ b/block.c
@@ -1168,7 +1168,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *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,
+bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
 bs->backing_blocker);
 out:
 bdrv_refresh_limits(bs, NULL);
@@ -2251,8 +2251,8 @@ int bdrv_commit(BlockDriverState *bs)
 return -ENOTSUP;
 }
 
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, NULL) ||
-bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, NULL)) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, NULL) ||
+bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET, NULL)) 
{
 return -EBUSY;
 }
 
diff --git a/blockdev.c b/blockdev.c
index e919566..095804a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1956,7 +1956,7 @@ void qmp_block_commit(const char *device,
 return;
 }
 
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
 return;
 }
 
@@ -1985,6 +1985,10 @@ void qmp_block_commit(const char *device,
 return;
 }
 
+if (bdrv_op_is_blocked(base_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) {
+return;
+}
+
 /* Do not allow attempts to commit an image into itself */
 if (top_bs == base_bs) {
 error_setg(errp, "cannot commit an image into itself");
diff --git a/include/block/block.h b/include/block/block.h
index 8f4ad16..3b0d39d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -165,7 +165,8 @@ typedef enum BlockOpType {
 BLOCK_OP_TYPE_BACKUP_SOURCE,
 BLOCK_OP_TYPE_BACKUP_TARGET,
 BLOCK_OP_TYPE_CHANGE,
-BLOCK_OP_TYPE_COMMIT,
+BLOCK_OP_TYPE_COMMIT_SOURCE,
+BLOCK_OP_TYPE_COMMIT_TARGET,
 BLOCK_OP_TYPE_DATAPLANE,
 BLOCK_OP_TYPE_DRIVE_DEL,
 BLOCK_OP_TYPE_EJECT,
-- 
1.9.3

Re: [Qemu-devel] [PATCH] block: Split BLOCK_OP_TYPE_COMMIT to BLOCK_OP_TYPE_COMMIT_{SOURCE, TARGET}

[Stefan Hajnoczi]

On Thu, Sep 11, 2014 at 01:14:00PM +0800, Fam Zheng wrote:
> Like BLOCK_OP_TYPE_BACKUP_SOURCE and BLOCK_OP_TYPE_BACKUP_TARGET,
> block-commit involves two asymmetric devices.
> 
> This change is not user-visible (yet), because commit only works with
> device names.
> 
> But once we enable backing reference in blockdev-add, or specifying
> node-name in block-commit command, we don't want the user to start two
> commit jobs on the same backing chain, which will corrupt things because
> of the final bdrv_swap.
> 
> Before we have per category blockers, splitting this type is still
> better.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block.c   | 6 +++---
>  blockdev.c| 6 +-
>  include/block/block.h | 3 ++-
>  3 files changed, 10 insertions(+), 5 deletions(-)

Sorry for missing this patch, it was not on my radar since it has no
Reveiwed-by.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


pgp1uov5IUcS9.pgp
Description: PGP signature