* [Qemu-devel] [PATCH] add support for protocol driver create_options
@ 2010-05-20 5:36 MORITA Kazutaka
2010-05-20 5:40 ` [Qemu-devel] Kvm device passthrough Mu Lin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: MORITA Kazutaka @ 2010-05-20 5:36 UTC (permalink / raw)
To: kwolf, hch; +Cc: sheepdog, qemu-devel, kvm
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@lab.ntt.co.jp>
---
block.c | 7 +++----
block.h | 1 +
qemu-img.c | 49 ++++++++++++++++++++++++++++++++++---------------
qemu-option.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-------
qemu-option.h | 2 ++
5 files changed, 85 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index 48d8468..0ab9424 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];
@@ -469,7 +468,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 d3c30a7..8ae7184 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..3ef4abc 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -346,6 +346,50 @@ 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;
+ }
+ 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 +409,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 +422,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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] Kvm device passthrough
2010-05-20 5:36 [Qemu-devel] [PATCH] add support for protocol driver create_options MORITA Kazutaka
@ 2010-05-20 5:40 ` Mu Lin
2010-05-21 11:40 ` [Qemu-devel] Re: [PATCH] add support for protocol driver create_options Kevin Wolf
2010-05-21 16:57 ` Kevin Wolf
2 siblings, 0 replies; 9+ messages in thread
From: Mu Lin @ 2010-05-20 5:40 UTC (permalink / raw)
To: kvm@vger.kernel.org, qemu-devel@nongnu.org; +Cc: Mu Lin
Hi, Folks:
Could you provide pointer to the kvm device passthrough howto/FAQ?
I have two questions:
1. my host os, the Linux doesn't have the native device driver for some home grown pci devices, the driver is in the guest os, does device passthrough work in this case? Assuming I have VT-d.
2. How about i2c devices?
Thanks
Mu
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-20 5:36 [Qemu-devel] [PATCH] add support for protocol driver create_options MORITA Kazutaka
2010-05-20 5:40 ` [Qemu-devel] Kvm device passthrough Mu Lin
@ 2010-05-21 11:40 ` Kevin Wolf
2010-05-24 6:21 ` MORITA Kazutaka
2010-05-21 16:57 ` Kevin Wolf
2 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2010-05-21 11:40 UTC (permalink / raw)
To: MORITA Kazutaka; +Cc: sheepdog, hch, kvm, qemu-devel
Am 20.05.2010 07:36, schrieb MORITA 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@lab.ntt.co.jp>
Hm, this is not stackable, right? Though I do see that making it
stackable would require some bigger changes, so maybe we can get away
with claiming that this approach covers everything that happens in practice.
If we accept that this is the desired behaviour, the code looks good to me.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-20 5:36 [Qemu-devel] [PATCH] add support for protocol driver create_options MORITA Kazutaka
2010-05-20 5:40 ` [Qemu-devel] Kvm device passthrough Mu Lin
2010-05-21 11:40 ` [Qemu-devel] Re: [PATCH] add support for protocol driver create_options Kevin Wolf
@ 2010-05-21 16:57 ` Kevin Wolf
2010-05-24 6:34 ` MORITA Kazutaka
2 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2010-05-21 16:57 UTC (permalink / raw)
To: MORITA Kazutaka; +Cc: sheepdog, hch, kvm, qemu-devel
Am 20.05.2010 07:36, schrieb MORITA 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@lab.ntt.co.jp>
> ---
> block.c | 7 +++----
> block.h | 1 +
> qemu-img.c | 49 ++++++++++++++++++++++++++++++++++---------------
> qemu-option.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-------
> qemu-option.h | 2 ++
> 5 files changed, 85 insertions(+), 26 deletions(-)
>
> diff --git a/block.c b/block.c
> index 48d8468..0ab9424 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];
> @@ -469,7 +468,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 d3c30a7..8ae7184 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..3ef4abc 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -346,6 +346,50 @@ 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;
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.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-21 11:40 ` [Qemu-devel] Re: [PATCH] add support for protocol driver create_options Kevin Wolf
@ 2010-05-24 6:21 ` MORITA Kazutaka
0 siblings, 0 replies; 9+ messages in thread
From: MORITA Kazutaka @ 2010-05-24 6:21 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, sheepdog, hch, MORITA Kazutaka, kvm
At Fri, 21 May 2010 13:40:31 +0200,
Kevin Wolf wrote:
>
> Am 20.05.2010 07:36, schrieb MORITA 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@lab.ntt.co.jp>
>
> Hm, this is not stackable, right? Though I do see that making it
> stackable would require some bigger changes, so maybe we can get away
> with claiming that this approach covers everything that happens in practice.
>
> If we accept that this is the desired behaviour, the code looks good to me.
>
As you say, this patch isn't stackable; we must specify a image name
with at most 1 format and 1 protocol. I cannot think of a situation
where we want to use more than one protocol to create qemu images, so
this seems to be enough to me.
Thanks,
Kazutaka
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-21 16:57 ` Kevin Wolf
@ 2010-05-24 6:34 ` MORITA Kazutaka
2010-05-25 13:43 ` Kevin Wolf
0 siblings, 1 reply; 9+ messages in thread
From: MORITA Kazutaka @ 2010-05-24 6:34 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, sheepdog, hch, MORITA Kazutaka, kvm
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@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 202f895..3ed35ed 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];
@@ -469,7 +468,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 b95a9c0..bd11cc0 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 d3c30a7..8ae7184 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..147bc04 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-24 6:34 ` MORITA Kazutaka
@ 2010-05-25 13:43 ` Kevin Wolf
2010-05-26 2:35 ` MORITA Kazutaka
0 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2010-05-25 13:43 UTC (permalink / raw)
To: MORITA Kazutaka; +Cc: sheepdog, hch, kvm, qemu-devel
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@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.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-25 13:43 ` Kevin Wolf
@ 2010-05-26 2:35 ` MORITA Kazutaka
2010-05-26 9:02 ` Kevin Wolf
0 siblings, 1 reply; 9+ messages in thread
From: MORITA Kazutaka @ 2010-05-26 2:35 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, sheepdog, hch, MORITA Kazutaka, kvm
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@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@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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: [PATCH] add support for protocol driver create_options
2010-05-26 2:35 ` MORITA Kazutaka
@ 2010-05-26 9:02 ` Kevin Wolf
0 siblings, 0 replies; 9+ messages in thread
From: Kevin Wolf @ 2010-05-26 9:02 UTC (permalink / raw)
To: MORITA Kazutaka; +Cc: sheepdog, hch, kvm, qemu-devel
Am 26.05.2010 04:35, schrieb MORITA Kazutaka:
> 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@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, now it looks good and passes all qemu-iotests tests again.
Applied the patch to the block branch.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-05-26 9:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-20 5:36 [Qemu-devel] [PATCH] add support for protocol driver create_options MORITA Kazutaka
2010-05-20 5:40 ` [Qemu-devel] Kvm device passthrough Mu Lin
2010-05-21 11:40 ` [Qemu-devel] Re: [PATCH] add support for protocol driver create_options Kevin Wolf
2010-05-24 6:21 ` MORITA Kazutaka
2010-05-21 16:57 ` Kevin Wolf
2010-05-24 6:34 ` MORITA Kazutaka
2010-05-25 13:43 ` Kevin Wolf
2010-05-26 2:35 ` MORITA Kazutaka
2010-05-26 9:02 ` 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).