[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] backup: Fail early if cannot determine cluster
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH] backup: Fail early if cannot determine cluster size |
Date: |
Wed, 18 May 2016 09:34:48 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, May 18, 2016 at 01:53:27PM +0800, Fam Zheng wrote:
> Otherwise the job is orphaned and block_job_cancel_sync in
> bdrv_close_all() when quiting will hang.
>
> A simple reproducer is running blockdev-backup from null-co:// to
> null-co://.
>
> Cc: address@hidden
> Signed-off-by: Fam Zheng <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
> ---
> block/backup.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 491fd14..3ff48c6 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -505,6 +505,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState
> *target,
> int64_t len;
> BlockDriverInfo bdi;
> int ret;
> + int64_t cluster_size;
>
> assert(bs);
> assert(target);
> @@ -568,23 +569,10 @@ void backup_start(BlockDriverState *bs,
> BlockDriverState *target,
> goto error;
> }
>
> - BackupBlockJob *job = block_job_create(&backup_job_driver, bs, speed,
> - cb, opaque, errp);
> - if (!job) {
> - goto error;
> - }
> -
> - job->on_source_error = on_source_error;
> - job->on_target_error = on_target_error;
> - job->target = target;
> - job->sync_mode = sync_mode;
> - job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
> - sync_bitmap : NULL;
> -
> /* If there is no backing file on the target, we cannot rely on COW if
> our
> * backup cluster size is smaller than the target cluster size. Even for
> * targets with a backing file, try to avoid COW if possible. */
> - ret = bdrv_get_info(job->target, &bdi);
> + ret = bdrv_get_info(target, &bdi);
> if (ret < 0 && !target->backing) {
> error_setg_errno(errp, -ret,
> "Couldn't determine the cluster size of the target image, "
> @@ -594,11 +582,25 @@ void backup_start(BlockDriverState *bs,
> BlockDriverState *target,
> goto error;
> } else if (ret < 0 && target->backing) {
> /* Not fatal; just trudge on ahead. */
> - job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
> + cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
> } else {
> - job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT,
> bdi.cluster_size);
> + cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
> }
>
> + BackupBlockJob *job = block_job_create(&backup_job_driver, bs, speed,
> + cb, opaque, errp);
> + if (!job) {
> + goto error;
> + }
> +
> + job->on_source_error = on_source_error;
> + job->on_target_error = on_target_error;
> + job->target = target;
> + job->sync_mode = sync_mode;
> + job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
> + sync_bitmap : NULL;
> + job->cluster_size = cluster_size;
> +
> bdrv_op_block_all(target, job->common.blocker);
> job->common.len = len;
> job->common.co = qemu_coroutine_create(backup_run);
> --
> 2.8.2
>