Commit Graph

8 Commits

Author SHA1 Message Date
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
Markus Armbruster
1a9076919f tests/check-qnum: Cover qnum_to_string() for "unround" argument
qnum_to_string() has a FIXME comment about rounding errors due to
insufficient precision.  Cover it: 2.718281828459045 gets converted to
"2.718282".  The next commit will fix it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201210161452.2813491-6-armbru@redhat.com>
2020-12-19 10:37:16 +01:00
Marc-André Lureau
cb3e7f08ae qobject: Replace qobject_incref/QINCREF qobject_decref/QDECREF
Now that we can safely call QOBJECT() on QObject * as well as its
subtypes, we can have macros qobject_ref() / qobject_unref() that work
everywhere instead of having to use QINCREF() / QDECREF() for QObject
and qobject_incref() / qobject_decref() for its subtypes.

The replacement is mechanical, except I broke a long line, and added a
cast in monitor_qmp_cleanup_req_queue_locked().  Unlike
qobject_decref(), qobject_unref() doesn't accept void *.

Note that the new macros evaluate their argument exactly once, thus no
need to shout them.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased, semantic conflict resolved, commit message improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-05-04 08:27:53 +02:00
Max Reitz
7dc847ebba qapi: Replace qobject_to_X(o) by qobject_to(X, o)
This patch was generated using the following Coccinelle script:

@@
expression Obj;
@@
(
- qobject_to_qnum(Obj)
+ qobject_to(QNum, Obj)
|
- qobject_to_qstring(Obj)
+ qobject_to(QString, Obj)
|
- qobject_to_qdict(Obj)
+ qobject_to(QDict, Obj)
|
- qobject_to_qlist(Obj)
+ qobject_to(QList, Obj)
|
- qobject_to_qbool(Obj)
+ qobject_to(QBool, Obj)
)

and a bit of manual fix-up for overly long lines and three places in
tests/check-qjson.c that Coccinelle did not find.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20180224154033.29559-4-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: swap order from qobject_to(o, X), rebase to master, also a fix
to latent false-positive compiler complaint about hw/i386/acpi-build.c]
Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-19 14:58:36 -05:00
Markus Armbruster
e688df6bc4 Include qapi/error.h exactly where needed
This cleanup makes the number of objects depending on qapi/error.h
drop from 1910 (out of 4743) to 1612 in my "build everything" tree.

While there, separate #include from file comment with a blank line,
and drop a useless comment on why qemu/osdep.h is included first.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-5-armbru@redhat.com>
[Semantic conflict with commit 34e304e975 resolved, OSX breakage fixed]
2018-02-09 13:50:17 +01:00
Marc-André Lureau
61a8f418b2 qnum: add uint type
In order to store integer values between INT64_MAX and UINT64_MAX, add
a uint64_t internal representation.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170607163635.17635-10-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-06-20 14:31:31 +02:00
Marc-André Lureau
36aeb6094f tests: remove /{qnum, qlist, dict}/destroy test
The tests aren't really useful, or already covered by other simple tests.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170607163635.17635-9-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-06-20 14:31:31 +02:00
Marc-André Lureau
01b2ffcedd qapi: merge QInt and QFloat in QNum
We would like to use a same QObject type to represent numbers, whether
they are int, uint, or floats. Getters will allow some compatibility
between the various types if the number fits other representations.

Add a few more tests while at it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170607163635.17635-7-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[parse_stats_intervals() simplified a bit, comment in
test_visitor_in_int_overflow() tidied up, suppress bogus warnings]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-06-20 14:31:31 +02:00