On 11/06/2015 05:09 PM, Stefan Hajnoczi wrote:
On Wed, Nov 04, 2015 at 08:19:34PM +0300, Denis V. Lunev wrote:
to delete snapshots from all loaded block drivers.
The patch also ensures proper locking.
Signed-off-by: Denis V. Lunev <d...@openvz.org>
CC: Juan Quintela <quint...@redhat.com>
CC: Stefan Hajnoczi <stefa...@redhat.com>
CC: Kevin Wolf <kw...@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 d729c76..1b4b846 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -384,3 +384,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 ((bs = bdrv_next(bs)) && ret == 0) {
If ret != 0 we will iterate to the next bs. first_bad_bs will be
incorrect.
+ 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;
+}
Similar approach without the int return value:
bool bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
Error **err)
{
Error *local_err = NULL;
BlockDriverState *bs = NULL;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
while ((bs = bdrv_next(bs)) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(bs);
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0) {
bdrv_snapshot_delete_by_id_or_name(bs, name, &local_err);
}
aio_context_release(bs);
if (local_err) {
error_propagate(err, local_err);
return false;
}
}
return true;
}
there are 2 notes.
Personally I do not like 'bool' functions as it is unclear
whether true means success or failure. Usually false is
failure but I have faced awkward situation when conditions
where reversed.
<0 and >=0 conditions are alive for 10th of years.
secondly the you will have to assign first_bad_bs two times.
I have started from this version of code and merged them
later to the current approach. Ok, I have made a mistake :(
Den
