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>,
"Denis V. Lunev" <den@virtuozzo.com>,
qemu-block@nongnu.org, "Juan Quintela" <quintela@redhat.com>,
"John Snow" <jsnow@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
"Max Reitz" <mreitz@redhat.com>,
"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>
Subject: [PATCH v7 04/11] block: add ability to specify list of blockdevs during snapshot
Date: Wed, 21 Oct 2020 17:26:57 +0100 [thread overview]
Message-ID: <20201021162704.3652555-5-berrange@redhat.com> (raw)
In-Reply-To: <20201021162704.3652555-1-berrange@redhat.com>
When running snapshot operations, there are various rules for which
blockdevs are included/excluded. While this provides reasonable default
behaviour, there are scenarios that are not well handled by the default
logic. Some of the conditions do not have a single correct answer.
Thus there needs to be a way for the mgmt app to provide an explicit
list of blockdevs to perform snapshots across. This can be achieved
by passing a list of node names that should be used.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
block/monitor/block-hmp-cmds.c | 4 +-
block/snapshot.c | 172 ++++++++++++++++++++++++---------
include/block/snapshot.h | 22 +++--
migration/savevm.c | 16 +--
monitor/hmp-cmds.c | 2 +-
replay/replay-debugging.c | 4 +-
6 files changed, 158 insertions(+), 62 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 548bb6b5e3..ebb6ae0333 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -901,7 +901,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
SnapshotEntry *snapshot_entry;
Error *err = NULL;
- bs = bdrv_all_find_vmstate_bs(&err);
+ bs = bdrv_all_find_vmstate_bs(false, NULL, &err);
if (!bs) {
error_report_err(err);
return;
@@ -953,7 +953,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, NULL) == 0) {
+ if (bdrv_all_find_snapshot(sn_tab[i].name, false, NULL, NULL) == 0) {
global_snapshots[total] = i;
total++;
QTAILQ_FOREACH(image_entry, &image_list, next) {
diff --git a/block/snapshot.c b/block/snapshot.c
index 482e3fc7b7..220173deae 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -447,6 +447,41 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
return ret;
}
+
+static int bdrv_all_get_snapshot_devices(bool has_devices, strList *devices,
+ GList **all_bdrvs,
+ Error **errp)
+{
+ g_autoptr(GList) bdrvs = NULL;
+
+ if (has_devices) {
+ if (!devices) {
+ error_setg(errp, "At least one device is required for snapshot");
+ return -1;
+ }
+
+ while (devices) {
+ BlockDriverState *bs = bdrv_find_node(devices->value);
+ if (!bs) {
+ error_setg(errp, "No block device node '%s'", devices->value);
+ return -1;
+ }
+ bdrvs = g_list_append(bdrvs, bs);
+ devices = devices->next;
+ }
+ } else {
+ BlockDriverState *bs;
+ BdrvNextIterator it;
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ bdrvs = g_list_append(bdrvs, bs);
+ }
+ }
+
+ *all_bdrvs = g_steal_pointer(&bdrvs);
+ return 0;
+}
+
+
static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
{
if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
@@ -462,43 +497,59 @@ 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(Error **errp)
+bool bdrv_all_can_snapshot(bool has_devices, strList *devices,
+ Error **errp)
{
- BlockDriverState *bs;
- BdrvNextIterator it;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return false;
+ }
+
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
bool ok = true;
aio_context_acquire(ctx);
- if (bdrv_all_snapshots_includes_bs(bs)) {
+ if (devices || bdrv_all_snapshots_includes_bs(bs)) {
ok = bdrv_can_snapshot(bs);
}
aio_context_release(ctx);
if (!ok) {
error_setg(errp, "Device '%s' is writable but does not support "
"snapshots", bdrv_get_device_or_node_name(bs));
- bdrv_next_cleanup(&it);
return false;
}
+
+ iterbdrvs = iterbdrvs->next;
}
return true;
}
-int bdrv_all_delete_snapshot(const char *name, Error **errp)
+int bdrv_all_delete_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp)
{
- BlockDriverState *bs;
- BdrvNextIterator it;
- QEMUSnapshotInfo sn1, *snapshot = &sn1;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
+
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return -1;
+ }
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
+ QEMUSnapshotInfo sn1, *snapshot = &sn1;
int ret = 0;
aio_context_acquire(ctx);
- if (bdrv_all_snapshots_includes_bs(bs) &&
+ if ((devices || bdrv_all_snapshots_includes_bs(bs)) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0)
{
ret = bdrv_snapshot_delete(bs, snapshot->id_str,
@@ -508,61 +559,80 @@ int bdrv_all_delete_snapshot(const char *name, Error **errp)
if (ret < 0) {
error_prepend(errp, "Could not delete snapshot '%s' on '%s': ",
name, bdrv_get_device_or_node_name(bs));
- bdrv_next_cleanup(&it);
return -1;
}
+
+ iterbdrvs = iterbdrvs->next;
}
return 0;
}
-int bdrv_all_goto_snapshot(const char *name, Error **errp)
+int bdrv_all_goto_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp)
{
- BlockDriverState *bs;
- BdrvNextIterator it;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return -1;
+ }
+
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
aio_context_acquire(ctx);
- if (bdrv_all_snapshots_includes_bs(bs)) {
+ if (devices || bdrv_all_snapshots_includes_bs(bs)) {
ret = bdrv_snapshot_goto(bs, name, errp);
}
aio_context_release(ctx);
if (ret < 0) {
error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
name, bdrv_get_device_or_node_name(bs));
- bdrv_next_cleanup(&it);
return -1;
}
+
+ iterbdrvs = iterbdrvs->next;
}
return 0;
}
-int bdrv_all_find_snapshot(const char *name, Error **errp)
+int bdrv_all_find_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp)
{
- QEMUSnapshotInfo sn;
- BlockDriverState *bs;
- BdrvNextIterator it;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
+
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return -1;
+ }
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
+ QEMUSnapshotInfo sn;
int ret = 0;
aio_context_acquire(ctx);
- if (bdrv_all_snapshots_includes_bs(bs)) {
+ if (devices || bdrv_all_snapshots_includes_bs(bs)) {
ret = bdrv_snapshot_find(bs, &sn, name);
}
aio_context_release(ctx);
if (ret < 0) {
error_setg(errp, "Could not find snapshot '%s' on '%s'",
name, bdrv_get_device_or_node_name(bs));
- bdrv_next_cleanup(&it);
return -1;
}
+
+ iterbdrvs = iterbdrvs->next;
}
return 0;
@@ -571,12 +641,19 @@ int bdrv_all_find_snapshot(const char *name, Error **errp)
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *vm_state_bs,
uint64_t vm_state_size,
+ bool has_devices, strList *devices,
Error **errp)
{
- BlockDriverState *bs;
- BdrvNextIterator it;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
+
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return -1;
+ }
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
@@ -584,7 +661,7 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
if (bs == vm_state_bs) {
sn->vm_state_size = vm_state_size;
ret = bdrv_snapshot_create(bs, sn);
- } else if (bdrv_all_snapshots_includes_bs(bs)) {
+ } else if (devices || bdrv_all_snapshots_includes_bs(bs)) {
sn->vm_state_size = 0;
ret = bdrv_snapshot_create(bs, sn);
}
@@ -592,34 +669,43 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
if (ret < 0) {
error_setg(errp, "Could not create snapshot '%s' on '%s'",
sn->name, bdrv_get_device_or_node_name(bs));
- bdrv_next_cleanup(&it);
return -1;
}
+
+ iterbdrvs = iterbdrvs->next;
}
return 0;
}
-BlockDriverState *bdrv_all_find_vmstate_bs(Error **errp)
+BlockDriverState *bdrv_all_find_vmstate_bs(bool has_devices, strList *devices,
+ Error **errp)
{
- BlockDriverState *bs;
- BdrvNextIterator it;
+ g_autoptr(GList) bdrvs = NULL;
+ GList *iterbdrvs;
- for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) < 0) {
+ return NULL;
+ }
+
+ iterbdrvs = bdrvs;
+ while (iterbdrvs) {
+ BlockDriverState *bs = iterbdrvs->data;
AioContext *ctx = bdrv_get_aio_context(bs);
- bool found;
+ bool found = false;
aio_context_acquire(ctx);
- found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
+ found = (devices || bdrv_all_snapshots_includes_bs(bs)) &&
+ bdrv_can_snapshot(bs);
aio_context_release(ctx);
if (found) {
- bdrv_next_cleanup(&it);
- break;
+ return bs;
}
+
+ iterbdrvs = iterbdrvs->next;
}
- if (!bs) {
- error_setg(errp, "No block device supports snapshots");
- }
- return bs;
+
+ error_setg(errp, "No block device supports snapshots");
+ return NULL;
}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 5cb2b696ad..2569a903f2 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"
@@ -77,15 +77,25 @@ 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(Error **errp);
-int bdrv_all_delete_snapshot(const char *name, Error **errp);
-int bdrv_all_goto_snapshot(const char *name, Error **errp);
-int bdrv_all_find_snapshot(const char *name, Error **errp);
+bool bdrv_all_can_snapshot(bool has_devices, strList *devices,
+ Error **errp);
+int bdrv_all_delete_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp);
+int bdrv_all_goto_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp);
+int bdrv_all_find_snapshot(const char *name,
+ bool has_devices, strList *devices,
+ Error **errp);
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *vm_state_bs,
uint64_t vm_state_size,
+ bool has_devices,
+ strList *devices,
Error **errp);
-BlockDriverState *bdrv_all_find_vmstate_bs(Error **errp);
+BlockDriverState *bdrv_all_find_vmstate_bs(bool has_devices, strList *devices,
+ Error **errp);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 531bb2eca1..efbe390671 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2680,18 +2680,18 @@ bool save_snapshot(const char *name, Error **errp)
return false;
}
- if (!bdrv_all_can_snapshot(errp)) {
+ if (!bdrv_all_can_snapshot(false, NULL, errp)) {
return false;
}
/* Delete old snapshots of the same name */
if (name) {
- if (bdrv_all_delete_snapshot(name, errp) < 0) {
+ if (bdrv_all_delete_snapshot(name, false, NULL, errp) < 0) {
return false;
}
}
- bs = bdrv_all_find_vmstate_bs(errp);
+ bs = bdrv_all_find_vmstate_bs(false, NULL, errp);
if (bs == NULL) {
return false;
}
@@ -2762,7 +2762,7 @@ bool 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, errp);
+ ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, false, NULL, errp);
if (ret < 0) {
goto the_end;
}
@@ -2873,15 +2873,15 @@ bool load_snapshot(const char *name, Error **errp)
AioContext *aio_context;
MigrationIncomingState *mis = migration_incoming_get_current();
- if (!bdrv_all_can_snapshot(errp)) {
+ if (!bdrv_all_can_snapshot(false, NULL, errp)) {
return false;
}
- ret = bdrv_all_find_snapshot(name, errp);
+ ret = bdrv_all_find_snapshot(name, false, NULL, errp);
if (ret < 0) {
return false;
}
- bs_vm_state = bdrv_all_find_vmstate_bs(errp);
+ bs_vm_state = bdrv_all_find_vmstate_bs(false, NULL, errp);
if (!bs_vm_state) {
return false;
}
@@ -2908,7 +2908,7 @@ bool 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, errp);
+ ret = bdrv_all_goto_snapshot(name, false, NULL, errp);
if (ret < 0) {
goto err_drain;
}
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 521fcf96eb..8fef194d80 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1142,7 +1142,7 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
Error *err = NULL;
const char *name = qdict_get_str(qdict, "name");
- bdrv_all_delete_snapshot(name, &err);
+ bdrv_all_delete_snapshot(name, false, NULL, &err);
hmp_handle_error(mon, err);
}
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 5458a73fce..a17c1d3578 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -149,7 +149,7 @@ static char *replay_find_nearest_snapshot(int64_t icount,
*snapshot_icount = -1;
- bs = bdrv_all_find_vmstate_bs(NULL);
+ bs = bdrv_all_find_vmstate_bs(false, NULL, NULL);
if (!bs) {
goto fail;
}
@@ -160,7 +160,7 @@ static char *replay_find_nearest_snapshot(int64_t icount,
aio_context_release(aio_context);
for (i = 0; i < nb_sns; i++) {
- if (bdrv_all_find_snapshot(sn_tab[i].name, NULL) == 0) {
+ if (bdrv_all_find_snapshot(sn_tab[i].name, false, NULL, NULL) == 0) {
if (sn_tab[i].icount != -1ULL
&& sn_tab[i].icount <= icount
&& (!nearest || nearest->icount < sn_tab[i].icount)) {
--
2.26.2
next prev parent reply other threads:[~2020-10-21 16:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-21 16:26 [PATCH v7 00/11] migration: bring improved savevm/loadvm/delvm to QMP Daniel P. Berrangé
2020-10-21 16:26 ` [PATCH v7 01/11] block: push error reporting into bdrv_all_*_snapshot functions Daniel P. Berrangé
2020-10-21 16:26 ` [PATCH v7 02/11] migration: Make save_snapshot() return bool, not 0/-1 Daniel P. Berrangé
2020-10-21 16:26 ` [PATCH v7 03/11] migration: stop returning errno from load_snapshot() Daniel P. Berrangé
2020-10-21 16:26 ` Daniel P. Berrangé [this message]
2020-10-21 16:26 ` [PATCH v7 05/11] block: allow specifying name of block device for vmstate storage Daniel P. Berrangé
2020-10-21 16:26 ` [PATCH v7 06/11] block: rename and alter bdrv_all_find_snapshot semantics Daniel P. Berrangé
2020-10-21 16:27 ` [PATCH v7 07/11] migration: control whether snapshots are ovewritten Daniel P. Berrangé
2020-10-21 16:27 ` [PATCH v7 08/11] migration: wire up support for snapshot device selection Daniel P. Berrangé
2020-10-21 16:38 ` Philippe Mathieu-Daudé
2020-10-21 16:27 ` [PATCH v7 09/11] migration: introduce a delete_snapshot wrapper Daniel P. Berrangé
2020-10-21 16:39 ` Philippe Mathieu-Daudé
2020-10-21 16:27 ` [PATCH v7 10/11] iotests: add support for capturing and matching QMP events Daniel P. Berrangé
2020-10-21 16:27 ` [PATCH v7 11/11] migration: introduce snapshot-{save, load, delete} QMP commands 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=20201021162704.3652555-5-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=den@virtuozzo.com \
--cc=dgilbert@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pavel.dovgaluk@ispras.ru \
--cc=pbonzini@redhat.com \
--cc=philmd@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.