1. Patchew
  2. QEMU
  3. [PATCH 0/6] block: add block-dirty-bitmap-populate job
  4. View patch

[PATCH 2/6] qmp: expose block-dirty-bitmap-populate

John Snow posted 6 patches 5 years, 8 months ago
Maintainers: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Eric Blake <eblake@redhat.com>, Cleber Rosa <crosa@redhat.com>, Max Reitz <mreitz@redhat.com>
There is a newer version of this series
[PATCH 2/6] qmp: expose block-dirty-bitmap-populate
Posted by John Snow 5 years, 8 months ago 
This is a new job-creating command.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qapi/block-core.json  | 18 ++++++++++
 qapi/transaction.json |  2 ++
 blockdev.c            | 78 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index df1797681a..a8be1fb36b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2293,6 +2293,24 @@
             '*auto-finalize': 'bool',
             '*auto-dismiss': 'bool' } }
 
+##
+# @block-dirty-bitmap-populate:
+#
+# Creates a new job that writes a pattern into a dirty bitmap.
+#
+# Since: 5.0
+#
+# Example:
+#
+# -> { "execute": "block-dirty-bitmap-populate",
+#      "arguments": { "node": "drive0", "target": "bitmap0",
+#                     "job-id": "job0", "pattern": "allocate-top" } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'block-dirty-bitmap-populate', 'boxed': true,
+  'data': 'BlockDirtyBitmapPopulate' }
+
 ##
 # @BlockDirtyBitmapSha256:
 #
diff --git a/qapi/transaction.json b/qapi/transaction.json
index 04301f1be7..28521d5c7f 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -50,6 +50,7 @@
 # - @block-dirty-bitmap-enable: since 4.0
 # - @block-dirty-bitmap-disable: since 4.0
 # - @block-dirty-bitmap-merge: since 4.0
+# - @block-dirty-bitmap-populate: since 5.0
 # - @blockdev-backup: since 2.3
 # - @blockdev-snapshot: since 2.5
 # - @blockdev-snapshot-internal-sync: since 1.7
@@ -67,6 +68,7 @@
        'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
        'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
        'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
+       'block-dirty-bitmap-populate': 'BlockDirtyBitmapPopulate',
        'blockdev-backup': 'BlockdevBackup',
        'blockdev-snapshot': 'BlockdevSnapshot',
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/blockdev.c b/blockdev.c
index 011dcfec27..33c0e35399 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2314,6 +2314,67 @@ static void block_dirty_bitmap_remove_commit(BlkActionState *common)
     bdrv_release_dirty_bitmap(state->bitmap);
 }
 
+static void block_dirty_bitmap_populate_prepare(BlkActionState *common, Error **errp)
+{
+    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    BlockDirtyBitmapPopulate *bitpop;
+    BlockDriverState *bs;
+    AioContext *aio_context;
+    BdrvDirtyBitmap *bmap = NULL;
+    int job_flags = JOB_DEFAULT;
+
+    assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE);
+    bitpop = common->action->u.block_dirty_bitmap_populate.data;
+
+    bs = bdrv_lookup_bs(bitpop->node, bitpop->node, errp);
+    if (!bs) {
+        return;
+    }
+
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+    state->bs = bs;
+
+    bmap = bdrv_find_dirty_bitmap(bs, bitpop->name);
+    if (!bmap) {
+        error_setg(errp, "Bitmap '%s' could not be found", bitpop->name);
+        return;
+    }
+
+    /* Paired with .clean() */
+    bdrv_drained_begin(state->bs);
+
+    if (!bitpop->has_on_error) {
+        bitpop->on_error = BLOCKDEV_ON_ERROR_REPORT;
+    }
+    if (!bitpop->has_auto_finalize) {
+        bitpop->auto_finalize = true;
+    }
+    if (!bitpop->has_auto_dismiss) {
+        bitpop->auto_dismiss = true;
+    }
+
+    if (!bitpop->auto_finalize) {
+        job_flags |= JOB_MANUAL_FINALIZE;
+    }
+    if (!bitpop->auto_dismiss) {
+        job_flags |= JOB_MANUAL_DISMISS;
+    }
+
+    state->job = bitpop_job_create(
+        bitpop->job_id,
+        bs,
+        bmap,
+        bitpop->pattern,
+        bitpop->on_error,
+        job_flags,
+        NULL, NULL,
+        common->block_job_txn,
+        errp);
+
+    aio_context_release(aio_context);
+}
+
 static void abort_prepare(BlkActionState *common, Error **errp)
 {
     error_setg(errp, "Transaction aborted using Abort action");
@@ -2397,6 +2458,13 @@ static const BlkActionOps actions[] = {
         .commit = block_dirty_bitmap_remove_commit,
         .abort = block_dirty_bitmap_remove_abort,
     },
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE] = {
+        .instance_size = sizeof(BlockdevBackupState),
+        .prepare = block_dirty_bitmap_populate_prepare,
+        .commit = blockdev_backup_commit,
+        .abort = blockdev_backup_abort,
+        .clean = blockdev_backup_clean,
+    },
     /* Where are transactions for MIRROR, COMMIT and STREAM?
      * Although these blockjobs use transaction callbacks like the backup job,
      * these jobs do not necessarily adhere to transaction semantics.
@@ -3225,6 +3293,16 @@ void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
     do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
 
+void qmp_block_dirty_bitmap_populate(BlockDirtyBitmapPopulate *bitpop,
+                                     Error **errp)
+{
+    TransactionAction action = {
+        .type = TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE,
+        .u.block_dirty_bitmap_populate.data = bitpop,
+    };
+    blockdev_do_action(&action, errp);
+}
+
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
                                                               const char *name,
                                                               Error **errp)
-- 
2.21.1
Re: [PATCH 2/6] qmp: expose block-dirty-bitmap-populate
Posted by Vladimir Sementsov-Ogievskiy 5 years, 8 months ago 
25.02.2020 3:56, John Snow wrote:
> This is a new job-creating command.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   qapi/block-core.json  | 18 ++++++++++
>   qapi/transaction.json |  2 ++
>   blockdev.c            | 78 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 98 insertions(+)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index df1797681a..a8be1fb36b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2293,6 +2293,24 @@
>               '*auto-finalize': 'bool',
>               '*auto-dismiss': 'bool' } }
>   
> +##
> +# @block-dirty-bitmap-populate:
> +#
> +# Creates a new job that writes a pattern into a dirty bitmap.
> +#
> +# Since: 5.0
> +#
> +# Example:
> +#
> +# -> { "execute": "block-dirty-bitmap-populate",
> +#      "arguments": { "node": "drive0", "target": "bitmap0",
> +#                     "job-id": "job0", "pattern": "allocate-top" } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'block-dirty-bitmap-populate', 'boxed': true,
> +  'data': 'BlockDirtyBitmapPopulate' }
> +
>   ##
>   # @BlockDirtyBitmapSha256:
>   #
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index 04301f1be7..28521d5c7f 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -50,6 +50,7 @@
>   # - @block-dirty-bitmap-enable: since 4.0
>   # - @block-dirty-bitmap-disable: since 4.0
>   # - @block-dirty-bitmap-merge: since 4.0
> +# - @block-dirty-bitmap-populate: since 5.0
>   # - @blockdev-backup: since 2.3
>   # - @blockdev-snapshot: since 2.5
>   # - @blockdev-snapshot-internal-sync: since 1.7
> @@ -67,6 +68,7 @@
>          'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
>          'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
>          'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
> +       'block-dirty-bitmap-populate': 'BlockDirtyBitmapPopulate',
>          'blockdev-backup': 'BlockdevBackup',
>          'blockdev-snapshot': 'BlockdevSnapshot',
>          'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
> diff --git a/blockdev.c b/blockdev.c
> index 011dcfec27..33c0e35399 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2314,6 +2314,67 @@ static void block_dirty_bitmap_remove_commit(BlkActionState *common)
>       bdrv_release_dirty_bitmap(state->bitmap);
>   }
>   
> +static void block_dirty_bitmap_populate_prepare(BlkActionState *common, Error **errp)

over80 line (not the only)

> +{
> +    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

At first glance using *Backup* looks like a mistake. May be rename it, or at least add a comment.

> +    BlockDirtyBitmapPopulate *bitpop;
> +    BlockDriverState *bs;
> +    AioContext *aio_context;
> +    BdrvDirtyBitmap *bmap = NULL;
> +    int job_flags = JOB_DEFAULT;
> +
> +    assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE);
> +    bitpop = common->action->u.block_dirty_bitmap_populate.data;
> +
> +    bs = bdrv_lookup_bs(bitpop->node, bitpop->node, errp);
> +    if (!bs) {
> +        return;
> +    }
> +
> +    aio_context = bdrv_get_aio_context(bs);
> +    aio_context_acquire(aio_context);
> +    state->bs = bs;
> +
> +    bmap = bdrv_find_dirty_bitmap(bs, bitpop->name);

Could we use block_dirty_bitmap_lookup ?

> +    if (!bmap) {
> +        error_setg(errp, "Bitmap '%s' could not be found", bitpop->name);
> +        return;

aio context lock leaked

> +    }
> +
> +    /* Paired with .clean() */
> +    bdrv_drained_begin(state->bs);
> +
> +    if (!bitpop->has_on_error) {
> +        bitpop->on_error = BLOCKDEV_ON_ERROR_REPORT;
> +    }
> +    if (!bitpop->has_auto_finalize) {
> +        bitpop->auto_finalize = true;
> +    }
> +    if (!bitpop->has_auto_dismiss) {
> +        bitpop->auto_dismiss = true;
> +    }
> +
> +    if (!bitpop->auto_finalize) {
> +        job_flags |= JOB_MANUAL_FINALIZE;
> +    }
> +    if (!bitpop->auto_dismiss) {
> +        job_flags |= JOB_MANUAL_DISMISS;
> +    }
> +
> +    state->job = bitpop_job_create(
> +        bitpop->job_id,
> +        bs,
> +        bmap,
> +        bitpop->pattern,
> +        bitpop->on_error,
> +        job_flags,
> +        NULL, NULL,
> +        common->block_job_txn,
> +        errp);
> +
> +    aio_context_release(aio_context);
> +}
> +
>   static void abort_prepare(BlkActionState *common, Error **errp)
>   {
>       error_setg(errp, "Transaction aborted using Abort action");
> @@ -2397,6 +2458,13 @@ static const BlkActionOps actions[] = {
>           .commit = block_dirty_bitmap_remove_commit,
>           .abort = block_dirty_bitmap_remove_abort,
>       },
> +    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE] = {
> +        .instance_size = sizeof(BlockdevBackupState),
> +        .prepare = block_dirty_bitmap_populate_prepare,
> +        .commit = blockdev_backup_commit,
> +        .abort = blockdev_backup_abort,
> +        .clean = blockdev_backup_clean,
> +    },
>       /* Where are transactions for MIRROR, COMMIT and STREAM?
>        * Although these blockjobs use transaction callbacks like the backup job,
>        * these jobs do not necessarily adhere to transaction semantics.
> @@ -3225,6 +3293,16 @@ void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
>       do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
>   }
>   
> +void qmp_block_dirty_bitmap_populate(BlockDirtyBitmapPopulate *bitpop,
> +                                     Error **errp)
> +{
> +    TransactionAction action = {
> +        .type = TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE,
> +        .u.block_dirty_bitmap_populate.data = bitpop,
> +    };
> +    blockdev_do_action(&action, errp);
> +}
> +
>   BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
>                                                                 const char *name,
>                                                                 Error **errp)
> 


-- 
Best regards,
Vladimir
Re: [PATCH 2/6] qmp: expose block-dirty-bitmap-populate
Posted by John Snow 5 years, 8 months ago 

On 2/27/20 5:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> 25.02.2020 3:56, John Snow wrote:
>> This is a new job-creating command.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   qapi/block-core.json  | 18 ++++++++++
>>   qapi/transaction.json |  2 ++
>>   blockdev.c            | 78 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 98 insertions(+)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index df1797681a..a8be1fb36b 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2293,6 +2293,24 @@
>>               '*auto-finalize': 'bool',
>>               '*auto-dismiss': 'bool' } }
>>   +##
>> +# @block-dirty-bitmap-populate:
>> +#
>> +# Creates a new job that writes a pattern into a dirty bitmap.
>> +#
>> +# Since: 5.0
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "block-dirty-bitmap-populate",
>> +#      "arguments": { "node": "drive0", "target": "bitmap0",
>> +#                     "job-id": "job0", "pattern": "allocate-top" } }
>> +# <- { "return": {} }
>> +#
>> +##
>> +{ 'command': 'block-dirty-bitmap-populate', 'boxed': true,
>> +  'data': 'BlockDirtyBitmapPopulate' }
>> +
>>   ##
>>   # @BlockDirtyBitmapSha256:
>>   #
>> diff --git a/qapi/transaction.json b/qapi/transaction.json
>> index 04301f1be7..28521d5c7f 100644
>> --- a/qapi/transaction.json
>> +++ b/qapi/transaction.json
>> @@ -50,6 +50,7 @@
>>   # - @block-dirty-bitmap-enable: since 4.0
>>   # - @block-dirty-bitmap-disable: since 4.0
>>   # - @block-dirty-bitmap-merge: since 4.0
>> +# - @block-dirty-bitmap-populate: since 5.0
>>   # - @blockdev-backup: since 2.3
>>   # - @blockdev-snapshot: since 2.5
>>   # - @blockdev-snapshot-internal-sync: since 1.7
>> @@ -67,6 +68,7 @@
>>          'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
>>          'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
>>          'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
>> +       'block-dirty-bitmap-populate': 'BlockDirtyBitmapPopulate',
>>          'blockdev-backup': 'BlockdevBackup',
>>          'blockdev-snapshot': 'BlockdevSnapshot',
>>          'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
>> diff --git a/blockdev.c b/blockdev.c
>> index 011dcfec27..33c0e35399 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -2314,6 +2314,67 @@ static void
>> block_dirty_bitmap_remove_commit(BlkActionState *common)
>>       bdrv_release_dirty_bitmap(state->bitmap);
>>   }
>>   +static void block_dirty_bitmap_populate_prepare(BlkActionState
>> *common, Error **errp)
> 
> over80 line (not the only)
> 

OK, will fix all instances.

>> +{
>> +    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState,
>> common, common);
> 
> At first glance using *Backup* looks like a mistake. May be rename it,
> or at least add a comment.
> 

You're right, it's tricky. A rename would be helpful.

>> +    BlockDirtyBitmapPopulate *bitpop;
>> +    BlockDriverState *bs;
>> +    AioContext *aio_context;
>> +    BdrvDirtyBitmap *bmap = NULL;
>> +    int job_flags = JOB_DEFAULT;
>> +
>> +    assert(common->action->type ==
>> TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE);
>> +    bitpop = common->action->u.block_dirty_bitmap_populate.data;
>> +
>> +    bs = bdrv_lookup_bs(bitpop->node, bitpop->node, errp);
>> +    if (!bs) {
>> +        return;
>> +    }
>> +
>> +    aio_context = bdrv_get_aio_context(bs);
>> +    aio_context_acquire(aio_context);
>> +    state->bs = bs;
>> +
>> +    bmap = bdrv_find_dirty_bitmap(bs, bitpop->name);
> 
> Could we use block_dirty_bitmap_lookup ?
> 

Yes.

>> +    if (!bmap) {
>> +        error_setg(errp, "Bitmap '%s' could not be found",
>> bitpop->name);
>> +        return;
> 
> aio context lock leaked
> 

Good spot.

>> +    }
>> +
>> +    /* Paired with .clean() */
>> +    bdrv_drained_begin(state->bs);
>> +
>> +    if (!bitpop->has_on_error) {
>> +        bitpop->on_error = BLOCKDEV_ON_ERROR_REPORT;
>> +    }
>> +    if (!bitpop->has_auto_finalize) {
>> +        bitpop->auto_finalize = true;
>> +    }
>> +    if (!bitpop->has_auto_dismiss) {
>> +        bitpop->auto_dismiss = true;
>> +    }
>> +
>> +    if (!bitpop->auto_finalize) {
>> +        job_flags |= JOB_MANUAL_FINALIZE;
>> +    }
>> +    if (!bitpop->auto_dismiss) {
>> +        job_flags |= JOB_MANUAL_DISMISS;
>> +    }
>> +
>> +    state->job = bitpop_job_create(
>> +        bitpop->job_id,
>> +        bs,
>> +        bmap,
>> +        bitpop->pattern,
>> +        bitpop->on_error,
>> +        job_flags,
>> +        NULL, NULL,
>> +        common->block_job_txn,
>> +        errp);
>> +
>> +    aio_context_release(aio_context);
>> +}
>> +
>>   static void abort_prepare(BlkActionState *common, Error **errp)
>>   {
>>       error_setg(errp, "Transaction aborted using Abort action");
>> @@ -2397,6 +2458,13 @@ static const BlkActionOps actions[] = {
>>           .commit = block_dirty_bitmap_remove_commit,
>>           .abort = block_dirty_bitmap_remove_abort,
>>       },
>> +    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE] = {
>> +        .instance_size = sizeof(BlockdevBackupState),
>> +        .prepare = block_dirty_bitmap_populate_prepare,
>> +        .commit = blockdev_backup_commit,
>> +        .abort = blockdev_backup_abort,
>> +        .clean = blockdev_backup_clean,
>> +    },
>>       /* Where are transactions for MIRROR, COMMIT and STREAM?
>>        * Although these blockjobs use transaction callbacks like the
>> backup job,
>>        * these jobs do not necessarily adhere to transaction semantics.
>> @@ -3225,6 +3293,16 @@ void qmp_block_dirty_bitmap_merge(const char
>> *node, const char *target,
>>       do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
>>   }
>>   +void qmp_block_dirty_bitmap_populate(BlockDirtyBitmapPopulate *bitpop,
>> +                                     Error **errp)
>> +{
>> +    TransactionAction action = {
>> +        .type = TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_POPULATE,
>> +        .u.block_dirty_bitmap_populate.data = bitpop,
>> +    };
>> +    blockdev_do_action(&action, errp);
>> +}
>> +
>>   BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const
>> char *node,
>>                                                                 const
>> char *name,
>>                                                                 Error
>> **errp)
>>
> 
> 

-- 
—js

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.