qemu/qobject/qnum.c
Markus Armbruster f917eed306 qobject: Fix qnum_to_string() to use sufficient precision
We should serialize numbers to JSON so that they deserialize back to
the same number.  We fail to do so.

The culprit is qnum_to_string(): it uses format %f with trailing '0'
trimmed.  Results in pretty output for "nice" numbers, but is prone to
nasty rounding errors.  For instance, numbers between 0 and 0.0000005
get flushed to zero.

Where exactly the incorrect rounding can bite is tiresome to gauge.
Here's my take.

* In QMP output, type 'number':

  - query-blockstats value avg_rd_queue_depth

  - QMP query-migrate values mbps, cache-miss-rate, encoding-rate,
    busy-rate, compression-rate.

  Relatively harmless, I guess.

* In tracing QMP input.  Harmless.

* In qemu-ga output, type 'number': guest-get-users value login-time.
  Harmless.

* In output of HMP qom-get.  Harmless.

Not affected, because double values don't actually occur there (I
think):

* QMP output, type 'any':

  * qom-get value

  * qom-list, qom-list-properties value default-value

  * query-cpu-model-comparison, query-cpu-model-baseline,
    query-cpu-model-expansion value props.

* qemu-img --output json output.

* "json:" pseudo-filenames generated by bdrv_refresh_filename().

* The rbd block driver's "=keyvalue-pairs" hack.

* In -object help on property default values.  Aside: use of JSON
  feels inappropriate here.

* Output of HMP qom-get.

* Argument conversion to QemuOpts for qdev_device_add() and HMP with
  qemu_opts_from_qdict()

  QMP and HMP device_add, virtio-net failover primary creation,
  xen-usb "usb-host" creation, HMP netdev_add, object_add.

* The uses of qobject_input_visitor_new_flat_confused()

  As far as I can tell, none of the visited types contain double
  values.

* Dumping ImageInfoSpecific with dump_qobject()

Fix by formatting with %.17g.  17 decimal digits always suffice for
IEEE double.

The change to expected test output illustrates the effect: the
rounding errors are gone, but some seemingly "nice" numbers now get
converted to not so nice strings, e.g. 0.42 to "0.41999999999999998".
This is because 0.42 is not representable exactly in double.  It's
more accurate in this example than strictly necessary, though.

If ugly accuracy bothers us, we can we can try using the least number
of digits that still converts back to the same double.  In this
example, "0.42" would do.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201210161452.2813491-7-armbru@redhat.com>
2020-12-19 10:37:16 +01:00

245 lines
5.3 KiB
C

/*
* QNum Module
*
* Copyright (C) 2009 Red Hat Inc.
*
* Authors:
* Luiz Capitulino <lcapitulino@redhat.com>
* Anthony Liguori <aliguori@us.ibm.com>
* Marc-André Lureau <marcandre.lureau@redhat.com>
*
* This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
* See the COPYING.LIB file in the top-level directory.
*/
#include "qemu/osdep.h"
#include "qapi/qmp/qnum.h"
/**
* qnum_from_int(): Create a new QNum from an int64_t
*
* Return strong reference.
*/
QNum *qnum_from_int(int64_t value)
{
QNum *qn = g_new(QNum, 1);
qobject_init(QOBJECT(qn), QTYPE_QNUM);
qn->kind = QNUM_I64;
qn->u.i64 = value;
return qn;
}
/**
* qnum_from_uint(): Create a new QNum from an uint64_t
*
* Return strong reference.
*/
QNum *qnum_from_uint(uint64_t value)
{
QNum *qn = g_new(QNum, 1);
qobject_init(QOBJECT(qn), QTYPE_QNUM);
qn->kind = QNUM_U64;
qn->u.u64 = value;
return qn;
}
/**
* qnum_from_double(): Create a new QNum from a double
*
* Return strong reference.
*/
QNum *qnum_from_double(double value)
{
QNum *qn = g_new(QNum, 1);
qobject_init(QOBJECT(qn), QTYPE_QNUM);
qn->kind = QNUM_DOUBLE;
qn->u.dbl = value;
return qn;
}
/**
* qnum_get_try_int(): Get an integer representation of the number
*
* Return true on success.
*/
bool qnum_get_try_int(const QNum *qn, int64_t *val)
{
switch (qn->kind) {
case QNUM_I64:
*val = qn->u.i64;
return true;
case QNUM_U64:
if (qn->u.u64 > INT64_MAX) {
return false;
}
*val = qn->u.u64;
return true;
case QNUM_DOUBLE:
return false;
}
assert(0);
return false;
}
/**
* qnum_get_int(): Get an integer representation of the number
*
* assert() on failure.
*/
int64_t qnum_get_int(const QNum *qn)
{
int64_t val;
bool success = qnum_get_try_int(qn, &val);
assert(success);
return val;
}
/**
* qnum_get_uint(): Get an unsigned integer from the number
*
* Return true on success.
*/
bool qnum_get_try_uint(const QNum *qn, uint64_t *val)
{
switch (qn->kind) {
case QNUM_I64:
if (qn->u.i64 < 0) {
return false;
}
*val = qn->u.i64;
return true;
case QNUM_U64:
*val = qn->u.u64;
return true;
case QNUM_DOUBLE:
return false;
}
assert(0);
return false;
}
/**
* qnum_get_uint(): Get an unsigned integer from the number
*
* assert() on failure.
*/
uint64_t qnum_get_uint(const QNum *qn)
{
uint64_t val;
bool success = qnum_get_try_uint(qn, &val);
assert(success);
return val;
}
/**
* qnum_get_double(): Get a float representation of the number
*
* qnum_get_double() loses precision for integers beyond 53 bits.
*/
double qnum_get_double(QNum *qn)
{
switch (qn->kind) {
case QNUM_I64:
return qn->u.i64;
case QNUM_U64:
return qn->u.u64;
case QNUM_DOUBLE:
return qn->u.dbl;
}
assert(0);
return 0.0;
}
char *qnum_to_string(QNum *qn)
{
switch (qn->kind) {
case QNUM_I64:
return g_strdup_printf("%" PRId64, qn->u.i64);
case QNUM_U64:
return g_strdup_printf("%" PRIu64, qn->u.u64);
case QNUM_DOUBLE:
/* FIXME: g_strdup_printf() is locale dependent; but JSON requires
* numbers to be formatted as if in the C locale. Dependence
* on C locale is a pervasive issue in QEMU. */
/* FIXME: This risks printing Inf or NaN, which are not valid
* JSON values. */
/* 17 digits suffice for IEEE double */
return g_strdup_printf("%.17g", qn->u.dbl);
}
assert(0);
return NULL;
}
/**
* qnum_is_equal(): Test whether the two QNums are equal
*
* Negative integers are never considered equal to unsigned integers,
* but positive integers in the range [0, INT64_MAX] are considered
* equal independently of whether the QNum's kind is i64 or u64.
*
* Doubles are never considered equal to integers.
*/
bool qnum_is_equal(const QObject *x, const QObject *y)
{
QNum *num_x = qobject_to(QNum, x);
QNum *num_y = qobject_to(QNum, y);
switch (num_x->kind) {
case QNUM_I64:
switch (num_y->kind) {
case QNUM_I64:
/* Comparison in native int64_t type */
return num_x->u.i64 == num_y->u.i64;
case QNUM_U64:
/* Implicit conversion of x to uin64_t, so we have to
* check its sign before */
return num_x->u.i64 >= 0 && num_x->u.i64 == num_y->u.u64;
case QNUM_DOUBLE:
return false;
}
abort();
case QNUM_U64:
switch (num_y->kind) {
case QNUM_I64:
return qnum_is_equal(y, x);
case QNUM_U64:
/* Comparison in native uint64_t type */
return num_x->u.u64 == num_y->u.u64;
case QNUM_DOUBLE:
return false;
}
abort();
case QNUM_DOUBLE:
switch (num_y->kind) {
case QNUM_I64:
case QNUM_U64:
return false;
case QNUM_DOUBLE:
/* Comparison in native double type */
return num_x->u.dbl == num_y->u.dbl;
}
abort();
}
abort();
}
/**
* qnum_destroy_obj(): Free all memory allocated by a
* QNum object
*/
void qnum_destroy_obj(QObject *obj)
{
assert(obj != NULL);
g_free(qobject_to(QNum, obj));
}