Patchwork [1/3] block: Image file option amendment

login
register
mail settings
Submitter Max Reitz
Date Aug. 28, 2013, 8:08 a.m.
Message ID <1377677336-15804-2-git-send-email-mreitz@redhat.com>
Download mbox | patch
Permalink /patch/270384/
State New
Headers show

Comments

Max Reitz - Aug. 28, 2013, 8:08 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                | 90 +++++++++++++++++++++++++++++++++++++++++++++++
 qemu-img.texi             |  5 +++
 6 files changed, 114 insertions(+)
Kevin Wolf - Aug. 28, 2013, 10:03 a.m.
Am 28.08.2013 um 10:08 hat Max Reitz geschrieben:
> 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                | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-img.texi             |  5 +++
>  6 files changed, 114 insertions(+)

> +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;
> +    BlockDriver *bdrv;
> +
> +    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;
> +    }
> +
> +    bdrv = bs->drv;
> +    if (!bdrv) {

This can't happen. A successful bdrv_open() always results in a
BlockDriverState that has a driver assigned.

> +        error_report("No driver found for this image file.");
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    fmt = bdrv->format_name;
> +
> +    if (is_help_option(options)) {
> +        ret = print_block_option_help(filename, fmt);
> +        goto out;
> +    }
> +
> +    create_options = append_option_parameters(create_options,
> +            bdrv->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);
> +    }
> +    if (ret) {
> +        return 1;
> +    }
> +    return 0;
> +}

Some free_option_parameters() calls are missing here.

Kevin
Eric Blake - Aug. 28, 2013, 12:22 p.m.
On 08/28/2013 02:08 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.
> 

> +
> +@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.

The flattening of implicit zero clusters into explicit may take a
significant amount of time; please consider adding support for -p
(progress meter) as part of this addition.  (I'm already bothered by the
fact that the 'commit' operation lacks a progress meter; and even
'check' might have cases where it could be long-running)
Kevin Wolf - Aug. 28, 2013, 12:26 p.m.
Am 28.08.2013 um 14:22 hat Eric Blake geschrieben:
> On 08/28/2013 02:08 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.
> > 
> 
> > +
> > +@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.
> 
> The flattening of implicit zero clusters into explicit may take a
> significant amount of time; please consider adding support for -p
> (progress meter) as part of this addition.  (I'm already bothered by the
> fact that the 'commit' operation lacks a progress meter; and even
> 'check' might have cases where it could be long-running)

Good point. I think we can keep that separate, though. Let's get the
basic functionality in first, and then add a series on top that adds a
progress meter to those qemu-img subcommands for which it makes sense.

Kevin

Patch

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..bb8eabb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2308,6 +2308,96 @@  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;
+    BlockDriver *bdrv;
+
+    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;
+    }
+
+    bdrv = bs->drv;
+    if (!bdrv) {
+        error_report("No driver found for this image file.");
+        ret = -1;
+        goto out;
+    }
+
+    fmt = bdrv->format_name;
+
+    if (is_help_option(options)) {
+        ret = print_block_option_help(filename, fmt);
+        goto out;
+    }
+
+    create_options = append_option_parameters(create_options,
+            bdrv->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);
+    }
+    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