diff mbox series

[v2,02/15] block: Inactivate external snapshot overlays when necessary

Message ID 20250130171240.286878-3-kwolf@redhat.com (mailing list archive)
State New
Headers show
Series block: Managing inactive nodes (QSD migration) | expand

Commit Message

Kevin Wolf Jan. 30, 2025, 5:12 p.m. UTC
Putting an active block node on top of an inactive one is strictly
speaking an invalid configuration and the next patch will turn it into a
hard error.

However, taking a snapshot while disk images are inactive after
completing migration has an important use case: After migrating to a
file, taking an external snapshot is what is needed to take a full VM
snapshot.

In order for this to keep working after the later patches, change
creating a snapshot such that it automatically inactivates an overlay
that is added on top of an already inactive node.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Eric Blake Jan. 30, 2025, 7:46 p.m. UTC | #1
On Thu, Jan 30, 2025 at 06:12:33PM +0100, Kevin Wolf wrote:
> Putting an active block node on top of an inactive one is strictly
> speaking an invalid configuration and the next patch will turn it into a
> hard error.
> 
> However, taking a snapshot while disk images are inactive after
> completing migration has an important use case: After migrating to a
> file, taking an external snapshot is what is needed to take a full VM
> snapshot.
> 
> In order for this to keep working after the later patches, change
> creating a snapshot such that it automatically inactivates an overlay
> that is added on top of an already inactive node.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/blockdev.c b/blockdev.c
index 218024497b..eb2517f1dd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1497,6 +1497,22 @@  static void external_snapshot_action(TransactionAction *action,
         return;
     }
 
+    /*
+     * Older QEMU versions have allowed adding an active parent node to an
+     * inactive child node. This is unsafe in the general case, but there is an
+     * important use case, which is taking a VM snapshot with migration to file
+     * and then adding an external snapshot while the VM is still stopped and
+     * images are inactive. Requiring the user to explicitly create the overlay
+     * as inactive would break compatibility, so just do it automatically here
+     * to keep this working.
+     */
+    if (bdrv_is_inactive(state->old_bs) && !bdrv_is_inactive(state->new_bs)) {
+        ret = bdrv_inactivate(state->new_bs, errp);
+        if (ret < 0) {
+            return;
+        }
+    }
+
     ret = bdrv_append(state->new_bs, state->old_bs, errp);
     if (ret < 0) {
         return;