Message ID | 1291799612-5145-1-git-send-email-Jes.Sorensen@redhat.com |
---|---|
State | New |
Headers | show |
Jes.Sorensen@redhat.com writes: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > This cleans up the handling of image size in img_create() by parsing > the value early, and then only setting it once if a value has been > added as the last argument to the command line. > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > qemu-img.c | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > Patch conflicts with commit c2abccec. > diff --git a/qemu-img.c b/qemu-img.c > index d146d8c..9986004 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -282,6 +282,7 @@ static int add_old_style_options(const char *fmt, QEMUOptionParameter *list, > static int img_create(int argc, char **argv) > { > int c, ret = 0; > + uint64_t img_size = -1; > const char *fmt = "raw"; > const char *base_fmt = NULL; > const char *filename; > @@ -329,6 +330,11 @@ static int img_create(int argc, char **argv) > } > filename = argv[optind++]; > > + /* Get image size, if specified */ > + if (optind < argc) { > + img_size = strtosz(argv[optind++], NULL); strtosz() can fail. More below. > + } > + > if (options && !strcmp(options, "?")) { > ret = print_block_option_help(filename, fmt); > goto out; > @@ -356,7 +362,8 @@ static int img_create(int argc, char **argv) > > /* Create parameter list with default values */ > param = parse_option_parameters("", create_options, param); > - set_option_parameter_int(param, BLOCK_OPT_SIZE, -1); > + > + set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > > /* Parse -o options */ > if (options) { > @@ -368,11 +375,6 @@ static int img_create(int argc, char **argv) > } > } > > - /* Add size to parameters */ > - if (optind < argc) { > - set_option_parameter(param, BLOCK_OPT_SIZE, argv[optind++]); > - } > - > /* Add old-style options to parameters */ > ret = add_old_style_options(fmt, param, base_filename, base_fmt); > if (ret < 0) { This switches parsing of the size argument from parse_option_size() (via set_option_parameter()) to strtosz(). I'm fine with that, but: * Before: $ qemu-img create xxx xxx Parameter 'size' expects a size You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. qemu-img: Image creation needs a size parameter * After: $ qemu-img create xxx xxx qemu-img: Image creation needs a size parameter Intentional?
On 12/15/10 17:47, Markus Armbruster wrote: > Jes.Sorensen@redhat.com writes: > >> From: Jes Sorensen <Jes.Sorensen@redhat.com> >> >> This cleans up the handling of image size in img_create() by parsing >> the value early, and then only setting it once if a value has been >> added as the last argument to the command line. >> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> >> --- >> qemu-img.c | 14 ++++++++------ >> 1 files changed, 8 insertions(+), 6 deletions(-) >> > > Patch conflicts with commit c2abccec. ARGH :( > This switches parsing of the size argument from parse_option_size() (via > set_option_parameter()) to strtosz(). I'm fine with that, but: > > * Before: > > $ qemu-img create xxx xxx > Parameter 'size' expects a size > You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. > qemu-img: Image creation needs a size parameter > > * After: > > $ qemu-img create xxx xxx > qemu-img: Image creation needs a size parameter > > Intentional? This was addressed in the later revision when I introduced strtosz_suffix() Cheers, Jes
Jes Sorensen <Jes.Sorensen@redhat.com> writes: > On 12/15/10 17:47, Markus Armbruster wrote: >> Jes.Sorensen@redhat.com writes: >> >>> From: Jes Sorensen <Jes.Sorensen@redhat.com> >>> >>> This cleans up the handling of image size in img_create() by parsing >>> the value early, and then only setting it once if a value has been >>> added as the last argument to the command line. >>> >>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> >>> --- >>> qemu-img.c | 14 ++++++++------ >>> 1 files changed, 8 insertions(+), 6 deletions(-) >>> >> >> Patch conflicts with commit c2abccec. > > ARGH :( > >> This switches parsing of the size argument from parse_option_size() (via >> set_option_parameter()) to strtosz(). I'm fine with that, but: >> >> * Before: >> >> $ qemu-img create xxx xxx >> Parameter 'size' expects a size >> You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. >> qemu-img: Image creation needs a size parameter >> >> * After: >> >> $ qemu-img create xxx xxx >> qemu-img: Image creation needs a size parameter >> >> Intentional? > > This was addressed in the later revision when I introduced strtosz_suffix() Looks like I'm getting lost in my post-vacation mail backlog %-}
diff --git a/qemu-img.c b/qemu-img.c index d146d8c..9986004 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -282,6 +282,7 @@ static int add_old_style_options(const char *fmt, QEMUOptionParameter *list, static int img_create(int argc, char **argv) { int c, ret = 0; + uint64_t img_size = -1; const char *fmt = "raw"; const char *base_fmt = NULL; const char *filename; @@ -329,6 +330,11 @@ static int img_create(int argc, char **argv) } filename = argv[optind++]; + /* Get image size, if specified */ + if (optind < argc) { + img_size = strtosz(argv[optind++], NULL); + } + if (options && !strcmp(options, "?")) { ret = print_block_option_help(filename, fmt); goto out; @@ -356,7 +362,8 @@ static int img_create(int argc, char **argv) /* Create parameter list with default values */ param = parse_option_parameters("", create_options, param); - set_option_parameter_int(param, BLOCK_OPT_SIZE, -1); + + set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); /* Parse -o options */ if (options) { @@ -368,11 +375,6 @@ static int img_create(int argc, char **argv) } } - /* Add size to parameters */ - if (optind < argc) { - set_option_parameter(param, BLOCK_OPT_SIZE, argv[optind++]); - } - /* Add old-style options to parameters */ ret = add_old_style_options(fmt, param, base_filename, base_fmt); if (ret < 0) {