| Message ID | 1405802159-2355-3-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/19/2014 02:35 PM, Max Reitz wrote: > qemu-img amend may extensively modify the target image, depending on the > options to be amended (e.g. conversion to qcow2 compat level 0.10 from > 1.1 for an image with many unallocated zero clusters). Therefore it > makes sense to allow the user to specify the cache mode to be used. Extensive modifications implies long-running operation - the 'amend' subcommand is a good candidate for the -p progress meter option. But that would be a separate patch. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > qemu-img-cmds.hx | 4 ++-- > qemu-img.c | 19 +++++++++++++++---- > qemu-img.texi | 2 +- > 3 files changed, 18 insertions(+), 7 deletions(-) > > > + cache = BDRV_DEFAULT_CACHE; > for (;;) { > - c = getopt(argc, argv, "hqf:o:"); > + c = getopt(argc, argv, "hqf:t:o:"); > if (c == -1) { > break; > } > @@ -2805,6 +2807,9 @@ static int img_amend(int argc, char **argv) > case 'f': > fmt = optarg; > break; > + case 't': > + cache = optarg; > + break; > case 'q': Pre-existing, so I won't hold up review, but I'm a big fan of having the switch block in the same order as the getopt string (that is, we listed 'q' before 'f' in the string above, so the cases are out-of-order with respect to that string). The fix can go either way (reshuffle the case statements, or reorder the optstring above). [and for the truly OCD, I prefer the optstring in case-insensitive alphabetical order "f:ho:qt:", because then it's easier to scan the string to see what letters are still available for new options - but that's asking a bit much] Reviewed-by: Eric Blake <eblake@redhat.com>
On 21.07.2014 17:57, Eric Blake wrote: > On 07/19/2014 02:35 PM, Max Reitz wrote: >> qemu-img amend may extensively modify the target image, depending on the >> options to be amended (e.g. conversion to qcow2 compat level 0.10 from >> 1.1 for an image with many unallocated zero clusters). Therefore it >> makes sense to allow the user to specify the cache mode to be used. > Extensive modifications implies long-running operation - the 'amend' > subcommand is a good candidate for the -p progress meter option. But > that would be a separate patch. Yes, this is on my to do list anyway. >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> qemu-img-cmds.hx | 4 ++-- >> qemu-img.c | 19 +++++++++++++++---- >> qemu-img.texi | 2 +- >> 3 files changed, 18 insertions(+), 7 deletions(-) >> >> >> + cache = BDRV_DEFAULT_CACHE; >> for (;;) { >> - c = getopt(argc, argv, "hqf:o:"); >> + c = getopt(argc, argv, "hqf:t:o:"); >> if (c == -1) { >> break; >> } >> @@ -2805,6 +2807,9 @@ static int img_amend(int argc, char **argv) >> case 'f': >> fmt = optarg; >> break; >> + case 't': >> + cache = optarg; >> + break; >> case 'q': > > Pre-existing, so I won't hold up review, but I'm a big fan of having the > switch block in the same order as the getopt string (that is, we listed > 'q' before 'f' in the string above, so the cases are out-of-order with > respect to that string). The fix can go either way (reshuffle the case > statements, or reorder the optstring above). [and for the truly OCD, I > prefer the optstring in case-insensitive alphabetical order "f:ho:qt:", > because then it's easier to scan the string to see what letters are > still available for new options - but that's asking a bit much] Right, in fact I thought of you when I touched this block; since it was already completely out of order, I decided for the minimal change, however. I'll redo it in v2. I personally somehow like to have common switches like -q at the end, I don't know why. For now, I'll just reshuffle the getopt() string. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks, Max
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 5613628..20a21ff 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -70,8 +70,8 @@ STEXI ETEXI DEF("amend", img_amend, - "amend [-q] [-f fmt] -o options filename") + "amend [-q] [-f fmt] [-t cache] -o options filename") STEXI -@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename} +@item amend [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename} @end table ETEXI diff --git a/qemu-img.c b/qemu-img.c index 845b8c7..df6cb46 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2773,12 +2773,14 @@ static int img_amend(int argc, char **argv) char *options = NULL; QemuOptsList *create_opts = NULL; QemuOpts *opts = NULL; - const char *fmt = NULL, *filename; + const char *fmt = NULL, *filename, *cache; + int flags; bool quiet = false; BlockDriverState *bs = NULL; + cache = BDRV_DEFAULT_CACHE; for (;;) { - c = getopt(argc, argv, "hqf:o:"); + c = getopt(argc, argv, "hqf:t:o:"); if (c == -1) { break; } @@ -2805,6 +2807,9 @@ static int img_amend(int argc, char **argv) case 'f': fmt = optarg; break; + case 't': + cache = optarg; + break; case 'q': quiet = true; break; @@ -2827,8 +2832,14 @@ static int img_amend(int argc, char **argv) error_exit("Expecting one image file name"); } - bs = bdrv_new_open("image", filename, fmt, - BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet); + flags = BDRV_O_FLAGS | BDRV_O_RDWR; + ret = bdrv_parse_cache_flags(cache, &flags); + if (ret < 0) { + error_report("Invalid cache option: %s", cache); + goto out; + } + + bs = bdrv_new_open("image", filename, fmt, flags, true, quiet); if (!bs) { error_report("Could not open image '%s'", filename); ret = -1; diff --git a/qemu-img.texi b/qemu-img.texi index 688b28d..cb68948 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -397,7 +397,7 @@ After using this command to grow a disk image, you must use file system and partitioning tools inside the VM to actually begin using the new space on the device. -@item amend [-f @var{fmt}] -o @var{options} @var{filename} +@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename} Amends the image format specific @var{options} for the image file @var{filename}. Not all file formats support this operation.
qemu-img amend may extensively modify the target image, depending on the options to be amended (e.g. conversion to qcow2 compat level 0.10 from 1.1 for an image with many unallocated zero clusters). Therefore it makes sense to allow the user to specify the cache mode to be used. Signed-off-by: Max Reitz <mreitz@redhat.com> --- qemu-img-cmds.hx | 4 ++-- qemu-img.c | 19 +++++++++++++++---- qemu-img.texi | 2 +- 3 files changed, 18 insertions(+), 7 deletions(-)