diff mbox

[v5,09/13] blockdev: Block device IO during external snapshot transaction

Message ID 1432102576-6637-10-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng May 20, 2015, 6:16 a.m. UTC
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Max Reitz May 23, 2015, 4:43 p.m. UTC | #1
On 20.05.2015 08:16, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   blockdev.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)

Can this be pulled into external_snapshot_commit()? Or might that pose 
problems with other operations in the same transaction?

> diff --git a/blockdev.c b/blockdev.c
> index 7f763d9..923fc90 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1404,6 +1404,7 @@ typedef struct ExternalSnapshotState {
>       BlockDriverState *old_bs;
>       BlockDriverState *new_bs;
>       AioContext *aio_context;
> +    Error *blocker;
>   } ExternalSnapshotState;
>   
>   static void external_snapshot_prepare(BlkTransactionState *common,
> @@ -1525,6 +1526,9 @@ static void external_snapshot_prepare(BlkTransactionState *common,
>       if (ret != 0) {
>           error_propagate(errp, local_err);
>       }
> +
> +    error_setg(&state->blocker, "snapshot in progress");
> +    bdrv_op_block(state->old_bs, BLOCK_OP_TYPE_DEVICE_IO, state->blocker);
>   }
>   
>   static void external_snapshot_commit(BlkTransactionState *common)
> @@ -1541,8 +1545,6 @@ static void external_snapshot_commit(BlkTransactionState *common)
>        * don't want to abort all of them if one of them fails the reopen */
>       bdrv_reopen(state->new_bs, state->new_bs->open_flags & ~BDRV_O_RDWR,
>                   NULL);
> -
> -    aio_context_release(state->aio_context);
>   }
>   
>   static void external_snapshot_abort(BlkTransactionState *common)
> @@ -1552,6 +1554,17 @@ static void external_snapshot_abort(BlkTransactionState *common)
>       if (state->new_bs) {
>           bdrv_unref(state->new_bs);
>       }
> +}
> +
> +static void external_snapshot_clean(BlkTransactionState *common)
> +{
> +    ExternalSnapshotState *state =
> +                             DO_UPCAST(ExternalSnapshotState, common, common);
> +
> +    if (state->old_bs) {

I think it would be better to make this state->blocker.

> +        bdrv_op_unblock(state->old_bs, BLOCK_OP_TYPE_DEVICE_IO, state->blocker);
> +        error_free(state->blocker);
> +    }
>       if (state->aio_context) {
>           aio_context_release(state->aio_context);
>       }
> @@ -1716,6 +1729,7 @@ static const BdrvActionOps actions[] = {
>           .prepare  = external_snapshot_prepare,
>           .commit   = external_snapshot_commit,
>           .abort = external_snapshot_abort,
> +        .clean = external_snapshot_clean,

Oh. Er, that's nice...

Max

>       },
>       [TRANSACTION_ACTION_KIND_DRIVE_BACKUP] = {
>           .instance_size = sizeof(DriveBackupState),
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 7f763d9..923fc90 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1404,6 +1404,7 @@  typedef struct ExternalSnapshotState {
     BlockDriverState *old_bs;
     BlockDriverState *new_bs;
     AioContext *aio_context;
+    Error *blocker;
 } ExternalSnapshotState;
 
 static void external_snapshot_prepare(BlkTransactionState *common,
@@ -1525,6 +1526,9 @@  static void external_snapshot_prepare(BlkTransactionState *common,
     if (ret != 0) {
         error_propagate(errp, local_err);
     }
+
+    error_setg(&state->blocker, "snapshot in progress");
+    bdrv_op_block(state->old_bs, BLOCK_OP_TYPE_DEVICE_IO, state->blocker);
 }
 
 static void external_snapshot_commit(BlkTransactionState *common)
@@ -1541,8 +1545,6 @@  static void external_snapshot_commit(BlkTransactionState *common)
      * don't want to abort all of them if one of them fails the reopen */
     bdrv_reopen(state->new_bs, state->new_bs->open_flags & ~BDRV_O_RDWR,
                 NULL);
-
-    aio_context_release(state->aio_context);
 }
 
 static void external_snapshot_abort(BlkTransactionState *common)
@@ -1552,6 +1554,17 @@  static void external_snapshot_abort(BlkTransactionState *common)
     if (state->new_bs) {
         bdrv_unref(state->new_bs);
     }
+}
+
+static void external_snapshot_clean(BlkTransactionState *common)
+{
+    ExternalSnapshotState *state =
+                             DO_UPCAST(ExternalSnapshotState, common, common);
+
+    if (state->old_bs) {
+        bdrv_op_unblock(state->old_bs, BLOCK_OP_TYPE_DEVICE_IO, state->blocker);
+        error_free(state->blocker);
+    }
     if (state->aio_context) {
         aio_context_release(state->aio_context);
     }
@@ -1716,6 +1729,7 @@  static const BdrvActionOps actions[] = {
         .prepare  = external_snapshot_prepare,
         .commit   = external_snapshot_commit,
         .abort = external_snapshot_abort,
+        .clean = external_snapshot_clean,
     },
     [TRANSACTION_ACTION_KIND_DRIVE_BACKUP] = {
         .instance_size = sizeof(DriveBackupState),