| Message ID | 1377775241-26278-2-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/29/2013 05:20 AM, Max Reitz wrote: > This patch adds the "amend" option to qemu-img which allows changing > image options on existing image files. It also adds the generic bdrv > implementation which is basically just a wrapper for the image format > specific function. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > > +static int img_amend(int argc, char **argv) > +{ > + int c, ret = 0; > + char *options = NULL; > + QEMUOptionParameter *create_options = NULL, *options_param = NULL; > + const char *fmt = NULL, *filename; > + bool quiet = false; > + BlockDriverState *bs = NULL; > + > + for (;;) { > + c = getopt(argc, argv, "h?qf:o:"); ? is not usually listed in the string to getopt() (doing so is not portable to POSIX). Besides, use of an unknown option (such as -x) will get mapped to '?' by getopt anyways, so your goal... > + if (c == -1) { > + break; > + } > + > + switch (c) { > + case 'h': > + case '?': > + help(); > + break; ...of printing help on unknown options is already met without the need for an explicit '-?' option. > + > + filename = argv[argc - 1]; > + > + bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet); > + if (!bs) { > + error_report("Could not open image."); We generally avoid trailing '.' in error messages; it might also be nice to report WHICH image could not be opened. > + options_param = parse_option_parameters(options, create_options, > + options_param); > + if (options_param == NULL) { > + error_report("Invalid options for file format '%s'.", fmt); again, no trailing '.' > +++ b/qemu-img.texi > @@ -282,6 +282,11 @@ sizes accordingly. Failure to do so will result in data loss! > 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} > + > +Amends the image format specific @var{options} for the image file > +@var{filename}. Only the format @code{qcow2} supports this. We might add support for other file formats in the future; we'd have to remember to update this sentence at that time. Could you use a more generic statement, such as "not all file formats support this", so we don't end up with stale docs if we forget to touch it down the road?
Am 29.08.2013 14:38, schrieb Eric Blake: > On 08/29/2013 05:20 AM, Max Reitz wrote: >> This patch adds the "amend" option to qemu-img which allows changing >> image options on existing image files. It also adds the generic bdrv >> implementation which is basically just a wrapper for the image format >> specific function. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> >> +static int img_amend(int argc, char **argv) >> +{ >> + int c, ret = 0; >> + char *options = NULL; >> + QEMUOptionParameter *create_options = NULL, *options_param = NULL; >> + const char *fmt = NULL, *filename; >> + bool quiet = false; >> + BlockDriverState *bs = NULL; >> + >> + for (;;) { >> + c = getopt(argc, argv, "h?qf:o:"); > ? is not usually listed in the string to getopt() (doing so is not > portable to POSIX). Besides, use of an unknown option (such as -x) will > get mapped to '?' by getopt anyways, so your goal... > >> + if (c == -1) { >> + break; >> + } >> + >> + switch (c) { >> + case 'h': >> + case '?': >> + help(); >> + break; > ...of printing help on unknown options is already met without the need > for an explicit '-?' option. Ah, okay, thanks. >> + >> + filename = argv[argc - 1]; >> + >> + bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet); >> + if (!bs) { >> + error_report("Could not open image."); > We generally avoid trailing '.' in error messages; it might also be nice > to report WHICH image could not be opened. Yes, I'll fix it. >> + options_param = parse_option_parameters(options, create_options, >> + options_param); >> + if (options_param == NULL) { >> + error_report("Invalid options for file format '%s'.", fmt); > again, no trailing '.' > >> +++ b/qemu-img.texi >> @@ -282,6 +282,11 @@ sizes accordingly. Failure to do so will result in data loss! >> 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} >> + >> +Amends the image format specific @var{options} for the image file >> +@var{filename}. Only the format @code{qcow2} supports this. > We might add support for other file formats in the future; we'd have to > remember to update this sentence at that time. Could you use a more > generic statement, such as "not all file formats support this", so we > don't end up with stale docs if we forget to touch it down the road? Of course. Max
diff --git a/block.c b/block.c index a387c1a..9c40a15 100644 --- a/block.c +++ b/block.c @@ -4674,3 +4674,11 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs, { notifier_with_return_list_add(&bs->before_write_notifiers, notifier); } + +int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options) +{ + if (bs->drv->bdrv_amend_options == NULL) { + return -ENOTSUP; + } + return bs->drv->bdrv_amend_options(bs, options); +} diff --git a/include/block/block.h b/include/block/block.h index 742fce5..40fad1b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -223,6 +223,8 @@ typedef enum { int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix); +int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options); + /* async block I/O */ typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, int sector_num); diff --git a/include/block/block_int.h b/include/block/block_int.h index 8012e25..3c93766 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -205,6 +205,9 @@ struct BlockDriver { int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result, BdrvCheckMode fix); + int (*bdrv_amend_options)(BlockDriverState *bs, + QEMUOptionParameter *options); + void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event); /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */ diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 4ca7e95..5a066b5 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -61,5 +61,11 @@ DEF("resize", img_resize, "resize [-q] filename [+ | -]size") STEXI @item resize [-q] @var{filename} [+ | -]@var{size} +ETEXI + +DEF("amend", img_amend, + "amend [-q] [-f fmt] -o options filename") +STEXI +@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename} @end table ETEXI diff --git a/qemu-img.c b/qemu-img.c index b9a848d..cc02b5b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2308,6 +2308,90 @@ out: return 0; } +static int img_amend(int argc, char **argv) +{ + int c, ret = 0; + char *options = NULL; + QEMUOptionParameter *create_options = NULL, *options_param = NULL; + const char *fmt = NULL, *filename; + bool quiet = false; + BlockDriverState *bs = NULL; + + for (;;) { + c = getopt(argc, argv, "h?qf:o:"); + if (c == -1) { + break; + } + + switch (c) { + case 'h': + case '?': + help(); + break; + case 'o': + options = optarg; + break; + case 'f': + fmt = optarg; + break; + case 'q': + quiet = true; + break; + } + } + + if (optind != argc - 1) { + help(); + } + + if (!options) { + help(); + } + + filename = argv[argc - 1]; + + bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet); + if (!bs) { + error_report("Could not open image."); + ret = -1; + goto out; + } + + fmt = bs->drv->format_name; + + if (is_help_option(options)) { + ret = print_block_option_help(filename, fmt); + goto out; + } + + create_options = append_option_parameters(create_options, + bs->drv->create_options); + options_param = parse_option_parameters(options, create_options, + options_param); + if (options_param == NULL) { + error_report("Invalid options for file format '%s'.", fmt); + ret = -1; + goto out; + } + + ret = bdrv_amend_options(bs, options_param); + if (ret < 0) { + error_report("Error while amending options: %s", strerror(-ret)); + goto out; + } + +out: + if (bs) { + bdrv_delete(bs); + } + free_option_parameters(create_options); + free_option_parameters(options_param); + if (ret) { + return 1; + } + return 0; +} + static const img_cmd_t img_cmds[] = { #define DEF(option, callback, arg_string) \ { option, callback }, diff --git a/qemu-img.texi b/qemu-img.texi index 69f1bda..4e39933 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -282,6 +282,11 @@ sizes accordingly. Failure to do so will result in data loss! 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} + +Amends the image format specific @var{options} for the image file +@var{filename}. Only the format @code{qcow2} supports this. @end table @c man end
This patch adds the "amend" option to qemu-img which allows changing image options on existing image files. It also adds the generic bdrv implementation which is basically just a wrapper for the image format specific function. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 8 +++++ include/block/block.h | 2 ++ include/block/block_int.h | 3 ++ qemu-img-cmds.hx | 6 ++++ qemu-img.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ qemu-img.texi | 5 +++ 6 files changed, 108 insertions(+)