[Qemu-devel] [PATCH v4 0/2] Clean up img_create() and introduce strtosz_suffix()

[Qemu-devel] [PATCH v4 0/2] Clean up img_create() and introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch set introduces strtosz_suffix() which is needed to be able
to use strtosz parsing with a non MB default suffix. This is used to
clean up qemu-img.c:img_create().

Kevin asked me to rebase this instead of applying the other patches on
top, so please discard the previous versions. Sorry for the patch
noise.

Jes Sorensen (2):
  Introduce strtosz_suffix()
  qemu-img.c: Clean up handling of image size in img_create()

 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 qemu-img.c    |   23 +++++++++++++++++------
 3 files changed, 38 insertions(+), 9 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/2] Introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces strtosz_suffix() which allows the caller to specify a
default suffix in case the non default of MB is wanted.

strtosz() is kept as a wrapper for strtosz_suffix() which keeps it's
current default of MB.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/cutils.c b/cutils.c
index 28089aa..1d24d9a 100644
--- a/cutils.c
+++ b/cutils.c
@@ -291,10 +291,10 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     ssize_t retval = -1;
-    char *endptr, c;
+    char *endptr, c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -313,10 +313,16 @@ ssize_t strtosz(const char *nptr, char **end)
      * part of a multi token argument.
      */
     c = *endptr;
+    d = c;
     if (isspace(c) || c == '\0' || c == ',') {
         c = 0;
+        if (default_suffix) {
+            d = default_suffix;
+        } else {
+            d = c;
+        }
     }
-    switch (c) {
+    switch (d) {
     case 'B':
     case 'b':
         mul = 1;
@@ -371,3 +377,8 @@ fail:
 
     return retval;
 }
+
+ssize_t strtosz(const char *nptr, char **end)
+{
+    return strtosz_suffix(nptr, end, 0);
+}
diff --git a/qemu-common.h b/qemu-common.h
index de82c2e..dc44cd6 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+
+#define STRTOSZ_DEFSUFFIX_TB	'T'
+#define STRTOSZ_DEFSUFFIX_GB	'G'
+#define STRTOSZ_DEFSUFFIX_MB	'M'
+#define STRTOSZ_DEFSUFFIX_KB	'K'
+#define STRTOSZ_DEFSUFFIX_B	'B'
 ssize_t strtosz(const char *nptr, char **end);
+ssize_t strtosz_suffix(const char *nptr, char **end, const char);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.3.2

[Qemu-devel] [PATCH 2/2] qemu-img.c: Clean up handling of image size in img_create()

[Jes.Sorensen]

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 |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index d146d8c..f078718 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,20 @@ static int img_create(int argc, char **argv)
     }
     filename = argv[optind++];
 
+    /* Get image size, if specified */
+    if (optind < argc) {
+        ssize_t sval;
+        sval = strtosz_suffix(argv[optind++], NULL, STRTOSZ_DEFSUFFIX_B);
+        if (sval < 0) {
+            error("Invalid image size specified! You may use k, M, G or "
+                  "T suffixes for ");
+            error("kilobytes, megabytes, gigabytes and terabytes.");
+            ret = -1;
+            goto out;
+        }
+        img_size = (uint64_t)sval;
+    }
+
     if (options && !strcmp(options, "?")) {
         ret = print_block_option_help(filename, fmt);
         goto out;
@@ -356,7 +371,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 +384,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) {
-- 
1.7.3.2

[Qemu-devel] Re: [PATCH 1/2] Introduce strtosz_suffix()

[Stefan Hajnoczi]

On Thu, Dec 09, 2010 at 01:13:33PM +0100, Jes.Sorensen@redhat.com wrote:
> @@ -371,3 +377,8 @@ fail:
> 
>      return retval;
>  }
> +
> +ssize_t strtosz(const char *nptr, char **end)
> +{
> +    return strtosz_suffix(nptr, end, 0);

This obscures what the default is, please use STRTOSZ_DEFSUFFIX_MB.  0
isn't very meaningful and requires the reader to dig into strtosz().

> diff --git a/qemu-common.h b/qemu-common.h
> index de82c2e..dc44cd6 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
>  int qemu_fls(int i);
>  int qemu_fdatasync(int fd);
>  int fcntl_setfl(int fd, int flag);
> +
> +#define STRTOSZ_DEFSUFFIX_TB	'T'
> +#define STRTOSZ_DEFSUFFIX_GB	'G'
> +#define STRTOSZ_DEFSUFFIX_MB	'M'
> +#define STRTOSZ_DEFSUFFIX_KB	'K'
> +#define STRTOSZ_DEFSUFFIX_B	'B'
>  ssize_t strtosz(const char *nptr, char **end);
> +ssize_t strtosz_suffix(const char *nptr, char **end, const char);

An argument name would be nice: const char unit?

Stefan

[Qemu-devel] Re: [PATCH 1/2] Introduce strtosz_suffix()

[Jes Sorensen]

On 12/09/10 13:53, Stefan Hajnoczi wrote:
> On Thu, Dec 09, 2010 at 01:13:33PM +0100, Jes.Sorensen@redhat.com wrote:
>> @@ -371,3 +377,8 @@ fail:
>>
>>      return retval;
>>  }
>> +
>> +ssize_t strtosz(const char *nptr, char **end)
>> +{
>> +    return strtosz_suffix(nptr, end, 0);
> 
> This obscures what the default is, please use STRTOSZ_DEFSUFFIX_MB.  0
> isn't very meaningful and requires the reader to dig into strtosz().

It doesn't make much different to me, but ok, that is easy to fix.

>> diff --git a/qemu-common.h b/qemu-common.h
>> index de82c2e..dc44cd6 100644
>> --- a/qemu-common.h
>> +++ b/qemu-common.h
>> @@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
>>  int qemu_fls(int i);
>>  int qemu_fdatasync(int fd);
>>  int fcntl_setfl(int fd, int flag);
>> +
>> +#define STRTOSZ_DEFSUFFIX_TB	'T'
>> +#define STRTOSZ_DEFSUFFIX_GB	'G'
>> +#define STRTOSZ_DEFSUFFIX_MB	'M'
>> +#define STRTOSZ_DEFSUFFIX_KB	'K'
>> +#define STRTOSZ_DEFSUFFIX_B	'B'
>>  ssize_t strtosz(const char *nptr, char **end);
>> +ssize_t strtosz_suffix(const char *nptr, char **end, const char);
> 
> An argument name would be nice: const char unit?

I'll add that in the same round.

Cheers,
Jes