block: Error out on load_vm with active dirty bitmaps
cover block: Error out on load_vm with active dirty bitmaps
Following the discussing at
https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg03572.html
this implements the error return for loading a snapshot while dirty bitmaps are active.
Kevin Wolf (3):
block: Add errp to bdrv_snapshot_goto()
block: Add errp to bdrv_all_goto_snapshot()
block: Error out on load_vm with active dirty bitmaps
include/block/snapshot.h | 6 ++++-- block/snapshot.c | 43 +++++++++++++++++++++++-------------------- migration/savevm.c | 6 +++--- qemu-img.c | 6 +++--- 4 files changed, 33 insertions(+), 28 deletions(-)
Signature · 1 line
2.13.6
PATCH 1/3 block: Add errp to bdrv_snapshot_goto()
Signed-off-by Kevin Wolf <[email protected]>
--- include/block/snapshot.h | 3 ++- block/snapshot.c | 21 ++++++++++++++++----- qemu-img.c | 6 +++--- 3 files changed, 21 insertions(+), 9 deletions(-)
include/block/snapshot.h +2 -1
@@ -57,7 +57,8 @@ int bdrv_can_snapshot(BlockDriverState *bs);int bdrv_snapshot_create (BlockDriverState *bs,QEMUSnapshotInfo *sn_info);int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id); +const char *snapshot_id, +Error **errp); int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, const char *name,
block/snapshot.c +16 -5
@@ -177,18 +177,21 @@ int bdrv_snapshot_create(BlockDriverState *bs, }int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id) +const char *snapshot_id, +Error **errp) {BlockDriver *drv = bs->drv;int ret, open_ret;int64_t len;if (!drv) { +error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } len =bdrv_getlength (bs);if (len <0 ) { +error_setg_errno (errp, -len,"Cannot get block device size" );return len; }/* We should set all bits in all enabled dirty bitmaps, because dirty
@@ -200,13 +203,18 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { -return drv->bdrv_snapshot_goto (bs, snapshot_id); + ret = drv->bdrv_snapshot_goto (bs, snapshot_id); +if (ret <0 ) { +error_setg_errno (errp, -ret,"Failed to load snapshot" ); + } +return ret; }if (bs->file) {BlockDriverState *file;QDict *options =qdict_clone_shallow (bs->options);QDict *file_options; +Error *local_err =NULL ; file = bs->file->bs;/* Prevent it from getting deleted when detached from bs */
@@ -220,12 +228,14 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_unref_child (bs, bs->file); bs->file =NULL ; - ret =bdrv_snapshot_goto (file, snapshot_id); - open_ret = drv->bdrv_open (bs, options, bs->open_flags,NULL ); + ret =bdrv_snapshot_goto (file, snapshot_id, errp); + open_ret = drv->bdrv_open (bs, options, bs->open_flags, &local_err);QDECREF (options);if (open_ret <0 ) {bdrv_unref (file); bs->drv =NULL ; +/* A bdrv_snapshot_goto() error takes precedence */ +error_propagate (errp, local_err);return open_ret; }
@@ -234,6 +244,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,return ret; } +error_setg (errp,"Block driver does not support snapshots" );return -ENOTSUP ; }
@@ -467,7 +478,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)aio_context_acquire (ctx);if (bdrv_can_snapshot (bs)) { - err =bdrv_snapshot_goto (bs, name); + err =bdrv_snapshot_goto (bs, name,NULL ); }aio_context_release (ctx);if (err <0 ) {
qemu-img.c +3 -3
@@ -2989,10 +2989,10 @@ static int img_snapshot(int argc, char **argv)break ;case SNAPSHOT_APPLY : - ret =bdrv_snapshot_goto (bs, snapshot_name); + ret =bdrv_snapshot_goto (bs, snapshot_name, &err);if (ret) { -error_report ("Could not apply snapshot '%s': %d (%s)" , - snapshot_name, ret,strerror (-ret)); +error_reportf_err (err,"Could not apply snapshot '%s': " , + snapshot_name); }break ;
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH for-2.11 1/3] block: Add errp to bdrv_snapshot_goto()
Quoted 71 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- include/block/snapshot.h | 3 ++- block/snapshot.c | 21 ++++++++++++++++----- qemu-img.c | 6 +++--- 3 files changed, 21 insertions(+), 9 deletions(-)include/block/snapshot.h +2
-1@@ -57,7 +57,8 @@ int bdrv_can_snapshot(BlockDriverState *bs);int bdrv_snapshot_create (BlockDriverState *bs,QEMUSnapshotInfo *sn_info);int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id); +const char *snapshot_id, +Error **errp); int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, const char *name,block/snapshot.c +10
-2@@ -177,18 +177,21 @@ int bdrv_snapshot_create(BlockDriverState *bs, }int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id) +const char *snapshot_id, +Error **errp) {BlockDriver *drv = bs->drv;int ret, open_ret;int64_t len;if (!drv) { +error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } len =bdrv_getlength (bs);if (len <0 ) { +error_setg_errno (errp, -len,"Cannot get block device size" );return len; }/* We should set all bits in all enabled dirty bitmaps, because dirty @@ -200,13 +203,18 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { -return drv->bdrv_snapshot_goto (bs, snapshot_id); + ret = drv->bdrv_snapshot_goto (bs, snapshot_id); +if (ret <0 ) { +error_setg_errno (errp, -ret,"Failed to load snapshot" ); + } +return ret; }if (bs->file) {BlockDriverState *file;QDict *options =qdict_clone_shallow (bs->options);QDict *file_options; +Error *local_err =NULL ; file = bs->file->bs;/* Prevent it from getting deleted when detached from bs */ @@ -220,12 +228,14 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_unref_child (bs, bs->file); bs->file =NULL ;
block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 237
Quoted 10 lines
block/snapshot.c +3
-2- ret =bdrv_snapshot_goto (file, snapshot_id); - open_ret = drv->bdrv_open (bs, options, bs->open_flags,NULL ); + ret =bdrv_snapshot_goto (file, snapshot_id, errp); + open_ret = drv->bdrv_open (bs, options, bs->open_flags, &local_err);QDECREF (options);if (open_ret <0 ) {bdrv_unref (file); bs->drv =NULL ; +/* A bdrv_snapshot_goto() error takes precedence */
you return open_ret from bdrv_open and err msg from bdrv_snapshot_goto. the may not correspond to each other.
Quoted 39 lines
block/snapshot.c +3
-1+error_propagate (errp, local_err);return open_ret; }@@ -234,6 +244,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,return ret; } +error_setg (errp,"Block driver does not support snapshots" );return -ENOTSUP ; }@@ -467,7 +478,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)aio_context_acquire (ctx);if (bdrv_can_snapshot (bs)) { - err =bdrv_snapshot_goto (bs, name); + err =bdrv_snapshot_goto (bs, name,NULL ); }aio_context_release (ctx);if (err <0 ) {qemu-img.c +3
-3@@ -2989,10 +2989,10 @@ static int img_snapshot(int argc, char **argv)break ;case SNAPSHOT_APPLY : - ret =bdrv_snapshot_goto (bs, snapshot_name); + ret =bdrv_snapshot_goto (bs, snapshot_name, &err);if (ret) { -error_report ("Could not apply snapshot '%s': %d (%s)" , - snapshot_name, ret,strerror (-ret)); +error_reportf_err (err,"Could not apply snapshot '%s': " , + snapshot_name); }break ;
Signature · 2 lines
Best regards, Vladimir
Re: [Qemu-devel] [PATCH for-2.11 1/3] block: Add errp to bdrv_snapshot_goto()
Am 20.11.2017 um 17:07 hat Vladimir Sementsov-Ogievskiy geschrieben:
Quoted 70 lines
Quoted 69 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- include/block/snapshot.h | 3 ++- block/snapshot.c | 21 ++++++++++++++++----- qemu-img.c | 6 +++--- 3 files changed, 21 insertions(+), 9 deletions(-)include/block/snapshot.h +2
-1@@ -57,7 +57,8 @@ int bdrv_can_snapshot(BlockDriverState *bs);int bdrv_snapshot_create (BlockDriverState *bs,QEMUSnapshotInfo *sn_info);int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id); +const char *snapshot_id, +Error **errp); int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, const char *name,block/snapshot.c +12
-4@@ -177,18 +177,21 @@ int bdrv_snapshot_create(BlockDriverState *bs, }int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id) +const char *snapshot_id, +Error **errp) {BlockDriver *drv = bs->drv;int ret, open_ret;int64_t len;if (!drv) { +error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } len =bdrv_getlength (bs);if (len <0 ) { +error_setg_errno (errp, -len,"Cannot get block device size" );return len; }/* We should set all bits in all enabled dirty bitmaps, because dirty @@ -200,13 +203,18 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { -return drv->bdrv_snapshot_goto (bs, snapshot_id); + ret = drv->bdrv_snapshot_goto (bs, snapshot_id); +if (ret <0 ) { +error_setg_errno (errp, -ret,"Failed to load snapshot" ); + } +return ret; }if (bs->file) {BlockDriverState *file;QDict *options =qdict_clone_shallow (bs->options);QDict *file_options; +Error *local_err =NULL ; file = bs->file->bs;/* Prevent it from getting deleted when detached from bs */ @@ -220,12 +228,14 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_unref_child (bs, bs->file); bs->file =NULL ; - ret =bdrv_snapshot_goto (file, snapshot_id); - open_ret = drv->bdrv_open (bs, options, bs->open_flags,NULL ); + ret =bdrv_snapshot_goto (file, snapshot_id, errp); + open_ret = drv->bdrv_open (bs, options, bs->open_flags, &local_err);
Quoted 8 lines
block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 237
Quoted 5 lines
block/snapshot.c +1
-0QDECREF (options);if (open_ret <0 ) {bdrv_unref (file); bs->drv =NULL ; +/* A bdrv_snapshot_goto() error takes precedence */ you return
open_retfrombdrv_openand err msg frombdrv_snapshot_goto. the may not correspond to each other.
Hm, yes. Is this a problem, though?
If it is, I guess we can change the return value below:
Quoted 2 lines
block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 239
Quoted 2 lines
block/snapshot.c +1
-0+error_propagate (errp, local_err);return open_ret;
return ret < 0 ? ret : open_ret;
Would people prefer this?
Kevin
Vladimir Sementsov-Ogievskiy
Reviewed-byRe: [Qemu-devel] [PATCH for-2.11 1/3] block: Add errp to bdrv_snapshot_goto()
Quoted 80 lines · Kevin Wolf
Am 20.11.2017 um 17:07 hat Vladimir Sementsov-Ogievskiy geschrieben:
Quoted 69 lines
Quoted 68 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- include/block/snapshot.h | 3 ++- block/snapshot.c | 21 ++++++++++++++++----- qemu-img.c | 6 +++--- 3 files changed, 21 insertions(+), 9 deletions(-)include/block/snapshot.h +2
-1@@ -57,7 +57,8 @@ int bdrv_can_snapshot(BlockDriverState *bs);int bdrv_snapshot_create (BlockDriverState *bs,QEMUSnapshotInfo *sn_info);int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id); +const char *snapshot_id, +Error **errp); int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, const char *name,block/snapshot.c +11
-4@@ -177,18 +177,21 @@ int bdrv_snapshot_create(BlockDriverState *bs, }int bdrv_snapshot_goto (BlockDriverState *bs, -const char *snapshot_id) +const char *snapshot_id, +Error **errp) {BlockDriver *drv = bs->drv;int ret, open_ret;int64_t len;if (!drv) { +error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } len =bdrv_getlength (bs);if (len <0 ) { +error_setg_errno (errp, -len,"Cannot get block device size" );return len; }/* We should set all bits in all enabled dirty bitmaps, because dirty @@ -200,13 +203,18 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { -return drv->bdrv_snapshot_goto (bs, snapshot_id); + ret = drv->bdrv_snapshot_goto (bs, snapshot_id); +if (ret <0 ) { +error_setg_errno (errp, -ret,"Failed to load snapshot" ); + } +return ret; }if (bs->file) {BlockDriverState *file;QDict *options =qdict_clone_shallow (bs->options);QDict *file_options; +Error *local_err =NULL ; file = bs->file->bs;/* Prevent it from getting deleted when detached from bs */ @@ -220,12 +228,14 @@ int bdrv_snapshot_goto(BlockDriverState *bs,bdrv_unref_child (bs, bs->file); bs->file =NULL ; - ret =bdrv_snapshot_goto (file, snapshot_id); - open_ret = drv->bdrv_open (bs, options, bs->open_flags,NULL ); + ret =bdrv_snapshot_goto (file, snapshot_id, errp);Quoted 8 lines
block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 237
Quoted 6 lines
block/snapshot.c +2
-0+ open_ret = drv->bdrv_open (bs, options, bs->open_flags, &local_err);QDECREF (options);if (open_ret <0 ) {bdrv_unref (file); bs->drv =NULL ; +/* A bdrv_snapshot_goto() error takes precedence */ you return
open_retfrombdrv_openand err msg frombdrv_snapshot_goto. the may not correspond to each other.Hm, yes. Is this a problem, though?
Quoted 7 lines
If it is, I guess we can change the return value below:
Quoted 2 lines
block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 239
Quoted 2 lines
block/snapshot.c +1
-0+error_propagate (errp, local_err);return open_ret;return ret < 0 ? ret :
open_ret;Would people prefer this?
looks good,
Reviewed-by Vladimir Sementsov-Ogievskiy <[email protected]>
Quoted 2 lines
Kevin
Signature · 2 lines
Best regards, Vladimir
PATCH 2/3 block: Add errp to bdrv_all_goto_snapshot()
In reply to Kevin WolfSigned-off-by Kevin Wolf <[email protected]>
--- include/block/snapshot.h | 3 ++- block/snapshot.c | 11 ++++++----- migration/savevm.c | 6 +++--- 3 files changed, 11 insertions(+), 9 deletions(-)
include/block/snapshot.h +2 -1
@@ -84,7 +84,8 @@ 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); +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); int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState *vm_state_bs,
block/snapshot.c +6 -5
@@ -467,9 +467,10 @@ fail: } -int bdrv_all_goto_snapshot (const char *name,BlockDriverState **first_bad_bs) +int bdrv_all_goto_snapshot (const char *name,BlockDriverState **first_bad_bs, +Error **errp) { -int err =0 ; +int ret =0 ;BlockDriverState *bs;BdrvNextIterator it;
@@ -478,10 +479,10 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)aio_context_acquire (ctx);if (bdrv_can_snapshot (bs)) { - err =bdrv_snapshot_goto (bs, name,NULL ); + ret =bdrv_snapshot_goto (bs, name, errp); }aio_context_release (ctx); -if (err <0 ) { +if (ret <0 ) {bdrv_next_cleanup (&it); goto fail; }
@@ -489,7 +490,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs) fail: *first_bad_bs = bs; -return err; +return ret; }int bdrv_all_find_snapshot (const char *name,BlockDriverState **first_bad_bs)
migration/savevm.c +3 -3
@@ -2346,10 +2346,10 @@ 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); + ret =bdrv_all_goto_snapshot (name, &bs, errp);if (ret <0 ) { -error_setg (errp,"Error %d while activating snapshot '%s' on '%s'" , - ret, name,bdrv_get_device_name (bs)); +error_prepend (errp,"Could not load snapshot '%s' on '%s': " , + name,bdrv_get_device_name (bs)); goto err_drain; }
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH for-2.11 2/3] block: Add errp to bdrv_all_goto_snapshot()
Quoted 65 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- include/block/snapshot.h | 3 ++- block/snapshot.c | 11 ++++++----- migration/savevm.c | 6 +++--- 3 files changed, 11 insertions(+), 9 deletions(-)include/block/snapshot.h +2
-1@@ -84,7 +84,8 @@ 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); +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); int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState *vm_state_bs,block/snapshot.c +6
-5@@ -467,9 +467,10 @@ fail: } -int bdrv_all_goto_snapshot (const char *name,BlockDriverState **first_bad_bs) +int bdrv_all_goto_snapshot (const char *name,BlockDriverState **first_bad_bs, +Error **errp) { -int err =0 ; +int ret =0 ;BlockDriverState *bs;BdrvNextIterator it;@@ -478,10 +479,10 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)aio_context_acquire (ctx);if (bdrv_can_snapshot (bs)) { - err =bdrv_snapshot_goto (bs, name,NULL ); + ret =bdrv_snapshot_goto (bs, name, errp); }aio_context_release (ctx); -if (err <0 ) { +if (ret <0 ) {bdrv_next_cleanup (&it); goto fail; }@@ -489,7 +490,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs) fail: *first_bad_bs = bs; -return err; +return ret; }int bdrv_all_find_snapshot (const char *name,BlockDriverState **first_bad_bs)migration/savevm.c +0
-0
migration/savevm.c int load_snapshot(const char *name, Error **errp) · line 2352
Quoted 10 lines
migration/savevm.c +3
-3@@ -2346,10 +2346,10 @@ 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); + ret =bdrv_all_goto_snapshot (name, &bs, errp);if (ret <0 ) { -error_setg (errp,"Error %d while activating snapshot '%s' on '%s'" , - ret, name,bdrv_get_device_name (bs)); +error_prepend (errp,"Could not load snapshot '%s' on '%s': " , + name,bdrv_get_device_name (bs));
possible resulting msg: "Could not load snapshot 'snap0' on 'dev0': Failed to load snapshot". a bit weird. with this fixed or not:
Reviewed-by Vladimir Sementsov-Ogievskiy <[email protected]>
Quoted 3 lines
migration/savevm.c +0
-0goto err_drain; }
Signature · 2 lines
Best regards, Vladimir
PATCH 3/3 block: Error out on load_vm with active dirty bitmaps
In reply to Kevin WolfLoading a snapshot invalidates the bitmap. Just marking all blocks dirty is not a useful response in practice, instead the user needs to be aware that we switch to a completely different state. If they are okay with losing the dirty bitmap, they can just explicitly delete it.
This effectively reverts commit 04dec3c3ae5.
Signed-off-by Kevin Wolf <[email protected]>
--- block/snapshot.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)
block/snapshot.c +3 -12
@@ -182,25 +182,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs, {BlockDriver *drv = bs->drv;int ret, open_ret; -int64_t len;if (!drv) {error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } - len =bdrv_getlength (bs); -if (len <0 ) { -error_setg_errno (errp, -len,"Cannot get block device size" ); -return len; +if (!QLIST_EMPTY (&bs->dirty_bitmaps)) { +error_setg (errp,"Device has active dirty bitmaps" ); +return -EBUSY ; } -/* We should set all bits in all enabled dirty bitmaps, because dirty -* bitmaps reflect active state of disk and snapshot switch operation -* actually dirties active state. -* TODO: It may make sense not to set all bits but analyze block status of -* current state and destination snapshot and do not set bits corresponding -* to both-zero or both-unallocated areas. */ -bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { ret = drv->bdrv_snapshot_goto (bs, snapshot_id);
Signature · 1 line
2.13.6
Quoted 8 lines · Kevin Wolf
Loading a snapshot invalidates the bitmap. Just marking all blocks dirty is not a useful response in practice, instead the user needs to be aware that we switch to a completely different state. If they are okay with losing the dirty bitmap, they can just explicitly delete it.
This effectively reverts commit
04dec3c3ae5.Signed-off-by Kevin Wolf <[email protected]>
Reviewed-by Vladimir Sementsov-Ogievskiy <[email protected]>
Quoted 37 lines
--- block/snapshot.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)block/snapshot.c +3
-12@@ -182,25 +182,16 @@ int bdrv_snapshot_goto(BlockDriverState *bs, {BlockDriver *drv = bs->drv;int ret, open_ret; -int64_t len;if (!drv) {error_setg (errp,"Block driver is closed" );return -ENOMEDIUM ; } - len =bdrv_getlength (bs); -if (len <0 ) { -error_setg_errno (errp, -len,"Cannot get block device size" ); -return len; +if (!QLIST_EMPTY (&bs->dirty_bitmaps)) { +error_setg (errp,"Device has active dirty bitmaps" ); +return -EBUSY ; } -/* We should set all bits in all enabled dirty bitmaps, because dirty -* bitmaps reflect active state of disk and snapshot switch operation -* actually dirties active state. -* TODO: It may make sense not to set all bits but analyze block status of -* current state and destination snapshot and do not set bits corresponding -* to both-zero or both-unallocated areas. */ -bdrv_set_dirty (bs,0 , len);if (drv->bdrv_snapshot_goto) { ret = drv->bdrv_snapshot_goto (bs, snapshot_id);
Signature · 2 lines
Best regards, Vladimir
Quoted 9 lines · Kevin Wolf
Following the discussing at
https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg03572.html
this implements the error return for loading a snapshot while dirty bitmaps are active.Kevin Wolf (3):
block: Add errp tobdrv_snapshot_goto()
block: Add errp tobdrv_all_goto_snapshot()
block: Error out onload_vmwith active dirty bitmaps
Reviewed-by Eric Blake <[email protected]>
Quoted 7 lines
include/block/snapshot.h | 6 ++++-- block/snapshot.c | 43 +++++++++++++++++++++++-------------------- migration/savevm.c | 6 +++--- qemu-img.c | 6 +++--- 4 files changed, 33 insertions(+), 28 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 10 lines · Kevin Wolf
Following the discussing at
https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg03572.html
this implements the error return for loading a snapshot while dirty bitmaps are active.Kevin Wolf (3):
block: Add errp tobdrv_snapshot_goto()
block: Add errp tobdrv_all_goto_snapshot()
block: Error out onload_vmwith active dirty bitmaps
Quoted 6 lines
include/block/snapshot.h | 6 ++++-- block/snapshot.c | 43 +++++++++++++++++++++++-------------------- migration/savevm.c | 6 +++--- qemu-img.c | 6 +++--- 4 files changed, 33 insertions(+), 28 deletions(-)
Reviewed-by Denis V. Lunev <[email protected]>
Quoted 10 lines · Kevin Wolf
Following the discussing at
https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg03572.html
this implements the error return for loading a snapshot while dirty bitmaps are active.Kevin Wolf (3):
block: Add errp tobdrv_snapshot_goto()
block: Add errp tobdrv_all_goto_snapshot()
block: Error out onload_vmwith active dirty bitmaps
Quoted 6 lines
include/block/snapshot.h | 6 ++++-- block/snapshot.c | 43 +++++++++++++++++++++++-------------------- migration/savevm.c | 6 +++--- qemu-img.c | 6 +++--- 4 files changed, 33 insertions(+), 28 deletions(-)
You have enough reviews, but just so it's unambiguous, I'm fine with this approach in lieu of Vladimir's; so:
Reviewed-by John Snow <[email protected]>