Message ID | 1342617786-22107-1-git-send-email-wdongxu@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Am 18.07.2012 15:23, schrieb Dong Xu Wang: > qemu-img not only suports k/K/M/G/T/b, but also supports m/g/t/B. So correct > it in help message. > > Also use the same parser in parse_option_size function. This is not what the patch does. It uses a parser that seems slightly more compatible with strtosz_suffix() than before, but it still doesn't use the same one. It really should call strtosz_suffix() instead of implementing a parser here. Kevin
On 07/18/2012 07:23 AM, Dong Xu Wang wrote: > qemu-img not only suports k/K/M/G/T/b, but also supports m/g/t/B. So correct > it in help message. > > +++ b/qemu-img.c > @@ -69,8 +69,9 @@ static void help(void) > " options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" > " 'directsync' and 'unsafe' (default for convert)\n" > " 'size' is the disk image size in bytes. Optional suffixes\n" > - " 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M)\n" > - " and T (terabyte, 1024G) are supported. 'b' is ignored.\n" > + " 'k' or 'K' (kilobyte, 1024), 'm' or 'M' (megabyte, 1024k),\n" > + " 'g' or 'G' (gigabyte, 1024M) and 't' or 'T' (terabyte, 1024G) are supported.\n" > + " 'b' or 'B' is ignored.\n" Technically, 'kilobyte' is only 1000 bytes; the correct term for 1024 bytes is 'kibibyte'. Likewise for 'megabyte' (1000000) vs. 'mebibyte' (1024k, or 1,048,576 bytes); and so on for gibibytes and tebibytes. Since disk manufacturers have already forced the rest of the world to ask whether the number of bytes they are looking at is a power of 10 or a power of 2 suffix, we might as well be precise in our naming to document that we really are using powers of 2. Furthermore, I think you can compress this by mentioning that the parse is case-insensitive, instead of spelling out all the options: 'size' is the disk image size in bytes, scaled by an optional case-insensitive suffix: 'k' (kibibyte, 1024), 'M' (mebibyte, 1024k), 'G' (gibibyte, 1024M), 'T' (tebibyte, 1024G), or 'b' (no scaling). > @@ -341,8 +342,8 @@ static int img_create(int argc, char **argv) > char *end; > sval = strtosz_suffix(argv[optind++], &end, STRTOSZ_DEFSUFFIX_B); > if (sval < 0 || *end) { > - error_report("Invalid image size specified! You may use k, M, G or " > - "T suffixes for "); > + error_report("Invalid image size specified! You may use k/K, m/M, " > + "g/G or t/T suffixes for "); I personally dislike this change. Just because we're lenient in what we accept does not mean we have to document all of the possibilities that we parse when correcting a user error; rather, we need only document the preferred possibilities. > default: > error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size"); > - error_printf_unless_qmp("You may use k, M, G or T suffixes for " > + error_printf_unless_qmp("You may use K/k, M/m, G/g or T/t suffixes for " Again, in an error message, I'd only document the preferred capitalization.
diff --git a/qemu-img.c b/qemu-img.c index 80cfb9b..7f2fde4 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -69,8 +69,9 @@ static void help(void) " options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" " 'directsync' and 'unsafe' (default for convert)\n" " 'size' is the disk image size in bytes. Optional suffixes\n" - " 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M)\n" - " and T (terabyte, 1024G) are supported. 'b' is ignored.\n" + " 'k' or 'K' (kilobyte, 1024), 'm' or 'M' (megabyte, 1024k),\n" + " 'g' or 'G' (gigabyte, 1024M) and 't' or 'T' (terabyte, 1024G) are supported.\n" + " 'b' or 'B' is ignored.\n" " 'output_filename' is the destination disk image filename\n" " 'output_fmt' is the destination format\n" " 'options' is a comma separated list of format specific options in a\n" @@ -341,8 +342,8 @@ static int img_create(int argc, char **argv) char *end; sval = strtosz_suffix(argv[optind++], &end, STRTOSZ_DEFSUFFIX_B); if (sval < 0 || *end) { - error_report("Invalid image size specified! You may use k, M, G or " - "T suffixes for "); + error_report("Invalid image size specified! You may use k/K, m/M, " + "g/G or t/T suffixes for "); error_report("kilobytes, megabytes, gigabytes and terabytes."); ret = -1; goto out; diff --git a/qemu-option.c b/qemu-option.c index bb3886c..abc3b90 100644 --- a/qemu-option.c +++ b/qemu-option.c @@ -213,25 +213,29 @@ static void parse_option_size(const char *name, const char *value, sizef = strtod(value, &postfix); switch (*postfix) { case 'T': + case 't': sizef *= 1024; /* fall through */ case 'G': + case 'g': sizef *= 1024; /* fall through */ case 'M': + case 'm': sizef *= 1024; /* fall through */ case 'K': case 'k': sizef *= 1024; /* fall through */ + case 'B': case 'b': case '\0': *ret = (uint64_t) sizef; break; default: error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size"); - error_printf_unless_qmp("You may use k, M, G or T suffixes for " + error_printf_unless_qmp("You may use K/k, M/m, G/g or T/t suffixes for " "kilobytes, megabytes, gigabytes and terabytes.\n"); return; }
qemu-img not only suports k/K/M/G/T/b, but also supports m/g/t/B. So correct it in help message. Also use the same parser in parse_option_size function. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> CC: riegamaths@gmail.com --- v1->v2: also correct error reporting. v2->v3: use the same parser in parse_option_size function. qemu-img.c | 9 +++++---- qemu-option.c | 6 +++++- 2 files changed, 10 insertions(+), 5 deletions(-)