Skip to content

Commit

Permalink
qapi: update the qobject visitor to use QNUM_U64
Browse files Browse the repository at this point in the history
Switch to use QNum/uint where appropriate to remove i64 limitation.

The input visitor will cast i64 input to u64 for compatibility
reasons (existing json QMP client already use negative i64 for large
u64, and expect an implicit cast in qemu).

Note: before the patch, uint64_t values above INT64_MAX are sent over
json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the
patch, they are sent unmodified.  Clearly a bug fix, but we have to
consider compatibility issues anyway.  libvirt should cope fine,
because its parsing of unsigned integers accepts negative values
modulo 2^64.  There's hope that other clients will, too.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170607163635.17635-12-marcandre.lureau@redhat.com>
[check_native_list() tweaked for consistency with signed case]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
  • Loading branch information
elmarco authored and Markus Armbruster committed Jun 20, 2017
1 parent 2bc7cfe commit 5923f85
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 17 deletions.
2 changes: 1 addition & 1 deletion hw/i386/acpi-build.c
Expand Up @@ -2614,7 +2614,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
if (!o) {
return false;
}
mcfg->mcfg_base = qnum_get_int(qobject_to_qnum(o));
mcfg->mcfg_base = qnum_get_uint(qobject_to_qnum(o));
qobject_decref(o);

o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
Expand Down
21 changes: 16 additions & 5 deletions qapi/qobject-input-visitor.c
Expand Up @@ -417,7 +417,6 @@ static void qobject_input_type_int64_keyval(Visitor *v, const char *name,
static void qobject_input_type_uint64(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
{
/* FIXME: qobject_to_qnum mishandles values over INT64_MAX */
QObjectInputVisitor *qiv = to_qiv(v);
QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
QNum *qnum;
Expand All @@ -427,11 +426,23 @@ static void qobject_input_type_uint64(Visitor *v, const char *name,
return;
}
qnum = qobject_to_qnum(qobj);
if (!qnum || !qnum_get_try_int(qnum, &val)) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
full_name(qiv, name), "integer");
if (!qnum) {
goto err;
}

if (qnum_get_try_uint(qnum, obj)) {
return;
}
*obj = val;

/* Need to accept negative values for backward compatibility */
if (qnum_get_try_int(qnum, &val)) {
*obj = val;
return;
}

err:
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
full_name(qiv, name), "uint64");
}

static void qobject_input_type_uint64_keyval(Visitor *v, const char *name,
Expand Down
3 changes: 1 addition & 2 deletions qapi/qobject-output-visitor.c
Expand Up @@ -150,9 +150,8 @@ static void qobject_output_type_int64(Visitor *v, const char *name,
static void qobject_output_type_uint64(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
{
/* FIXME values larger than INT64_MAX become negative */
QObjectOutputVisitor *qov = to_qov(v);
qobject_output_add(qov, name, qnum_from_int(*obj));
qobject_output_add(qov, name, qnum_from_uint(*obj));
}

static void qobject_output_type_bool(Visitor *v, const char *name, bool *obj,
Expand Down
7 changes: 2 additions & 5 deletions tests/test-qobject-input-visitor.c
Expand Up @@ -123,7 +123,6 @@ static void test_visitor_in_int(TestInputVisitorData *data,
static void test_visitor_in_uint(TestInputVisitorData *data,
const void *unused)
{
Error *err = NULL;
uint64_t res = 0;
int64_t i64;
double dbl;
Expand All @@ -147,12 +146,10 @@ static void test_visitor_in_uint(TestInputVisitorData *data,
visit_type_uint64(v, NULL, &res, &error_abort);
g_assert_cmpuint(res, ==, (uint64_t)-value);

/* BUG: value between INT64_MAX+1 and UINT64_MAX rejected */

v = visitor_input_test_init(data, "18446744073709551574");

visit_type_uint64(v, NULL, &res, &err);
error_free_or_abort(&err);
visit_type_uint64(v, NULL, &res, &error_abort);
g_assert_cmpuint(res, ==, 18446744073709551574U);

visit_type_number(v, NULL, &dbl, &error_abort);
g_assert_cmpfloat(dbl, ==, 18446744073709552000.0);
Expand Down
22 changes: 18 additions & 4 deletions tests/test-qobject-output-visitor.c
Expand Up @@ -602,14 +602,28 @@ static void check_native_list(QObject *qobj,
qlist = qlist_copy(qobject_to_qlist(qdict_get(qdict, "data")));

switch (kind) {
case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
case USER_DEF_NATIVE_LIST_UNION_KIND_U8:
case USER_DEF_NATIVE_LIST_UNION_KIND_U16:
case USER_DEF_NATIVE_LIST_UNION_KIND_U32:
case USER_DEF_NATIVE_LIST_UNION_KIND_U64:
for (i = 0; i < 32; i++) {
QObject *tmp;
QNum *qvalue;
uint64_t val;

tmp = qlist_peek(qlist);
g_assert(tmp);
qvalue = qobject_to_qnum(tmp);
g_assert(qnum_get_try_uint(qvalue, &val));
g_assert_cmpint(val, ==, i);
qobject_decref(qlist_pop(qlist));
}
break;

case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
/*
* All integer elements in JSON arrays get stored into QNums
* when we convert to QObjects, so we can check them all in
Expand Down

0 comments on commit 5923f85

Please sign in to comment.