1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/4] transaction support for bitmap merge
  4. View patch

[Qemu-devel] [PATCH v2 4/4] qapi: add transaction support for x-block-dirty-bitmap-merge

Vladimir Sementsov-Ogievskiy posted 4 patches 7 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 4/4] qapi: add transaction support for x-block-dirty-bitmap-merge
Posted by Vladimir Sementsov-Ogievskiy 7 years, 4 months ago 
New action is like clean action: do the whole thing in .prepare and
undo in .abort. This behavior for bitmap-changing actions is needed
because backup job actions use bitmap in .prepare.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/transaction.json |  2 ++
 blockdev.c            | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/qapi/transaction.json b/qapi/transaction.json
index d7e4274550..5875cdb16c 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -48,6 +48,7 @@
 # - @block-dirty-bitmap-clear: since 2.5
 # - @x-block-dirty-bitmap-enable: since 3.0
 # - @x-block-dirty-bitmap-disable: since 3.0
+# - @x-block-dirty-bitmap-merge: since 3.1
 # - @blockdev-backup: since 2.3
 # - @blockdev-snapshot: since 2.5
 # - @blockdev-snapshot-internal-sync: since 1.7
@@ -63,6 +64,7 @@
        'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
        'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
        'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+       'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
        'blockdev-backup': 'BlockdevBackup',
        'blockdev-snapshot': 'BlockdevSnapshot',
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/blockdev.c b/blockdev.c
index 5348e8ba9b..feebbb9a9a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1940,6 +1940,7 @@ static void blockdev_backup_clean(BlkActionState *common)
 typedef struct BlockDirtyBitmapState {
     BlkActionState common;
     BdrvDirtyBitmap *bitmap;
+    BdrvDirtyBitmap *merge_source;
     BlockDriverState *bs;
     HBitmap *backup;
     bool prepared;
@@ -2112,6 +2113,35 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
     }
 }
 
+static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
+                                             Error **errp)
+{
+    BlockDirtyBitmapMerge *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    if (action_check_completion_mode(common, errp) < 0) {
+        return;
+    }
+
+    action = common->action->u.x_block_dirty_bitmap_merge.data;
+    state->bitmap = block_dirty_bitmap_lookup(action->node,
+                                              action->dst_name,
+                                              &state->bs,
+                                              errp);
+    if (!state->bitmap) {
+        return;
+    }
+
+    state->merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
+    if (!state->merge_source) {
+        return;
+    }
+
+    bdrv_merge_dirty_bitmap(state->bitmap, state->merge_source, &state->backup,
+                            errp);
+}
+
 static void abort_prepare(BlkActionState *common, Error **errp)
 {
     error_setg(errp, "Transaction aborted using Abort action");
@@ -2182,7 +2212,13 @@ static const BlkActionOps actions[] = {
         .instance_size = sizeof(BlockDirtyBitmapState),
         .prepare = block_dirty_bitmap_disable_prepare,
         .abort = block_dirty_bitmap_disable_abort,
-     }
+    },
+    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_merge_prepare,
+        .commit = block_dirty_bitmap_free_backup,
+        .abort = block_dirty_bitmap_restore,
+    }
 };
 
 /**
-- 
2.11.1
Re: [Qemu-devel] [PATCH v2 4/4] qapi: add transaction support for x-block-dirty-bitmap-merge
Posted by John Snow 7 years, 1 month ago 

On 07/06/2018 07:36 AM, Vladimir Sementsov-Ogievskiy wrote:
> New action is like clean action: do the whole thing in .prepare and
> undo in .abort. This behavior for bitmap-changing actions is needed
> because backup job actions use bitmap in .prepare.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/transaction.json |  2 ++
>  blockdev.c            | 38 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index d7e4274550..5875cdb16c 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -48,6 +48,7 @@
>  # - @block-dirty-bitmap-clear: since 2.5
>  # - @x-block-dirty-bitmap-enable: since 3.0
>  # - @x-block-dirty-bitmap-disable: since 3.0
> +# - @x-block-dirty-bitmap-merge: since 3.1
>  # - @blockdev-backup: since 2.3
>  # - @blockdev-snapshot: since 2.5
>  # - @blockdev-snapshot-internal-sync: since 1.7
> @@ -63,6 +64,7 @@
>         'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
>         'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
>         'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
> +       'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
>         'blockdev-backup': 'BlockdevBackup',
>         'blockdev-snapshot': 'BlockdevSnapshot',
>         'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
> diff --git a/blockdev.c b/blockdev.c
> index 5348e8ba9b..feebbb9a9a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1940,6 +1940,7 @@ static void blockdev_backup_clean(BlkActionState *common)
>  typedef struct BlockDirtyBitmapState {
>      BlkActionState common;
>      BdrvDirtyBitmap *bitmap;
> +    BdrvDirtyBitmap *merge_source;

Is this necessary?

>      BlockDriverState *bs;
>      HBitmap *backup;
>      bool prepared;
> @@ -2112,6 +2113,35 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
>      }
>  }
>  
> +static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
> +                                             Error **errp)
> +{
> +    BlockDirtyBitmapMerge *action;
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    if (action_check_completion_mode(common, errp) < 0) {
> +        return;
> +    }
> +
> +    action = common->action->u.x_block_dirty_bitmap_merge.data;
> +    state->bitmap = block_dirty_bitmap_lookup(action->node,
> +                                              action->dst_name,
> +                                              &state->bs,
> +                                              errp);
> +    if (!state->bitmap) {
> +        return;
> +    }
> +
> +    state->merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
> +    if (!state->merge_source) {
> +        return;
> +    }
> +
> +    bdrv_merge_dirty_bitmap(state->bitmap, state->merge_source, &state->backup,
> +                            errp);
> +}
> +
>  static void abort_prepare(BlkActionState *common, Error **errp)
>  {
>      error_setg(errp, "Transaction aborted using Abort action");
> @@ -2182,7 +2212,13 @@ static const BlkActionOps actions[] = {
>          .instance_size = sizeof(BlockDirtyBitmapState),
>          .prepare = block_dirty_bitmap_disable_prepare,
>          .abort = block_dirty_bitmap_disable_abort,
> -     }
> +    },
> +    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
> +        .instance_size = sizeof(BlockDirtyBitmapState),
> +        .prepare = block_dirty_bitmap_merge_prepare,
> +        .commit = block_dirty_bitmap_free_backup,
> +        .abort = block_dirty_bitmap_restore,
> +    }
>  };
>  
>  /**
> 

If the new state is not necessary and you remove it:

Reviewed-by: John Snow <jsnow@redhat.com>
Re: [Qemu-devel] [PATCH v2 4/4] qapi: add transaction support for x-block-dirty-bitmap-merge
Posted by Vladimir Sementsov-Ogievskiy 7 years, 1 month ago 
11.09.2018 22:45, John Snow wrote:
>
> On 07/06/2018 07:36 AM, Vladimir Sementsov-Ogievskiy wrote:
>> New action is like clean action: do the whole thing in .prepare and
>> undo in .abort. This behavior for bitmap-changing actions is needed
>> because backup job actions use bitmap in .prepare.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   qapi/transaction.json |  2 ++
>>   blockdev.c            | 38 +++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/transaction.json b/qapi/transaction.json
>> index d7e4274550..5875cdb16c 100644
>> --- a/qapi/transaction.json
>> +++ b/qapi/transaction.json
>> @@ -48,6 +48,7 @@
>>   # - @block-dirty-bitmap-clear: since 2.5
>>   # - @x-block-dirty-bitmap-enable: since 3.0
>>   # - @x-block-dirty-bitmap-disable: since 3.0
>> +# - @x-block-dirty-bitmap-merge: since 3.1
>>   # - @blockdev-backup: since 2.3
>>   # - @blockdev-snapshot: since 2.5
>>   # - @blockdev-snapshot-internal-sync: since 1.7
>> @@ -63,6 +64,7 @@
>>          'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
>>          'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
>>          'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
>> +       'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
>>          'blockdev-backup': 'BlockdevBackup',
>>          'blockdev-snapshot': 'BlockdevSnapshot',
>>          'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
>> diff --git a/blockdev.c b/blockdev.c
>> index 5348e8ba9b..feebbb9a9a 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -1940,6 +1940,7 @@ static void blockdev_backup_clean(BlkActionState *common)
>>   typedef struct BlockDirtyBitmapState {
>>       BlkActionState common;
>>       BdrvDirtyBitmap *bitmap;
>> +    BdrvDirtyBitmap *merge_source;
> Is this necessary?

looks like it isn't, will drop it.

>
>>       BlockDriverState *bs;
>>       HBitmap *backup;
>>       bool prepared;
>> @@ -2112,6 +2113,35 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
>>       }
>>   }
>>   
>> +static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
>> +                                             Error **errp)
>> +{
>> +    BlockDirtyBitmapMerge *action;
>> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
>> +                                             common, common);
>> +
>> +    if (action_check_completion_mode(common, errp) < 0) {
>> +        return;
>> +    }
>> +
>> +    action = common->action->u.x_block_dirty_bitmap_merge.data;
>> +    state->bitmap = block_dirty_bitmap_lookup(action->node,
>> +                                              action->dst_name,
>> +                                              &state->bs,
>> +                                              errp);
>> +    if (!state->bitmap) {
>> +        return;
>> +    }
>> +
>> +    state->merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
>> +    if (!state->merge_source) {
>> +        return;
>> +    }
>> +
>> +    bdrv_merge_dirty_bitmap(state->bitmap, state->merge_source, &state->backup,
>> +                            errp);
>> +}
>> +
>>   static void abort_prepare(BlkActionState *common, Error **errp)
>>   {
>>       error_setg(errp, "Transaction aborted using Abort action");
>> @@ -2182,7 +2212,13 @@ static const BlkActionOps actions[] = {
>>           .instance_size = sizeof(BlockDirtyBitmapState),
>>           .prepare = block_dirty_bitmap_disable_prepare,
>>           .abort = block_dirty_bitmap_disable_abort,
>> -     }
>> +    },
>> +    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
>> +        .instance_size = sizeof(BlockDirtyBitmapState),
>> +        .prepare = block_dirty_bitmap_merge_prepare,
>> +        .commit = block_dirty_bitmap_free_backup,
>> +        .abort = block_dirty_bitmap_restore,
>> +    }
>>   };
>>   
>>   /**
>>
> If the new state is not necessary and you remove it:
>
> Reviewed-by: John Snow <jsnow@redhat.com>


-- 
Best regards,
Vladimir

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.