[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/6] qmp: expose block-dirty-bitmap-populate
From: |
John Snow |
Subject: |
Re: [PATCH 2/6] qmp: expose block-dirty-bitmap-populate |
Date: |
Tue, 3 Mar 2020 16:48:36 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
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 <address@hidden>
>> ---
>> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 2/6] qmp: expose block-dirty-bitmap-populate,
John Snow <=