* [Qemu-devel] [PATCH 01/10] snapshot: create helper to test that block drivers supports snapshots
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 02/10] snapshot: return error code from bdrv_snapshot_delete_by_id_or_name Denis V. Lunev
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
The patch enforces proper locking for this operation.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 24 ++++++++++++++++++++++++
include/block/snapshot.h | 8 ++++++++
migration/savevm.c | 17 ++++-------------
3 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 89500f2..d929d08 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -356,3 +356,27 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
return ret;
}
+
+
+/* Group operations. All block drivers are involved.
+ * 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 ok = true;
+ BlockDriverState *bs = NULL;
+
+ while (ok && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
+ ok = bdrv_can_snapshot(bs);
+ }
+ aio_context_release(ctx);
+ }
+
+ *first_bad_bs = bs;
+ return ok;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 770d9bb..6195c9c 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -75,4 +75,12 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
const char *id_or_name,
Error **errp);
+
+
+/* Group operations. All block drivers are involved.
+ * 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);
+
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 9f2230f..c212288 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1290,19 +1290,10 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
const char *name = qdict_get_try_str(qdict, "name");
Error *local_err = NULL;
- /* Verify if there is a device that doesn't support snapshots and is writable */
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
-
- if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
- continue;
- }
-
- if (!bdrv_can_snapshot(bs)) {
- monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
- bdrv_get_device_name(bs));
- return;
- }
+ if (!bdrv_all_can_snapshot(&bs)) {
+ monitor_printf(mon, "Device '%s' is writable but does not "
+ "support snapshots.\n", bdrv_get_device_name(bs));
+ return;
}
bs = find_vmstate_bs();
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 02/10] snapshot: return error code from bdrv_snapshot_delete_by_id_or_name
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 01/10] snapshot: create helper to test that block drivers supports snapshots Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 03/10] snapshot: create bdrv_all_delete_snapshot helper Denis V. Lunev
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
this will make code better in the next patch
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 7 ++++---
include/block/snapshot.h | 6 +++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index d929d08..ed0422d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -253,9 +253,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
return -ENOTSUP;
}
-void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
- const char *id_or_name,
- Error **errp)
+int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp)
{
int ret;
Error *local_err = NULL;
@@ -270,6 +270,7 @@ void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
if (ret < 0) {
error_propagate(errp, local_err);
}
+ return ret;
}
int bdrv_snapshot_list(BlockDriverState *bs,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 6195c9c..9ddfd42 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -63,9 +63,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
const char *snapshot_id,
const char *name,
Error **errp);
-void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
- const char *id_or_name,
- Error **errp);
+int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp);
int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int bdrv_snapshot_load_tmp(BlockDriverState *bs,
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 03/10] snapshot: create bdrv_all_delete_snapshot helper
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 01/10] snapshot: create helper to test that block drivers supports snapshots Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 02/10] snapshot: return error code from bdrv_snapshot_delete_by_id_or_name Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 04/10] snapshot: create bdrv_all_goto_snapshot helper Denis V. Lunev
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
to delete snapshots from all loaded block drivers.
The patch also ensures proper locking.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 22 ++++++++++++++++++++
include/block/snapshot.h | 2 ++
migration/savevm.c | 54 +++++++++---------------------------------------
3 files changed, 34 insertions(+), 44 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index ed0422d..61a6ad1 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -381,3 +381,25 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
*first_bad_bs = bs;
return ok;
}
+
+int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
+ Error **err)
+{
+ int ret = 0;
+ BlockDriverState *bs = NULL;
+ QEMUSnapshotInfo sn1, *snapshot = &sn1;
+
+ while (ret == 0 && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_can_snapshot(bs) &&
+ bdrv_snapshot_find(bs, snapshot, name) >= 0) {
+ ret = bdrv_snapshot_delete_by_id_or_name(bs, name, err);
+ }
+ aio_context_release(ctx);
+ }
+
+ *first_bad_bs = bs;
+ return ret;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 9ddfd42..d02d2b1 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -82,5 +82,7 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
* 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 **err);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index c212288..1157a6f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1248,35 +1248,6 @@ static BlockDriverState *find_vmstate_bs(void)
return NULL;
}
-/*
- * Deletes snapshots of a given name in all opened images.
- */
-static int del_existing_snapshots(Monitor *mon, const char *name)
-{
- BlockDriverState *bs;
- QEMUSnapshotInfo sn1, *snapshot = &sn1;
- Error *err = NULL;
-
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
- if (bdrv_can_snapshot(bs) &&
- bdrv_snapshot_find(bs, snapshot, name) >= 0) {
- bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
- if (err) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s':"
- " %s\n",
- bdrv_get_device_name(bs),
- error_get_pretty(err));
- error_free(err);
- return -1;
- }
- }
- }
-
- return 0;
-}
-
void hmp_savevm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
@@ -1334,7 +1305,11 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
}
/* Delete old snapshots of the same name */
- if (name && del_existing_snapshots(mon, name) < 0) {
+ if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
+ monitor_printf(mon,
+ "Error while deleting snapshot on device '%s': %s\n",
+ bdrv_get_device_name(bs1), error_get_pretty(local_err));
+ error_free(local_err);
goto the_end;
}
@@ -1494,20 +1469,11 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
return;
}
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
- if (bdrv_can_snapshot(bs)) {
- err = NULL;
- bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
- if (err) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s':"
- " %s\n",
- bdrv_get_device_name(bs),
- error_get_pretty(err));
- error_free(err);
- }
- }
+ if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
+ monitor_printf(mon,
+ "Error while deleting snapshot on device '%s': %s\n",
+ bdrv_get_device_name(bs), error_get_pretty(err));
+ error_free(err);
}
}
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 04/10] snapshot: create bdrv_all_goto_snapshot helper
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (2 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 03/10] snapshot: create bdrv_all_delete_snapshot helper Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 05/10] snapshot: create bdrv_all_find_snapshot helper Denis V. Lunev
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
to switch to snapshot on all loaded block drivers.
The patch also ensures proper locking.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 20 ++++++++++++++++++++
include/block/snapshot.h | 1 +
migration/savevm.c | 15 +++++----------
3 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 61a6ad1..9f07a63 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -403,3 +403,23 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
*first_bad_bs = bs;
return ret;
}
+
+
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
+{
+ int err = 0;
+ BlockDriverState *bs = NULL;
+
+ while (err == 0 && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_can_snapshot(bs)) {
+ err = bdrv_snapshot_goto(bs, name);
+ }
+ aio_context_release(ctx);
+ }
+
+ *first_bad_bs = bs;
+ return err;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d02d2b1..0a176c7 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -84,5 +84,6 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
Error **err);
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 1157a6f..d18ff13 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1425,16 +1425,11 @@ int load_vmstate(const char *name)
/* Flush all IO requests so they don't interfere with the new state. */
bdrv_drain_all();
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
- if (bdrv_can_snapshot(bs)) {
- ret = bdrv_snapshot_goto(bs, name);
- if (ret < 0) {
- error_report("Error %d while activating snapshot '%s' on '%s'",
- ret, name, bdrv_get_device_name(bs));
- return ret;
- }
- }
+ ret = bdrv_all_goto_snapshot(name, &bs);
+ if (ret < 0) {
+ error_report("Error %d while activating snapshot '%s' on '%s'",
+ ret, name, bdrv_get_device_name(bs));
+ return ret;
}
/* restore the VM state */
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 05/10] snapshot: create bdrv_all_find_snapshot helper
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (3 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 04/10] snapshot: create bdrv_all_goto_snapshot helper Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-09 17:27 ` Stefan Hajnoczi
2015-11-07 15:54 ` [Qemu-devel] [PATCH 06/10] migration: drop find_vmstate_bs check in hmp_delvm Denis V. Lunev
` (5 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
to check that snapshot is available for all loaded block drivers. The
ability to switch to snapshot is verified separately using
bdrv_all_can_snapshot.
The patch also ensures proper locking.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 20 ++++++++++++++++++
include/block/snapshot.h | 1 +
migration/savevm.c | 55 +++++++++++++-----------------------------------
3 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 9f07a63..a7f1f8d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -423,3 +423,23 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
*first_bad_bs = bs;
return err;
}
+
+int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
+{
+ QEMUSnapshotInfo sn;
+ int err = 0;
+ BlockDriverState *bs = NULL;
+
+ while (err == 0 && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
+ err = bdrv_snapshot_find(bs, &sn, name);
+ }
+ aio_context_release(ctx);
+ }
+
+ *first_bad_bs = bs;
+ return err;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 0a176c7..10ee582 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -85,5 +85,6 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
Error **err);
int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
+int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index d18ff13..a9c1130 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1383,6 +1383,18 @@ int load_vmstate(const char *name)
QEMUFile *f;
int ret;
+ if (!bdrv_all_can_snapshot(&bs)) {
+ error_report("Device '%s' is writable but does not support snapshots.",
+ bdrv_get_device_name(bs));
+ return -ENOTSUP;
+ }
+ ret = bdrv_all_find_snapshot(name, &bs);
+ if (ret < 0) {
+ error_report("Device '%s' does not have the requested snapshot '%s'",
+ bdrv_get_device_name(bs), name);
+ return ret;
+ }
+
bs_vm_state = find_vmstate_bs();
if (!bs_vm_state) {
error_report("No block device supports snapshots");
@@ -1399,29 +1411,6 @@ int load_vmstate(const char *name)
return -EINVAL;
}
- /* Verify if there is any device that doesn't support snapshots and is
- writable and check if the requested snapshot is available too. */
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
-
- if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
- continue;
- }
-
- if (!bdrv_can_snapshot(bs)) {
- error_report("Device '%s' is writable but does not support snapshots.",
- bdrv_get_device_name(bs));
- return -ENOTSUP;
- }
-
- ret = bdrv_snapshot_find(bs, &sn, name);
- if (ret < 0) {
- error_report("Device '%s' does not have the requested snapshot '%s'",
- bdrv_get_device_name(bs), name);
- return ret;
- }
- }
-
/* Flush all IO requests so they don't interfere with the new state. */
bdrv_drain_all();
@@ -1475,8 +1464,8 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
- QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
- int nb_sns, i, ret, available;
+ QEMUSnapshotInfo *sn_tab, *sn;
+ int nb_sns, i;
int total;
int *available_snapshots;
@@ -1500,21 +1489,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
available_snapshots = g_new0(int, nb_sns);
total = 0;
for (i = 0; i < nb_sns; i++) {
- sn = &sn_tab[i];
- available = 1;
- bs1 = NULL;
-
- while ((bs1 = bdrv_next(bs1))) {
- if (bdrv_can_snapshot(bs1) && bs1 != bs) {
- ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
- if (ret < 0) {
- available = 0;
- break;
- }
- }
- }
-
- if (available) {
+ if (bdrv_all_find_snapshot(sn_tab[i].id_str, &bs1) == 0) {
available_snapshots[total] = i;
total++;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 05/10] snapshot: create bdrv_all_find_snapshot helper
2015-11-07 15:54 ` [Qemu-devel] [PATCH 05/10] snapshot: create bdrv_all_find_snapshot helper Denis V. Lunev
@ 2015-11-09 17:27 ` Stefan Hajnoczi
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2015-11-09 17:27 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Kevin Wolf, qemu-devel, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]
On Sat, Nov 07, 2015 at 06:54:55PM +0300, Denis V. Lunev wrote:
This:
> +int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
> +{
> + QEMUSnapshotInfo sn;
> + int err = 0;
> + BlockDriverState *bs = NULL;
> +
> + while (err == 0 && (bs = bdrv_next(bs))) {
> + AioContext *ctx = bdrv_get_aio_context(bs);
> +
> + aio_context_acquire(ctx);
> + if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
> + err = bdrv_snapshot_find(bs, &sn, name);
> + }
> + aio_context_release(ctx);
> + }
> +
> + *first_bad_bs = bs;
> + return err;
> +}
and this:
> @@ -1500,21 +1489,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
> available_snapshots = g_new0(int, nb_sns);
> total = 0;
> for (i = 0; i < nb_sns; i++) {
> - sn = &sn_tab[i];
> - available = 1;
> - bs1 = NULL;
> -
> - while ((bs1 = bdrv_next(bs1))) {
> - if (bdrv_can_snapshot(bs1) && bs1 != bs) {
> - ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
> - if (ret < 0) {
> - available = 0;
> - break;
> - }
> - }
> - }
> -
> - if (available) {
> + if (bdrv_all_find_snapshot(sn_tab[i].id_str, &bs1) == 0) {
> available_snapshots[total] = i;
> total++;
> }
is not equivalent. hmp_info_snapshots() skips devices that are inserted
and writeable but do not support snapshots. The new code will fail in
that case.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 06/10] migration: drop find_vmstate_bs check in hmp_delvm
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (4 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 05/10] snapshot: create bdrv_all_find_snapshot helper Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper Denis V. Lunev
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Denis V. Lunev, qemu-devel, Juan Quintela
There is no much sense to do the check and write warning.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
---
migration/savevm.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index a9c1130..0c2890d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1448,11 +1448,6 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
Error *err;
const char *name = qdict_get_str(qdict, "name");
- if (!find_vmstate_bs()) {
- monitor_printf(mon, "No block device supports snapshots\n");
- return;
- }
-
if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
monitor_printf(mon,
"Error while deleting snapshot on device '%s': %s\n",
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (5 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 06/10] migration: drop find_vmstate_bs check in hmp_delvm Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-09 17:33 ` Stefan Hajnoczi
2015-11-07 15:54 ` [Qemu-devel] [PATCH 08/10] migration: reorder processing in hmp_savevm Denis V. Lunev
` (3 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
to create snapshot for all loaded block drivers.
The patch also ensures proper locking.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 22 ++++++++++++++++++++++
include/block/snapshot.h | 1 +
migration/savevm.c | 20 +++++---------------
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index a7f1f8d..4daf9d4 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -443,3 +443,25 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
*first_bad_bs = bs;
return err;
}
+
+int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
+{
+ int err = 0;
+ BlockDriverState *bs = NULL;
+
+ while (err == 0 && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_can_snapshot(bs)) {
+ err = bdrv_snapshot_create(bs, sn);
+ /* Tricky part here. First image contains VM state. The behavior
+ * is matched one in bdrv_all_find_vmstate_bs */
+ sn->vm_state_size = 0;
+ }
+ aio_context_release(ctx);
+ }
+
+ *bad = bs;
+ return err;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 10ee582..df443b2 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -86,5 +86,6 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
Error **err);
int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
+int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad);
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 0c2890d..f72349d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1255,7 +1255,6 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
int ret;
QEMUFile *f;
int saved_vm_running;
- uint64_t vm_state_size;
qemu_timeval tv;
struct tm tm;
const char *name = qdict_get_try_str(qdict, "name");
@@ -1320,7 +1319,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
goto the_end;
}
ret = qemu_savevm_state(f, &local_err);
- vm_state_size = qemu_ftell(f);
+ sn->vm_state_size = qemu_ftell(f);
qemu_fclose(f);
if (ret < 0) {
monitor_printf(mon, "%s\n", error_get_pretty(local_err));
@@ -1328,19 +1327,10 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
goto the_end;
}
- /* create the snapshots */
-
- bs1 = NULL;
- while ((bs1 = bdrv_next(bs1))) {
- if (bdrv_can_snapshot(bs1)) {
- /* Write VM state size only to the image that contains the state */
- sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
- ret = bdrv_snapshot_create(bs1, sn);
- if (ret < 0) {
- monitor_printf(mon, "Error while creating snapshot on '%s'\n",
- bdrv_get_device_name(bs1));
- }
- }
+ ret = bdrv_all_create_snapshot(sn, &bs);
+ if (ret < 0) {
+ monitor_printf(mon, "Error while creating snapshot on '%s'\n",
+ bdrv_get_device_name(bs));
}
the_end:
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper
2015-11-07 15:54 ` [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper Denis V. Lunev
@ 2015-11-09 17:33 ` Stefan Hajnoczi
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2015-11-09 17:33 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Kevin Wolf, qemu-devel, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
On Sat, Nov 07, 2015 at 06:54:57PM +0300, Denis V. Lunev wrote:
> +int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
> +{
> + int err = 0;
> + BlockDriverState *bs = NULL;
> +
> + while (err == 0 && (bs = bdrv_next(bs))) {
> + AioContext *ctx = bdrv_get_aio_context(bs);
> +
> + aio_context_acquire(ctx);
> + if (bdrv_can_snapshot(bs)) {
> + err = bdrv_snapshot_create(bs, sn);
> + /* Tricky part here. First image contains VM state. The behavior
> + * is matched one in bdrv_all_find_vmstate_bs */
> + sn->vm_state_size = 0;
Please avoid the tricky part by passing in vm_state_bs and
vm_state_size. Then this function can do:
/* Write VM state size only to the image that contains the state */
sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
without making assumptions about the algorithm for choosing the device
to store vmstate data on.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 08/10] migration: reorder processing in hmp_savevm
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (6 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:54 ` [Qemu-devel] [PATCH 09/10] migration: implement bdrv_all_find_vmstate_bs helper Denis V. Lunev
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Denis V. Lunev, qemu-devel, Juan Quintela
State deletion can be performed on running VM which reduces VM downtime
This approach looks a bit more natural.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
---
migration/savevm.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index f72349d..b4a3930 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1266,6 +1266,15 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
return;
}
+ /* Delete old snapshots of the same name */
+ if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
+ monitor_printf(mon,
+ "Error while deleting snapshot on device '%s': %s\n",
+ bdrv_get_device_name(bs1), error_get_pretty(local_err));
+ error_free(local_err);
+ return;
+ }
+
bs = find_vmstate_bs();
if (!bs) {
monitor_printf(mon, "No block device can accept snapshots\n");
@@ -1303,15 +1312,6 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm);
}
- /* Delete old snapshots of the same name */
- if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s': %s\n",
- bdrv_get_device_name(bs1), error_get_pretty(local_err));
- error_free(local_err);
- goto the_end;
- }
-
/* save the VM state */
f = qemu_fopen_bdrv(bs, 1);
if (!f) {
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 09/10] migration: implement bdrv_all_find_vmstate_bs helper
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (7 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 08/10] migration: reorder processing in hmp_savevm Denis V. Lunev
@ 2015-11-07 15:54 ` Denis V. Lunev
2015-11-07 15:55 ` [Qemu-devel] [PATCH 10/10] migration: normalize locking in migration/savevm.c Denis V. Lunev
2015-11-09 17:37 ` [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Stefan Hajnoczi
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:54 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
The patch also ensures proper locking for the operation.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 15 +++++++++++++++
include/block/snapshot.h | 2 ++
migration/savevm.c | 19 ++++---------------
3 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 4daf9d4..337ac8e 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -465,3 +465,18 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
*bad = bs;
return err;
}
+
+BlockDriverState *bdrv_all_find_vmstate_bs(void)
+{
+ bool not_found = true;
+ BlockDriverState *bs = NULL;
+
+ while (not_found && (bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ not_found = !bdrv_can_snapshot(bs);
+ aio_context_release(ctx);
+ }
+ return bs;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index df443b2..543cbb3 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -88,4 +88,6 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad);
+BlockDriverState *bdrv_all_find_vmstate_bs(void);
+
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index b4a3930..6be30cb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1237,17 +1237,6 @@ out:
return ret;
}
-static BlockDriverState *find_vmstate_bs(void)
-{
- BlockDriverState *bs = NULL;
- while ((bs = bdrv_next(bs))) {
- if (bdrv_can_snapshot(bs)) {
- return bs;
- }
- }
- return NULL;
-}
-
void hmp_savevm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
@@ -1275,8 +1264,8 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
return;
}
- bs = find_vmstate_bs();
- if (!bs) {
+ bs = bdrv_all_find_vmstate_bs();
+ if (bs == NULL) {
monitor_printf(mon, "No block device can accept snapshots\n");
return;
}
@@ -1385,7 +1374,7 @@ int load_vmstate(const char *name)
return ret;
}
- bs_vm_state = find_vmstate_bs();
+ bs_vm_state = bdrv_all_find_vmstate_bs();
if (!bs_vm_state) {
error_report("No block device supports snapshots");
return -ENOTSUP;
@@ -1454,7 +1443,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
int total;
int *available_snapshots;
- bs = find_vmstate_bs();
+ bs = bdrv_all_find_vmstate_bs();
if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n");
return;
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 10/10] migration: normalize locking in migration/savevm.c
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (8 preceding siblings ...)
2015-11-07 15:54 ` [Qemu-devel] [PATCH 09/10] migration: implement bdrv_all_find_vmstate_bs helper Denis V. Lunev
@ 2015-11-07 15:55 ` Denis V. Lunev
2015-11-09 17:37 ` [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Stefan Hajnoczi
10 siblings, 0 replies; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-07 15:55 UTC (permalink / raw)
Cc: Kevin Wolf, Denis V. Lunev, qemu-devel, Stefan Hajnoczi, Juan Quintela
basically all bdrv_* operations must be called under aio_context_acquire
except ones with bdrv_all prefix.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
migration/savevm.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 6be30cb..5e6ff55 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1248,6 +1248,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
struct tm tm;
const char *name = qdict_get_try_str(qdict, "name");
Error *local_err = NULL;
+ AioContext *aio_context;
if (!bdrv_all_can_snapshot(&bs)) {
monitor_printf(mon, "Device '%s' is writable but does not "
@@ -1269,6 +1270,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "No block device can accept snapshots\n");
return;
}
+ aio_context = bdrv_get_aio_context(bs);
saved_vm_running = runstate_is_running();
@@ -1279,6 +1281,8 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
}
vm_stop(RUN_STATE_SAVE_VM);
+ aio_context_acquire(aio_context);
+
memset(sn, 0, sizeof(*sn));
/* fill auxiliary fields */
@@ -1323,6 +1327,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
}
the_end:
+ aio_context_release(aio_context);
if (saved_vm_running) {
vm_start();
}
@@ -1361,6 +1366,7 @@ int load_vmstate(const char *name)
QEMUSnapshotInfo sn;
QEMUFile *f;
int ret;
+ AioContext *aio_context;
if (!bdrv_all_can_snapshot(&bs)) {
error_report("Device '%s' is writable but does not support snapshots.",
@@ -1379,9 +1385,12 @@ int load_vmstate(const char *name)
error_report("No block device supports snapshots");
return -ENOTSUP;
}
+ aio_context = bdrv_get_aio_context(bs);
/* Don't even try to load empty VM states */
+ aio_context_acquire(aio_context);
ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
+ aio_context_release(aio_context);
if (ret < 0) {
return ret;
} else if (sn.vm_state_size == 0) {
@@ -1409,9 +1418,12 @@ int load_vmstate(const char *name)
qemu_system_reset(VMRESET_SILENT);
migration_incoming_state_new(f);
- ret = qemu_loadvm_state(f);
+ aio_context_acquire(aio_context);
+ ret = qemu_loadvm_state(f);
qemu_fclose(f);
+ aio_context_release(aio_context);
+
migration_incoming_state_destroy();
if (ret < 0) {
error_report("Error %d while loading VM state", ret);
@@ -1442,14 +1454,19 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
int nb_sns, i;
int total;
int *available_snapshots;
+ AioContext *aio_context;
bs = bdrv_all_find_vmstate_bs();
if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n");
return;
}
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+ aio_context_release(aio_context);
+
if (nb_sns < 0) {
monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
return;
--
2.5.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes
2015-11-07 15:54 [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Denis V. Lunev
` (9 preceding siblings ...)
2015-11-07 15:55 ` [Qemu-devel] [PATCH 10/10] migration: normalize locking in migration/savevm.c Denis V. Lunev
@ 2015-11-09 17:37 ` Stefan Hajnoczi
2015-11-09 17:57 ` Denis V. Lunev
10 siblings, 1 reply; 16+ messages in thread
From: Stefan Hajnoczi @ 2015-11-09 17:37 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Kevin Wolf, qemu-devel, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 884 bytes --]
On Sat, Nov 07, 2015 at 06:54:50PM +0300, Denis V. Lunev wrote:
> with test
> while /bin/true ; do
> virsh snapshot-create rhel7
> sleep 10
> virsh snapshot-delete rhel7 --current
> done
> with enabled iothreads on a running VM leads to a lot of troubles: hangs,
> asserts, errors.
>
> Anyway, I think that the construction like
> assert(aio_context_is_locked(aio_context));
> should be widely used to ensure proper locking.
>
> Changes from v5:
> - dropped already merged patch 11
> - fixed spelling in patch 1
> - changed order of condition in loops in all patches. Thank you Stefan :)
> - dropped patch 9
> - aio_context is not acquired any more in bdrv_all_find_vmstate_bs by request
> of Stefan
> - patch 10 is implemented in completely different way
I left comments on specific patches. Besides that, I'm happy.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes
2015-11-09 17:37 ` [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes Stefan Hajnoczi
@ 2015-11-09 17:57 ` Denis V. Lunev
2015-11-10 9:45 ` Stefan Hajnoczi
0 siblings, 1 reply; 16+ messages in thread
From: Denis V. Lunev @ 2015-11-09 17:57 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Juan Quintela
On 11/09/2015 08:37 PM, Stefan Hajnoczi wrote:
> On Sat, Nov 07, 2015 at 06:54:50PM +0300, Denis V. Lunev wrote:
>> with test
>> while /bin/true ; do
>> virsh snapshot-create rhel7
>> sleep 10
>> virsh snapshot-delete rhel7 --current
>> done
>> with enabled iothreads on a running VM leads to a lot of troubles: hangs,
>> asserts, errors.
>>
>> Anyway, I think that the construction like
>> assert(aio_context_is_locked(aio_context));
>> should be widely used to ensure proper locking.
>>
>> Changes from v5:
>> - dropped already merged patch 11
>> - fixed spelling in patch 1
>> - changed order of condition in loops in all patches. Thank you Stefan :)
>> - dropped patch 9
>> - aio_context is not acquired any more in bdrv_all_find_vmstate_bs by request
>> of Stefan
>> - patch 10 is implemented in completely different way
> I left comments on specific patches. Besides that, I'm happy.
OK. that sounds good enough to me. These changes
are not a problem at all.
Should we ask Juan that this is good for him?
Den
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH for 2.5 v6 0/10] dataplane snapshot fixes
2015-11-09 17:57 ` Denis V. Lunev
@ 2015-11-10 9:45 ` Stefan Hajnoczi
0 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2015-11-10 9:45 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Kevin Wolf, qemu-devel, Juan Quintela
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
On Mon, Nov 09, 2015 at 08:57:26PM +0300, Denis V. Lunev wrote:
> On 11/09/2015 08:37 PM, Stefan Hajnoczi wrote:
> >On Sat, Nov 07, 2015 at 06:54:50PM +0300, Denis V. Lunev wrote:
> >>with test
> >> while /bin/true ; do
> >> virsh snapshot-create rhel7
> >> sleep 10
> >> virsh snapshot-delete rhel7 --current
> >> done
> >>with enabled iothreads on a running VM leads to a lot of troubles: hangs,
> >>asserts, errors.
> >>
> >>Anyway, I think that the construction like
> >> assert(aio_context_is_locked(aio_context));
> >>should be widely used to ensure proper locking.
> >>
> >>Changes from v5:
> >>- dropped already merged patch 11
> >>- fixed spelling in patch 1
> >>- changed order of condition in loops in all patches. Thank you Stefan :)
> >>- dropped patch 9
> >>- aio_context is not acquired any more in bdrv_all_find_vmstate_bs by request
> >> of Stefan
> >>- patch 10 is implemented in completely different way
> >I left comments on specific patches. Besides that, I'm happy.
> OK. that sounds good enough to me. These changes
> are not a problem at all.
>
> Should we ask Juan that this is good for him?
I think the requested changes won't be controversial. Sending the next
revision should be fine.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread