qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: qemu-devel@nongnu.org
Cc: Jeff Cody <jcody@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org, jsnow@redhat.com, qemu-stable@nongnu.org
Subject: [Qemu-devel] [PATCH] backup: Fail early if cannot determine cluster size
Date: Wed, 18 May 2016 13:53:27 +0800	[thread overview]
Message-ID: <1463550807-2750-1-git-send-email-famz@redhat.com> (raw)

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: qemu-stable@nongnu.org
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 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

             reply	other threads:[~2016-05-18  5:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-18  5:53 Fam Zheng [this message]
2016-05-18 13:34 ` [Qemu-devel] [PATCH] backup: Fail early if cannot determine cluster size Jeff Cody
2016-05-18 14:00 ` John Snow
2016-05-18 14:25 ` Eric Blake
2016-05-19  0:54   ` Fam Zheng
2016-06-14 14:49 ` Max Reitz
2016-06-15  1:53   ` Fam Zheng
2016-06-15  2:05   ` Fam Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1463550807-2750-1-git-send-email-famz@redhat.com \
    --to=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).