[Qemu-devel] [PATCH 0/3] Re-factor img_create() and add live snapshots

[Qemu-devel] [PATCH 0/3] Re-factor img_create() and add live snapshots

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

This set of patches re-factors img_create() and moves the core part of
it into block.c so it can be accessed from qemu as well as
qemu-img. The second patch adds basic live snapshots support to the
code, however only snapshots to external QCOW2 images is supported for
now. QED support should be trivial once the QED patches go into
upstream.

The last patch fixes a small gotcha which is present in the old code
as well. Try to catch cases where a user tries to create an image with
itself as the backing file. QEMU does 'interesting' things when you do
this.....

Many thanks to Kevin for his help with block layer internals!

Cheers,
Jes


Jes Sorensen (3):
  qemu-img.c: Re-factor img_create()
  Introduce do_snapshot_blkdev() and monitor command to handle it.
  Prevent creating an image with the same filename as backing file

 block.c         |  148 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block.h         |    4 ++
 blockdev.c      |   61 +++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   19 +++++++
 qemu-img.c      |  106 +--------------------------------------
 6 files changed, 235 insertions(+), 104 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/3] qemu-img.c: Re-factor img_create()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch re-factors img_create() moving the code doing the actual
work into block.c where it can be shared with QEMU. This is needed to
be able to create images from QEMU to be used for live snapshots.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block.c    |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block.h    |    4 ++
 qemu-img.c |  106 +--------------------------------------------
 3 files changed, 147 insertions(+), 104 deletions(-)

diff --git a/block.c b/block.c
index 63effd8..3ab062c 100644
--- a/block.c
+++ b/block.c
@@ -2742,3 +2742,144 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs)
 {
     return bs->dirty_count;
 }
+
+int bdrv_img_create(const char *filename, const char *fmt,
+                    const char *base_filename, const char *base_fmt,
+                    char *options, uint64_t img_size, int flags)
+{
+    QEMUOptionParameter *param = NULL, *create_options = NULL;
+    BlockDriverState *bs = NULL;
+    BlockDriver *drv, *proto_drv;
+    int ret = 0;
+
+    /* Find driver and parse its options */
+    drv = bdrv_find_format(fmt);
+    if (!drv) {
+        error_report("Unknown file format '%s'", fmt);
+        ret = -1;
+        goto out;
+    }
+
+    proto_drv = bdrv_find_protocol(filename);
+    if (!proto_drv) {
+        error_report("Unknown protocol '%s'", filename);
+        ret = -1;
+        goto out;
+    }
+
+    create_options = append_option_parameters(create_options,
+                                              drv->create_options);
+    create_options = append_option_parameters(create_options,
+                                              proto_drv->create_options);
+
+    /* Create parameter list with default values */
+    param = parse_option_parameters("", create_options, param);
+
+    set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
+
+    /* Parse -o options */
+    if (options) {
+        param = parse_option_parameters(options, create_options, param);
+        if (param == NULL) {
+            error_report("Invalid options for file format '%s'.", fmt);
+            ret = -1;
+            goto out;
+        }
+    }
+
+    if (base_filename) {
+        if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
+                                 base_filename)) {
+            error_report("Backing file not supported for file format '%s'",
+                         fmt);
+            ret = -1;
+            goto out;
+        }
+    }
+    if (base_fmt) {
+        if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+            error_report("Backing file format not supported for file "
+                         "format '%s'", fmt);
+            ret = -1;
+            goto out;
+        }
+    }
+
+    // The size for the image must always be specified, with one exception:
+    // If we are using a backing file, we can obtain the size from there
+    if (get_option_parameter(param, BLOCK_OPT_SIZE)->value.n == -1) {
+        QEMUOptionParameter *backing_file =
+            get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
+        QEMUOptionParameter *backing_fmt =
+            get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
+
+        if (backing_file && backing_file->value.s) {
+            uint64_t size;
+            const char *fmt = NULL;
+            char buf[32];
+
+            if (backing_fmt && backing_fmt->value.s) {
+                if (bdrv_find_format(backing_fmt->value.s)) {
+                    fmt = backing_fmt->value.s;
+                } else {
+                    error_report("Unknown backing file format '%s'",
+                                 backing_fmt->value.s);
+                    ret = -1;
+                    goto out;
+                }
+            }
+
+            bs = bdrv_new("");
+            if (!bs) {
+                error_report("Not enough memory to allocate BlockDriverState");
+                ret = -1;
+                goto out;
+            }
+            ret = bdrv_open(bs, backing_file->value.s, flags, drv);
+            if (ret < 0) {
+                error_report("Could not open '%s'", filename);
+                ret = -1;
+                goto out;
+            }
+            bdrv_get_geometry(bs, &size);
+            size *= 512;
+
+            snprintf(buf, sizeof(buf), "%" PRId64, size);
+            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
+        } else {
+            error_report("Image creation needs a size parameter");
+            ret = -1;
+            goto out;
+        }
+    }
+
+    printf("Formatting '%s', fmt=%s ", filename, fmt);
+    print_option_parameters(param);
+    puts("");
+
+    ret = bdrv_create(drv, filename, param);
+    free_option_parameters(create_options);
+    free_option_parameters(param);
+
+    if (ret < 0) {
+        if (ret == -ENOTSUP) {
+            error_report("Formatting or formatting option not supported for "
+                         "file format '%s'", fmt);
+        } else if (ret == -EFBIG) {
+            error_report("The image size is too large for file format '%s'",
+                         fmt);
+        } else {
+            error_report("%s: error while creating %s: %s", filename, fmt,
+                         strerror(-ret));
+        }
+    }
+
+out:
+    if (bs) {
+        bdrv_delete(bs);
+    }
+    if (ret) {
+        return 1;
+    }
+    return 0;
+}
diff --git a/block.h b/block.h
index 78ecfac..b812172 100644
--- a/block.h
+++ b/block.h
@@ -227,6 +227,10 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
 int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
                       int64_t pos, int size);
 
+int bdrv_img_create(const char *filename, const char *fmt,
+                    const char *base_filename, const char *base_fmt,
+                    char *options, uint64_t img_size, int flags);
+
 #define BDRV_SECTORS_PER_DIRTY_CHUNK 2048
 
 void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable);
diff --git a/qemu-img.c b/qemu-img.c
index f078718..603bdb3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -287,8 +287,6 @@ static int img_create(int argc, char **argv)
     const char *base_fmt = NULL;
     const char *filename;
     const char *base_filename = NULL;
-    BlockDriver *drv, *proto_drv;
-    QEMUOptionParameter *param = NULL, *create_options = NULL;
     char *options = NULL;
 
     for(;;) {
@@ -349,108 +347,8 @@ static int img_create(int argc, char **argv)
         goto out;
     }
 
-    /* Find driver and parse its options */
-    drv = bdrv_find_format(fmt);
-    if (!drv) {
-        error("Unknown file format '%s'", fmt);
-        ret = -1;
-        goto out;
-    }
-
-    proto_drv = bdrv_find_protocol(filename);
-    if (!proto_drv) {
-        error("Unknown protocol '%s'", filename);
-        ret = -1;
-        goto out;
-    }
-
-    create_options = append_option_parameters(create_options,
-                                              drv->create_options);
-    create_options = append_option_parameters(create_options,
-                                              proto_drv->create_options);
-
-    /* Create parameter list with default values */
-    param = parse_option_parameters("", create_options, param);
-
-    set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
-
-    /* Parse -o options */
-    if (options) {
-        param = parse_option_parameters(options, create_options, param);
-        if (param == NULL) {
-            error("Invalid options for file format '%s'.", fmt);
-            ret = -1;
-            goto out;
-        }
-    }
-
-    /* Add old-style options to parameters */
-    ret = add_old_style_options(fmt, param, base_filename, base_fmt);
-    if (ret < 0) {
-        goto out;
-    }
-
-    // The size for the image must always be specified, with one exception:
-    // If we are using a backing file, we can obtain the size from there
-    if (get_option_parameter(param, BLOCK_OPT_SIZE)->value.n == -1) {
-
-        QEMUOptionParameter *backing_file =
-            get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
-        QEMUOptionParameter *backing_fmt =
-            get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
-
-        if (backing_file && backing_file->value.s) {
-            BlockDriverState *bs;
-            uint64_t size;
-            const char *fmt = NULL;
-            char buf[32];
-
-            if (backing_fmt && backing_fmt->value.s) {
-                 if (bdrv_find_format(backing_fmt->value.s)) {
-                     fmt = backing_fmt->value.s;
-                } else {
-                     error("Unknown backing file format '%s'",
-                        backing_fmt->value.s);
-                     ret = -1;
-                     goto out;
-                }
-            }
-
-            bs = bdrv_new_open(backing_file->value.s, fmt, BDRV_O_FLAGS);
-            if (!bs) {
-                ret = -1;
-                goto out;
-            }
-            bdrv_get_geometry(bs, &size);
-            size *= 512;
-            bdrv_delete(bs);
-
-            snprintf(buf, sizeof(buf), "%" PRId64, size);
-            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
-        } else {
-            error("Image creation needs a size parameter");
-            ret = -1;
-            goto out;
-        }
-    }
-
-    printf("Formatting '%s', fmt=%s ", filename, fmt);
-    print_option_parameters(param);
-    puts("");
-
-    ret = bdrv_create(drv, filename, param);
-    free_option_parameters(create_options);
-    free_option_parameters(param);
-
-    if (ret < 0) {
-        if (ret == -ENOTSUP) {
-            error("Formatting or formatting option not supported for file format '%s'", fmt);
-        } else if (ret == -EFBIG) {
-            error("The image size is too large for file format '%s'", fmt);
-        } else {
-            error("%s: error while creating %s: %s", filename, fmt, strerror(-ret));
-        }
-    }
+    ret = bdrv_img_create(filename, fmt, base_filename, base_fmt,
+                          options, img_size, BDRV_O_FLAGS);
 out:
     if (ret) {
         return 1;
-- 
1.7.3.2

[Qemu-devel] [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

The monitor command is:
snapshot_blkdev <device> [snapshot-file] [format]

Default format is qcow2. For now snapshots without a snapshot-file, eg
internal snapshots, are not supported.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 blockdev.c      |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   19 +++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f6ac439..1ea24d7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -514,6 +514,67 @@ void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
+int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *filename = qdict_get_try_str(qdict, "snapshot_file");
+    const char *format = qdict_get_try_str(qdict, "format");
+    const char format_qcow2[] = "qcow2";
+    BlockDriverState *bs;
+    BlockDriver *drv, *proto_drv;
+    int ret = 0;
+    int flags;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, device);
+        ret = -1;
+        goto out;
+    }
+
+    if (!format) {
+        format = format_qcow2;
+    }
+
+    drv = bdrv_find_format(format);
+    if (!drv) {
+        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
+        ret = -1;
+        goto out;
+    }
+
+    proto_drv = bdrv_find_protocol(filename);
+    if (!proto_drv) {
+        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
+        ret = -1;
+        goto out;
+    }
+
+    ret = bdrv_img_create(filename, format, bs->filename,
+                          bs->drv->format_name, NULL, -1, bs->open_flags);
+    if (ret) {
+        goto out;
+    }
+
+    qemu_aio_flush();
+    bdrv_flush(bs);
+
+    flags = bs->open_flags;
+    bdrv_close(bs);
+    ret = bdrv_open(bs, filename, flags, drv);
+    /*
+     * If reopening the image file we just created fails, we really
+     * are in trouble :(
+     */
+    assert(ret == 0);
+out:
+    if (ret) {
+        ret = 1;
+    }
+
+    return ret;
+}
+
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 {
     if (!force) {
diff --git a/blockdev.h b/blockdev.h
index 4cb8ca9..4536b5c 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -52,5 +52,6 @@ int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 23024ba..97e744e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -801,6 +801,25 @@ STEXI
 Set maximum tolerated downtime (in seconds) for migration.
 ETEXI
 
+    {
+        .name       = "snapshot_blkdev",
+        .args_type  = "device:s,snapshot_file:s?,format:s?",
+        .params     = "device [snapshot-file] [format]",
+        .help       = "initiates a live snapshot\n\t\t\t"
+                      "of device. If a snapshot file is specified, the\n\t\t\t"
+                      "snapshot file will become the new root image. If\n\t\t\t"
+                      "format is specified, the snapshot file will be\n\t\t\t"
+                      "created in that format. Otherwise the snapshot\n\t\t\t"
+                      "will be internal! (currently unsupported)",
+        .mhandler.cmd_new = do_snapshot_blkdev,
+    },
+
+STEXI
+@item snapshot_blkdev
+@findex snapshot_blkdev
+Snapshot device, using snapshot file as target if provided
+ETEXI
+
 #if defined(TARGET_I386)
     {
         .name       = "drive_add",
-- 
1.7.3.2

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Kevin Wolf]

Am 13.12.2010 08:32, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> The monitor command is:
> snapshot_blkdev <device> [snapshot-file] [format]
> 
> Default format is qcow2. For now snapshots without a snapshot-file, eg
> internal snapshots, are not supported.
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  blockdev.c      |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  blockdev.h      |    1 +
>  hmp-commands.hx |   19 +++++++++++++++++
>  3 files changed, 81 insertions(+), 0 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index f6ac439..1ea24d7 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -514,6 +514,67 @@ void do_commit(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *filename = qdict_get_try_str(qdict, "snapshot_file");
> +    const char *format = qdict_get_try_str(qdict, "format");
> +    const char format_qcow2[] = "qcow2";
> +    BlockDriverState *bs;
> +    BlockDriver *drv, *proto_drv;
> +    int ret = 0;
> +    int flags;
> +
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        qerror_report(QERR_DEVICE_NOT_FOUND, device);
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    if (!format) {
> +        format = format_qcow2;
> +    }
> +
> +    drv = bdrv_find_format(format);
> +    if (!drv) {
> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    proto_drv = bdrv_find_protocol(filename);
> +    if (!proto_drv) {
> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    ret = bdrv_img_create(filename, format, bs->filename,
> +                          bs->drv->format_name, NULL, -1, bs->open_flags);
> +    if (ret) {
> +        goto out;
> +    }
> +
> +    qemu_aio_flush();
> +    bdrv_flush(bs);
> +
> +    flags = bs->open_flags;
> +    bdrv_close(bs);
> +    ret = bdrv_open(bs, filename, flags, drv);
> +    /*
> +     * If reopening the image file we just created fails, we really
> +     * are in trouble :(
> +     */
> +    assert(ret == 0);
> +out:
> +    if (ret) {
> +        ret = 1;

I seem to remember that errors are always -1 for monitor commands.

Kevin

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Jes Sorensen]

On 12/15/10 17:55, Kevin Wolf wrote:
>> +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +{
>> +    const char *device = qdict_get_str(qdict, "device");
>> +    const char *filename = qdict_get_try_str(qdict, "snapshot_file");
>> +    const char *format = qdict_get_try_str(qdict, "format");
>> +    const char format_qcow2[] = "qcow2";
>> +    BlockDriverState *bs;
>> +    BlockDriver *drv, *proto_drv;
>> +    int ret = 0;
>> +    int flags;
>> +
>> +    bs = bdrv_find(device);
>> +    if (!bs) {
>> +        qerror_report(QERR_DEVICE_NOT_FOUND, device);
>> +        ret = -1;
>> +        goto out;
>> +    }
>> +
>> +    if (!format) {
>> +        format = format_qcow2;
>> +    }
>> +
>> +    drv = bdrv_find_format(format);
>> +    if (!drv) {
>> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
>> +        ret = -1;
>> +        goto out;
>> +    }
>> +
>> +    proto_drv = bdrv_find_protocol(filename);
>> +    if (!proto_drv) {
>> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
>> +        ret = -1;
>> +        goto out;
>> +    }
>> +
>> +    ret = bdrv_img_create(filename, format, bs->filename,
>> +                          bs->drv->format_name, NULL, -1, bs->open_flags);
>> +    if (ret) {
>> +        goto out;
>> +    }
>> +
>> +    qemu_aio_flush();
>> +    bdrv_flush(bs);
>> +
>> +    flags = bs->open_flags;
>> +    bdrv_close(bs);
>> +    ret = bdrv_open(bs, filename, flags, drv);
>> +    /*
>> +     * If reopening the image file we just created fails, we really
>> +     * are in trouble :(
>> +     */
>> +    assert(ret == 0);
>> +out:
>> +    if (ret) {
>> +        ret = 1;
> 
> I seem to remember that errors are always -1 for monitor commands.

I mapped it after something else, but admitted I cannot remember where -
can someone clarify?

Cheers,
Jes

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Luiz Capitulino]

On Wed, 15 Dec 2010 17:57:25 +0100
Jes Sorensen <Jes.Sorensen@redhat.com> wrote:

> On 12/15/10 17:55, Kevin Wolf wrote:
> >> +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> +{
> >> +    const char *device = qdict_get_str(qdict, "device");
> >> +    const char *filename = qdict_get_try_str(qdict, "snapshot_file");
> >> +    const char *format = qdict_get_try_str(qdict, "format");
> >> +    const char format_qcow2[] = "qcow2";
> >> +    BlockDriverState *bs;
> >> +    BlockDriver *drv, *proto_drv;
> >> +    int ret = 0;
> >> +    int flags;
> >> +
> >> +    bs = bdrv_find(device);
> >> +    if (!bs) {
> >> +        qerror_report(QERR_DEVICE_NOT_FOUND, device);
> >> +        ret = -1;
> >> +        goto out;
> >> +    }
> >> +
> >> +    if (!format) {
> >> +        format = format_qcow2;
> >> +    }
> >> +
> >> +    drv = bdrv_find_format(format);
> >> +    if (!drv) {
> >> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> >> +        ret = -1;
> >> +        goto out;
> >> +    }
> >> +
> >> +    proto_drv = bdrv_find_protocol(filename);
> >> +    if (!proto_drv) {
> >> +        qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> >> +        ret = -1;
> >> +        goto out;
> >> +    }
> >> +
> >> +    ret = bdrv_img_create(filename, format, bs->filename,
> >> +                          bs->drv->format_name, NULL, -1, bs->open_flags);
> >> +    if (ret) {
> >> +        goto out;
> >> +    }
> >> +
> >> +    qemu_aio_flush();
> >> +    bdrv_flush(bs);
> >> +
> >> +    flags = bs->open_flags;
> >> +    bdrv_close(bs);
> >> +    ret = bdrv_open(bs, filename, flags, drv);
> >> +    /*
> >> +     * If reopening the image file we just created fails, we really
> >> +     * are in trouble :(
> >> +     */
> >> +    assert(ret == 0);
> >> +out:
> >> +    if (ret) {
> >> +        ret = 1;
> > 
> > I seem to remember that errors are always -1 for monitor commands.
> 
> I mapped it after something else, but admitted I cannot remember where -
> can someone clarify?

-1

> 
> Cheers,
> Jes
> 

[Qemu-devel] [PATCH 3/3] Prevent creating an image with the same filename as backing file

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 3ab062c..403a434 100644
--- a/block.c
+++ b/block.c
@@ -2752,6 +2752,13 @@ int bdrv_img_create(const char *filename, const char *fmt,
     BlockDriver *drv, *proto_drv;
     int ret = 0;
 
+    if (!strcmp(filename, base_filename)) {
+        error_report("Error: Trying to create a snapshot with the same "
+                     "filename as the backing file");
+        ret = -1;
+        goto out;
+    }
+
     /* Find driver and parse its options */
     drv = bdrv_find_format(fmt);
     if (!drv) {
-- 
1.7.3.2

[Qemu-devel] Re: [PATCH 0/3] Re-factor img_create() and add live snapshots

[Kevin Wolf]

Am 13.12.2010 08:32, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> This set of patches re-factors img_create() and moves the core part of
> it into block.c so it can be accessed from qemu as well as
> qemu-img. The second patch adds basic live snapshots support to the
> code, however only snapshots to external QCOW2 images is supported for
> now. QED support should be trivial once the QED patches go into
> upstream.
> 
> The last patch fixes a small gotcha which is present in the old code
> as well. Try to catch cases where a user tries to create an image with
> itself as the backing file. QEMU does 'interesting' things when you do
> this.....
> 
> Many thanks to Kevin for his help with block layer internals!
> 
> Cheers,
> Jes
> 
> 
> Jes Sorensen (3):
>   qemu-img.c: Re-factor img_create()
>   Introduce do_snapshot_blkdev() and monitor command to handle it.
>   Prevent creating an image with the same filename as backing file

This doesn't seem to apply cleanly any more. Can you please rebase
(ideally on top of my block branch)?

Kevin

[Qemu-devel] Re: [PATCH 0/3] Re-factor img_create() and add live snapshots

[Jes Sorensen]

On 12/15/10 17:52, Kevin Wolf wrote:
> Am 13.12.2010 08:32, schrieb Jes.Sorensen@redhat.com:
>> Jes Sorensen (3):
>>   qemu-img.c: Re-factor img_create()
>>   Introduce do_snapshot_blkdev() and monitor command to handle it.
>>   Prevent creating an image with the same filename as backing file
> 
> This doesn't seem to apply cleanly any more. Can you please rebase
> (ideally on top of my block branch)?
> 
> Kevin

Darn! I'll try and rebase it tomorrow.

Thanks,
Jes

[Qemu-devel] [PATCH v2 0/3] Re-factor img_create() and add live snapshots

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

This set of patches re-factors img_create() and moves the core part of
it into block.c so it can be accessed from qemu as well as
qemu-img. The second patch adds basic live snapshots support to the
code, however only snapshots to external QCOW2 images is supported for
now. QED support should be trivial once the QED patches go into
upstream.

The last patch fixes a small gotcha which is present in the old code
as well. Try to catch cases where a user tries to create an image with
itself as the backing file. QEMU does 'interesting' things when you do
this.....

Many thanks to Kevin for his help with block layer internals!

New in v2:
 - Fix error return value in monitor command
 - Clarify help message for command
 - Fix patch conflict against block tree. It's all Stefan's fault :)
   f8feb11f4d76f390dddc5cc5345abf99f7659a78

Cheers,
Jes

Jes Sorensen (3):
  qemu-img.c: Re-factor img_create()
  Introduce do_snapshot_blkdev() and monitor command to handle it.
  Prevent creating an image with the same filename as backing file

 block.c         |  151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block.h         |    4 ++
 blockdev.c      |   61 ++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   19 +++++++
 qemu-img.c      |  108 +--------------------------------------
 6 files changed, 238 insertions(+), 106 deletions(-)

-- 
1.7.3.3

[Qemu-devel] [PATCH 3/3] Prevent creating an image with the same filename as backing file

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 765f9f3..027dc6a 100644
--- a/block.c
+++ b/block.c
@@ -2769,6 +2769,13 @@ int bdrv_img_create(const char *filename, const char *fmt,
     BlockDriver *drv, *proto_drv;
     int ret = 0;
 
+    if (!strcmp(filename, base_filename)) {
+        error_report("Error: Trying to create a snapshot with the same "
+                     "filename as the backing file");
+        ret = -1;
+        goto out;
+    }
+
     /* Find driver and parse its options */
     drv = bdrv_find_format(fmt);
     if (!drv) {
-- 
1.7.3.3

Re: [Qemu-devel] [PATCH 3/3] Prevent creating an image with the same filename as backing file

[Stefan Hajnoczi]

On Thu, Dec 16, 2010 at 11:04 AM,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  block.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/block.c b/block.c
> index 765f9f3..027dc6a 100644
> --- a/block.c
> +++ b/block.c
> @@ -2769,6 +2769,13 @@ int bdrv_img_create(const char *filename, const char *fmt,
>     BlockDriver *drv, *proto_drv;
>     int ret = 0;
>
> +    if (!strcmp(filename, base_filename)) {
> +        error_report("Error: Trying to create a snapshot with the same "
> +                     "filename as the backing file");

Can we avoid using the word "snapshot" here?  Just "image" would make sense too.

Users could hit this if they incorrectly create an image manually.
They might not be thinking in terms of "snapshot" and that word is
already overloaded in QEMU.

Stefan