Patchwork Fix -snapshot deleting images on disk change

login
register
mail settings
Submitter Blue Swirl
Date July 25, 2010, 8:49 p.m.
Message ID <AANLkTimFu_z+HLRw+O2KM9EwJVS7U1DC1Adt4UKZxRjs@mail.gmail.com>
Download mbox | patch
Permalink /patch/59881/
State New
Headers show

Comments

Blue Swirl - July 25, 2010, 8:49 p.m.
Block device change command did not copy BDRV_O_SNAPSHOT flag. Thus
the new image did not have this flag and the file got deleted during
opening.

Fix by copying BDRV_O_SNAPSHOT flag.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 block.c    |    5 +++++
 block.h    |    1 +
 blockdev.c |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)
Markus Armbruster - July 26, 2010, 6:58 a.m.
Blue Swirl <blauwirbel@gmail.com> writes:

> Block device change command did not copy BDRV_O_SNAPSHOT flag. Thus
> the new image did not have this flag and the file got deleted during
> opening.
>
> Fix by copying BDRV_O_SNAPSHOT flag.

Aha: the file gets deleted because bs->is_temporary survives change, but
BDRV_O_SNAPSHOT does not.  Thus, the new image gets opened without a
snapshot file wrapped around it.  But since is_temporary is still true,
we attempt to delete the (nonexistant) snapshot file, and end up
deleting the image file instead.

Your fix makes sense.  But I recommend to additionally set
bs->is_temporary unconditionally in bdrv_open().  Or get rid of it and
test bs->open_flags instead.
Kevin Wolf - July 26, 2010, 9:03 a.m.
Am 25.07.2010 22:49, schrieb Blue Swirl:
> Block device change command did not copy BDRV_O_SNAPSHOT flag. Thus
> the new image did not have this flag and the file got deleted during
> opening.
> 
> Fix by copying BDRV_O_SNAPSHOT flag.
> 
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

Thanks, applied to the block branch.

> diff --git a/block.h b/block.h
> index c2a7e4c..db131a3 100644
> --- a/block.h
> +++ b/block.h
> @@ -202,6 +202,7 @@ const char
> *bdrv_get_encrypted_filename(BlockDriverState *bs);

Please avoid wrapped lines when you send patches, e.g. by using git
send-email.

Kevin

Patch

diff --git a/block.c b/block.c
index f837876..14cfed1 100644
--- a/block.c
+++ b/block.c
@@ -1800,6 +1800,11 @@  int bdrv_can_snapshot(BlockDriverState *bs)
     return 1;
 }

+int bdrv_is_snapshot(BlockDriverState *bs)
+{
+    return !!(bs->open_flags & BDRV_O_SNAPSHOT);
+}
+
 BlockDriverState *bdrv_snapshots(void)
 {
     BlockDriverState *bs;
diff --git a/block.h b/block.h
index c2a7e4c..db131a3 100644
--- a/block.h
+++ b/block.h
@@ -202,6 +202,7 @@  const char
*bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 int bdrv_can_snapshot(BlockDriverState *bs);
+int bdrv_is_snapshot(BlockDriverState *bs);
 BlockDriverState *bdrv_snapshots(void);
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info);
diff --git a/blockdev.c b/blockdev.c
index 0a9dec3..01e402b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -590,6 +590,7 @@  int do_change_block(Monitor *mon, const char *device,
         return -1;
     }
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
     if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
         qerror_report(QERR_OPEN_FILE_FAILED, filename);
         return -1;