[v2,1/3] block: Image file option amendment

Submitted by Max Reitz on Aug. 29, 2013, 11:20 a.m.

Details

Message ID 1377775241-26278-2-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Aug. 29, 2013, 11:20 a.m.
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(+)

Comments

Eric Blake Aug. 29, 2013, 12:38 p.m.
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?
Max Reitz Aug. 29, 2013, 12:40 p.m.
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

Patch hide | download patch | download mbox

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