qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, hreitz@redhat.com, stefanha@redhat.com,
	pkrempa@redhat.com, peterx@redhat.com, farosas@suse.de,
	qemu-devel@nongnu.org
Subject: [PATCH v3 03/16] block: Inactivate external snapshot overlays when necessary
Date: Tue,  4 Feb 2025 22:13:54 +0100	[thread overview]
Message-ID: <20250204211407.381505-4-kwolf@redhat.com> (raw)
In-Reply-To: <20250204211407.381505-1-kwolf@redhat.com>

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>
Acked-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

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;
-- 
2.48.1



  parent reply	other threads:[~2025-02-04 21:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-04 21:13 [PATCH v3 00/16] block: Managing inactive nodes (QSD migration) Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 01/16] block: Add 'active' field to BlockDeviceInfo Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 02/16] block: Allow inactivating already inactive nodes Kevin Wolf
2025-02-04 21:13 ` Kevin Wolf [this message]
2025-02-04 21:13 ` [PATCH v3 04/16] migration/block-active: Remove global active flag Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 05/16] block: Don't attach inactive child to active node Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 06/16] block: Fix crash on block_resize on inactive node Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 07/16] block: Add option to create inactive nodes Kevin Wolf
2025-02-04 21:13 ` [PATCH v3 08/16] block: Add blockdev-set-active QMP command Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 09/16] block: Support inactive nodes in blk_insert_bs() Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 10/16] block/export: Don't ignore image activation error in blk_exp_add() Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 11/16] block: Drain nodes before inactivating them Kevin Wolf
2025-02-05 20:42   ` Eric Blake
2025-02-04 21:14 ` [PATCH v3 12/16] block/export: Add option to allow export of inactive nodes Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 13/16] nbd/server: Support " Kevin Wolf
2025-02-05 20:43   ` Eric Blake
2025-02-04 21:14 ` [PATCH v3 14/16] iotests: Add filter_qtest() Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 15/16] iotests: Add qsd-migrate case Kevin Wolf
2025-02-05 20:46   ` Eric Blake
2025-02-24 10:23   ` Thomas Huth
2025-02-24 13:13     ` Kevin Wolf
2025-02-04 21:14 ` [PATCH v3 16/16] iotests: Add (NBD-based) tests for inactive nodes Kevin Wolf
2025-02-05 20:49   ` Eric Blake
2025-02-05 15:35 ` [PATCH v3 00/16] block: Managing inactive nodes (QSD migration) Stefan Hajnoczi

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=20250204211407.381505-4-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=farosas@suse.de \
    --cc=hreitz@redhat.com \
    --cc=peterx@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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).