At Tue, 25 May 2010 15:43:17 +0200,
Kevin Wolf wrote:
>
> Am 24.05.2010 08:34, schrieb MORITA Kazutaka:
> > At Fri, 21 May 2010 18:57:36 +0200,
> > Kevin Wolf wrote:
> >>
> >> Am 20.05.2010 07:36, schrieb MORITA Kazutaka:
> >>> +
> >>> +/*
> >>> + * Append an option list (list) to an option list (dest).
> >>> + *
> >>> + * If dest is NULL, a new copy of list is created.
> >>> + *
> >>> + * Returns a pointer to the first element of dest (or the newly allocated copy)
> >>> + */
> >>> +QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest,
> >>> + QEMUOptionParameter *list)
> >>> +{
> >>> + size_t num_options, num_dest_options;
> >>> +
> >>> + num_options = count_option_parameters(dest);
> >>> + num_dest_options = num_options;
> >>> +
> >>> + num_options += count_option_parameters(list);
> >>> +
> >>> + dest = qemu_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter));
> >>> +
> >>> + while (list && list->name) {
> >>> + if (get_option_parameter(dest, list->name) == NULL) {
> >>> + dest[num_dest_options++] = *list;
> >>
> >> You need to add a dest[num_dest_options].name = NULL; here. Otherwise
> >> the next loop iteration works on uninitialized memory and possibly an
> >> unterminated list. I got a segfault for that reason.
> >>
> >
> > I forgot to add it, sorry.
> > Fixed version is below.
> >
> > Thanks,
> >
> > Kazutaka
> >
> > ==
> > This patch enables protocol drivers to use their create options which
> > are not supported by the format. For example, protcol drivers can use
> > a backing_file option with raw format.
> >
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
>
> $ ./qemu-img create -f qcow2 -o cluster_size=4k /tmp/test.qcow2 4G
> Unknown option 'cluster_size'
> qemu-img: Invalid options for file format 'qcow2'.
>
> I think you added another num_dest_options++ which shouldn't be there.
>
Sorry again. I wrongly added `dest[num_dest_options++].name = NULL;'
instead of `dest[num_dest_options].name = NULL;'.
Thanks,
Kazutaka
==
This patch enables protocol drivers to use their create options which
are not supported by the format. For example, protcol drivers can use
a backing_file option with raw format.
Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---
block.c | 7 +++----
block.h | 1 +
qemu-img.c | 49 ++++++++++++++++++++++++++++++++++---------------
qemu-option.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
qemu-option.h | 2 ++
5 files changed, 86 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index 6e7766a..f881f10 100644
--- a/block.c
+++ b/block.c
@@ -56,7 +56,6 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
-static BlockDriver *find_protocol(const char *filename);
static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -210,7 +209,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
{
BlockDriver *drv;
- drv = find_protocol(filename);
+ drv = bdrv_find_protocol(filename);
if (drv == NULL) {
drv = bdrv_find_format("file");
}
@@ -283,7 +282,7 @@ static BlockDriver *find_hdev_driver(const char *filename)
return drv;
}
-static BlockDriver *find_protocol(const char *filename)
+BlockDriver *bdrv_find_protocol(const char *filename)
{
BlockDriver *drv1;
char protocol[128];
@@ -478,7 +477,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
BlockDriver *drv;
int ret;
- drv = find_protocol(filename);
+ drv = bdrv_find_protocol(filename);
if (!drv) {
return -ENOENT;
}
diff --git a/block.h b/block.h
index 24efeb6..9034ebb 100644
--- a/block.h
+++ b/block.h
@@ -54,6 +54,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data);
void bdrv_init(void);
void bdrv_init_with_whitelist(void);
+BlockDriver *bdrv_find_protocol(const char *filename);
BlockDriver *bdrv_find_format(const char *format_name);
BlockDriver *bdrv_find_whitelisted_format(const char *format_name);
int bdrv_create(BlockDriver *drv, const char* filename,
diff --git a/qemu-img.c b/qemu-img.c
index cb007b7..ea091f0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -252,8 +252,8 @@ static int img_create(int argc, char **argv)
const char *base_fmt = NULL;
const char *filename;
const char *base_filename = NULL;
- BlockDriver *drv;
- QEMUOptionParameter *param = NULL;
+ BlockDriver *drv, *proto_drv;
+ QEMUOptionParameter *param = NULL, *create_options = NULL;
char *options = NULL;
flags = 0;
@@ -286,33 +286,42 @@ static int img_create(int argc, char **argv)
}
}
+ /* Get the filename */
+ if (optind >= argc)
+ help();
+ filename = argv[optind++];
+
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
if (!drv)
error("Unknown file format '%s'", fmt);
+ proto_drv = bdrv_find_protocol(filename);
+ if (!proto_drv)
+ error("Unknown protocol '%s'", filename);
+
+ create_options = append_option_parameters(create_options,
+ drv->create_options);
+ create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
+
if (options && !strcmp(options, "?")) {
- print_option_help(drv->create_options);
+ print_option_help(create_options);
return 0;
}
/* Create parameter list with default values */
- param = parse_option_parameters("", drv->create_options, param);
+ param = parse_option_parameters("", create_options, param);
set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
/* Parse -o options */
if (options) {
- param = parse_option_parameters(options, drv->create_options, param);
+ param = parse_option_parameters(options, create_options, param);
if (param == NULL) {
error("Invalid options for file format '%s'.", fmt);
}
}
- /* Get the filename */
- if (optind >= argc)
- help();
- filename = argv[optind++];
-
/* Add size to parameters */
if (optind < argc) {
set_option_parameter(param, BLOCK_OPT_SIZE, argv[optind++]);
@@ -362,6 +371,7 @@ static int img_create(int argc, char **argv)
puts("");
ret = bdrv_create(drv, filename, param);
+ free_option_parameters(create_options);
free_option_parameters(param);
if (ret < 0) {
@@ -543,14 +553,14 @@ static int img_convert(int argc, char **argv)
{
int c, ret, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
const char *fmt, *out_fmt, *out_baseimg, *out_filename;
- BlockDriver *drv;
+ BlockDriver *drv, *proto_drv;
BlockDriverState **bs, *out_bs;
int64_t total_sectors, nb_sectors, sector_num, bs_offset;
uint64_t bs_sectors;
uint8_t * buf;
const uint8_t *buf1;
BlockDriverInfo bdi;
- QEMUOptionParameter *param = NULL;
+ QEMUOptionParameter *param = NULL, *create_options = NULL;
char *options = NULL;
fmt = NULL;
@@ -615,19 +625,27 @@ static int img_convert(int argc, char **argv)
if (!drv)
error("Unknown file format '%s'", out_fmt);
+ proto_drv = bdrv_find_protocol(out_filename);
+ if (!proto_drv)
+ error("Unknown protocol '%s'", out_filename);
+
+ create_options = append_option_parameters(create_options,
+ drv->create_options);
+ create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
if (options && !strcmp(options, "?")) {
- print_option_help(drv->create_options);
+ print_option_help(create_options);
free(bs);
return 0;
}
if (options) {
- param = parse_option_parameters(options, drv->create_options, param);
+ param = parse_option_parameters(options, create_options, param);
if (param == NULL) {
error("Invalid options for file format '%s'.", out_fmt);
}
} else {
- param = parse_option_parameters("", drv->create_options, param);
+ param = parse_option_parameters("", create_options, param);
}
set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
@@ -649,6 +667,7 @@ static int img_convert(int argc, char **argv)
/* Create the new image */
ret = bdrv_create(drv, out_filename, param);
+ free_option_parameters(create_options);
free_option_parameters(param);
if (ret < 0) {
diff --git a/qemu-option.c b/qemu-option.c
index 076dddf..acd74f9 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -346,6 +346,51 @@ void free_option_parameters(QEMUOptionParameter *list)
}
/*
+ * Count valid options in list
+ */
+static size_t count_option_parameters(QEMUOptionParameter *list)
+{
+ size_t num_options = 0;
+
+ while (list && list->name) {
+ num_options++;
+ list++;
+ }
+
+ return num_options;
+}
+
+/*
+ * Append an option list (list) to an option list (dest).
+ *
+ * If dest is NULL, a new copy of list is created.
+ *
+ * Returns a pointer to the first element of dest (or the newly allocated copy)
+ */
+QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest,
+ QEMUOptionParameter *list)
+{
+ size_t num_options, num_dest_options;
+
+ num_options = count_option_parameters(dest);
+ num_dest_options = num_options;
+
+ num_options += count_option_parameters(list);
+
+ dest = qemu_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter));
+
+ while (list && list->name) {
+ if (get_option_parameter(dest, list->name) == NULL) {
+ dest[num_dest_options++] = *list;
+ dest[num_dest_options].name = NULL;
+ }
+ list++;
+ }
+
+ return dest;
+}
+
+/*
* 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
@@ -365,7 +410,6 @@ void free_option_parameters(QEMUOptionParameter *list)
QEMUOptionParameter *parse_option_parameters(const char *param,
QEMUOptionParameter *list, QEMUOptionParameter *dest)
{
- QEMUOptionParameter *cur;
QEMUOptionParameter *allocated = NULL;
char name[256];
char value[256];
@@ -379,12 +423,7 @@ QEMUOptionParameter *parse_option_parameters(const char *param,
if (dest == NULL) {
// Count valid options
- num_options = 0;
- cur = list;
- while (cur->name) {
- num_options++;
- cur++;
- }
+ num_options = count_option_parameters(list);
// Create a copy of the option list to fill in values
dest = qemu_mallocz((num_options + 1) * sizeof(QEMUOptionParameter));
diff --git a/qemu-option.h b/qemu-option.h
index 58136f3..4823219 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -70,6 +70,8 @@ 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 *append_option_parameters(QEMUOptionParameter *dest,
+ QEMUOptionParameter *list);
QEMUOptionParameter *parse_option_parameters(const char *param,
QEMUOptionParameter *list, QEMUOptionParameter *dest);
void free_option_parameters(QEMUOptionParameter *list);
--
1.5.6.5
|
|