On Thu, Feb 04, 2021 at 01:07:08PM -0600, Eric Blake wrote: > The value '1.1k' is inexact; 1126.4 bytes is not possible, so we > happen to truncate it to 1126. Our use of fractional sizes is > intended for convenience, but when a user specifies a fraction that is > not a clean translation to binary, truncating/rounding behind their > backs can cause confusion. Better is to deprecate inexact values, > which still leaves '1.5k' as valid, but alerts the user to spell out > their values as a precise byte number in cases where they are > currently being rounded. > > Note that values like '0.1G' in the testsuite need adjustment as a > result. > > Sadly, since qemu_strtosz() does not have an Err** parameter, we > pollute to stderr.
Does "deprecate" mean there's a plan to eventually remove this? If so I think it should say that (in docs/system/deprecated.rst I think). I think it would be better to remove this now or in the future since it's a trap for users. Rich. > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > tests/test-cutils.c | 4 ++-- > tests/test-keyval.c | 4 ++-- > tests/test-qemu-opts.c | 4 ++-- > util/cutils.c | 4 ++++ > 4 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/tests/test-cutils.c b/tests/test-cutils.c > index 0c2c89d6f113..ad51fb1baa51 100644 > --- a/tests/test-cutils.c > +++ b/tests/test-cutils.c > @@ -2095,14 +2095,14 @@ static void test_qemu_strtosz_units(void) > > static void test_qemu_strtosz_float(void) > { > - const char *str = "12.345M"; > + const char *str = "12.125M"; > int err; > const char *endptr; > uint64_t res = 0xbaadf00d; > > err = qemu_strtosz(str, &endptr, &res); > g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 12.345 * MiB); > + g_assert_cmpint(res, ==, 12.125 * MiB); > g_assert(endptr == str + 7); > } > > diff --git a/tests/test-keyval.c b/tests/test-keyval.c > index 13be763650b2..c951ac54cd23 100644 > --- a/tests/test-keyval.c > +++ b/tests/test-keyval.c > @@ -522,7 +522,7 @@ static void test_keyval_visit_size(void) > visit_free(v); > > /* Suffixes */ > - qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.1G,sz5=16777215T", > + qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.125G,sz5=16777215T", > NULL, NULL, &error_abort); > v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); > qobject_unref(qdict); > @@ -534,7 +534,7 @@ static void test_keyval_visit_size(void) > visit_type_size(v, "sz3", &sz, &error_abort); > g_assert_cmphex(sz, ==, 2 * MiB); > visit_type_size(v, "sz4", &sz, &error_abort); > - g_assert_cmphex(sz, ==, GiB / 10); > + g_assert_cmphex(sz, ==, GiB / 8); > visit_type_size(v, "sz5", &sz, &error_abort); > g_assert_cmphex(sz, ==, 16777215ULL * TiB); > visit_check_struct(v, &error_abort); > diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c > index f79b698e6715..6a1ea1d01c4f 100644 > --- a/tests/test-qemu-opts.c > +++ b/tests/test-qemu-opts.c > @@ -717,10 +717,10 @@ static void test_opts_parse_size(void) > g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, 8); > g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 1536); > g_assert_cmphex(qemu_opt_get_size(opts, "size3", 0), ==, 2 * MiB); > - opts = qemu_opts_parse(&opts_list_02, "size1=0.1G,size2=16777215T", > + opts = qemu_opts_parse(&opts_list_02, "size1=0.125G,size2=16777215T", > false, &error_abort); > g_assert_cmpuint(opts_count(opts), ==, 2); > - g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 10); > + g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 8); > g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215ULL * > TiB); > > /* Beyond limit with suffix */ > diff --git a/util/cutils.c b/util/cutils.c > index 75190565cbb5..5ec6101ae778 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -327,6 +327,10 @@ static int do_strtosz(const char *nptr, const char **end, > retval = -ERANGE; > goto out; > } > + if (mul_required && fraction * mul != (uint64_t) (fraction * mul)) { > + fprintf(stderr, "Using a fractional size that is not an exact byte " > + "multiple is deprecated: %s\n", nptr); > + } > *result = val * mul + (uint64_t) (fraction * mul); > retval = 0; > > -- > 2.30.0 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW