[Qemu-devel] [PATCH v2] blockdev: fix missed target unref for drive-backup

John Snow posted 1 patch 4 years, 11 months ago
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190513150638.14622-1-jsnow@redhat.com
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>
blockdev.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
[Qemu-devel] [PATCH v2] blockdev: fix missed target unref for drive-backup
Posted by John Snow 4 years, 11 months ago
If the bitmap can't be used for whatever reason, we skip putting down
the reference. Fix that.

In practice, this means that if you attempt to gracefully exit QEMU
after a backup command being rejected, bdrv_close_all will fail and
tell you some unpleasant things via assert().

Reported-by: aihua liang <aliang@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1703916
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 79fbac8450..e856ca4be9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3525,8 +3525,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
     if (set_backing_hd) {
         bdrv_set_backing_hd(target_bs, source, &local_err);
         if (local_err) {
-            bdrv_unref(target_bs);
-            goto out;
+            goto unref;
         }
     }
 
@@ -3534,11 +3533,10 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
         bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
         if (!bmap) {
             error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
-            bdrv_unref(target_bs);
-            goto out;
+            goto unref;
         }
         if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_DEFAULT, errp)) {
-            goto out;
+            goto unref;
         }
     }
     if (!backup->auto_finalize) {
@@ -3552,12 +3550,13 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
                             backup->sync, bmap, backup->compress,
                             backup->on_source_error, backup->on_target_error,
                             job_flags, NULL, NULL, txn, &local_err);
-    bdrv_unref(target_bs);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
-        goto out;
+        goto unref;
     }
 
+unref:
+    bdrv_unref(target_bs);
 out:
     aio_context_release(aio_context);
     return job;
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2] blockdev: fix missed target unref for drive-backup
Posted by John Snow 4 years, 11 months ago

On 5/13/19 11:06 AM, John Snow wrote:
> If the bitmap can't be used for whatever reason, we skip putting down
> the reference. Fix that.
> 
> In practice, this means that if you attempt to gracefully exit QEMU
> after a backup command being rejected, bdrv_close_all will fail and
> tell you some unpleasant things via assert().
> 
> Reported-by: aihua liang <aliang@redhat.com>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1703916
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

(I assume this can go through either Kevin or Max's tree?)

Re: [Qemu-devel] [PATCH v2] blockdev: fix missed target unref for drive-backup
Posted by Kevin Wolf 4 years, 11 months ago
Am 13.05.2019 um 17:06 hat John Snow geschrieben:
> If the bitmap can't be used for whatever reason, we skip putting down
> the reference. Fix that.
> 
> In practice, this means that if you attempt to gracefully exit QEMU
> after a backup command being rejected, bdrv_close_all will fail and
> tell you some unpleasant things via assert().
> 
> Reported-by: aihua liang <aliang@redhat.com>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1703916
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Thanks, applied to the block branch.

Kevin