Am 14.05.2020 um 05:49 hat John Snow geschrieben:
> This job copies the allocation map into a bitmap. It's a job because
> there's no guarantee that allocation interrogation will be quick (or
> won't hang), so it cannot be retrofit into block-dirty-bitmap-merge.
>
> It was designed with different possible population patterns in mind,
> but only top layer allocation was implemented for now.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> qapi/block-core.json | 48 +++++++++
> qapi/job.json | 2 +-
> include/block/block_int.h | 21 ++++
> block/bitmap-alloc.c | 207 ++++++++++++++++++++++++++++++++++++++
bitmap-populate.c to be more consistent with the actual job name?
> blockjob.c | 3 +-
> block/Makefile.objs | 1 +
> 6 files changed, 280 insertions(+), 2 deletions(-)
> create mode 100644 block/bitmap-alloc.c
[...]
> +BlockJob *bitpop_job_create(
> + const char *job_id,
> + BlockDriverState *bs,
> + BdrvDirtyBitmap *target_bitmap,
> + BitmapPattern pattern,
> + BlockdevOnError on_error,
> + int creation_flags,
> + BlockCompletionFunc *cb,
> + void *opaque,
> + JobTxn *txn,
> + Error **errp)
> +{
> + int64_t len;
> + BitpopBlockJob *job = NULL;
> + int64_t cluster_size;
> + BdrvDirtyBitmap *new_bitmap = NULL;
> +
> + assert(bs);
> + assert(target_bitmap);
> +
> + if (!bdrv_is_inserted(bs)) {
> + error_setg(errp, "Device is not inserted: %s",
> + bdrv_get_device_name(bs));
> + return NULL;
> + }
> +
> + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
> + return NULL;
> + }
What does this protect? And why does BACKUP_SOURCE describe acccurately
what this job does?
> + if (bdrv_dirty_bitmap_check(target_bitmap, BDRV_BITMAP_DEFAULT, errp)) {
> + return NULL;
> + }
> +
> + if (pattern != BITMAP_PATTERN_ALLOCATION_TOP) {
> + error_setg(errp, "Unrecognized bitmap pattern");
> + return NULL;
> + }
> +
> + len = bdrv_getlength(bs);
> + if (len < 0) {
> + error_setg_errno(errp, -len, "unable to get length for '%s'",
> + bdrv_get_device_name(bs));
This operates on the node level, so bdrv_get_device_or_node_name() is
necessary to avoid empty strings in the message.
> + return NULL;
> + }
> +
> + /* NB: new bitmap is anonymous and enabled */
> + cluster_size = bdrv_dirty_bitmap_granularity(target_bitmap);
> + new_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
> + if (!new_bitmap) {
> + return NULL;
> + }
> +
> + /* Take ownership; we reserve the right to write into this on-commit. */
> + bdrv_dirty_bitmap_set_busy(target_bitmap, true);
> +
> + job = block_job_create(job_id, &bitpop_job_driver, txn, bs,
> + BLK_PERM_CONSISTENT_READ,
I don't think we actually rely on CONSISTENT_READ, but then, using the
job on inconsistent nodes probably makes little sense and we can always
relax the restriction later if necessary.
> + BLK_PERM_ALL & ~BLK_PERM_RESIZE,
> + 0, creation_flags,
> + cb, opaque, errp);
> + if (!job) {
> + bdrv_dirty_bitmap_set_busy(target_bitmap, false);
> + bdrv_release_dirty_bitmap(new_bitmap);
> + return NULL;
> + }
> +
> + job->bs = bs;
> + job->on_error = on_error;
> + job->target_bitmap = target_bitmap;
> + job->new_bitmap = new_bitmap;
> + job->len = len;
> + job_progress_set_remaining(&job->common.job, job->len);
> +
> + return &job->common;
> +}
Kevin
