[PATCH v2 4/4] utils: Deprecate inexact fractional suffix sizes

[Eric Blake]

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.

Since qemu_strtosz() does not have an Err** parameter, and plumbing
that in would be a much larger task, we instead go with just directly
emitting the deprecation warning to stderr.

Signed-off-by: Eric Blake <eblake at redhat.com>

---

I'm not a fan of this patch, but am proposing it for discussion purposes.
---
 docs/system/deprecated.rst | 9 +++++++++
 tests/test-cutils.c        | 6 +++---
 tests/test-keyval.c        | 4 ++--
 tests/test-qemu-opts.c     | 4 ++--
 util/cutils.c              | 9 +++++++--
 5 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 113c2e933f1b..2c9cb849eec5 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -154,6 +154,15 @@ Input parameters that take a size value should only use a size suffix
 the value is hexadecimal.  That is, '0x20M' is deprecated, and should
 be written either as '32M' or as '0x2000000'.

+inexact sizes via scaled fractions (since 6.0)
+''''''''''''''''''''''''''''''''''''''''''''''
+
+Input parameters that take a size value should only use a fractional
+size (such as '1.5M') that will result in an exact byte value.  The
+use of inexact values (such as '1.1M') that require truncation or
+rounding is deprecated, and you should instead consider writing your
+unusual size in bytes (here, '1153433' or '1153434' as desired).
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------

diff --git a/tests/test-cutils.c b/tests/test-cutils.c
index bad3a6099389..c6c33866277b 100644
--- a/tests/test-cutils.c
+++ b/tests/test-cutils.c
@@ -2124,11 +2124,11 @@ static void test_qemu_strtosz_float(void)
     g_assert_cmpint(res, ==, 1024);
     g_assert(endptr == str + 3);

-    /* For convenience, we permit values that are not byte-exact */
-    str = "12.345M";
+    /* Fractional values should still be byte-exact */
+    str = "12.125M";
     err = qemu_strtosz(str, &endptr, &res);
     g_assert_cmpint(err, ==, 0);
-    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB));
+    g_assert_cmpint(res, ==, (uint64_t) (12.125 * MiB));
     g_assert(endptr == str + 7);
 }

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index e20c07cf3ea5..7a45c22942e6 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -525,7 +525,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);
@@ -537,7 +537,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 6568e31a7229..549e994938fe 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -720,10 +720,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 6a8a175e0d71..1154b9de131a 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -246,12 +246,13 @@ static int64_t suffix_mul(char suffix, int64_t unit)
  * The size parsing supports the following syntaxes
  * - 12345 - decimal, scale determined by @default_suffix and @unit
  * - 12345{bBkKmMgGtTpPeE} - decimal, scale determined by suffix and @unit
- * - 12345.678{kKmMgGtTpPeE} - decimal, scale determined by suffix, and
- *   fractional portion is truncated to byte
+ * - 12345.678{kKmMgGtTpPeE} - decimal, scale determined by suffix, if
+ *   fractional portion is exact
  * - 0x7fEE - hexadecimal, unit determined by @default_suffix
  *
  * The following cause a deprecation warning, and may be removed in the future
  * - 0xabc{kKmMgGtTpP} - hex with scaling suffix
+ * - 12345.678{kKmMgGtTpPeE} - decimal with inexact fraction that caused truncation
  *
  * The following are intentionally not supported
  * - octal, such as 08
@@ -342,6 +343,10 @@ static int do_strtosz(const char *nptr, const char **end,
         retval = -ERANGE;
         goto out;
     }
+    if (mul_required && fraction * mul != (uint64_t) (fraction * mul)) {
+        warn_report("Using a fractional size that is not an exact byte "
+                    "multiple is deprecated: %s", nptr);
+    }
     *result = val * mul + (uint64_t) (fraction * mul);
     retval = 0;

-- 
2.30.1