All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Peter Krempa" <pkrempa@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-block@nongnu.org, "Juan Quintela" <quintela@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>
Subject: [PATCH 3/6] block: add ability to filter out blockdevs during snapshot
Date: Thu,  2 Jul 2020 18:57:51 +0100	[thread overview]
Message-ID: <20200702175754.2211821-4-berrange@redhat.com> (raw)
In-Reply-To: <20200702175754.2211821-1-berrange@redhat.com>

When executing snapshot logic, currently blockdevs are filtered out if
they are read-only or if there is no media present. In order to support
snapshotting when UEFI firmware is in use, we need the ability to filter
out the blockdev used for the firmware vars store. This can be achieved
by having a list of node names that should be excluded from
consideration for snapshots.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/monitor/block-hmp-cmds.c |  4 ++--
 block/snapshot.c               | 41 ++++++++++++++++++++++------------
 include/block/snapshot.h       | 19 +++++++++-------
 migration/savevm.c             | 18 +++++++--------
 4 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 4c8c375172..0ee6e7a4be 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -899,7 +899,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
     ImageEntry *image_entry, *next_ie;
     SnapshotEntry *snapshot_entry;
 
-    bs = bdrv_all_find_vmstate_bs();
+    bs = bdrv_all_find_vmstate_bs(NULL);
     if (!bs) {
         monitor_printf(mon, "No available block device supports snapshots\n");
         return;
@@ -951,7 +951,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
     total = 0;
     for (i = 0; i < nb_sns; i++) {
         SnapshotEntry *next_sn;
-        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs1) == 0) {
+        if (bdrv_all_find_snapshot(sn_tab[i].name, NULL, &bs1) == 0) {
             global_snapshots[total] = i;
             total++;
             QTAILQ_FOREACH(image_entry, &image_list, next) {
diff --git a/block/snapshot.c b/block/snapshot.c
index bd9fb01817..c8950b0b86 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -385,12 +385,22 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
     return ret;
 }
 
-static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
+static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs,
+                                           strList *exclude_bs)
 {
+    const char *node_name = bdrv_get_node_name(bs);
+
     if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
         return false;
     }
 
+    while (exclude_bs) {
+        if (g_str_equal(node_name, exclude_bs->value)) {
+            return false;
+        }
+        exclude_bs = exclude_bs->next;
+    }
+
     /* Include all nodes that are either in use by a BlockBackend, or that
      * aren't attached to any node, but owned by the monitor. */
     return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
@@ -400,7 +410,7 @@ static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
  * These functions will properly handle dataplane (take aio_context_acquire
  * when appropriate for appropriate block drivers) */
 
-bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
+bool bdrv_all_can_snapshot(strList *exclude_bs, BlockDriverState **first_bad_bs)
 {
     bool ok = true;
     BlockDriverState *bs;
@@ -410,7 +420,7 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_all_snapshots_includes_bs(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs, exclude_bs)) {
             ok = bdrv_can_snapshot(bs);
         }
         aio_context_release(ctx);
@@ -425,8 +435,8 @@ fail:
     return ok;
 }
 
-int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
-                             Error **errp)
+int bdrv_all_delete_snapshot(const char *name, strList *exclude_bs,
+                             BlockDriverState **first_bad_bs, Error **errp)
 {
     int ret = 0;
     BlockDriverState *bs;
@@ -437,7 +447,7 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_all_snapshots_includes_bs(bs) &&
+        if (bdrv_all_snapshots_includes_bs(bs, exclude_bs) &&
             bdrv_snapshot_find(bs, snapshot, name) >= 0)
         {
             ret = bdrv_snapshot_delete(bs, snapshot->id_str,
@@ -456,8 +466,8 @@ fail:
 }
 
 
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
-                           Error **errp)
+int bdrv_all_goto_snapshot(const char *name, strList *exclude_bs,
+                           BlockDriverState **first_bad_bs, Error **errp)
 {
     int ret = 0;
     BlockDriverState *bs;
@@ -467,7 +477,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_all_snapshots_includes_bs(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs, exclude_bs)) {
             ret = bdrv_snapshot_goto(bs, name, errp);
         }
         aio_context_release(ctx);
@@ -482,7 +492,8 @@ fail:
     return ret;
 }
 
-int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
+int bdrv_all_find_snapshot(const char *name, strList *exclude_bs,
+                           BlockDriverState **first_bad_bs)
 {
     QEMUSnapshotInfo sn;
     int err = 0;
@@ -493,7 +504,7 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_all_snapshots_includes_bs(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs, exclude_bs)) {
             err = bdrv_snapshot_find(bs, &sn, name);
         }
         aio_context_release(ctx);
@@ -511,6 +522,7 @@ fail:
 int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
                              BlockDriverState *vm_state_bs,
                              uint64_t vm_state_size,
+                             strList *exclude_bs,
                              BlockDriverState **first_bad_bs)
 {
     int err = 0;
@@ -524,7 +536,7 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
         if (bs == vm_state_bs) {
             sn->vm_state_size = vm_state_size;
             err = bdrv_snapshot_create(bs, sn);
-        } else if (bdrv_all_snapshots_includes_bs(bs)) {
+        } else if (bdrv_all_snapshots_includes_bs(bs, exclude_bs)) {
             sn->vm_state_size = 0;
             err = bdrv_snapshot_create(bs, sn);
         }
@@ -540,7 +552,7 @@ fail:
     return err;
 }
 
-BlockDriverState *bdrv_all_find_vmstate_bs(void)
+BlockDriverState *bdrv_all_find_vmstate_bs(strList *exclude_bs)
 {
     BlockDriverState *bs;
     BdrvNextIterator it;
@@ -550,7 +562,8 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
         bool found;
 
         aio_context_acquire(ctx);
-        found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
+        found = bdrv_all_snapshots_includes_bs(bs, exclude_bs) &&
+            bdrv_can_snapshot(bs);
         aio_context_release(ctx);
 
         if (found) {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 2bfcd57578..f20986ca37 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -25,7 +25,7 @@
 #ifndef SNAPSHOT_H
 #define SNAPSHOT_H
 
-
+#include "qapi/qapi-builtin-types.h"
 
 #define SNAPSHOT_OPT_BASE       "snapshot."
 #define SNAPSHOT_OPT_ID         "snapshot.id"
@@ -76,17 +76,20 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
  * These functions will properly handle dataplane (take aio_context_acquire
  * when appropriate for appropriate block drivers */
 
-bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
-int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
-                             Error **errp);
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
-                           Error **errp);
-int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
+bool bdrv_all_can_snapshot(strList *exclude_bs,
+                           BlockDriverState **first_bad_bs);
+int bdrv_all_delete_snapshot(const char *name, strList *exclude_bs,
+                             BlockDriverState **first_bsd_bs, Error **errp);
+int bdrv_all_goto_snapshot(const char *name, strList *exclude_bs,
+                           BlockDriverState **first_bad_bs, Error **errp);
+int bdrv_all_find_snapshot(const char *name, strList *exclude_bs,
+                           BlockDriverState **first_bad_bs);
 int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
                              BlockDriverState *vm_state_bs,
                              uint64_t vm_state_size,
+                             strList *exclude_bs,
                              BlockDriverState **first_bad_bs);
 
-BlockDriverState *bdrv_all_find_vmstate_bs(void);
+BlockDriverState *bdrv_all_find_vmstate_bs(strList *exclude_bs);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 53586a6406..4251aa0dde 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2646,7 +2646,7 @@ int save_snapshot(const char *name, Error **errp)
         return ret;
     }
 
-    if (!bdrv_all_can_snapshot(&bs)) {
+    if (!bdrv_all_can_snapshot(NULL, &bs)) {
         error_setg(errp, "Device '%s' is writable but does not support "
                    "snapshots", bdrv_get_device_or_node_name(bs));
         return ret;
@@ -2654,7 +2654,7 @@ int save_snapshot(const char *name, Error **errp)
 
     /* Delete old snapshots of the same name */
     if (name) {
-        ret = bdrv_all_delete_snapshot(name, &bs1, errp);
+        ret = bdrv_all_delete_snapshot(name, NULL, &bs1, errp);
         if (ret < 0) {
             error_prepend(errp, "Error while deleting snapshot on device "
                           "'%s': ", bdrv_get_device_or_node_name(bs1));
@@ -2662,7 +2662,7 @@ int save_snapshot(const char *name, Error **errp)
         }
     }
 
-    bs = bdrv_all_find_vmstate_bs();
+    bs = bdrv_all_find_vmstate_bs(NULL);
     if (bs == NULL) {
         error_setg(errp, "No block device can accept snapshots");
         return ret;
@@ -2725,7 +2725,7 @@ int save_snapshot(const char *name, Error **errp)
     aio_context_release(aio_context);
     aio_context = NULL;
 
-    ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, &bs);
+    ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, NULL, &bs);
     if (ret < 0) {
         error_setg(errp, "Error while creating snapshot on '%s'",
                    bdrv_get_device_or_node_name(bs));
@@ -2843,13 +2843,13 @@ int load_snapshot(const char *name, Error **errp)
         return -EINVAL;
     }
 
-    if (!bdrv_all_can_snapshot(&bs)) {
+    if (!bdrv_all_can_snapshot(NULL, &bs)) {
         error_setg(errp,
                    "Device '%s' is writable but does not support snapshots",
                    bdrv_get_device_or_node_name(bs));
         return -ENOTSUP;
     }
-    ret = bdrv_all_find_snapshot(name, &bs);
+    ret = bdrv_all_find_snapshot(name, NULL, &bs);
     if (ret < 0) {
         error_setg(errp,
                    "Device '%s' does not have the requested snapshot '%s'",
@@ -2857,7 +2857,7 @@ int load_snapshot(const char *name, Error **errp)
         return ret;
     }
 
-    bs_vm_state = bdrv_all_find_vmstate_bs();
+    bs_vm_state = bdrv_all_find_vmstate_bs(NULL);
     if (!bs_vm_state) {
         error_setg(errp, "No block device supports snapshots");
         return -ENOTSUP;
@@ -2879,7 +2879,7 @@ int load_snapshot(const char *name, Error **errp)
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all_begin();
 
-    ret = bdrv_all_goto_snapshot(name, &bs, errp);
+    ret = bdrv_all_goto_snapshot(name, NULL, &bs, errp);
     if (ret < 0) {
         error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
                       name, bdrv_get_device_or_node_name(bs));
@@ -2964,7 +2964,7 @@ void qmp_delvm(const char *tag, Error **errp)
 {
     BlockDriverState *bs;
 
-    if (bdrv_all_delete_snapshot(tag, &bs, errp) < 0) {
+    if (bdrv_all_delete_snapshot(tag, NULL, &bs, errp) < 0) {
         error_prepend(errp,
                       "deleting snapshot on device '%s': ",
                       bdrv_get_device_or_node_name(bs));
-- 
2.26.2



  parent reply	other threads:[~2020-07-02 18:01 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-02 17:57 [PATCH 0/6] migration: bring savevm/loadvm/delvm over to QMP Daniel P. Berrangé
2020-07-02 17:57 ` [PATCH 1/6] migration: improve error reporting of block driver state name Daniel P. Berrangé
2020-07-02 18:36   ` Eric Blake
2020-07-02 19:13     ` Dr. David Alan Gilbert
2020-07-02 17:57 ` [PATCH 2/6] migration: introduce savevm, loadvm, delvm QMP commands Daniel P. Berrangé
2020-07-02 18:12   ` Eric Blake
2020-07-02 18:24     ` Daniel P. Berrangé
2020-07-03 15:49       ` Dr. David Alan Gilbert
2020-07-03 16:02         ` Daniel P. Berrangé
2020-07-03 16:10           ` Dr. David Alan Gilbert
2020-07-03 16:16             ` Daniel P. Berrangé
2020-07-03 16:22               ` Dr. David Alan Gilbert
2020-07-03 16:49                 ` Daniel P. Berrangé
2020-07-03 17:00                   ` Dr. David Alan Gilbert
2020-07-03 17:10                     ` Daniel P. Berrangé
2020-07-03 17:26                       ` Dr. David Alan Gilbert
2020-07-03 16:24             ` Peter Krempa
2020-07-03 16:26               ` Dr. David Alan Gilbert
2020-07-06 16:15           ` Kevin Wolf
2020-07-07  6:38             ` Peter Krempa
2020-07-07 10:33               ` Kevin Wolf
2020-07-07 10:41                 ` Peter Krempa
2020-07-03 17:22   ` Denis V. Lunev
2020-07-02 17:57 ` Daniel P. Berrangé [this message]
2020-07-02 17:57 ` [PATCH 4/6] block: allow specifying name of block device for vmstate storage Daniel P. Berrangé
2020-07-02 17:57 ` [PATCH 5/6] migration: support excluding block devs in QMP snapshot commands Daniel P. Berrangé
2020-07-06 15:57   ` Kevin Wolf
2020-07-07  9:14     ` Daniel P. Berrangé
2020-07-07 10:11       ` Kevin Wolf
2020-07-02 17:57 ` [PATCH 6/6] migration: support picking vmstate disk " Daniel P. Berrangé
2020-07-02 18:19   ` Eric Blake
2020-07-03  8:37     ` Daniel P. Berrangé
2020-07-02 18:53 ` [PATCH 0/6] migration: bring savevm/loadvm/delvm over to QMP no-reply
2020-07-02 19:07 ` no-reply
2020-07-03 17:15 ` Denis V. Lunev
2020-07-03 17:22   ` Daniel P. Berrangé
2020-07-03 17:29     ` Denis V. Lunev
2020-07-06 14:28       ` Daniel P. Berrangé
2020-07-06 16:07         ` Denis V. Lunev
2020-07-06 15:27       ` Kevin Wolf
2020-07-06 15:29         ` Daniel P. Berrangé
2020-07-06 15:50           ` Kevin Wolf
2020-07-06 16:03             ` Daniel P. Berrangé
2020-07-06 16:10               ` Denis V. Lunev
2020-07-06 16:15                 ` Daniel P. Berrangé
2020-07-06 16:21               ` Kevin Wolf
2020-07-07  9:07                 ` Daniel P. Berrangé

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=20200702175754.2211821-4-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.