On 22.06.2017 01:17, Max Reitz wrote:
On 2017-06-13 14:16, Pavel Butsykin wrote:The flag as additional precaution of data loss. Perhaps in the future the operation shrink without this flag will be banned, but while we need to maintain compatibility.Signed-off-by: Pavel Butsykin <pbutsy...@virtuozzo.com> --- qemu-img-cmds.hx | 4 ++-- qemu-img.c | 15 +++++++++++++++ qemu-img.texi | 5 ++++- tests/qemu-iotests/102 | 4 ++-- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index a39fcdba71..3b2eab9d20 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -76,9 +76,9 @@ STEXI ETEXIDEF("resize", img_resize,- "resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size") + "resize [--object objectdef] [--image-opts] [-q] [--shrink] filename [+ | -]size") STEXI -@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+ | -]@var{size} +@item resize [--object @var{objectdef}] [--image-opts] [-q] [--shrink] @var{filename} [+ | -]@var{size} ETEXIDEF("amend", img_amend,diff --git a/qemu-img.c b/qemu-img.c index 0ad698d7f1..bfe5f61b0b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -61,6 +61,7 @@ enum { OPTION_FLUSH_INTERVAL = 261, OPTION_NO_DRAIN = 262, OPTION_TARGET_IMAGE_OPTS = 263, + OPTION_SHRINK = 264, };typedef enum OutputFormat {@@ -3452,6 +3453,7 @@ static int img_resize(int argc, char **argv) }, }; bool image_opts = false; + bool shrink = false;/* Remove size from argv manually so that negative numbers are not treated* as options by getopt. */ @@ -3469,6 +3471,7 @@ static int img_resize(int argc, char **argv) {"help", no_argument, 0, 'h'}, {"object", required_argument, 0, OPTION_OBJECT}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"shrink", no_argument, 0, OPTION_SHRINK}, {0, 0, 0, 0} }; c = getopt_long(argc, argv, ":f:hq", @@ -3503,6 +3506,9 @@ static int img_resize(int argc, char **argv) case OPTION_IMAGE_OPTS: image_opts = true; break; + case OPTION_SHRINK: + shrink = true; + break; } } if (optind != argc - 1) { @@ -3562,6 +3568,15 @@ static int img_resize(int argc, char **argv) goto out; }+ if (total_size < blk_getlength(blk) && !shrink) {+ qprintf(quiet, "Warning: shrinking of the image can lead to data loss. "I think this should always be printed to stderr, even if quiet is true; especially considering we (or at least I) plan to make it mandatory.
OK.
+ "Before performing shrink operation you must make sure "*Before performaing a shrink operation
s/performaing/performing/
Also, I'd rather use the imperative: "Before ... operation, make sure that the..." (English isn't my native language either, but as far as I remember "must" is generally used for external influences. But it's not like a force of nature is forcing you to confirm there's no important data; you can just ignore this advice and see all of your non-backed-up childhood pictures go to /dev/null.)
Yes, It should be just a recommendation. (As you might have already guessed, English isn't my native language too :) I'm glad you are understanding. Really thanks for helping to improve the text.)
+ "that the shrink part of image doesn't contain important"Hmm... I don't think "shrink part" really works. Maybe the following would work better: Warning: Shrinking an image will delete all data beyond the shrunken image's end. Before performing such an operation, make sure there is no important data there.+ " data.\n"); + qprintf(quiet, + "If you don't want to see this message use --shrink option.\n");You should make a note that --shrink may (and I hope it will) become necessary in the future, like: Using the --shrink option will suppress this message. Note that future versions of qemu-img may refuse to shrink images without this option!
will fix.
+ } + ret = blk_truncate(blk, total_size, &err); if (!ret) { qprintf(quiet, "Image resized.\n"); diff --git a/qemu-img.texi b/qemu-img.texi index 5b925ecf41..c2b694cd00 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -499,7 +499,7 @@ qemu-img rebase -b base.img diff.qcow2 At this point, @code{modified.img} can be discarded, since @code{base.img + diff.qcow2} contains the same information.-@item resize @var{filename} [+ | -]@var{size}+@item resize [--shrink] @var{filename} [+ | -]@var{size}Change the disk image as if it had been created with @var{size}. @@ -507,6 +507,9 @@ Before using this command to shrink a disk image, you MUST use file system andpartitioning tools inside the VM to reduce allocated file systems and partition sizes accordingly. Failure to do so will result in data loss!+If @code{--shrink} is specified, warning about data loss doesn't print for+the shrink operation. +Maybe rather: @code{--shrink} informs qemu-img that the user is certain about wanting to shrink an image and is aware that any data beyond the truncated image's end will be lost. Trying to shrink an image without this option results in a warning; future versions may make it an error.
good.
The functional changes look good to me; even though I'd rather have it an error for qcow2 now (even if that means having to check the image format in img_resize(), and being inconsistent because you wouldn't need --shrink for raw, but for qcow2 you would). But, well, I'm not going to stop this series over that.
Why shrink for qcow2 image is dangerous, but for raw is not dangerous? I think we should provide the same behavior for all formats. When --shrink option will become necessary, it also should be the same for all image formats.
Max
