From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: vsementsov@virtuozzo.com, berrange@redhat.com,
qemu-block@nongnu.org, tao3.xu@intel.com, rjones@redhat.com,
armbru@redhat.com
Subject: [PATCH 0/3] Improve do_strtosz precision
Date: Thu, 4 Feb 2021 13:07:05 -0600 [thread overview]
Message-ID: <20210204190708.1306296-1-eblake@redhat.com> (raw)
Recently, commit 8b1170012b tweaked the maximum size the block layer
will allow, which in turn affects nbdkit's testsuite of edge-case
behaviors, where Rich noted [1] that our use of double meant rounding
errors that cause spurious failures in qemu-io (among other places).
So I decided to fix that. In the process, I was reminded that we have
attempted this before, most recently in Dec 2019 [2], where Markus
(rightly, IMHO) said that any approach that tries to reimplement
strtoull() or which compares the amount of data consumed between
strtod() and strtoull() adds more complexity than it solves.
So first, our analysis of what we absolutely need:
- Existing clients expect decimal scaling to work (1M is a lot easier
to type than 1048576)
- Existing clients expect hex to work (my initial attempt disabled it,
and promptly hung in iotests when things like 0x10000 got parsed as
zero rather than their intended byte value) (although our existing
testsuite coverage was only via iotests, rather than unit tests)
- Existing clients expect sane decimal fractions to work (1.5k is
easier to type than 1536), although our testsuite coverage of this
was less apparant
- Existing clients are surprised when something that looks like a byte
value is truncated or rounded due to an intermediate pass through
double (hence Rich's bug report)
[1] https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg00812.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg00852.html
My solution, instead of parsing twice and picking the longest, is to
always parse the integral portion with strtoull(), then special case
on what is left: if it is 'x' or 'X', the user wanted hex (and does
NOT want a fraction, and in fact we can optionally warn about the use
of scaling suffixes in this case); if it is '.', the user wanted the
convenience of a floating point adjustment which we can do via
strtod() on the suffix (and where we can optionally warn about
fractions that are not evenly divisible into bytes). I also enhanced
the testsuite (our coverage for hex constants was implicit via
iotests, and now explicit in the unit test; and our testsuite did not
flag the fact that we were previously accepting nonsense like '0x1.8k'
for 1536, or '1.5e1M' for 53477376).
We may decide that one or both of patch 2 and 3 goes too far, which is
why I split them out from the main enhancement.
Eric Blake (3):
utils: Improve qemu_strtosz() to have 64 bits of precision
utils: Deprecate hex-with-suffix sizes
utils: Deprecate inexact fractional suffix sizes
docs/system/deprecated.rst | 8 ++++
tests/test-cutils.c | 83 ++++++++++++++++++++++++++++---------
tests/test-keyval.c | 28 +++++++++----
tests/test-qemu-opts.c | 24 +++++++----
util/cutils.c | 85 +++++++++++++++++++++++++++++---------
tests/qemu-iotests/049.out | 7 +++-
6 files changed, 178 insertions(+), 57 deletions(-)
--
2.30.0
next reply other threads:[~2021-02-04 19:32 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-04 19:07 Eric Blake [this message]
2021-02-04 19:07 ` [PATCH 1/3] utils: Improve qemu_strtosz() to have 64 bits of precision Eric Blake
2021-02-04 20:12 ` Eric Blake
2021-02-05 10:06 ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:18 ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:06 ` Eric Blake
2021-02-05 14:10 ` Daniel P. Berrangé
2021-02-05 10:07 ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:12 ` Eric Blake
2021-02-05 10:28 ` Richard W.M. Jones
2021-02-05 14:15 ` Eric Blake
2021-02-05 11:02 ` Daniel P. Berrangé
2021-02-05 14:27 ` Eric Blake
2021-02-05 14:36 ` Daniel P. Berrangé
2021-02-05 11:34 ` Daniel P. Berrangé
2021-02-05 14:36 ` Eric Blake
2021-02-04 19:07 ` [PATCH 2/3] utils: Deprecate hex-with-suffix sizes Eric Blake
2021-02-05 10:25 ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:31 ` Richard W.M. Jones
2021-02-05 13:38 ` Eric Blake
2021-02-05 11:13 ` Daniel P. Berrangé
2021-02-05 13:40 ` Eric Blake
2021-02-05 14:02 ` Daniel P. Berrangé
2021-02-04 19:07 ` [PATCH 3/3] utils: Deprecate inexact fractional suffix sizes Eric Blake
2021-02-04 20:02 ` Eric Blake
2021-02-05 10:34 ` Richard W.M. Jones
2021-02-05 14:19 ` Eric Blake
2021-02-05 10:38 ` Vladimir Sementsov-Ogievskiy
2021-02-05 11:10 ` Daniel P. Berrangé
2021-02-05 14:28 ` Eric Blake
2021-02-05 14:40 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210204190708.1306296-1-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rjones@redhat.com \
--cc=tao3.xu@intel.com \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).