[Qemu-devel] [PATCH 0/4] Make qemu-img create options generic

[Qemu-devel] [PATCH 0/4] Make qemu-img create options generic

[Kevin Wolf]

Currently, qemu-img and the bdrv_create() interface need to carry parameters
for every single option that is used by at least one file format. We already
have a few of such format specific options (encryption for qcow2, version 6
header for VMDK, backing file for some formats) and there are more to come
(like qcow cluster size).

This patch series changes this into a more generic approach: All block drivers
contain a data structure which describes all options this driver recognizes.
qemu-img can use this structure then instead of hard-coding every single
option. An -o option is introduced for these parameteres, the format is the
usual comma separated name=value style like in -drive.

Kevin Wolf (4):
  Create qemu-option.h
  Convert all block drivers to new bdrv_create
  Convert qemu-img create to new bdrv_create
  Convert qemu-img convert to new bdrv_create

 Makefile          |    2 +-
 block.c           |   44 +++++--
 block.h           |    6 +-
 block/cow.c       |   26 +++-
 block/qcow.c      |   28 ++++-
 block/qcow2.c     |   36 +++++-
 block/raw-posix.c |   37 ++++--
 block/raw-win32.c |   20 +++-
 block/vmdk.c      |   28 ++++-
 block/vpc.c       |   21 +++-
 block/vvfat.c     |    4 +-
 block_int.h       |   17 ++-
 qemu-img.c        |  188 ++++++++++++++++++++---------
 qemu-option.c     |  349 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-option.h     |   67 ++++++++++
 sysemu.h          |    2 -
 vl.c              |   38 +------
 17 files changed, 762 insertions(+), 151 deletions(-)
 create mode 100644 qemu-option.c
 create mode 100644 qemu-option.h

[Qemu-devel] [PATCH 1/4] Create qemu-option.h

[Kevin Wolf]

This patch creates a new header file and the corresponding implementation file
for parsing of parameter strings for options (like used in -drive). Part of
this is code moved from vl.c (so qemu-img can use it later).

The idea is to have a data structure describing all accepted parameters. When
parsing a parameter string, the structure is copied and filled with the
parameter values.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 Makefile      |    2 +-
 qemu-option.c |  349 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-option.h |   67 +++++++++++
 sysemu.h      |    2 -
 vl.c          |   38 +------
 5 files changed, 418 insertions(+), 40 deletions(-)
 create mode 100644 qemu-option.c
 create mode 100644 qemu-option.h

diff --git a/Makefile b/Makefile
index f303aff..fd0ddb1 100644
--- a/Makefile
+++ b/Makefile
@@ -63,7 +63,7 @@ recurse-all: $(SUBDIR_RULES)
 #######################################################################
 # BLOCK_OBJS is code used by both qemu system emulation and qemu-img
 
-BLOCK_OBJS=cutils.o cache-utils.o qemu-malloc.o module.o
+BLOCK_OBJS=cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
 BLOCK_OBJS+=block/cow.o block/qcow.o aes.o block/vmdk.o block/cloop.o
 BLOCK_OBJS+=block/dmg.o block/bochs.o block/vpc.o block/vvfat.o
 BLOCK_OBJS+=block/qcow2.o block/parallels.o block/nbd.o
diff --git a/qemu-option.c b/qemu-option.c
new file mode 100644
index 0000000..3cebdd5
--- /dev/null
+++ b/qemu-option.c
@@ -0,0 +1,349 @@
+/*
+ * Commandline option parsing functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2009 Kevin Wolf <kwolf@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "qemu-common.h"
+#include "qemu-option.h"
+
+/*
+ * Extracts the name of an option from the parameter string (p points at the
+ * first byte of the option name)
+ *
+ * The option name is delimited by delim (usually , or =) or the string end
+ * and is copied into buf. If the option name is longer than buf_size, it is
+ * truncated. buf is always zero terminated.
+ *
+ * The return value is the position of the delimiter/zero byte after the option
+ * name in p.
+ */
+const char *get_opt_name(char *buf, int buf_size, const char *p, char delim)
+{
+    char *q;
+
+    q = buf;
+    while (*p != '\0' && *p != delim) {
+        if (q && (q - buf) < buf_size - 1)
+            *q++ = *p;
+        p++;
+    }
+    if (q)
+        *q = '\0';
+
+    return p;
+}
+
+/*
+ * Extracts the value of an option from the parameter string p (p points at the
+ * first byte of the option value)
+ *
+ * This function is comparable to get_opt_name with the difference that the
+ * delimiter is fixed to be comma which starts a new option. To specify an
+ * option value that contains commas, double each comma.
+ */
+const char *get_opt_value(char *buf, int buf_size, const char *p)
+{
+    char *q;
+
+    q = buf;
+    while (*p != '\0') {
+        if (*p == ',') {
+            if (*(p + 1) != ',')
+                break;
+            p++;
+        }
+        if (q && (q - buf) < buf_size - 1)
+            *q++ = *p;
+        p++;
+    }
+    if (q)
+        *q = '\0';
+
+    return p;
+}
+
+/*
+ * Searches an option list for an option with the given name
+ */
+QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list,
+    const char *name)
+{
+    while (list && list->name) {
+        if (!strcmp(list->name, name)) {
+            return list;
+        }
+        list++;
+    }
+
+    return NULL;
+}
+
+/*
+ * Sets the value of a parameter in a given option list. The parsing of the
+ * value depends on the type of option:
+ *
+ * OPT_FLAG (uses value.n):
+ *      If no value is given, the flag is set to 1.
+ *      Otherwise the value must be "on" (set to 1) or "off" (set to 0)
+ *
+ * OPT_STRING (uses value.s):
+ *      value is strdup()ed and assigned as option value
+ *
+ * OPT_SIZE (uses value.n):
+ *      The value is converted to an integer. Suffixes for kilobytes etc. are
+ *      allowed (powers of 1024).
+ *
+ * Returns 0 on succes, -1 in error cases
+ */
+int set_option_parameter(QEMUOptionParameter *list, const char *name,
+    const char *value)
+{
+    // Find a matching parameter
+    list = get_option_parameter(list, name);
+    if (list == NULL) {
+        fprintf(stderr, "Unknown option '%s'\n", name);
+        return -1;
+    }
+
+    // Process parameter
+    switch (list->type) {
+    case OPT_FLAG:
+        if (value != NULL) {
+            if (!strcmp(value, "on")) {
+                list->value.n = 1;
+            } else if (!strcmp(value, "off")) {
+                list->value.n = 0;
+            } else {
+                fprintf(stderr, "Option '%s': Use 'on' or 'off'\n", name);
+                return -1;
+            }
+        } else {
+            list->value.n = 1;
+        }
+        break;
+
+    case OPT_STRING:
+        if (value != NULL) {
+            list->value.s = strdup(value);
+        } else {
+            fprintf(stderr, "Option '%s' needs a parameter\n", name);
+            return -1;
+        }
+        break;
+
+    case OPT_SIZE:
+        if (value != NULL) {
+            double sizef = strtod(value, (char**) &value);
+
+            switch (*value) {
+            case 'T':
+                sizef *= 1024;
+            case 'G':
+                sizef *= 1024;
+            case 'M':
+                sizef *= 1024;
+            case 'K':
+            case 'k':
+                sizef *= 1024;
+            case 'b':
+            case '\0':
+                list->value.n = (uint64_t) sizef;
+                break;
+            default:
+                fprintf(stderr, "Option '%s' needs size as parameter\n", name);
+                fprintf(stderr, "You may use k, M, G or T suffixes for "
+                    "kilobytes, megabytes, gigabytes and terabytes.\n");
+                return -1;
+            }
+        } else {
+            fprintf(stderr, "Option '%s' needs a parameter\n", name);
+            return -1;
+        }
+        break;
+    default:
+        fprintf(stderr, "Bug: Option '%s' has an unknown type\n", name);
+        return -1;
+    }
+
+    return 0;
+}
+
+/*
+ * Sets the given parameter to an integer instead of a string.
+ * This function cannot be used to set string options.
+ *
+ * Returns 0 on success, -1 in error cases
+ */
+int set_option_parameter_int(QEMUOptionParameter *list, const char *name,
+    uint64_t value)
+{
+    // Find a matching parameter
+    list = get_option_parameter(list, name);
+    if (list == NULL) {
+        fprintf(stderr, "Unknown option '%s'\n", name);
+        return -1;
+    }
+
+    // Process parameter
+    switch (list->type) {
+    case OPT_FLAG:
+    case OPT_NUMBER:
+    case OPT_SIZE:
+        list->value.n = value;
+        break;
+
+    default:
+        return -1;
+    }
+
+    return 0;
+}
+
+/*
+ * Frees a option list. If it contains strings, the strings are freed as well.
+ */
+void free_option_parameters(QEMUOptionParameter *list)
+{
+    QEMUOptionParameter *cur = list;
+
+    while (cur && cur->name) {
+        if (cur->type == OPT_STRING) {
+            free(cur->value.s);
+        }
+        cur++;
+    }
+
+    free(list);
+}
+
+/*
+ * Parses a parameter string (param) into an option list (dest).
+ *
+ * list is the templace is. If dest is NULL, a new copy of list is created for
+ * it. If list is NULL, this function fails.
+ *
+ * A parameter string consists of one or more parameters, separated by commas.
+ * Each parameter consists of its name and possibly of a value. In the latter
+ * case, the value is delimited by an = character. To specify a value which
+ * contains commas, double each comma so it won't be recognized as the end of
+ * the parameter.
+ *
+ * For more details of the parsing see above.
+ *
+ * Returns a pointer to the first element of dest (or the newly allocated copy)
+ * or NULL in error cases
+ */
+QEMUOptionParameter *parse_option_parameters(const char *param,
+    QEMUOptionParameter *list, QEMUOptionParameter *dest)
+{
+    QEMUOptionParameter *cur;
+    QEMUOptionParameter *allocated = NULL;
+    char name[256];
+    char value[256];
+    char *param_delim, *value_delim;
+    char next_delim;
+    size_t num_options;
+
+    if (list == NULL) {
+        return NULL;
+    }
+
+    if (dest == NULL) {
+        // Count valid options
+        num_options = 0;
+        cur = list;
+        while (cur->name) {
+            num_options++;
+            cur++;
+        }
+
+        // Create a copy of the option list to fill in values
+        dest = qemu_mallocz((num_options + 1) * sizeof(QEMUOptionParameter));
+        allocated = dest;
+        memcpy(dest, list, (num_options + 1) * sizeof(QEMUOptionParameter));
+    }
+
+    while (*param) {
+
+        // Find parameter name and value in the string
+        param_delim = strchr(param, ',');
+        value_delim = strchr(param, '=');
+
+        if (value_delim && (value_delim < param_delim || !param_delim)) {
+            next_delim = '=';
+        } else {
+            next_delim = ',';
+            value_delim = NULL;
+        }
+
+        param = get_opt_name(name, sizeof(name), param, next_delim);
+        if (value_delim) {
+            param = get_opt_value(value, sizeof(value), param + 1);
+        }
+        if (*param != '\0') {
+            param++;
+        }
+
+        // Set the parameter
+        if (set_option_parameter(dest, name, value_delim ? value : NULL)) {
+            goto fail;
+        }
+    }
+
+    return dest;
+
+fail:
+    // Only free the list if it was newly allocated
+    free_option_parameters(allocated);
+    return NULL;
+}
+
+/*
+ * Prints all options of a list that have a value to stdout
+ */
+void print_option_parameters(QEMUOptionParameter *list)
+{
+    while (list && list->name) {
+        switch (list->type) {
+            case OPT_STRING:
+                 if (list->value.s != NULL) {
+                     printf("%s='%s' ", list->name, list->value.s);
+                 }
+                break;
+            case OPT_FLAG:
+                printf("%s=%s ", list->name, list->value.n ? "on" : "off");
+                break;
+            case OPT_SIZE:
+            case OPT_NUMBER:
+                printf("%s=%" PRId64 " ", list->name, list->value.n);
+                break;
+            default:
+                printf("%s=(unkown type) ", list->name);
+                break;
+        }
+        list++;
+    }
+}
diff --git a/qemu-option.h b/qemu-option.h
new file mode 100644
index 0000000..ac24694
--- /dev/null
+++ b/qemu-option.h
@@ -0,0 +1,67 @@
+/*
+ * Commandline option parsing functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2009 Kevin Wolf <kwolf@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_OPTIONS_H
+#define QEMU_OPTIONS_H
+
+enum QEMUOptionParType {
+    OPT_FLAG,
+    OPT_NUMBER,
+    OPT_SIZE,
+    OPT_STRING,
+};
+
+typedef struct QEMUOptionParameter {
+    const char *name;
+    enum QEMUOptionParType type;
+    union {
+        uint64_t n;
+        char* s;
+    } value;
+} QEMUOptionParameter;
+
+
+const char *get_opt_name(char *buf, int buf_size, const char *p, char delim);
+const char *get_opt_value(char *buf, int buf_size, const char *p);
+
+
+/*
+ * The following functions take a parameter list as input. This is a pointer to
+ * the first element of a QEMUOptionParameter array which is terminated by an
+ * entry with entry->name == NULL.
+ */
+
+QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list,
+    const char *name);
+int set_option_parameter(QEMUOptionParameter *list, const char *name,
+    const char *value);
+int set_option_parameter_int(QEMUOptionParameter *list, const char *name,
+    uint64_t value);
+QEMUOptionParameter *parse_option_parameters(const char *param,
+    QEMUOptionParameter *list, QEMUOptionParameter *dest);
+void free_option_parameters(QEMUOptionParameter *list);
+void print_option_parameters(QEMUOptionParameter *list);
+
+#endif
diff --git a/sysemu.h b/sysemu.h
index 2ff6194..8b1af6b 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -256,8 +256,6 @@ void do_usb_add(Monitor *mon, const char *devname);
 void do_usb_del(Monitor *mon, const char *devname);
 void usb_info(Monitor *mon);
 
-const char *get_opt_name(char *buf, int buf_size, const char *p, char delim);
-const char *get_opt_value(char *buf, int buf_size, const char *p);
 int get_param_value(char *buf, int buf_size,
                     const char *tag, const char *str);
 int check_params(const char * const *params, const char *str);
diff --git a/vl.c b/vl.c
index 40b1d8b..ca04c49 100644
--- a/vl.c
+++ b/vl.c
@@ -156,6 +156,7 @@ int main(int argc, char **argv)
 #include "migration.h"
 #include "kvm.h"
 #include "balloon.h"
+#include "qemu-option.h"
 
 #include "disas.h"
 
@@ -1806,43 +1807,6 @@ static int socket_init(void)
 }
 #endif
 
-const char *get_opt_name(char *buf, int buf_size, const char *p, char delim)
-{
-    char *q;
-
-    q = buf;
-    while (*p != '\0' && *p != delim) {
-        if (q && (q - buf) < buf_size - 1)
-            *q++ = *p;
-        p++;
-    }
-    if (q)
-        *q = '\0';
-
-    return p;
-}
-
-const char *get_opt_value(char *buf, int buf_size, const char *p)
-{
-    char *q;
-
-    q = buf;
-    while (*p != '\0') {
-        if (*p == ',') {
-            if (*(p + 1) != ',')
-                break;
-            p++;
-        }
-        if (q && (q - buf) < buf_size - 1)
-            *q++ = *p;
-        p++;
-    }
-    if (q)
-        *q = '\0';
-
-    return p;
-}
-
 int get_param_value(char *buf, int buf_size,
                     const char *tag, const char *str)
 {
-- 
1.6.0.6

[Qemu-devel] [PATCH 2/4] Convert all block drivers to new bdrv_create

[Kevin Wolf]

Now we can make use of the newly introduced option structures. Instead of
having bdrv_create carry more and more parameters (which are format specific in
most cases), just pass a option structure as defined by the driver itself.

bdrv_create2() contains an emulation of the old interface to simplify the
transition.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c           |   44 +++++++++++++++++++++++++++++++++-----------
 block.h           |    6 +++---
 block/cow.c       |   26 +++++++++++++++++++++-----
 block/qcow.c      |   28 ++++++++++++++++++++++++++--
 block/qcow2.c     |   36 +++++++++++++++++++++++++++++++-----
 block/raw-posix.c |   37 +++++++++++++++++++++++++++----------
 block/raw-win32.c |   20 ++++++++++++++++----
 block/vmdk.c      |   28 ++++++++++++++++++++++++++--
 block/vpc.c       |   21 +++++++++++++++++----
 block/vvfat.c     |    4 ++--
 block_int.h       |   17 +++++++++++------
 qemu-img.c        |    2 +-
 12 files changed, 214 insertions(+), 55 deletions(-)

diff --git a/block.c b/block.c
index 980fbec..7f4cf9e 100644
--- a/block.c
+++ b/block.c
@@ -189,22 +189,44 @@ int bdrv_create2(BlockDriver *drv,
                 const char *backing_file, const char *backing_format,
                 int flags)
 {
-    if (drv->bdrv_create2)
-        return drv->bdrv_create2(filename, size_in_sectors, backing_file,
-                                 backing_format, flags);
-    if (drv->bdrv_create)
-        return drv->bdrv_create(filename, size_in_sectors, backing_file,
-                                flags);
-    return -ENOTSUP;
+    QEMUOptionParameter *options;
+
+    options = parse_option_parameters("", drv->create_options, NULL);
+
+    // Process flags
+    if (flags & ~(BLOCK_FLAG_ENCRYPT | BLOCK_FLAG_COMPAT6 | BLOCK_FLAG_COMPRESS)) {
+        return -ENOTSUP;
+    }
+
+    if (flags & BLOCK_FLAG_ENCRYPT) {
+        set_option_parameter_int(options, BLOCK_OPT_ENCRYPT, 1);
+    }
+    if (flags & BLOCK_FLAG_COMPAT6) {
+        set_option_parameter_int(options, BLOCK_OPT_COMPAT6, 1);
+    }
+
+    // Add size to options
+    set_option_parameter_int(options, BLOCK_OPT_SIZE, size_in_sectors * 512);
+
+    // Backing files
+    if ((backing_file != NULL && set_option_parameter(options,
+            BLOCK_OPT_BACKING_FILE, backing_file))
+        || (backing_format != NULL && set_option_parameter(options,
+            BLOCK_OPT_BACKING_FMT, backing_format)))
+    {
+        return -ENOTSUP;
+    }
+
+    return bdrv_create(drv, filename, options);
 }
 
-int bdrv_create(BlockDriver *drv,
-                const char *filename, int64_t size_in_sectors,
-                const char *backing_file, int flags)
+int bdrv_create(BlockDriver *drv, const char* filename,
+    QEMUOptionParameter *options)
 {
     if (!drv->bdrv_create)
         return -ENOTSUP;
-    return drv->bdrv_create(filename, size_in_sectors, backing_file, flags);
+
+    return drv->bdrv_create(filename, options);
 }
 
 #ifdef _WIN32
diff --git a/block.h b/block.h
index 22df8ca..0acac63 100644
--- a/block.h
+++ b/block.h
@@ -3,6 +3,7 @@
 
 #include "qemu-aio.h"
 #include "qemu-common.h"
+#include "qemu-option.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
@@ -45,9 +46,8 @@ void bdrv_info_stats(Monitor *mon);
 
 void bdrv_init(void);
 BlockDriver *bdrv_find_format(const char *format_name);
-int bdrv_create(BlockDriver *drv,
-                const char *filename, int64_t size_in_sectors,
-                const char *backing_file, int flags);
+int bdrv_create(BlockDriver *drv, const char* filename,
+    QEMUOptionParameter *options);
 int bdrv_create2(BlockDriver *drv,
                  const char *filename, int64_t size_in_sectors,
                  const char *backing_file, const char *backing_format,
diff --git a/block/cow.c b/block/cow.c
index 94b3549..41d292a 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -202,15 +202,23 @@ static void cow_close(BlockDriverState *bs)
     close(s->fd);
 }
 
-static int cow_create(const char *filename, int64_t image_sectors,
-                      const char *image_filename, int flags)
+static int cow_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd, cow_fd;
     struct cow_header_v2 cow_header;
     struct stat st;
-
-    if (flags)
-        return -ENOTSUP;
+    int64_t image_sectors = 0;
+    const char *image_filename = NULL;
+
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            image_sectors = options->value.n / 512;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+            image_filename = options->value.s;
+        }
+        options++;
+    }
 
     cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
@@ -253,6 +261,12 @@ static void cow_flush(BlockDriverState *bs)
     fsync(s->fd);
 }
 
+static QEMUOptionParameter cow_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { BLOCK_OPT_BACKING_FILE,   OPT_STRING },
+    { NULL }
+};
+
 static BlockDriver bdrv_cow = {
     .format_name	= "cow",
     .instance_size	= sizeof(BDRVCowState),
@@ -264,6 +278,8 @@ static BlockDriver bdrv_cow = {
     .bdrv_create	= cow_create,
     .bdrv_flush		= cow_flush,
     .bdrv_is_allocated	= cow_is_allocated,
+
+    .create_options = cow_create_options,
 };
 
 static void bdrv_cow_init(void)
diff --git a/block/qcow.c b/block/qcow.c
index 1cf7c3b..6ecf2e8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -767,12 +767,26 @@ static void qcow_close(BlockDriverState *bs)
     bdrv_delete(s->hd);
 }
 
-static int qcow_create(const char *filename, int64_t total_size,
-                      const char *backing_file, int flags)
+static int qcow_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd, header_size, backing_filename_len, l1_size, i, shift;
     QCowHeader header;
     uint64_t tmp;
+    int64_t total_size = 0;
+    const char *backing_file = NULL;
+    int flags = 0;
+
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / 512;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+            backing_file = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
+            flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
+        }
+        options++;
+    }
 
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
     if (fd < 0)
@@ -918,6 +932,14 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
+
+static QEMUOptionParameter qcow_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { BLOCK_OPT_BACKING_FILE,   OPT_STRING },
+    { BLOCK_OPT_ENCRYPT,        OPT_FLAG },
+    { NULL }
+};
+
 static BlockDriver bdrv_qcow = {
     .format_name	= "qcow",
     .instance_size	= sizeof(BDRVQcowState),
@@ -935,6 +957,8 @@ static BlockDriver bdrv_qcow = {
     .aiocb_size		= sizeof(QCowAIOCB),
     .bdrv_write_compressed = qcow_write_compressed,
     .bdrv_get_info	= qcow_get_info,
+
+    .create_options = qcow_create_options,
 };
 
 static void bdrv_qcow_init(void)
diff --git a/block/qcow2.c b/block/qcow2.c
index a6de9b6..b6171d2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1696,10 +1696,28 @@ static int qcow_create2(const char *filename, int64_t total_size,
     return 0;
 }
 
-static int qcow_create(const char *filename, int64_t total_size,
-                       const char *backing_file, int flags)
-{
-    return qcow_create2(filename, total_size, backing_file, NULL, flags);
+static int qcow_create(const char *filename, QEMUOptionParameter *options)
+{
+    const char *backing_file = NULL;
+    const char *backing_fmt = NULL;
+    uint64_t sectors = 0;
+    int flags = 0;
+
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            sectors = options->value.n / 512;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+            backing_file = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
+            backing_fmt = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
+            flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
+        }
+        options++;
+    }
+
+    return qcow_create2(filename, sectors, backing_file, backing_fmt, flags);
 }
 
 static int qcow_make_empty(BlockDriverState *bs)
@@ -2892,6 +2910,14 @@ static int qcow_get_buffer(BlockDriverState *bs, uint8_t *buf,
     return ret;
 }
 
+static QEMUOptionParameter qcow_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { BLOCK_OPT_BACKING_FILE,   OPT_STRING },
+    { BLOCK_OPT_BACKING_FMT,    OPT_STRING },
+    { BLOCK_OPT_ENCRYPT,        OPT_FLAG },
+    { NULL }
+};
+
 static BlockDriver bdrv_qcow2 = {
     .format_name	= "qcow2",
     .instance_size	= sizeof(BDRVQcowState),
@@ -2919,7 +2945,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_put_buffer    = qcow_put_buffer,
     .bdrv_get_buffer    = qcow_get_buffer,
 
-    .bdrv_create2 = qcow_create2,
+    .create_options = qcow_create_options,
     .bdrv_check = qcow_check,
 };
 
diff --git a/block/raw-posix.c b/block/raw-posix.c
index f3a9476..86e3067 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -823,13 +823,18 @@ again:
 }
 #endif
 
-static int raw_create(const char *filename, int64_t total_size,
-                      const char *backing_file, int flags)
+static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd;
+    int64_t total_size = 0;
 
-    if (flags || backing_file)
-        return -ENOTSUP;
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / 512;
+        }
+        options++;
+    }
 
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
@@ -846,6 +851,12 @@ static void raw_flush(BlockDriverState *bs)
     fsync(s->fd);
 }
 
+
+static QEMUOptionParameter raw_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { NULL }
+};
+
 static BlockDriver bdrv_raw = {
     .format_name = "raw",
     .instance_size = sizeof(BDRVRawState),
@@ -866,6 +877,8 @@ static BlockDriver bdrv_raw = {
 
     .bdrv_truncate = raw_truncate,
     .bdrv_getlength = raw_getlength,
+
+    .create_options = raw_create_options,
 };
 
 /***********************************************/
@@ -1364,15 +1377,20 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs,
 #endif /* !linux && !FreeBSD */
 
 #if defined(__linux__) || defined(__FreeBSD__)
-static int hdev_create(const char *filename, int64_t total_size,
-                       const char *backing_file, int flags)
+static int hdev_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd;
     int ret = 0;
     struct stat stat_buf;
+    int64_t total_size = 0;
 
-    if (flags || backing_file)
-        return -ENOTSUP;
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, "size")) {
+            total_size = options->value.n / 512;
+        }
+        options++;
+    }
 
     fd = open(filename, O_WRONLY | O_BINARY);
     if (fd < 0)
@@ -1391,8 +1409,7 @@ static int hdev_create(const char *filename, int64_t total_size,
 
 #else  /* !(linux || freebsd) */
 
-static int hdev_create(const char *filename, int64_t total_size,
-                       const char *backing_file, int flags)
+static int hdev_create(const char *filename, QEMUOptionParameter *options)
 {
     return -ENOTSUP;
 }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 15f3ec4..6e5c09b 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -210,13 +210,18 @@ static int64_t raw_getlength(BlockDriverState *bs)
     return l.QuadPart;
 }
 
-static int raw_create(const char *filename, int64_t total_size,
-                      const char *backing_file, int flags)
+static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd;
+    int64_t total_size = 0;
 
-    if (flags || backing_file)
-        return -ENOTSUP;
+    /* Read out options */
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / 512;
+        }
+        options++;
+    }
 
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
               0644);
@@ -228,6 +233,11 @@ static int raw_create(const char *filename, int64_t total_size,
     return 0;
 }
 
+static QEMUOptionParameter raw_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { NULL }
+};
+
 static BlockDriver bdrv_raw = {
     .format_name	= "raw",
     .instance_size	= sizeof(BDRVRawState),
@@ -239,6 +249,8 @@ static BlockDriver bdrv_raw = {
     .bdrv_write		= raw_write,
     .bdrv_truncate	= raw_truncate,
     .bdrv_getlength	= raw_getlength,
+
+    .create_options = raw_create_options,
 };
 
 /***********************************************/
diff --git a/block/vmdk.c b/block/vmdk.c
index 13866e9..b3ea686 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -687,8 +687,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static int vmdk_create(const char *filename, int64_t total_size,
-                       const char *backing_file, int flags)
+static int vmdk_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd, i;
     VMDK4Header header;
@@ -713,6 +712,21 @@ static int vmdk_create(const char *filename, int64_t total_size,
         "ddb.adapterType = \"ide\"\n";
     char desc[1024];
     const char *real_filename, *temp_str;
+    int64_t total_size = 0;
+    const char *backing_file = NULL;
+    int flags = 0;
+
+    // Read out options
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            total_size = options->value.n / 512;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+            backing_file = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
+            flags |= options->value.n ? BLOCK_FLAG_COMPAT6: 0;
+        }
+        options++;
+    }
 
     /* XXX: add support for backing file */
     if (backing_file) {
@@ -812,6 +826,14 @@ static void vmdk_flush(BlockDriverState *bs)
     bdrv_flush(s->hd);
 }
 
+
+static QEMUOptionParameter vmdk_create_options[] = {
+    { BLOCK_OPT_SIZE,           OPT_SIZE },
+    { BLOCK_OPT_BACKING_FILE,   OPT_STRING },
+    { BLOCK_OPT_COMPAT6,        OPT_FLAG },
+    { NULL }
+};
+
 static BlockDriver bdrv_vmdk = {
     .format_name	= "vmdk",
     .instance_size	= sizeof(BDRVVmdkState),
@@ -823,6 +845,8 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_create	= vmdk_create,
     .bdrv_flush		= vmdk_flush,
     .bdrv_is_allocated	= vmdk_is_allocated,
+
+    .create_options = vmdk_create_options,
 };
 
 static void bdrv_vmdk_init(void)
diff --git a/block/vpc.c b/block/vpc.c
index 211ae5c..662a6f6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -477,8 +477,7 @@ static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
     return 0;
 }
 
-static int vpc_create(const char *filename, int64_t total_sectors,
-    const char *backing_file, int flags)
+static int vpc_create(const char *filename, QEMUOptionParameter *options)
 {
     uint8_t buf[1024];
     struct vhd_footer* footer = (struct vhd_footer*) buf;
@@ -489,10 +488,17 @@ static int vpc_create(const char *filename, int64_t total_sectors,
     uint8_t heads;
     uint8_t secs_per_cyl;
     size_t block_size, num_bat_entries;
+    int64_t total_sectors = 0;
 
-    if (backing_file != NULL)
-        return -ENOTSUP;
+    // Read out options
+    while (options && options->name) {
+        if (!strcmp(options->name, "size")) {
+            total_sectors = options->value.n / 512;
+        }
+        options++;
+    }
 
+    // Create the file
     fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
     if (fd < 0)
         return -EIO;
@@ -587,6 +593,11 @@ static void vpc_close(BlockDriverState *bs)
     bdrv_delete(s->hd);
 }
 
+static QEMUOptionParameter vpc_create_options[] = {
+    { "size", OPT_SIZE },
+    { NULL }
+};
+
 static BlockDriver bdrv_vpc = {
     .format_name	= "vpc",
     .instance_size	= sizeof(BDRVVPCState),
@@ -596,6 +607,8 @@ static BlockDriver bdrv_vpc = {
     .bdrv_write		= vpc_write,
     .bdrv_close		= vpc_close,
     .bdrv_create	= vpc_create,
+
+    .create_options = vpc_create_options,
 };
 
 static void bdrv_vpc_init(void)
diff --git a/block/vvfat.c b/block/vvfat.c
index 2a8feb3..13960e9 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2777,8 +2777,8 @@ static int enable_write_target(BDRVVVFATState *s)
 
     s->qcow_filename = qemu_malloc(1024);
     get_tmp_filename(s->qcow_filename, 1024);
-    if (bdrv_create(bdrv_find_format("qcow"),
-		s->qcow_filename, s->sector_count, "fat:", 0) < 0)
+    if (bdrv_create2(bdrv_find_format("qcow"),
+		s->qcow_filename, s->sector_count, "fat:", NULL, 0) < 0)
 	return -1;
     s->qcow = bdrv_new("");
     if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0) < 0)
diff --git a/block_int.h b/block_int.h
index 9d11940..782de6c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -25,11 +25,18 @@
 #define BLOCK_INT_H
 
 #include "block.h"
+#include "qemu-option.h"
 
 #define BLOCK_FLAG_ENCRYPT	1
 #define BLOCK_FLAG_COMPRESS	2
 #define BLOCK_FLAG_COMPAT6	4
 
+#define BLOCK_OPT_SIZE          "size"
+#define BLOCK_OPT_ENCRYPT       "encryption"
+#define BLOCK_OPT_COMPAT6       "compat6"
+#define BLOCK_OPT_BACKING_FILE  "backing_file"
+#define BLOCK_OPT_BACKING_FMT   "backing_fmt"
+
 typedef struct AIOPool {
     void (*cancel)(BlockDriverAIOCB *acb);
     int aiocb_size;
@@ -46,8 +53,7 @@ struct BlockDriver {
     int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
                       const uint8_t *buf, int nb_sectors);
     void (*bdrv_close)(BlockDriverState *bs);
-    int (*bdrv_create)(const char *filename, int64_t total_sectors,
-                       const char *backing_file, int flags);
+    int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
     void (*bdrv_flush)(BlockDriverState *bs);
     int (*bdrv_is_allocated)(BlockDriverState *bs, int64_t sector_num,
                              int nb_sectors, int *pnum);
@@ -97,10 +103,9 @@ struct BlockDriver {
 
     AIOPool aio_pool;
 
-    /* new create with backing file format */
-    int (*bdrv_create2)(const char *filename, int64_t total_sectors,
-                        const char *backing_file, const char *backing_format,
-                        int flags);
+    /* List of options for creating images, terminated by name == NULL */
+    QEMUOptionParameter *create_options;
+
 
     /* Returns number of errors in image, -errno for internal errors */
     int (*bdrv_check)(BlockDriverState* bs);
diff --git a/qemu-img.c b/qemu-img.c
index a3d15e7..190153f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -551,7 +551,7 @@ static int img_convert(int argc, char **argv)
     if (flags & BLOCK_FLAG_ENCRYPT && flags & BLOCK_FLAG_COMPRESS)
         error("Compression and encryption not supported at the same time");
 
-    ret = bdrv_create(drv, out_filename, total_sectors, out_baseimg, flags);
+    ret = bdrv_create2(drv, out_filename, total_sectors, out_baseimg, NULL, flags);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", out_fmt);
-- 
1.6.0.6

[Qemu-devel] [PATCH 3/4] Convert qemu-img create to new bdrv_create

[Kevin Wolf]

This patch changes qemu-img to actually use the new bdrv_create interface. It
translates the old-style qemu-img options which have been bdrv_create2
parameters or flags so far to option structures. As the generic approach, it
introduces an -o option which accepts any parameter the driver knows.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c |  131 +++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 86 insertions(+), 45 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 190153f..0032979 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu-common.h"
+#include "qemu-option.h"
 #include "osdep.h"
 #include "block_int.h"
 #include <stdio.h>
@@ -221,14 +222,13 @@ static int img_create(int argc, char **argv)
     const char *base_fmt = NULL;
     const char *filename;
     const char *base_filename = NULL;
-    uint64_t size;
-    double sizef;
-    const char *p;
     BlockDriver *drv;
+    QEMUOptionParameter *param = NULL;
+    char *options = NULL;
 
     flags = 0;
     for(;;) {
-        c = getopt(argc, argv, "F:b:f:he6");
+        c = getopt(argc, argv, "F:b:f:he6o:");
         if (c == -1)
             break;
         switch(c) {
@@ -250,59 +250,100 @@ static int img_create(int argc, char **argv)
         case '6':
             flags |= BLOCK_FLAG_COMPAT6;
             break;
+        case 'o':
+            options = optarg;
+            break;
         }
     }
     if (optind >= argc)
         help();
     filename = argv[optind++];
-    size = 0;
-    if (base_filename) {
-        BlockDriverState *bs;
-        BlockDriver *base_drv = NULL;
 
-        if (base_fmt) {
-            base_drv = bdrv_find_format(base_fmt);
-            if (base_drv == NULL)
-                error("Unknown basefile format '%s'", base_fmt);
-        }
+    /* Find driver and parse its options */
+    drv = bdrv_find_format(fmt);
+    if (!drv)
+        error("Unknown file format '%s'", fmt);
 
-        bs = bdrv_new_open(base_filename, base_fmt);
-        bdrv_get_geometry(bs, &size);
-        size *= 512;
-        bdrv_delete(bs);
+    if (options) {
+        param = parse_option_parameters(options, drv->create_options, param);
+        if (param == NULL) {
+            error("Invalid options for file format '%s'.", fmt);
+        }
     } else {
-        if (optind >= argc)
-            help();
-        p = argv[optind];
-        sizef = strtod(p, (char **)&p);
-        if (*p == 'M') {
-            size = (uint64_t)(sizef * 1024 * 1024);
-        } else if (*p == 'G') {
-            size = (uint64_t)(sizef * 1024 * 1024 * 1024);
-        } else if (*p == 'k' || *p == 'K' || *p == '\0') {
-            size = (uint64_t)(sizef * 1024);
-        } else {
-            help();
+        param = parse_option_parameters("", drv->create_options, param);
+    }
+
+    /* Add size to parameters */
+    if (optind < argc) {
+        set_option_parameter(param, BLOCK_OPT_SIZE, argv[optind++]);
+    }
+
+    /* Add old-style options to parameters */
+    if (flags & BLOCK_FLAG_ENCRYPT) {
+        if (set_option_parameter(param, BLOCK_OPT_ENCRYPT, "on")) {
+            error("Encryption not supported for file format '%s'", fmt);
         }
     }
-    drv = bdrv_find_format(fmt);
-    if (!drv)
-        error("Unknown file format '%s'", fmt);
-    printf("Formatting '%s', fmt=%s",
-           filename, fmt);
-    if (flags & BLOCK_FLAG_ENCRYPT)
-        printf(", encrypted");
-    if (flags & BLOCK_FLAG_COMPAT6)
-        printf(", compatibility level=6");
+    if (flags & BLOCK_FLAG_COMPAT6) {
+        if (set_option_parameter(param, BLOCK_OPT_COMPAT6, "on")) {
+            error("VMDK version 6 not supported for file format '%s'", fmt);
+        }
+    }
+
     if (base_filename) {
-        printf(", backing_file=%s",
-               base_filename);
-         if (base_fmt)
-             printf(", backing_fmt=%s",
-                    base_fmt);
-    }
-    printf(", size=%" PRIu64 " kB\n", size / 1024);
-    ret = bdrv_create2(drv, filename, size / 512, base_filename, base_fmt, flags);
+        if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, base_filename)) {
+            error("Backing file not supported for file format '%s'", fmt);
+        }
+    }
+    if (base_fmt) {
+        if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+            error("Backing file format not supported for file format '%s'", 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 == 0) {
+
+        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);
+                }
+            }
+
+            bs = bdrv_new_open(backing_file->value.s, fmt);
+            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");
+        }
+    }
+
+    printf("Formatting '%s', fmt=%s ", filename, fmt);
+    print_option_parameters(param);
+    puts("");
+
+    ret = bdrv_create(drv, filename, param);
+    free_option_parameters(param);
+
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting or formatting option not supported for file format '%s'", fmt);
-- 
1.6.0.6

[Qemu-devel] [PATCH 4/4] Convert qemu-img convert to new bdrv_create

[Kevin Wolf]

This is part two of the qemu-img conversion. This really works the same as the
previous conversion of qemu-img create: It introduces a new -o option for the
generic approach and adds the old-style options to this option set.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c |   97 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 65 insertions(+), 32 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0032979..3edf25a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -215,6 +215,32 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     return bs;
 }
 
+static void add_old_style_options(const char *fmt, QEMUOptionParameter *list,
+    int flags, const char *base_filename, const char *base_fmt)
+{
+    if (flags & BLOCK_FLAG_ENCRYPT) {
+        if (set_option_parameter(list, BLOCK_OPT_ENCRYPT, "on")) {
+            error("Encryption not supported for file format '%s'", fmt);
+        }
+    }
+    if (flags & BLOCK_FLAG_COMPAT6) {
+        if (set_option_parameter(list, BLOCK_OPT_COMPAT6, "on")) {
+            error("VMDK version 6 not supported for file format '%s'", fmt);
+        }
+    }
+
+    if (base_filename) {
+        if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, base_filename)) {
+            error("Backing file not supported for file format '%s'", fmt);
+        }
+    }
+    if (base_fmt) {
+        if (set_option_parameter(list, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+            error("Backing file format not supported for file format '%s'", fmt);
+        }
+    }
+}
+
 static int img_create(int argc, char **argv)
 {
     int c, ret, flags;
@@ -279,27 +305,7 @@ static int img_create(int argc, char **argv)
     }
 
     /* Add old-style options to parameters */
-    if (flags & BLOCK_FLAG_ENCRYPT) {
-        if (set_option_parameter(param, BLOCK_OPT_ENCRYPT, "on")) {
-            error("Encryption not supported for file format '%s'", fmt);
-        }
-    }
-    if (flags & BLOCK_FLAG_COMPAT6) {
-        if (set_option_parameter(param, BLOCK_OPT_COMPAT6, "on")) {
-            error("VMDK version 6 not supported for file format '%s'", fmt);
-        }
-    }
-
-    if (base_filename) {
-        if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, base_filename)) {
-            error("Backing file not supported for file format '%s'", fmt);
-        }
-    }
-    if (base_fmt) {
-        if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
-            error("Backing file format not supported for file format '%s'", fmt);
-        }
-    }
+    add_old_style_options(fmt, param, flags, base_filename, base_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
@@ -525,13 +531,15 @@ static int img_convert(int argc, char **argv)
     uint8_t buf[IO_BUF_SIZE];
     const uint8_t *buf1;
     BlockDriverInfo bdi;
+    QEMUOptionParameter *param = NULL;
+    char *options = NULL;
 
     fmt = NULL;
     out_fmt = "raw";
     out_baseimg = NULL;
     flags = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:B:hce6");
+        c = getopt(argc, argv, "f:O:B:hce6o:");
         if (c == -1)
             break;
         switch(c) {
@@ -556,6 +564,9 @@ static int img_convert(int argc, char **argv)
         case '6':
             flags |= BLOCK_FLAG_COMPAT6;
             break;
+        case 'o':
+            options = optarg;
+            break;
         }
     }
 
@@ -580,19 +591,41 @@ static int img_convert(int argc, char **argv)
         total_sectors += bs_sectors;
     }
 
+    /* Find driver and parse its options */
     drv = bdrv_find_format(out_fmt);
     if (!drv)
         error("Unknown file format '%s'", out_fmt);
-    if (flags & BLOCK_FLAG_COMPRESS && strcmp(drv->format_name, "qcow") && strcmp(drv->format_name, "qcow2"))
-        error("Compression not supported for this file format");
-    if (flags & BLOCK_FLAG_ENCRYPT && strcmp(drv->format_name, "qcow") && strcmp(drv->format_name, "qcow2"))
-        error("Encryption not supported for this file format");
-    if (flags & BLOCK_FLAG_COMPAT6 && strcmp(drv->format_name, "vmdk"))
-        error("Alternative compatibility level not supported for this file format");
-    if (flags & BLOCK_FLAG_ENCRYPT && flags & BLOCK_FLAG_COMPRESS)
-        error("Compression and encryption not supported at the same time");
-
-    ret = bdrv_create2(drv, out_filename, total_sectors, out_baseimg, NULL, flags);
+
+    if (options) {
+        param = parse_option_parameters(options, drv->create_options, param);
+        if (param == NULL) {
+            error("Invalid options for file format '%s'.", out_fmt);
+        }
+    } else {
+        param = parse_option_parameters("", drv->create_options, param);
+    }
+
+    set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
+    add_old_style_options(out_fmt, param, flags, out_baseimg, NULL);
+
+    /* Check if compression is supported */
+    if (flags & BLOCK_FLAG_COMPRESS) {
+        QEMUOptionParameter *encryption =
+            get_option_parameter(param, BLOCK_OPT_ENCRYPT);
+
+        if (!drv->bdrv_write_compressed) {
+            error("Compression not supported for this file format");
+        }
+
+        if (encryption && encryption->value.n) {
+            error("Compression and encryption not supported at the same time");
+        }
+    }
+
+    /* Create the new image */
+    ret = bdrv_create(drv, out_filename, param);
+    free_option_parameters(param);
+
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", out_fmt);
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH 4/4] Convert qemu-img convert to new bdrv_create

[Christoph Hellwig]

On Mon, May 18, 2009 at 04:42:12PM +0200, Kevin Wolf wrote:
> This is part two of the qemu-img conversion. This really works the same as the
> previous conversion of qemu-img create: It introduces a new -o option for the
> generic approach and adds the old-style options to this option set.

Shouldn't we kill bdrv_create2 and friends after this?

Re: [Qemu-devel] [PATCH 4/4] Convert qemu-img convert to new bdrv_create

[Kevin Wolf]

Christoph Hellwig schrieb:
> On Mon, May 18, 2009 at 04:42:12PM +0200, Kevin Wolf wrote:
>> This is part two of the qemu-img conversion. This really works the same as the
>> previous conversion of qemu-img create: It introduces a new -o option for the
>> generic approach and adds the old-style options to this option set.
> 
> Shouldn't we kill bdrv_create2 and friends after this?

There are two references left, one in vvfat and one in block.c, both use
it for creating temporary qcow images (vvfat still uses qcow1, by the
way...)

Should be a small patch to kill them, so yes, we should do that. I will
send such a patch once this series is applied.

Kevin