[Qemu-devel] [PATCH v2] blockdev: loosen restrictions on drive-backup source node

John Snow posted 1 patch 4 years, 11 months ago
Test docker-clang@ubuntu passed
Test asan 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/20190521210053.8864-1-jsnow@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>, Markus Armbruster <armbru@redhat.com>
blockdev.c             | 7 ++++++-
tests/qemu-iotests/056 | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] blockdev: loosen restrictions on drive-backup source node
Posted by John Snow 4 years, 11 months ago
We mandate that the source node must be a root node; but there's no reason
I am aware of that it needs to be restricted to such. In some cases, we need
to make sure that there's a medium present, but in the general case we can
allow the backup job itself to do the graph checking.

This patch helps improve the error message when you try to backup from
the same node more than once, which is reflected in the change to test
056.

For backups with bitmaps, it will also show a better error message that
the bitmap is in use instead of giving you something cryptic like "need
a root node."

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1707303
Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c             | 7 ++++++-
 tests/qemu-iotests/056 | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 79fbac8450..bc339d7818 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3450,11 +3450,16 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
         backup->compress = false;
     }
 
-    bs = qmp_get_root_bs(backup->device, errp);
+    bs = bdrv_lookup_bs(backup->device, backup->device, errp);
     if (!bs) {
         return NULL;
     }
 
+    if (!bs->drv) {
+        error_setg(errp, "Device has no medium");
+        return NULL;
+    }
+
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index 3df323984d..f40fc11a09 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -214,7 +214,7 @@ class BackupTest(iotests.QMPTestCase):
         res = self.vm.qmp('query-block-jobs')
         self.assert_qmp(res, 'return[0]/status', 'concluded')
         # Leave zombie job un-dismissed, observe a failure:
-        res = self.qmp_backup_and_wait(serror='Need a root block node',
+        res = self.qmp_backup_and_wait(serror="Node 'drive0' is busy: block device is in use by block job: backup",
                                        device='drive0', format=iotests.imgfmt,
                                        sync='full', target=self.dest_img,
                                        auto_dismiss=False)
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2] blockdev: loosen restrictions on drive-backup source node
Posted by Max Reitz 4 years, 11 months ago
On 21.05.19 23:00, John Snow wrote:
> We mandate that the source node must be a root node; but there's no reason
> I am aware of that it needs to be restricted to such. In some cases, we need
> to make sure that there's a medium present, but in the general case we can
> allow the backup job itself to do the graph checking.
> 
> This patch helps improve the error message when you try to backup from
> the same node more than once, which is reflected in the change to test
> 056.
> 
> For backups with bitmaps, it will also show a better error message that
> the bitmap is in use instead of giving you something cryptic like "need
> a root node."
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1707303
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockdev.c             | 7 ++++++-
>  tests/qemu-iotests/056 | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Let’s see what breaks.

Max