[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/5] qemu_img: add error report to cvtnum
From: |
Eric Blake |
Subject: |
Re: [PATCH v2 2/5] qemu_img: add error report to cvtnum |
Date: |
Wed, 6 May 2020 16:59:39 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
All calls to cvtnum check the return value and print the same error message more
or less. And so error reporting moved to cvtnum to reduce duplicate code and
provide a single error message.
Acked-by: Mark Kanda <address@hidden>
Signed-off-by: Eyal Moscovici <address@hidden>
---
qemu-img.c | 63 ++++++++++++++++----------------------
tests/qemu-iotests/049.out | 4 +--
2 files changed, 28 insertions(+), 39 deletions(-)
- err = qemu_strtosz(s, NULL, &value);
- if (err < 0) {
+ err = qemu_strtosz(arg_value, NULL, &value);
+ if (err < 0 && err != -ERANGE) {
+ error_report("Invalid %s specified! You may use "
+ "k, M, G, T, P or E suffixes for ", arg_name);
+ error_report("kilobytes, megabytes, gigabytes, terabytes, "
+ "petabytes and exabytes.");
return err;
}
- if (value > INT64_MAX) {
+ if (err == -ERANGE || value > INT64_MAX) {
+ error_report("Invalid %s specified! Must be less than 8 EiB!",
Copied from our pre-existing errors, but why are we shouting at our
user? This would be a good time to s/!/./ to tone it down a bit.
@@ -4491,10 +4488,12 @@ static int img_dd_bs(const char *arg,
{
int64_t res;
- res = cvtnum(arg);
+ res = cvtnum("bs", arg);
- if (res <= 0) {
- error_report("invalid number: '%s'", arg);
+ if (res < 0) {
+ return 1;
+ } else if (res == 0) {
+ error_report("Invalid bs specified! It cannot be 0.");
Maybe it's worth two functions:
int64_t cvtnum_full(const char *name, const char *value, int64_t min,
int64_t max)
and then a common helper:
int64_t cvtnum(const char *name, const char *value) {
return cvtnum_full(name, value, 0, INT64_MAX);
}
many existing callers remain with cvtnum(), but callers like this could
be cvtnum("bs", arg, 1, INT64_MAX). You'd still have to special-case
other restrictions, such as whether a number must a power-of-2, but
that's fewer places.
+++ b/tests/qemu-iotests/049.out
@@ -92,13 +92,13 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1649267441664
cluster_size=65536 l
== 3. Invalid sizes ==
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024
-qemu-img: Image size must be less than 8 EiB!
+qemu-img: Invalid image size specified! Must be less than 8 EiB!
Nice that you checked for iotest fallout. Is this really the only
impacted test?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v2 0/5] Additional parameters for qemu_img map, Eyal Moscovici, 2020/05/06
- [PATCH v2 2/5] qemu_img: add error report to cvtnum, Eyal Moscovici, 2020/05/06
- Re: [PATCH v2 2/5] qemu_img: add error report to cvtnum,
Eric Blake <=
- [PATCH v2 3/5] qemu-img: validate image length in img_map, Eyal Moscovici, 2020/05/06
- [PATCH v2 4/5] qemu-img: refactor dump_map_entry JSON format output, Eyal Moscovici, 2020/05/06
- [PATCH v2 5/5] qemu-img: Add --start-offset and --max-length to map, Eyal Moscovici, 2020/05/06
- Re: [PATCH v2 0/5] Additional parameters for qemu_img map, Eric Blake, 2020/05/06