qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots
@ 2010-12-16 12:52 Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 1/4] qemu-img.c: Re-factor img_create() Jes.Sorensen
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jes.Sorensen @ 2010-12-16 12:52 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, armbru, stefanha

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

New in v3:
 - Address issues pointed out by Stefan and Kevin
 - Additional patch to return proper -errno error values on error in
   bdrv_img_create() as suggested by Kevin.

Jes Sorensen (4):
  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
  bdrv_img_create() use proper errno return values

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

-- 
1.7.3.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 1/4] qemu-img.c: Re-factor img_create()
  2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
@ 2010-12-16 12:52 ` Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 2/4] Introduce do_snapshot_blkdev() and monitor command to handle it Jes.Sorensen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jes.Sorensen @ 2010-12-16 12:52 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, armbru, stefanha

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 |  108 +---------------------------------------------
 3 files changed, 147 insertions(+), 106 deletions(-)

diff --git a/block.c b/block.c
index b4aaf41..a48b30c 100644
--- a/block.c
+++ b/block.c
@@ -2758,3 +2758,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;
+    QEMUOptionParameter *backing_fmt;
+    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;
+        }
+    }
+
+    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
+    if (backing_fmt && backing_fmt->value.s) {
+        if (!bdrv_find_format(backing_fmt->value.s)) {
+            error_report("Unknown backing file format '%s'",
+                         backing_fmt->value.s);
+            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);
+
+        if (backing_file && backing_file->value.s) {
+            uint64_t size;
+            const char *fmt = NULL;
+            char buf[32];
+
+            if (backing_fmt && backing_fmt->value.s) {
+                fmt = backing_fmt->value.s;
+            }
+
+            bs = bdrv_new("");
+
+            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);
+
+    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:
+    free_option_parameters(create_options);
+    free_option_parameters(param);
+
+    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 1d936ed..603bdb3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -287,9 +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;
-    QEMUOptionParameter *backing_fmt = NULL;
     char *options = NULL;
 
     for(;;) {
@@ -350,110 +347,9 @@ 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;
-    }
-
-    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
-    if (backing_fmt && backing_fmt->value.s) {
-        if (!bdrv_find_format(backing_fmt->value.s)) {
-            error("Unknown backing file format '%s'",
-                  backing_fmt->value.s);
-            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);
-
-        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) {
-                fmt = backing_fmt->value.s;
-            }
-
-            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);
-
-    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:
-    free_option_parameters(create_options);
-    free_option_parameters(param);
     if (ret) {
         return 1;
     }
-- 
1.7.3.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 2/4] Introduce do_snapshot_blkdev() and monitor command to handle it.
  2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 1/4] qemu-img.c: Re-factor img_create() Jes.Sorensen
@ 2010-12-16 12:52 ` Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 3/4] Prevent creating an image with the same filename as backing file Jes.Sorensen
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jes.Sorensen @ 2010-12-16 12:52 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, armbru, stefanha

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      |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   19 ++++++++++++++++
 3 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b3b82d..d7add36 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -516,6 +516,68 @@ 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");
+    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 = "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 :(
+     */
+    if (ret != 0) {
+        abort();
+    }
+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..dd3db36 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 [new-image-file] [format]",
+        .help       = "initiates a live snapshot\n\t\t\t"
+                      "of device. If a new image file is specified, the\n\t\t\t"
+                      "new image file will become the new root image.\n\t\t\t"
+                      "If format is specified, the snapshot file will\n\t\t\t"
+                      "be created in that format. Otherwise the\n\t\t\t"
+                      "snapshot 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.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 3/4] Prevent creating an image with the same filename as backing file
  2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 1/4] qemu-img.c: Re-factor img_create() Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 2/4] Introduce do_snapshot_blkdev() and monitor command to handle it Jes.Sorensen
@ 2010-12-16 12:52 ` Jes.Sorensen
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 4/4] bdrv_img_create() use proper errno return values Jes.Sorensen
  2010-12-16 14:21 ` [Qemu-devel] Re: [PATCH v3 0/4] Re-factor img_create() and add live snapshots Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Jes.Sorensen @ 2010-12-16 12:52 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, armbru, stefanha

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

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

diff --git a/block.c b/block.c
index a48b30c..0c14eee 100644
--- a/block.c
+++ b/block.c
@@ -2764,7 +2764,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
                     char *options, uint64_t img_size, int flags)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
-    QEMUOptionParameter *backing_fmt;
+    QEMUOptionParameter *backing_fmt, *backing_file;
     BlockDriverState *bs = NULL;
     BlockDriver *drv, *proto_drv;
     int ret = 0;
@@ -2823,6 +2823,16 @@ int bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
+    backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
+    if (backing_file && backing_file->value.s) {
+        if (!strcmp(filename, backing_file->value.s)) {
+            error_report("Error: Trying to create an image with the "
+                         "same filename as the backing file");
+            ret = -1;
+            goto out;
+        }
+    }
+
     backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
     if (backing_fmt && backing_fmt->value.s) {
         if (!bdrv_find_format(backing_fmt->value.s)) {
@@ -2836,9 +2846,6 @@ int bdrv_img_create(const char *filename, const char *fmt,
     // 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);
-
         if (backing_file && backing_file->value.s) {
             uint64_t size;
             const char *fmt = NULL;
-- 
1.7.3.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 4/4] bdrv_img_create() use proper errno return values
  2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
                   ` (2 preceding siblings ...)
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 3/4] Prevent creating an image with the same filename as backing file Jes.Sorensen
@ 2010-12-16 12:52 ` Jes.Sorensen
  2010-12-16 14:21 ` [Qemu-devel] Re: [PATCH v3 0/4] Re-factor img_create() and add live snapshots Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Jes.Sorensen @ 2010-12-16 12:52 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, armbru, stefanha

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

Kevin suggested to have bdrv_img_create() return proper -errno values
on error.

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

diff --git a/block.c b/block.c
index 0c14eee..fe07d0b 100644
--- a/block.c
+++ b/block.c
@@ -2773,14 +2773,14 @@ int bdrv_img_create(const char *filename, const char *fmt,
     drv = bdrv_find_format(fmt);
     if (!drv) {
         error_report("Unknown file format '%s'", fmt);
-        ret = -1;
+        ret = -EINVAL;
         goto out;
     }
 
     proto_drv = bdrv_find_protocol(filename);
     if (!proto_drv) {
         error_report("Unknown protocol '%s'", filename);
-        ret = -1;
+        ret = -EINVAL;
         goto out;
     }
 
@@ -2799,7 +2799,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
         param = parse_option_parameters(options, create_options, param);
         if (param == NULL) {
             error_report("Invalid options for file format '%s'.", fmt);
-            ret = -1;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2809,7 +2809,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
                                  base_filename)) {
             error_report("Backing file not supported for file format '%s'",
                          fmt);
-            ret = -1;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2818,7 +2818,7 @@ int bdrv_img_create(const char *filename, const char *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;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2828,7 +2828,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
         if (!strcmp(filename, backing_file->value.s)) {
             error_report("Error: Trying to create an image with the "
                          "same filename as the backing file");
-            ret = -1;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2838,7 +2838,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
         if (!bdrv_find_format(backing_fmt->value.s)) {
             error_report("Unknown backing file format '%s'",
                          backing_fmt->value.s);
-            ret = -1;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2860,7 +2860,6 @@ int bdrv_img_create(const char *filename, const char *fmt,
             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);
@@ -2870,7 +2869,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
             set_option_parameter(param, BLOCK_OPT_SIZE, buf);
         } else {
             error_report("Image creation needs a size parameter");
-            ret = -1;
+            ret = -EINVAL;
             goto out;
         }
     }
@@ -2901,8 +2900,6 @@ out:
     if (bs) {
         bdrv_delete(bs);
     }
-    if (ret) {
-        return 1;
-    }
-    return 0;
+
+    return ret;
 }
-- 
1.7.3.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] Re: [PATCH v3 0/4] Re-factor img_create() and add live snapshots
  2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
                   ` (3 preceding siblings ...)
  2010-12-16 12:52 ` [Qemu-devel] [PATCH 4/4] bdrv_img_create() use proper errno return values Jes.Sorensen
@ 2010-12-16 14:21 ` Kevin Wolf
  4 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2010-12-16 14:21 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: qemu-devel, armbru, stefanha

Am 16.12.2010 13:52, 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!
> 
> 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
> 
> New in v3:
>  - Address issues pointed out by Stefan and Kevin
>  - Additional patch to return proper -errno error values on error in
>    bdrv_img_create() as suggested by Kevin.
> 
> Jes Sorensen (4):
>   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
>   bdrv_img_create() use proper errno return values
> 
>  block.c         |  145 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    4 ++
>  blockdev.c      |   62 +++++++++++++++++++++++
>  blockdev.h      |    1 +
>  hmp-commands.hx |   19 +++++++
>  qemu-img.c      |  108 +----------------------------------------
>  6 files changed, 233 insertions(+), 106 deletions(-)

Thanks, applied all to the block branch.

Kevin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-12-16 14:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-16 12:52 [Qemu-devel] [PATCH v3 0/4] Re-factor img_create() and add live snapshots Jes.Sorensen
2010-12-16 12:52 ` [Qemu-devel] [PATCH 1/4] qemu-img.c: Re-factor img_create() Jes.Sorensen
2010-12-16 12:52 ` [Qemu-devel] [PATCH 2/4] Introduce do_snapshot_blkdev() and monitor command to handle it Jes.Sorensen
2010-12-16 12:52 ` [Qemu-devel] [PATCH 3/4] Prevent creating an image with the same filename as backing file Jes.Sorensen
2010-12-16 12:52 ` [Qemu-devel] [PATCH 4/4] bdrv_img_create() use proper errno return values Jes.Sorensen
2010-12-16 14:21 ` [Qemu-devel] Re: [PATCH v3 0/4] Re-factor img_create() and add live snapshots Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).