yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
ddf19c
From 9ba321e18a357c1a3a238ceee301bbb174f96eee Mon Sep 17 00:00:00 2001
ddf19c
From: Kevin Wolf <kwolf@redhat.com>
ddf19c
Date: Fri, 13 Mar 2020 12:34:34 +0000
ddf19c
Subject: [PATCH 14/20] block: Relax restrictions for blockdev-snapshot
ddf19c
ddf19c
RH-Author: Kevin Wolf <kwolf@redhat.com>
ddf19c
Message-id: <20200313123439.10548-9-kwolf@redhat.com>
ddf19c
Patchwork-id: 94285
ddf19c
O-Subject: [RHEL-AV-8.2.0 qemu-kvm PATCH v2 08/13] block: Relax restrictions for blockdev-snapshot
ddf19c
Bugzilla: 1790482 1805143
ddf19c
RH-Acked-by: John Snow <jsnow@redhat.com>
ddf19c
RH-Acked-by: Daniel P. Berrange <berrange@redhat.com>
ddf19c
RH-Acked-by: Peter Krempa <pkrempa@redhat.com>
ddf19c
ddf19c
blockdev-snapshot returned an error if the overlay was already in use,
ddf19c
which it defined as having any BlockBackend parent. This is in fact both
ddf19c
too strict (some parents can tolerate the change of visible data caused
ddf19c
by attaching a backing file) and too loose (some non-BlockBackend
ddf19c
parents may not be happy with it).
ddf19c
ddf19c
One important use case that is prevented by the too strict check is live
ddf19c
storage migration with blockdev-mirror. Here, the target node is
ddf19c
usually opened without a backing file so that the active layer is
ddf19c
mirrored while its backing chain can be copied in the background.
ddf19c
ddf19c
The backing chain should be attached to the mirror target node when
ddf19c
finalising the job, just before switching the users of the source node
ddf19c
to the new copy (at which point the mirror job still has a reference to
ddf19c
the node). drive-mirror did this automatically, but with blockdev-mirror
ddf19c
this is the job of the QMP client, so it needs a way to do this.
ddf19c
ddf19c
blockdev-snapshot is the obvious way, so this patch makes it work in
ddf19c
this scenario. The new condition is that no parent uses CONSISTENT_READ
ddf19c
permissions. This will ensure that the operation will still be blocked
ddf19c
when the node is attached to the guest device, so blockdev-snapshot
ddf19c
remains safe.
ddf19c
ddf19c
(For the sake of completeness, x-blockdev-reopen can be used to achieve
ddf19c
the same, however it is a big hammer, performs the graph change
ddf19c
completely unchecked and is still experimental. So even with the option
ddf19c
of using x-blockdev-reopen, there are reasons why blockdev-snapshot
ddf19c
should be able to perform this operation.)
ddf19c
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
Message-Id: <20200310113831.27293-3-kwolf@redhat.com>
ddf19c
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
ddf19c
Tested-by: Peter Krempa <pkrempa@redhat.com>
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
(cherry picked from commit d29d3d1f80b3947fb26e7139645c83de66d146a9)
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
ddf19c
---
ddf19c
 blockdev.c                 | 14 ++++++++------
ddf19c
 tests/qemu-iotests/085.out |  4 ++--
ddf19c
 2 files changed, 10 insertions(+), 8 deletions(-)
ddf19c
ddf19c
diff --git a/blockdev.c b/blockdev.c
ddf19c
index 4cd9a58..7918533 100644
ddf19c
--- a/blockdev.c
ddf19c
+++ b/blockdev.c
ddf19c
@@ -1536,6 +1536,7 @@ static void external_snapshot_prepare(BlkActionState *common,
ddf19c
     TransactionAction *action = common->action;
ddf19c
     AioContext *aio_context;
ddf19c
     AioContext *old_context;
ddf19c
+    uint64_t perm, shared;
ddf19c
     int ret;
ddf19c
 
ddf19c
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
ddf19c
@@ -1656,16 +1657,17 @@ static void external_snapshot_prepare(BlkActionState *common,
ddf19c
         goto out;
ddf19c
     }
ddf19c
 
ddf19c
-    if (bdrv_has_blk(state->new_bs)) {
ddf19c
+    /*
ddf19c
+     * Allow attaching a backing file to an overlay that's already in use only
ddf19c
+     * if the parents don't assume that they are already seeing a valid image.
ddf19c
+     * (Specifically, allow it as a mirror target, which is write-only access.)
ddf19c
+     */
ddf19c
+    bdrv_get_cumulative_perm(state->new_bs, &perm, &shared);
ddf19c
+    if (perm & BLK_PERM_CONSISTENT_READ) {
ddf19c
         error_setg(errp, "The overlay is already in use");
ddf19c
         goto out;
ddf19c
     }
ddf19c
 
ddf19c
-    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
ddf19c
-                           errp)) {
ddf19c
-        goto out;
ddf19c
-    }
ddf19c
-
ddf19c
     if (state->new_bs->backing != NULL) {
ddf19c
         error_setg(errp, "The overlay already has a backing image");
ddf19c
         goto out;
ddf19c
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
ddf19c
index bb50227..487d920 100644
ddf19c
--- a/tests/qemu-iotests/085.out
ddf19c
+++ b/tests/qemu-iotests/085.out
ddf19c
@@ -82,7 +82,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
ddf19c
 === Invalid command - cannot create a snapshot using a file BDS ===
ddf19c
 
ddf19c
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node':'virtio0', 'overlay':'file_12' } }
ddf19c
-{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
ddf19c
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
ddf19c
 
ddf19c
 === Invalid command - snapshot node used as active layer ===
ddf19c
 
ddf19c
@@ -96,7 +96,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
ddf19c
 === Invalid command - snapshot node used as backing hd ===
ddf19c
 
ddf19c
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay':'snap_11' } }
ddf19c
-{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}}
ddf19c
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
ddf19c
 
ddf19c
 === Invalid command - snapshot node has a backing image ===
ddf19c
 
ddf19c
-- 
ddf19c
1.8.3.1
ddf19c