On Thu, Mar 11, 2021 at 02:07:02PM -0600, Eric Blake wrote: > Not all floating point fractions are precise. For example, the two > nearest 32-bit IEEE float values for 0.345 are 0.344999998808 and > 0.34500002861, with the lower one being closer. When our scaling unit > is 1000, that in turn can produce instances of double rounding (our > first when truncating to get the floating point fraction compared to > what the user typed, the second in converting the result of the > multiplication back to an integer), resulting in a final result 1 byte > smaller than the intuitive integer. > > For the actual test failure encountered on gitlab cross-i386-user, we > can clean things up by adding in DBL_EPSILON (with IEEE double, that > is 2^-53) for all values on a scale smaller than Petabytes (that is > 2^50), where our introduced error is not enough to add a full byte, > but will be enough to cause the subsequent multiplication to overshoot > rather than undershoot the nearest integer. And ultimately, we've > already documented that fractional values are for human convenience: > if a user is worried about byte-level precision when specifying more > than 50 bits of sizing, they should already be specifying things in > bytes rather than fractions. > > Fixes: cf923b783efd5 (utils: Improve qemu_strtosz() to have 64 bits of > precision) > Reported-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > > I'm not actually sure how to kick off a gitlab CI run of this to see if > it fixes the failure originally reported at > https://gitlab.com/qemu-project/qemu/-/jobs/1090685134 > Pointers welcome!
Create a gitlab.com account, and fork the QEMU repo. Then simply push your branch to your QEMU fork. Pipeline will run automagically and be visible in https://gitlab.com/<your user name>/qemu/-/pipelines > An alternative patch might be writing (uint64_t)(fraction * mul + 0.5) > (that is, introduce the fudge factor after the multiplication instead > of before). Preferences? No pref from me if both achieve the same end result. > util/cutils.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/util/cutils.c b/util/cutils.c > index d89a40a8c325..c124d8165f57 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -25,6 +25,7 @@ > #include "qemu/osdep.h" > #include "qemu/host-utils.h" > #include <math.h> > +#include <float.h> > > #include "qemu-common.h" > #include "qemu/sockets.h" > @@ -329,6 +330,15 @@ static int do_strtosz(const char *nptr, const char **end, > "is deprecated: %s", nptr); > } > endptr++; > + /* > + * Add in a fudge-factor (2^53 when double is IEEE format) for > + * all scales less than P (2^50), so that things like > + * 12.345M with unit 1000 produce 12345000 instead of > + * 12344999. > + */ > + if (mul > 1e49) { > + fraction += DBL_EPSILON; > + } Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
