* [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code
@ 2010-12-06 14:25 Jes.Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz() Jes.Sorensen
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Hi,
These patches applies a number of cleanups to qemu-img.c as well as a
minor bug in qemu-malloc.c.
The handling of block help printing is moved to shared code, which
allows the "?" detection to happen early in the parsing, instead of
half way down img_create() and img_convert(). I would like to see this
happen as I would like to pull some of the code out of img_create()
and into block.c so it can be shared with qemu and qemu-img.
In addition there is a couple of patches to clean up the error
handling in qemu-img.c and make it more consistent.
The formatting patch is solely because the last patch wanted to
change code next to the badly formatted code, and I didn't want to
pollute the patch with the formatting fixed.
The seventh patch fixes qemu-img to exit on detection of unknown
options instead of continuing with a potentially wrong set of
arguments.
v3 applies a number of changes discussed on irc and email. This is the
grow to seven from three patches series.
Cheers,
Jes
Jes Sorensen (7):
Add missing tracing to qemu_mallocz()
Use qemu_mallocz() instead of calloc() in img_convert()
img_convert(): Only try to free bs[] entries if bs is valid.
Make error handling more consistent in img_create() and img_resize()
Consolidate printing of block driver options
Fix formatting and missing braces in qemu-img.c
Fail if detecting an unknown option
qemu-img.c | 162 +++++++++++++++++++++++++++++++++++++++-----------------
qemu-malloc.c | 5 ++-
2 files changed, 117 insertions(+), 50 deletions(-)
--
1.7.3.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz()
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 14:42 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert() Jes.Sorensen
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-malloc.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 28fb05a..b9b3851 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -64,10 +64,13 @@ void *qemu_realloc(void *ptr, size_t size)
void *qemu_mallocz(size_t size)
{
+ void *ptr;
if (!size && !allow_zero_malloc()) {
abort();
}
- return qemu_oom_check(calloc(1, size ? size : 1));
+ ptr = qemu_oom_check(calloc(1, size ? size : 1));
+ trace_qemu_malloc(size, ptr);
+ return ptr;
}
char *qemu_strdup(const char *str)
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert()
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz() Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 14:43 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid Jes.Sorensen
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index fa77ac0..eca99c4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -699,11 +699,7 @@ static int img_convert(int argc, char **argv)
return 1;
}
- bs = calloc(bs_n, sizeof(BlockDriverState *));
- if (!bs) {
- error("Out of memory");
- return 1;
- }
+ bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *));
total_sectors = 0;
for (bs_i = 0; bs_i < bs_n; bs_i++) {
@@ -983,7 +979,7 @@ out:
bdrv_delete(bs[bs_i]);
}
}
- free(bs);
+ qemu_free(bs);
if (ret) {
return 1;
}
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid.
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz() Jes.Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert() Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 14:44 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
This allows for jumping to 'out:' consistently for error exit.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index eca99c4..aded72d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -696,7 +696,8 @@ static int img_convert(int argc, char **argv)
if (bs_n > 1 && out_baseimg) {
error("-B makes no sense when concatenating multiple input images");
- return 1;
+ ret = -1;
+ goto out;
}
bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *));
@@ -974,12 +975,14 @@ out:
if (out_bs) {
bdrv_delete(out_bs);
}
- for (bs_i = 0; bs_i < bs_n; bs_i++) {
- if (bs[bs_i]) {
- bdrv_delete(bs[bs_i]);
+ if (bs) {
+ for (bs_i = 0; bs_i < bs_n; bs_i++) {
+ if (bs[bs_i]) {
+ bdrv_delete(bs[bs_i]);
+ }
}
+ qemu_free(bs);
}
- qemu_free(bs);
if (ret) {
return 1;
}
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize()
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
` (2 preceding siblings ...)
2010-12-06 14:25 ` [Qemu-devel] [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 14:58 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 15:25 ` Kevin Wolf
2010-12-06 14:25 ` [Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options Jes.Sorensen
` (3 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index aded72d..7f4939e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -314,13 +314,15 @@ static int img_create(int argc, char **argv)
drv = bdrv_find_format(fmt);
if (!drv) {
error("Unknown file format '%s'", fmt);
- return 1;
+ ret = -1;
+ goto out;
}
proto_drv = bdrv_find_protocol(filename);
if (!proto_drv) {
error("Unknown protocol '%s'", filename);
- return 1;
+ ret = -1;
+ goto out;
}
create_options = append_option_parameters(create_options,
@@ -1483,14 +1485,16 @@ static int img_resize(int argc, char **argv)
param = parse_option_parameters("", resize_options, NULL);
if (set_option_parameter(param, BLOCK_OPT_SIZE, size)) {
/* Error message already printed when size parsing fails */
- exit(1);
+ ret = -1;
+ goto out;
}
n = get_option_parameter(param, BLOCK_OPT_SIZE)->value.n;
free_option_parameters(param);
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
if (!bs) {
- return 1;
+ ret = -1;
+ goto out;
}
if (relative) {
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
` (3 preceding siblings ...)
2010-12-06 14:25 ` [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 14:59 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 6/7] Fix formatting and missing braces in qemu-img.c Jes.Sorensen
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
This consolidates the printing of block driver options in
print_block_option_help() which is called from both img_create() and
img_convert().
This allows for the "?" detection to be done just after the parsing of
options and the filename, instead of half way down the codepath of
these functions.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7f4939e..c7d0ca8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,6 +188,33 @@ static int read_password(char *buf, int buf_size)
}
#endif
+static int print_block_option_help(const char *filename, const char *fmt)
+{
+ BlockDriver *drv, *proto_drv;
+ QEMUOptionParameter *create_options = NULL;
+
+ /* Find driver and parse its options */
+ drv = bdrv_find_format(fmt);
+ if (!drv) {
+ error("Unknown file format '%s'", fmt);
+ return 1;
+ }
+
+ proto_drv = bdrv_find_protocol(filename);
+ if (!proto_drv) {
+ error("Unknown protocol '%s'", filename);
+ return 1;
+ }
+
+ create_options = append_option_parameters(create_options,
+ drv->create_options);
+ create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
+ print_option_help(create_options);
+ free_option_parameters(create_options);
+ return 0;
+}
+
static BlockDriverState *bdrv_new_open(const char *filename,
const char *fmt,
int flags)
@@ -310,6 +337,11 @@ static int img_create(int argc, char **argv)
help();
filename = argv[optind++];
+ if (options && !strcmp(options, "?")) {
+ ret = print_block_option_help(filename, fmt);
+ goto out;
+ }
+
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
if (!drv) {
@@ -330,11 +362,6 @@ static int img_create(int argc, char **argv)
create_options = append_option_parameters(create_options,
proto_drv->create_options);
- if (options && !strcmp(options, "?")) {
- print_option_help(create_options);
- goto out;
- }
-
/* Create parameter list with default values */
param = parse_option_parameters("", create_options, param);
set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
@@ -696,6 +723,11 @@ static int img_convert(int argc, char **argv)
out_filename = argv[argc - 1];
+ if (options && !strcmp(options, "?")) {
+ ret = print_block_option_help(out_filename, out_fmt);
+ goto out;
+ }
+
if (bs_n > 1 && out_baseimg) {
error("-B makes no sense when concatenating multiple input images");
ret = -1;
@@ -748,10 +780,6 @@ static int img_convert(int argc, char **argv)
drv->create_options);
create_options = append_option_parameters(create_options,
proto_drv->create_options);
- if (options && !strcmp(options, "?")) {
- print_option_help(create_options);
- goto out;
- }
if (options) {
param = parse_option_parameters(options, create_options, param);
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 6/7] Fix formatting and missing braces in qemu-img.c
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
` (4 preceding siblings ...)
2010-12-06 14:25 ` [Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 15:00 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 7/7] Fail if detecting an unknown option Jes.Sorensen
2010-12-06 15:33 ` [Qemu-devel] Re: [PATCH v3 0/7] Cleanup qemu-img code Kevin Wolf
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
qemu-img.c | 77 +++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index c7d0ca8..d812db0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -305,8 +305,9 @@ static int img_create(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "F:b:f:he6o:");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -333,8 +334,9 @@ static int img_create(int argc, char **argv)
}
/* Get the filename */
- if (optind >= argc)
+ if (optind >= argc) {
help();
+ }
filename = argv[optind++];
if (options && !strcmp(options, "?")) {
@@ -473,8 +475,9 @@ static int img_check(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -484,8 +487,9 @@ static int img_check(int argc, char **argv)
break;
}
}
- if (optind >= argc)
+ if (optind >= argc) {
help();
+ }
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS);
@@ -549,8 +553,9 @@ static int img_commit(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -560,8 +565,9 @@ static int img_commit(int argc, char **argv)
break;
}
}
- if (optind >= argc)
+ if (optind >= argc) {
help();
+ }
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
@@ -685,8 +691,9 @@ static int img_convert(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "f:O:B:s:hce6o:");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -719,7 +726,9 @@ static int img_convert(int argc, char **argv)
}
bs_n = argc - optind - 1;
- if (bs_n < 1) help();
+ if (bs_n < 1) {
+ help();
+ }
out_filename = argv[argc - 1];
@@ -907,8 +916,9 @@ static int img_convert(int argc, char **argv)
}
assert (remainder == 0);
- if (n < cluster_sectors)
+ if (n < cluster_sectors) {
memset(buf + n * 512, 0, cluster_size - n * 512);
+ }
if (is_not_zero(buf, cluster_size)) {
ret = bdrv_write_compressed(out_bs, sector_num, buf,
cluster_sectors);
@@ -928,12 +938,14 @@ static int img_convert(int argc, char **argv)
sector_num = 0; // total number of sectors converted so far
for(;;) {
nb_sectors = total_sectors - sector_num;
- if (nb_sectors <= 0)
+ if (nb_sectors <= 0) {
break;
- if (nb_sectors >= (IO_BUF_SIZE / 512))
+ }
+ if (nb_sectors >= (IO_BUF_SIZE / 512)) {
n = (IO_BUF_SIZE / 512);
- else
+ } else {
n = nb_sectors;
+ }
while (sector_num - bs_offset >= bs_sectors) {
bs_i ++;
@@ -945,8 +957,9 @@ static int img_convert(int argc, char **argv)
sector_num, bs_i, bs_offset, bs_sectors); */
}
- if (n > bs_offset + bs_sectors - sector_num)
+ if (n > bs_offset + bs_sectors - sector_num) {
n = bs_offset + bs_sectors - sector_num;
+ }
if (has_zero_init) {
/* If the output image is being created as a copy on write image,
@@ -1082,8 +1095,9 @@ static int img_info(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -1093,8 +1107,9 @@ static int img_info(int argc, char **argv)
break;
}
}
- if (optind >= argc)
+ if (optind >= argc) {
help();
+ }
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
@@ -1105,11 +1120,12 @@ static int img_info(int argc, char **argv)
bdrv_get_geometry(bs, &total_sectors);
get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
allocated_size = get_allocated_file_size(filename);
- if (allocated_size < 0)
+ if (allocated_size < 0) {
snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
- else
+ } else {
get_human_readable_size(dsize_buf, sizeof(dsize_buf),
allocated_size);
+ }
printf("image: %s\n"
"file format: %s\n"
"virtual size: %s (%" PRId64 " bytes)\n"
@@ -1117,11 +1133,13 @@ static int img_info(int argc, char **argv)
filename, fmt_name, size_buf,
(total_sectors * 512),
dsize_buf);
- if (bdrv_is_encrypted(bs))
+ if (bdrv_is_encrypted(bs)) {
printf("encrypted: yes\n");
+ }
if (bdrv_get_info(bs, &bdi) >= 0) {
- if (bdi.cluster_size != 0)
+ if (bdi.cluster_size != 0) {
printf("cluster_size: %d\n", bdi.cluster_size);
+ }
}
bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
if (backing_filename[0] != '\0') {
@@ -1154,8 +1172,9 @@ static int img_snapshot(int argc, char **argv)
/* Parse commandline parameters */
for(;;) {
c = getopt(argc, argv, "la:c:d:h");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -1195,8 +1214,9 @@ static int img_snapshot(int argc, char **argv)
}
}
- if (optind >= argc)
+ if (optind >= argc) {
help();
+ }
filename = argv[optind++];
/* Open the image */
@@ -1220,23 +1240,26 @@ static int img_snapshot(int argc, char **argv)
sn.date_nsec = tv.tv_usec * 1000;
ret = bdrv_snapshot_create(bs, &sn);
- if (ret)
+ if (ret) {
error("Could not create snapshot '%s': %d (%s)",
snapshot_name, ret, strerror(-ret));
+ }
break;
case SNAPSHOT_APPLY:
ret = bdrv_snapshot_goto(bs, snapshot_name);
- if (ret)
+ if (ret) {
error("Could not apply snapshot '%s': %d (%s)",
snapshot_name, ret, strerror(-ret));
+ }
break;
case SNAPSHOT_DELETE:
ret = bdrv_snapshot_delete(bs, snapshot_name);
- if (ret)
+ if (ret) {
error("Could not delete snapshot '%s': %d (%s)",
snapshot_name, ret, strerror(-ret));
+ }
break;
}
@@ -1264,8 +1287,9 @@ static int img_rebase(int argc, char **argv)
for(;;) {
c = getopt(argc, argv, "uhf:F:b:");
- if (c == -1)
+ if (c == -1) {
break;
+ }
switch(c) {
case 'h':
help();
@@ -1285,8 +1309,9 @@ static int img_rebase(int argc, char **argv)
}
}
- if ((optind >= argc) || !out_baseimg)
+ if ((optind >= argc) || !out_baseimg) {
help();
+ }
filename = argv[optind++];
/*
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 7/7] Fail if detecting an unknown option
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
` (5 preceding siblings ...)
2010-12-06 14:25 ` [Qemu-devel] [PATCH 6/7] Fix formatting and missing braces in qemu-img.c Jes.Sorensen
@ 2010-12-06 14:25 ` Jes.Sorensen
2010-12-06 15:02 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 15:33 ` [Qemu-devel] Re: [PATCH v3 0/7] Cleanup qemu-img code Kevin Wolf
7 siblings, 1 reply; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 14:25 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
This patch changes qemu-img to exit if an unknown option is detected,
instead of trying to continue with a set of arguments which may be
incorrect.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index d812db0..f021a06 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -309,6 +309,7 @@ static int img_create(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
@@ -479,6 +480,7 @@ static int img_check(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
@@ -557,6 +559,7 @@ static int img_commit(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
@@ -695,6 +698,7 @@ static int img_convert(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
@@ -1099,6 +1103,7 @@ static int img_info(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
@@ -1176,6 +1181,7 @@ static int img_snapshot(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
return 0;
@@ -1291,6 +1297,7 @@ static int img_rebase(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
return 0;
@@ -1505,6 +1512,7 @@ static int img_resize(int argc, char **argv)
break;
}
switch(c) {
+ case '?':
case 'h':
help();
break;
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 1/7] Add missing tracing to qemu_mallocz()
2010-12-06 14:25 ` [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz() Jes.Sorensen
@ 2010-12-06 14:42 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 14:42 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:34PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-malloc.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert()
2010-12-06 14:25 ` [Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert() Jes.Sorensen
@ 2010-12-06 14:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 14:43 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:35PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 8 ++------
> 1 files changed, 2 insertions(+), 6 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid.
2010-12-06 14:25 ` [Qemu-devel] [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid Jes.Sorensen
@ 2010-12-06 14:44 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 14:44 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:36PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This allows for jumping to 'out:' consistently for error exit.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 4/7] Make error handling more consistent in img_create() and img_resize()
2010-12-06 14:25 ` [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
@ 2010-12-06 14:58 ` Stefan Hajnoczi
2010-12-06 15:25 ` Kevin Wolf
1 sibling, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 14:58 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:37PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 5/7] Consolidate printing of block driver options
2010-12-06 14:25 ` [Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options Jes.Sorensen
@ 2010-12-06 14:59 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 14:59 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:38PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This consolidates the printing of block driver options in
> print_block_option_help() which is called from both img_create() and
> img_convert().
>
> This allows for the "?" detection to be done just after the parsing of
> options and the filename, instead of half way down the codepath of
> these functions.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 46 +++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 37 insertions(+), 9 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 6/7] Fix formatting and missing braces in qemu-img.c
2010-12-06 14:25 ` [Qemu-devel] [PATCH 6/7] Fix formatting and missing braces in qemu-img.c Jes.Sorensen
@ 2010-12-06 15:00 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 15:00 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:39PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
> qemu-img.c | 77 +++++++++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 51 insertions(+), 26 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 7/7] Fail if detecting an unknown option
2010-12-06 14:25 ` [Qemu-devel] [PATCH 7/7] Fail if detecting an unknown option Jes.Sorensen
@ 2010-12-06 15:02 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2010-12-06 15:02 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: kwolf, qemu-devel
On Mon, Dec 06, 2010 at 03:25:40PM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This patch changes qemu-img to exit if an unknown option is detected,
> instead of trying to continue with a set of arguments which may be
> incorrect.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 4/7] Make error handling more consistent in img_create() and img_resize()
2010-12-06 14:25 ` [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
2010-12-06 14:58 ` [Qemu-devel] " Stefan Hajnoczi
@ 2010-12-06 15:25 ` Kevin Wolf
2010-12-06 15:32 ` Jes Sorensen
1 sibling, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2010-12-06 15:25 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: stefanha, qemu-devel
Am 06.12.2010 15:25, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> qemu-img.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index aded72d..7f4939e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -314,13 +314,15 @@ static int img_create(int argc, char **argv)
> drv = bdrv_find_format(fmt);
> if (!drv) {
> error("Unknown file format '%s'", fmt);
> - return 1;
> + ret = -1;
> + goto out;
> }
>
> proto_drv = bdrv_find_protocol(filename);
> if (!proto_drv) {
> error("Unknown protocol '%s'", filename);
> - return 1;
> + ret = -1;
> + goto out;
> }
>
> create_options = append_option_parameters(create_options,
> @@ -1483,14 +1485,16 @@ static int img_resize(int argc, char **argv)
> param = parse_option_parameters("", resize_options, NULL);
> if (set_option_parameter(param, BLOCK_OPT_SIZE, size)) {
> /* Error message already printed when size parsing fails */
> - exit(1);
> + ret = -1;
> + goto out;
bs isn't initialized here, so the bdrv_delete(bs) after out: will crash.
> }
> n = get_option_parameter(param, BLOCK_OPT_SIZE)->value.n;
> free_option_parameters(param);
>
> bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
> if (!bs) {
> - return 1;
> + ret = -1;
> + goto out;
> }
Same here.
Heh, wanted to try it out to be sure, but the compiler notices that, so
it doesn't even build:
cc1: warnings being treated as errors
qemu-img.c: In function 'img_resize':
qemu-img.c:1497: error: 'bs' may be used uninitialized in this function
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 4/7] Make error handling more consistent in img_create() and img_resize()
2010-12-06 15:25 ` Kevin Wolf
@ 2010-12-06 15:32 ` Jes Sorensen
0 siblings, 0 replies; 19+ messages in thread
From: Jes Sorensen @ 2010-12-06 15:32 UTC (permalink / raw)
To: Kevin Wolf; +Cc: stefanha, qemu-devel
On 12/06/10 16:25, Kevin Wolf wrote:
>> }
>> n = get_option_parameter(param, BLOCK_OPT_SIZE)->value.n;
>> free_option_parameters(param);
>>
>> bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
>> if (!bs) {
>> - return 1;
>> + ret = -1;
>> + goto out;
>> }
>
> Same here.
>
> Heh, wanted to try it out to be sure, but the compiler notices that, so
> it doesn't even build:
Grrrrr I am an idiot!
Sorry about the noise, I was sure I had tested that last change. Fix
coming up in a few minutes.
Cheers,
Jes
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH v3 0/7] Cleanup qemu-img code
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
` (6 preceding siblings ...)
2010-12-06 14:25 ` [Qemu-devel] [PATCH 7/7] Fail if detecting an unknown option Jes.Sorensen
@ 2010-12-06 15:33 ` Kevin Wolf
7 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2010-12-06 15:33 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: stefanha, qemu-devel
Am 06.12.2010 15:25, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Hi,
>
> These patches applies a number of cleanups to qemu-img.c as well as a
> minor bug in qemu-malloc.c.
>
> The handling of block help printing is moved to shared code, which
> allows the "?" detection to happen early in the parsing, instead of
> half way down img_create() and img_convert(). I would like to see this
> happen as I would like to pull some of the code out of img_create()
> and into block.c so it can be shared with qemu and qemu-img.
>
> In addition there is a couple of patches to clean up the error
> handling in qemu-img.c and make it more consistent.
>
> The formatting patch is solely because the last patch wanted to
> change code next to the badly formatted code, and I didn't want to
> pollute the patch with the formatting fixed.
>
> The seventh patch fixes qemu-img to exit on detection of unknown
> options instead of continuing with a potentially wrong set of
> arguments.
>
> v3 applies a number of changes discussed on irc and email. This is the
> grow to seven from three patches series.
>
> Cheers,
> Jes
>
> Jes Sorensen (7):
> Add missing tracing to qemu_mallocz()
> Use qemu_mallocz() instead of calloc() in img_convert()
> img_convert(): Only try to free bs[] entries if bs is valid.
> Make error handling more consistent in img_create() and img_resize()
> Consolidate printing of block driver options
> Fix formatting and missing braces in qemu-img.c
> Fail if detecting an unknown option
>
> qemu-img.c | 162 +++++++++++++++++++++++++++++++++++++++-----------------
> qemu-malloc.c | 5 ++-
> 2 files changed, 117 insertions(+), 50 deletions(-)
Thanks, applied all except patch 4, which breaks the build. Please
resend a new version of patch 4 as a single patch without the rest of
the series.
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize()
@ 2010-12-06 15:45 Jes.Sorensen
0 siblings, 0 replies; 19+ messages in thread
From: Jes.Sorensen @ 2010-12-06 15:45 UTC (permalink / raw)
To: kwolf; +Cc: stefanha, qemu-devel
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
qemu-img.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index aded72d..2deac67 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -314,13 +314,15 @@ static int img_create(int argc, char **argv)
drv = bdrv_find_format(fmt);
if (!drv) {
error("Unknown file format '%s'", fmt);
- return 1;
+ ret = -1;
+ goto out;
}
proto_drv = bdrv_find_protocol(filename);
if (!proto_drv) {
error("Unknown protocol '%s'", filename);
- return 1;
+ ret = -1;
+ goto out;
}
create_options = append_option_parameters(create_options,
@@ -1432,7 +1434,7 @@ static int img_resize(int argc, char **argv)
int c, ret, relative;
const char *filename, *fmt, *size;
int64_t n, total_size;
- BlockDriverState *bs;
+ BlockDriverState *bs = NULL;
QEMUOptionParameter *param;
QEMUOptionParameter resize_options[] = {
{
@@ -1483,14 +1485,16 @@ static int img_resize(int argc, char **argv)
param = parse_option_parameters("", resize_options, NULL);
if (set_option_parameter(param, BLOCK_OPT_SIZE, size)) {
/* Error message already printed when size parsing fails */
- exit(1);
+ ret = -1;
+ goto out;
}
n = get_option_parameter(param, BLOCK_OPT_SIZE)->value.n;
free_option_parameters(param);
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
if (!bs) {
- return 1;
+ ret = -1;
+ goto out;
}
if (relative) {
--
1.7.3.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-12-06 15:45 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-06 14:25 [Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code Jes.Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz() Jes.Sorensen
2010-12-06 14:42 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert() Jes.Sorensen
2010-12-06 14:43 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 3/7] img_convert(): Only try to free bs[] entries if bs is valid Jes.Sorensen
2010-12-06 14:44 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
2010-12-06 14:58 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 15:25 ` Kevin Wolf
2010-12-06 15:32 ` Jes Sorensen
2010-12-06 14:25 ` [Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options Jes.Sorensen
2010-12-06 14:59 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 6/7] Fix formatting and missing braces in qemu-img.c Jes.Sorensen
2010-12-06 15:00 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 14:25 ` [Qemu-devel] [PATCH 7/7] Fail if detecting an unknown option Jes.Sorensen
2010-12-06 15:02 ` [Qemu-devel] " Stefan Hajnoczi
2010-12-06 15:33 ` [Qemu-devel] Re: [PATCH v3 0/7] Cleanup qemu-img code Kevin Wolf
2010-12-06 15:45 [Qemu-devel] [PATCH 4/7] Make error handling more consistent in img_create() and img_resize() Jes.Sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.