block: commits of snapshots larger than backing files
v2 cover block: commits of snapshots larger than backing files
Changes, v1->v2:
Patch 1/3: Added error check for bdrv_getlength() return (Stefan) Patch 2/3: Restore flags in commit_active_start() on failures after
the bdrv_reopen() (Fam)
Patch 3/3: New patch, adds documentation clarification for the behavior
of both offline commit, and live block commit, as it pertains to image truncation. (Stefan)
If a snapshot is larger than a backing file, then the offline bdrv_commit and the live active layer commit will fail with an i/o error (usually). A live commit of a non-active layer will complete successfully, as it runs bdrv_truncate() on the backing image to resize it to the larger size.
For both bdrv_commit() and commit_active_start(), this series will resize the underlying base image if needed. If the resize fails, an error will be returned.
Jeff Cody (3):
block: resize backing file image during offline commit, if necessary
block: resize backing image during active layer commit, if needed
block: update block commit documentation regarding image truncation
block.c | 23 ++++++++++++++++++++--- block/mirror.c | 36 ++++++++++++++++++++++++++++++++++++ hmp-commands.hx | 5 +++++ qapi-schema.json | 7 +++++++ qemu-img.texi | 7 ++++++- qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 4 deletions(-)
Signature · 1 line
1.8.3.1
PATCH v2 1/3 block: resize backing file image during offline commit, if necessary
Currently, if an image file is logically larger than its backing file, commiting it via 'qemu-img commit' will fail.
For instance, if we have a base image with a virtual size 10G, and a snapshot image of size 20G, then committing the snapshot offline with 'qemu-img commit' will likely fail.
This will automatically attempt to resize the base image, if the snapshot image to be committed is larger.
--- block.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
block.c +20 -3
@@ -1876,10 +1876,10 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)int bdrv_commit (BlockDriverState *bs) {BlockDriver *drv = bs->drv; -int64_t sector, total_sectors; +int64_t sector, total_sectors, length, backing_length;int n, ro, open_flags;int ret =0 ; -uint8_t *buf; +uint8_t *buf =NULL ;char filename[PATH_MAX ];if (!drv)
@@ -1904,7 +1904,24 @@ int bdrv_commit(BlockDriverState *bs) } } - total_sectors =bdrv_getlength (bs) >>BDRV_SECTOR_BITS ; + length =bdrv_getlength (bs); + backing_length =bdrv_getlength (bs->backing_hd); + +if (length <0 || backing_length <0 ) { + goto ro_cleanup; + } + +/* If our top snapshot is larger than the backing file image, +* grow the backing file image if possible. If not possible, +* we must return an error */ +if (length > backing_length) { + ret =bdrv_truncate (bs->backing_hd, length); +if (ret <0 ) { + goto ro_cleanup; + } + } + + total_sectors = length >>BDRV_SECTOR_BITS ; buf =g_malloc (COMMIT_BUF_SECTORS *BDRV_SECTOR_SIZE );for (sector =0 ; sector < total_sectors; sector += n) {
Signature · 1 line
1.8.3.1
PATCH v2 3/3 block: update block commit documentation regarding image truncation
In reply to Jeff CodyThis updates the documentation for commiting snapshot images. Specifically, this highlights what happens when the base image is either smaller or larger than the snapshot image being committed.
In the case of the base image being smaller, it is resized to the larger size of the snapshot image. In the case of the base image being larger, it is not resized automatically, but once the commit has completed it is safe for the user to truncate the base image.
--- hmp-commands.hx | 5 +++++ qapi-schema.json | 7 +++++++ qemu-img.texi | 7 ++++++- qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-)
hmp-commands.hx +5 -0
@@ -35,6 +35,11 @@ STEXI @item commit @findex commit Commit changes to the disk images (if -snapshot is used) or backing files. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. ETEXI {
qapi-schema.json +7 -0
@@ -1973,6 +1973,13 @@ # user needs to complete the job with the block-job-complete # command after getting the ready event. (Since 2.0) # +# If the base image is smaller than top, then the base image +# will be resized to be the same size as top. If top is +# smaller than the base image, the base will not be +# truncated. If you want the base image size to match the +# size of the smaller top, you can safely truncate it +# yourself once the commit operation successfully completes. +# # # @speed: #optional the maximum speed, in bytes per second #
qemu-img.texi +6 -1
@@ -140,7 +140,12 @@ it doesn't need to be specified separately in this case. @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename} -Commit the changes recorded in @var{filename} in its base image. +Commit the changes recorded in @var{filename} in its base image or backing file. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
qmp-commands.hx +39 -0
@@ -965,6 +965,45 @@ EQMP .mhandler.cmd_new = qmp_marshal_input_block_commit, }, +SQMP +block-commit +------------ + +Live commit of data from overlay image nodes into backing nodes - i.e., writes +data between 'top' and 'base' into 'base'. + +Arguments: + +- "device": The device's ID, must be unique (json-string) +- "base": The file name of the backing image to write data into. + If not specified, this is the deepest backing image + (json-string, optional) +- "top": The file name of the backing image within the image chain, + which contains the topmost data to be committed down. + + If top == base, that is an error. + If top == active, the job will not be completed by itself, + user needs to complete the job with the block-job-complete + command after getting the ready event. (Since 2.0) + + If the base image is smaller than top, then the base image + will be resized to be the same size as top. If top is + smaller than the base image, the base will not be + truncated. If you want the base image size to match the + size of the smaller top, you can safely truncate it + yourself once the commit operation successfully completes. + (json-string) +- "speed": the maximum speed, in bytes per second (json-int, optional) + + +Example: + +-> { "execute": "block-commit", "arguments": { "device": "virtio0", + "top": "/tmp/snap1.qcow2" } } +<- { "return": {} } + +EQMP + { .name = "drive-backup", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH v2 3/3] block: update block commit documentation regarding image truncation
Quoted 13 lines · Jeff Cody
This updates the documentation for commiting snapshot images. Specifically, this highlights what happens when the base image is either smaller or larger than the snapshot image being committed.
In the case of the base image being smaller, it is resized to the larger size of the snapshot image. In the case of the base image being larger, it is not resized automatically, but once the commit has completed it is safe for the user to truncate the base image.
Signed-off-by Jeff Cody <[email protected]>---
+++b/qapi-schema.json
qapi-schema.json line 1978
Quoted 6 lines
qapi-schema.json +3
-0@@ -1973,6 +1973,13 @@ # user needs to complete the job with the block-job-complete # command after getting the ready event. (Since 2.0) # +# If the base image is smaller than top, then the base image +# will be resized to be the same size as top. If top is +# smaller than the base image, the base will not be
Spurious double space.
qmp-commands.hx EQMP · line 991
Quoted 9 lines
qmp-commands.hx +8
-0+SQMP +block-commit +------------ + + + If the base image is smaller than top, then the base image + will be resized to be the same size as top. If top is + smaller than the base image, the base will not be
and again.
But as that is minor,
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v2 1/3] block: resize backing file image during offline commit, if necessary
In reply to Jeff CodyQuoted 2 lines · Jeff Cody
Currently, if an image file is logically larger than its backing file, commiting it via 'qemu-img commit' will fail.
s/commiting/committing/
Quoted 5 lines
For instance, if we have a base image with a virtual size 10G, and a snapshot image of size 20G, then committing the snapshot offline with 'qemu-img commit' will likely fail.
Quoted 8 lines
This will automatically attempt to resize the base image, if the snapshot image to be committed is larger.
Signed-off-by Jeff Cody <[email protected]>--- block.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
The commit message change is trivial, so:
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH v2 2/3 block: resize backing image during active layer commit, if needed
In reply to Jeff CodyIf the top image to commit is the active layer, and also larger than the base image, then an I/O error will likely be returned during block-commit.
For instance, if we have a base image with a virtual size 10G, and a active layer image of size 20G, then committing the snapshot via 'block-commit' will likely fail.
This will automatically attempt to resize the base image, if the active layer image to be committed is larger.
--- block/mirror.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
block/mirror.c +36 -0
@@ -630,11 +630,47 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { +int64_t length, base_length; +int orig_base_flags; + + orig_base_flags =bdrv_get_flags (base); +if (bdrv_reopen (base, bs->open_flags, errp)) {return ; } + + length =bdrv_getlength (bs); + base_length =bdrv_getlength (base); + +if (length <0 || base_length <0 ) { + goto error_restore_flags; + } + +if (length > base_length) { +if (bdrv_truncate (base, length) <0 ) { +error_setg (errp,"Top image %s is larger than base image %s, and " +"resize of base image failed." , + bs->filename, base->filename); + goto error_restore_flags; + } + }else if (length <0 ) { + goto error_restore_flags; + } + +bdrv_ref (base);mirror_start_job (bs, base, speed,0 ,0 , on_error, on_error, cb, opaque, errp, &commit_active_job_driver, false, base); +if (error_is_set (errp)) { + goto error_restore_flags; + } + +return ; + +error_restore_flags: +/* ignore error and errp for bdrv_reopen, because we want to propagate +* the original error */ +bdrv_reopen (base, orig_base_flags,NULL ); +return ; }
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH v2 2/3] block: resize backing image during active layer commit, if needed
Quoted 17 lines · Jeff Cody
If the top image to commit is the active layer, and also larger than the base image, then an I/O error will likely be returned during block-commit.
For instance, if we have a base image with a virtual size 10G, and a active layer image of size 20G, then committing the snapshot via 'block-commit' will likely fail.
This will automatically attempt to resize the base image, if the active layer image to be committed is larger.
Signed-off-by Jeff Cody <[email protected]>--- block/mirror.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
block/mirror.c void commit_active_start(BlockDriverState *bs, BlockDriverState *base, · line 652
Quoted 4 lines
block/mirror.c +4
-0+if (length > base_length) { +if (bdrv_truncate (base, length) <0 ) { + error_setg(errp, "Top image %s is larger than base image %s, and " + "resize of base image failed.",
We typically don't use trailing '.' in error messages.
But that's small enough that I'm okay adding:
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v2 2/3] block: resize backing image during active layer commit, if needed
In reply to Jeff CodyQuoted 27 lines · Jeff Cody
If the top image to commit is the active layer, and also larger than the base image, then an I/O error will likely be returned during block-commit.
For instance, if we have a base image with a virtual size 10G, and a active layer image of size 20G, then committing the snapshot via 'block-commit' will likely fail.
This will automatically attempt to resize the base image, if the active layer image to be committed is larger.
Signed-off-by Jeff Cody <[email protected]>--- block/mirror.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)block/mirror.c +3
-0@@ -630,11 +630,47 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { +int64_t length, base_length; +int orig_base_flags; +
block/mirror.c void commit_active_start(BlockDriverState *bs, BlockDriverState *base, · line 645
Quoted 10 lines
block/mirror.c +7
-0+ orig_base_flags =bdrv_get_flags (base); +if (bdrv_reopen (base, bs->open_flags, errp)) {return ; } + + length =bdrv_getlength (bs); + base_length =bdrv_getlength (base); + +if (length <0 || base_length <0 ) {
I prefer to add an error to errp here, at least tell which bdrv_getlength failed helps.
Quoted 19 lines
block/mirror.c +15
-0+ goto error_restore_flags; + } + +if (length > base_length) { +if (bdrv_truncate (base, length) <0 ) { +error_setg (errp,"Top image %s is larger than base image %s, and " +"resize of base image failed." , + bs->filename, base->filename); + goto error_restore_flags; + } + }else if (length <0 ) { + goto error_restore_flags; + } + +bdrv_ref (base);mirror_start_job (bs, base, speed,0 ,0 , on_error, on_error, cb, opaque, errp, &commit_active_job_driver, false, base);
block/mirror.c void commit_active_start(BlockDriverState *bs, BlockDriverState *base, · line 674
Quoted 10 lines
block/mirror.c +10
-0+if (error_is_set (errp)) { + goto error_restore_flags; + } + +return ; + +error_restore_flags: +/* ignore error and errp for bdrv_reopen, because we want to propagate +* the original error */ +bdrv_reopen (base, orig_base_flags,NULL );
Well, I hope this never fail. (But if it does, should we do anything else than ignoring it?)
Thanks,
Fam
Quoted 5 lines
block/mirror.c +1
-0+return ; }Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH v2 1/3] block: resize backing file image during offline commit, if necessary
In reply to Jeff CodyQuoted 61 lines · Jeff Cody
Currently, if an image file is logically larger than its backing file, commiting it via 'qemu-img commit' will fail.
For instance, if we have a base image with a virtual size 10G, and a snapshot image of size 20G, then committing the snapshot offline with 'qemu-img commit' will likely fail.
This will automatically attempt to resize the base image, if the snapshot image to be committed is larger.
Signed-off-by Jeff Cody <[email protected]>--- block.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)block.c +20
-3@@ -1876,10 +1876,10 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)int bdrv_commit (BlockDriverState *bs) {BlockDriver *drv = bs->drv; -int64_t sector, total_sectors; +int64_t sector, total_sectors, length, backing_length;int n, ro, open_flags;int ret =0 ; -uint8_t *buf; +uint8_t *buf =NULL ;char filename[PATH_MAX ];if (!drv)@@ -1904,7 +1904,24 @@ int bdrv_commit(BlockDriverState *bs) } } - total_sectors =bdrv_getlength (bs) >>BDRV_SECTOR_BITS ; + length =bdrv_getlength (bs); + backing_length =bdrv_getlength (bs->backing_hd); + +if (length <0 || backing_length <0 ) { + goto ro_cleanup; + } + +/* If our top snapshot is larger than the backing file image, +* grow the backing file image if possible. If not possible, +* we must return an error */ +if (length > backing_length) { + ret =bdrv_truncate (bs->backing_hd, length); +if (ret <0 ) { + goto ro_cleanup; + } + } + + total_sectors = length >>BDRV_SECTOR_BITS ; buf =g_malloc (COMMIT_BUF_SECTORS *BDRV_SECTOR_SIZE );for (sector =0 ; sector < total_sectors; sector += n) {Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH v2 3/3] block: update block commit documentation regarding image truncation
In reply to Jeff CodyQuoted 122 lines · Jeff Cody
This updates the documentation for commiting snapshot images. Specifically, this highlights what happens when the base image is either smaller or larger than the snapshot image being committed.
In the case of the base image being smaller, it is resized to the larger size of the snapshot image. In the case of the base image being larger, it is not resized automatically, but once the commit has completed it is safe for the user to truncate the base image.
Signed-off-by Jeff Cody <[email protected]>--- hmp-commands.hx | 5 +++++ qapi-schema.json | 7 +++++++ qemu-img.texi | 7 ++++++- qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-)hmp-commands.hx +5
-0@@ -35,6 +35,11 @@ STEXI @item commit @findex commit Commit changes to the disk images (if -snapshot is used) or backing files. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. ETEXI {qapi-schema.json +7
-0@@ -1973,6 +1973,13 @@ # user needs to complete the job with the block-job-complete # command after getting the ready event. (Since 2.0) # +# If the base image is smaller than top, then the base image +# will be resized to be the same size as top. If top is +# smaller than the base image, the base will not be +# truncated. If you want the base image size to match the +# size of the smaller top, you can safely truncate it +# yourself once the commit operation successfully completes. +# # # @speed: #optional the maximum speed, in bytes per second #qemu-img.texi +6
-1@@ -140,7 +140,12 @@ it doesn't need to be specified separately in this case. @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename} -Commit the changes recorded in @var{filename} in its base image. +Commit the changes recorded in @var{filename} in its base image or backing file. +If the backing file is smaller than the snapshot, then the backing file will be +resized to be the same size as the snapshot. If the snapshot is smaller than +the backing file, the backing file will not be truncated. If you want the +backing file to match the size of the smaller snapshot, you can safely truncate +it yourself once the commit operation successfully completes. @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}qmp-commands.hx +39
-0@@ -965,6 +965,45 @@ EQMP .mhandler.cmd_new = qmp_marshal_input_block_commit, }, +SQMP +block-commit +------------ + +Live commit of data from overlay image nodes into backing nodes - i.e., writes +data between 'top' and 'base' into 'base'. + +Arguments: + +- "device": The device's ID, must be unique (json-string) +- "base": The file name of the backing image to write data into. + If not specified, this is the deepest backing image + (json-string, optional) +- "top": The file name of the backing image within the image chain, + which contains the topmost data to be committed down. + + If top == base, that is an error. + If top == active, the job will not be completed by itself, + user needs to complete the job with the block-job-complete + command after getting the ready event. (Since 2.0) + + If the base image is smaller than top, then the base image + will be resized to be the same size as top. If top is + smaller than the base image, the base will not be + truncated. If you want the base image size to match the + size of the smaller top, you can safely truncate it + yourself once the commit operation successfully completes. + (json-string) +- "speed": the maximum speed, in bytes per second (json-int, optional) + + +Example: + +-> { "execute": "block-commit", "arguments": { "device": "virtio0", + "top": "/tmp/snap1.qcow2" } } +<- { "return": {} } + +EQMP + { .name = "drive-backup", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH v2 2/3] block: resize backing image during active layer commit, if needed
In reply to Fam ZhengQuoted 33 lines · Fam Zheng
Quoted 32 lines · Jeff Cody
If the top image to commit is the active layer, and also larger than the base image, then an I/O error will likely be returned during block-commit.
For instance, if we have a base image with a virtual size 10G, and a active layer image of size 20G, then committing the snapshot via 'block-commit' will likely fail.
This will automatically attempt to resize the base image, if the active layer image to be committed is larger.
Signed-off-by Jeff Cody <[email protected]>--- block/mirror.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)block/mirror.c +5
-0@@ -630,11 +630,47 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { +int64_t length, base_length; +int orig_base_flags; + + orig_base_flags =bdrv_get_flags (base); +if (bdrv_reopen (base, bs->open_flags, errp)) {return ; }
Quoted 9 lines
block/mirror.c void commit_active_start(BlockDriverState *bs, BlockDriverState *base, · line 645
Quoted 5 lines
block/mirror.c +5
-0+ + length =bdrv_getlength (bs); + base_length =bdrv_getlength (base); + +if (length <0 || base_length <0 ) {I prefer to add an error to errp here, at least tell which
bdrv_getlengthfailed helps.
OK, I can do that.
Quoted 24 lines
Quoted 24 lines
block/mirror.c +20
-0+ goto error_restore_flags; + } + +if (length > base_length) { +if (bdrv_truncate (base, length) <0 ) { +error_setg (errp,"Top image %s is larger than base image %s, and " +"resize of base image failed." , + bs->filename, base->filename); + goto error_restore_flags; + } + }else if (length <0 ) { + goto error_restore_flags; + } + +bdrv_ref (base);mirror_start_job (bs, base, speed,0 ,0 , on_error, on_error, cb, opaque, errp, &commit_active_job_driver, false, base); +if (error_is_set (errp)) { + goto error_restore_flags; + } + +return ;
Quoted 9 lines
block/mirror.c void commit_active_start(BlockDriverState *bs, BlockDriverState *base, · line 674
Quoted 5 lines
block/mirror.c +5
-0+ +error_restore_flags: +/* ignore error and errp for bdrv_reopen, because we want to propagate +* the original error */ +bdrv_reopen (base, orig_base_flags,NULL );Well, I hope this never fail. (But if it does, should we do anything else than ignoring it?)
Me too :) There isn't a lot we can do but ignore it; the original error that lead us here is most likely more important and informative than the reopen error, so I don't think it makes sense to override errp. And failing to restore the original flags is likely not catastrophic (but symptomatic).
Quoted 6 lines
Quoted 6 lines
block/mirror.c +1
-0+return ; }Signature · 1 line
1.8.3.1