* [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors
@ 2012-06-08 15:35 Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t Andreas Färber
` (10 more replies)
0 siblings, 11 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel
Cc: Jan Kiszka, Michael Roth, Luiz Capitulino, Anthony Liguori,
Paolo Bonzini, Laszlo Ersek, Andreas Färber
Hello Anthony,
To get moving with the merge of qom-next into qemu.git, now that the Makefile
PULL is in, please pull a first small batch, grouping patches directly related
to fixed-width visitors.
Outlook:
A second batch would include the QBus refactoring - still cherry-picking,
reordering and re-testing.
I'm preparing a two-patch realize series that could get appended to the second.
Still open are the static property movements and their prerequisites.
CPU-related patches are split off into their own pull.
Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Jan, this does not include the big file movement but the largest part of the
qdev-properties.c changes and in particular the ones that affect PCI. I suggest
that you rebase on this one and me/Paolo the remaining parts on yours.
Cc: Jan Kiszka <jan.kiszka@siemens.com>
The following changes since commit fa79c914efd35cb60e0bc18512c03690c48b13e2:
Merge remote-tracking branch 'bonzini/nested-makefiles-3' into staging (2012-06-07 17:21:40 +0800)
are available in the git repository at:
git://repo.or.cz/qemu/afaerber.git qom-next-1
Andreas Färber (1):
target-i386: Use uint32 visitor for [x]level properties
Michael Roth (6):
qapi: Add Visitor interfaces for uint*_t and int*_t
qapi: Unit tests for visitor-based serialization
qapi: String visitor, use %f representation for floats
qapi: Add String visitor coverage to serialization unit tests
qdev: Use int32_t container for devfn property
qdev: Switch property accessors to fixed-width visitor interfaces
Paolo Bonzini (1):
qdev: Remove PropertyInfo range checking
hw/mc146818rtc.c | 7 -
hw/pci.c | 2 +-
hw/pci.h | 2 +-
hw/qdev-addr.c | 4 +-
hw/qdev-properties.c | 205 ++++------
hw/qdev.h | 4 +-
qapi/qapi-visit-core.c | 139 +++++++
qapi/qapi-visit-core.h | 16 +
qapi/string-output-visitor.c | 2 +-
target-i386/cpu.c | 42 +--
tests/Makefile | 4 +-
tests/test-string-output-visitor.c | 2 +-
tests/test-visitor-serialization.c | 784 ++++++++++++++++++++++++++++++++++++
13 files changed, 1041 insertions(+), 172 deletions(-)
create mode 100644 tests/test-visitor-serialization.c
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-09 15:03 ` Laszlo Ersek
2012-06-08 15:35 ` [Qemu-devel] [PATCH 2/8] qapi: Unit tests for visitor-based serialization Andreas Färber
` (9 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Laszlo Ersek, Michael Roth, Anthony Liguori,
Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
This adds visitor interfaces for fixed-width integers types.
Implementing these in visitors is optional, otherwise we fall back to
visit_type_int() (int64_t) with some additional bounds checking to avoid
integer overflows for cases where the value fetched exceeds the bounds
of our target C type.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[LE: exclude negative values in uint*_t Visitor interfaces]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
[AF: Merged fix by Laszlo]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
hw/mc146818rtc.c | 7 ---
qapi/qapi-visit-core.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++
qapi/qapi-visit-core.h | 16 ++++++
3 files changed, 155 insertions(+), 7 deletions(-)
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 9c64e0a..3777f85 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -599,13 +599,6 @@ static const MemoryRegionOps cmos_ops = {
.old_portio = cmos_portio
};
-// FIXME add int32 visitor
-static void visit_type_int32(Visitor *v, int *value, const char *name, Error **errp)
-{
- int64_t val = *value;
- visit_type_int(v, &val, name, errp);
-}
-
static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a4e088c..ffffbf7 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -97,6 +97,145 @@ void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
}
}
+void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_uint8) {
+ v->type_uint8(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT8_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint8_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_uint16) {
+ v->type_uint16(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT16_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint16_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_uint32) {
+ v->type_uint32(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT32_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint32_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_uint64) {
+ v->type_uint64(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_int8) {
+ v->type_int8(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT8_MIN || value > INT8_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int8_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_int16) {
+ v->type_int16(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT16_MIN || value > INT16_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int16_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
+{
+ int64_t value;
+ if (!error_is_set(errp)) {
+ if (v->type_int32) {
+ v->type_int32(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT32_MIN || value > INT32_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int32_t");
+ return;
+ }
+ *obj = value;
+ }
+ }
+}
+
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ if (v->type_int64) {
+ v->type_int64(v, obj, name, errp);
+ } else {
+ v->type_int(v, obj, name, errp);
+ }
+ }
+}
+
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
{
if (!error_is_set(errp)) {
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index e850746..a19d70c 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -52,6 +52,14 @@ struct Visitor
void (*start_handle)(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp);
void (*end_handle)(Visitor *v, Error **errp);
+ void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
+ void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
+ void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
+ void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+ void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
+ void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
+ void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
+ void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
};
void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
void visit_type_enum(Visitor *v, int *obj, const char *strings[],
const char *kind, const char *name, Error **errp);
void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
+void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/8] qapi: Unit tests for visitor-based serialization
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 3/8] qapi: String visitor, use %f representation for floats Andreas Färber
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Anthony Liguori, Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Currently we test our visitors individually, and seperately for input
vs. output. This is useful for validating internal representations
against the native C types and vice-versa, and other visitor-specific
testing, but it doesn't cover the potential use-case of using visitor
pairs for serialization/deserialization very well, and makes it
hard to easily extend the coverage for different C types / boundary
conditions.
To cover that we add a set of unit tests that takes a number of native C
values, passes them into an output visitor, extracts the values with an
input visitor, then compares the result to the original.
Plugging in new visitors to the test harness only requires a user to
implement the SerializeOps interface and add it to a list.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
tests/Makefile | 4 +-
tests/test-visitor-serialization.c | 744 ++++++++++++++++++++++++++++++++++++
2 files changed, 747 insertions(+), 1 deletions(-)
create mode 100644 tests/test-visitor-serialization.c
diff --git a/tests/Makefile b/tests/Makefile
index 2e754c3..d66ab19 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -13,6 +13,7 @@ check-unit-y += tests/test-qmp-commands$(EXESUF)
check-unit-y += tests/test-string-input-visitor$(EXESUF)
check-unit-y += tests/test-string-output-visitor$(EXESUF)
check-unit-y += tests/test-coroutine$(EXESUF)
+check-unit-y += tests/test-visitor-serialization$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -31,7 +32,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
tests/test-coroutine.o tests/test-string-output-visitor.o \
tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
- tests/test-qmp-commands.o
+ tests/test-qmp-commands.o tests/test-visitor-serialization.o
test-qapi-obj-y = $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y)
test-qapi-obj-y += tests/test-qapi-visit.o tests/test-qapi-types.o
@@ -64,6 +65,7 @@ tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-q
tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y)
tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y)
tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
+tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
new file mode 100644
index 0000000..6ef57d0
--- /dev/null
+++ b/tests/test-visitor-serialization.c
@@ -0,0 +1,744 @@
+/*
+ * Unit-tests for visitor-based serialization
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <float.h>
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+
+typedef struct PrimitiveType {
+ union {
+ const char *string;
+ bool boolean;
+ double number;
+ int64_t integer;
+ uint8_t u8;
+ uint16_t u16;
+ uint32_t u32;
+ uint64_t u64;
+ int8_t s8;
+ int16_t s16;
+ int32_t s32;
+ int64_t s64;
+ intmax_t max;
+ } value;
+ enum {
+ PTYPE_STRING = 0,
+ PTYPE_BOOLEAN,
+ PTYPE_NUMBER,
+ PTYPE_INTEGER,
+ PTYPE_U8,
+ PTYPE_U16,
+ PTYPE_U32,
+ PTYPE_U64,
+ PTYPE_S8,
+ PTYPE_S16,
+ PTYPE_S32,
+ PTYPE_S64,
+ PTYPE_EOL,
+ } type;
+ const char *description;
+} PrimitiveType;
+
+/* test helpers */
+
+static void visit_primitive_type(Visitor *v, void **native, Error **errp)
+{
+ PrimitiveType *pt = *native;
+ switch(pt->type) {
+ case PTYPE_STRING:
+ visit_type_str(v, (char **)&pt->value.string, NULL, errp);
+ break;
+ case PTYPE_BOOLEAN:
+ visit_type_bool(v, &pt->value.boolean, NULL, errp);
+ break;
+ case PTYPE_NUMBER:
+ visit_type_number(v, &pt->value.number, NULL, errp);
+ break;
+ case PTYPE_INTEGER:
+ visit_type_int(v, &pt->value.integer, NULL, errp);
+ break;
+ case PTYPE_U8:
+ visit_type_uint8(v, &pt->value.u8, NULL, errp);
+ break;
+ case PTYPE_U16:
+ visit_type_uint16(v, &pt->value.u16, NULL, errp);
+ break;
+ case PTYPE_U32:
+ visit_type_uint32(v, &pt->value.u32, NULL, errp);
+ break;
+ case PTYPE_U64:
+ visit_type_uint64(v, &pt->value.u64, NULL, errp);
+ break;
+ case PTYPE_S8:
+ visit_type_int8(v, &pt->value.s8, NULL, errp);
+ break;
+ case PTYPE_S16:
+ visit_type_int16(v, &pt->value.s16, NULL, errp);
+ break;
+ case PTYPE_S32:
+ visit_type_int32(v, &pt->value.s32, NULL, errp);
+ break;
+ case PTYPE_S64:
+ visit_type_int64(v, &pt->value.s64, NULL, errp);
+ break;
+ case PTYPE_EOL:
+ g_assert(false);
+ }
+}
+
+typedef struct TestStruct
+{
+ int64_t integer;
+ bool boolean;
+ char *string;
+} TestStruct;
+
+static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
+ const char *name, Error **errp)
+{
+ visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp);
+
+ visit_type_int(v, &(*obj)->integer, "integer", errp);
+ visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
+ visit_type_str(v, &(*obj)->string, "string", errp);
+
+ visit_end_struct(v, errp);
+}
+
+static TestStruct *struct_create(void)
+{
+ TestStruct *ts = g_malloc0(sizeof(*ts));
+ ts->integer = -42;
+ ts->boolean = true;
+ ts->string = strdup("test string");
+ return ts;
+}
+
+static void struct_compare(TestStruct *ts1, TestStruct *ts2)
+{
+ g_assert(ts1);
+ g_assert(ts2);
+ g_assert_cmpint(ts1->integer, ==, ts2->integer);
+ g_assert(ts1->boolean == ts2->boolean);
+ g_assert_cmpstr(ts1->string, ==, ts2->string);
+}
+
+static void struct_cleanup(TestStruct *ts)
+{
+ g_free(ts->string);
+ g_free(ts);
+}
+
+static void visit_struct(Visitor *v, void **native, Error **errp)
+{
+ visit_type_TestStruct(v, (TestStruct **)native, NULL, errp);
+}
+
+static UserDefNested *nested_struct_create(void)
+{
+ UserDefNested *udnp = g_malloc0(sizeof(*udnp));
+ udnp->string0 = strdup("test_string0");
+ udnp->dict1.string1 = strdup("test_string1");
+ udnp->dict1.dict2.userdef1 = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1.dict2.userdef1->integer = 42;
+ udnp->dict1.dict2.userdef1->string = strdup("test_string");
+ udnp->dict1.dict2.string2 = strdup("test_string2");
+ udnp->dict1.has_dict3 = true;
+ udnp->dict1.dict3.userdef2 = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1.dict3.userdef2->integer = 43;
+ udnp->dict1.dict3.userdef2->string = strdup("test_string");
+ udnp->dict1.dict3.string3 = strdup("test_string3");
+ return udnp;
+}
+
+static void nested_struct_compare(UserDefNested *udnp1, UserDefNested *udnp2)
+{
+ g_assert(udnp1);
+ g_assert(udnp2);
+ g_assert_cmpstr(udnp1->string0, ==, udnp2->string0);
+ g_assert_cmpstr(udnp1->dict1.string1, ==, udnp2->dict1.string1);
+ g_assert_cmpint(udnp1->dict1.dict2.userdef1->integer, ==,
+ udnp2->dict1.dict2.userdef1->integer);
+ g_assert_cmpstr(udnp1->dict1.dict2.userdef1->string, ==,
+ udnp2->dict1.dict2.userdef1->string);
+ g_assert_cmpstr(udnp1->dict1.dict2.string2, ==, udnp2->dict1.dict2.string2);
+ g_assert(udnp1->dict1.has_dict3 == udnp2->dict1.has_dict3);
+ g_assert_cmpint(udnp1->dict1.dict3.userdef2->integer, ==,
+ udnp2->dict1.dict3.userdef2->integer);
+ g_assert_cmpstr(udnp1->dict1.dict3.userdef2->string, ==,
+ udnp2->dict1.dict3.userdef2->string);
+ g_assert_cmpstr(udnp1->dict1.dict3.string3, ==, udnp2->dict1.dict3.string3);
+}
+
+static void nested_struct_cleanup(UserDefNested *udnp)
+{
+ qapi_free_UserDefNested(udnp);
+}
+
+static void visit_nested_struct(Visitor *v, void **native, Error **errp)
+{
+ visit_type_UserDefNested(v, (UserDefNested **)native, NULL, errp);
+}
+
+static void visit_nested_struct_list(Visitor *v, void **native, Error **errp)
+{
+ visit_type_UserDefNestedList(v, (UserDefNestedList **)native, NULL, errp);
+}
+
+/* test cases */
+
+typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp);
+
+typedef enum VisitorCapabilities {
+ VCAP_PRIMITIVES = 1,
+ VCAP_STRUCTURES = 2,
+ VCAP_LISTS = 4,
+} VisitorCapabilities;
+
+typedef struct SerializeOps {
+ void (*serialize)(void *native_in, void **datap,
+ VisitorFunc visit, Error **errp);
+ void (*deserialize)(void **native_out, void *datap,
+ VisitorFunc visit, Error **errp);
+ void (*cleanup)(void *datap);
+ const char *type;
+ VisitorCapabilities caps;
+} SerializeOps;
+
+typedef struct TestArgs {
+ const SerializeOps *ops;
+ void *test_data;
+} TestArgs;
+
+#define FLOAT_STRING_PRECISION 6 /* corresponding to n in %.nf formatting */
+static gsize calc_float_string_storage(double value)
+{
+ int whole_value = value;
+ gsize i = 0;
+ do {
+ i++;
+ } while (whole_value /= 10);
+ return i + 2 + FLOAT_STRING_PRECISION;
+}
+
+static void test_primitives(gconstpointer opaque)
+{
+ TestArgs *args = (TestArgs *) opaque;
+ const SerializeOps *ops = args->ops;
+ PrimitiveType *pt = args->test_data;
+ PrimitiveType *pt_copy = g_malloc0(sizeof(*pt_copy));
+ Error *err = NULL;
+ void *serialize_data;
+ char *double1, *double2;
+
+ pt_copy->type = pt->type;
+ ops->serialize(pt, &serialize_data, visit_primitive_type, &err);
+ ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type, &err);
+
+ g_assert(err == NULL);
+ g_assert(pt_copy != NULL);
+ if (pt->type == PTYPE_STRING) {
+ g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
+ } else if (pt->type == PTYPE_NUMBER) {
+ /* we serialize with %f for our reference visitors, so rather than fuzzy
+ * floating math to test "equality", just compare the formatted values
+ */
+ double1 = g_malloc0(calc_float_string_storage(pt->value.number));
+ double2 = g_malloc0(calc_float_string_storage(pt_copy->value.number));
+ g_assert_cmpstr(double1, ==, double2);
+ g_free(double1);
+ g_free(double2);
+ } else if (pt->type == PTYPE_BOOLEAN) {
+ g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max);
+ } else {
+ g_assert_cmpint(pt->value.max, ==, pt_copy->value.max);
+ }
+
+ ops->cleanup(serialize_data);
+ g_free(args);
+}
+
+static void test_struct(gconstpointer opaque)
+{
+ TestArgs *args = (TestArgs *) opaque;
+ const SerializeOps *ops = args->ops;
+ TestStruct *ts = struct_create();
+ TestStruct *ts_copy = NULL;
+ Error *err = NULL;
+ void *serialize_data;
+
+ ops->serialize(ts, &serialize_data, visit_struct, &err);
+ ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err);
+
+ g_assert(err == NULL);
+ struct_compare(ts, ts_copy);
+
+ struct_cleanup(ts);
+ struct_cleanup(ts_copy);
+
+ ops->cleanup(serialize_data);
+ g_free(args);
+}
+
+static void test_nested_struct(gconstpointer opaque)
+{
+ TestArgs *args = (TestArgs *) opaque;
+ const SerializeOps *ops = args->ops;
+ UserDefNested *udnp = nested_struct_create();
+ UserDefNested *udnp_copy = NULL;
+ Error *err = NULL;
+ void *serialize_data;
+
+ ops->serialize(udnp, &serialize_data, visit_nested_struct, &err);
+ ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct, &err);
+
+ g_assert(err == NULL);
+ nested_struct_compare(udnp, udnp_copy);
+
+ nested_struct_cleanup(udnp);
+ nested_struct_cleanup(udnp_copy);
+
+ ops->cleanup(serialize_data);
+ g_free(args);
+}
+
+static void test_nested_struct_list(gconstpointer opaque)
+{
+ TestArgs *args = (TestArgs *) opaque;
+ const SerializeOps *ops = args->ops;
+ UserDefNestedList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
+ Error *err = NULL;
+ void *serialize_data;
+ int i = 0;
+
+ for (i = 0; i < 8; i++) {
+ tmp = g_malloc0(sizeof(UserDefNestedList));
+ tmp->value = nested_struct_create();
+ tmp->next = listp;
+ listp = tmp;
+ }
+
+ ops->serialize(listp, &serialize_data, visit_nested_struct_list, &err);
+ ops->deserialize((void **)&listp_copy, serialize_data,
+ visit_nested_struct_list, &err);
+
+ g_assert(err == NULL);
+
+ tmp = listp;
+ tmp_copy = listp_copy;
+ while (listp_copy) {
+ g_assert(listp);
+ nested_struct_compare(listp->value, listp_copy->value);
+ listp = listp->next;
+ listp_copy = listp_copy->next;
+ }
+
+ qapi_free_UserDefNestedList(tmp);
+ qapi_free_UserDefNestedList(tmp_copy);
+
+ ops->cleanup(serialize_data);
+ g_free(args);
+}
+
+PrimitiveType pt_values[] = {
+ /* string tests */
+ {
+ .description = "string_empty",
+ .type = PTYPE_STRING,
+ .value.string = "",
+ },
+ {
+ .description = "string_whitespace",
+ .type = PTYPE_STRING,
+ .value.string = "a b c\td",
+ },
+ {
+ .description = "string_newlines",
+ .type = PTYPE_STRING,
+ .value.string = "a\nb\n",
+ },
+ {
+ .description = "string_commas",
+ .type = PTYPE_STRING,
+ .value.string = "a,b, c,d",
+ },
+ {
+ .description = "string_single_quoted",
+ .type = PTYPE_STRING,
+ .value.string = "'a b',cd",
+ },
+ {
+ .description = "string_double_quoted",
+ .type = PTYPE_STRING,
+ .value.string = "\"a b\",cd",
+ },
+ /* boolean tests */
+ {
+ .description = "boolean_true1",
+ .type = PTYPE_BOOLEAN,
+ .value.boolean = true,
+ },
+ {
+ .description = "boolean_true2",
+ .type = PTYPE_BOOLEAN,
+ .value.boolean = 8,
+ },
+ {
+ .description = "boolean_true3",
+ .type = PTYPE_BOOLEAN,
+ .value.boolean = -1,
+ },
+ {
+ .description = "boolean_false1",
+ .type = PTYPE_BOOLEAN,
+ .value.boolean = false,
+ },
+ {
+ .description = "boolean_false2",
+ .type = PTYPE_BOOLEAN,
+ .value.boolean = 0,
+ },
+ /* number tests (double) */
+ /* note: we format these to %.6f before comparing, since that's how
+ * we serialize them and it doesn't make sense to check precision
+ * beyond that.
+ */
+ {
+ .description = "number_sanity1",
+ .type = PTYPE_NUMBER,
+ .value.number = -1,
+ },
+ {
+ .description = "number_sanity2",
+ .type = PTYPE_NUMBER,
+ .value.number = 3.14159265,
+ },
+ {
+ .description = "number_min",
+ .type = PTYPE_NUMBER,
+ .value.number = DBL_MIN,
+ },
+ {
+ .description = "number_max",
+ .type = PTYPE_NUMBER,
+ .value.number = DBL_MAX,
+ },
+ /* integer tests (int64) */
+ {
+ .description = "integer_sanity1",
+ .type = PTYPE_INTEGER,
+ .value.integer = -1,
+ },
+ {
+ .description = "integer_sanity2",
+ .type = PTYPE_INTEGER,
+ .value.integer = INT64_MAX / 2 + 1,
+ },
+ {
+ .description = "integer_min",
+ .type = PTYPE_INTEGER,
+ .value.integer = INT64_MIN,
+ },
+ {
+ .description = "integer_max",
+ .type = PTYPE_INTEGER,
+ .value.integer = INT64_MAX,
+ },
+ /* uint8 tests */
+ {
+ .description = "uint8_sanity1",
+ .type = PTYPE_U8,
+ .value.u8 = 1,
+ },
+ {
+ .description = "uint8_sanity2",
+ .type = PTYPE_U8,
+ .value.u8 = UINT8_MAX / 2 + 1,
+ },
+ {
+ .description = "uint8_min",
+ .type = PTYPE_U8,
+ .value.u8 = 0,
+ },
+ {
+ .description = "uint8_max",
+ .type = PTYPE_U8,
+ .value.u8 = UINT8_MAX,
+ },
+ /* uint16 tests */
+ {
+ .description = "uint16_sanity1",
+ .type = PTYPE_U16,
+ .value.u16 = 1,
+ },
+ {
+ .description = "uint16_sanity2",
+ .type = PTYPE_U16,
+ .value.u16 = UINT16_MAX / 2 + 1,
+ },
+ {
+ .description = "uint16_min",
+ .type = PTYPE_U16,
+ .value.u16 = 0,
+ },
+ {
+ .description = "uint16_max",
+ .type = PTYPE_U16,
+ .value.u16 = UINT16_MAX,
+ },
+ /* uint32 tests */
+ {
+ .description = "uint32_sanity1",
+ .type = PTYPE_U32,
+ .value.u32 = 1,
+ },
+ {
+ .description = "uint32_sanity2",
+ .type = PTYPE_U32,
+ .value.u32 = UINT32_MAX / 2 + 1,
+ },
+ {
+ .description = "uint32_min",
+ .type = PTYPE_U32,
+ .value.u32 = 0,
+ },
+ {
+ .description = "uint32_max",
+ .type = PTYPE_U32,
+ .value.u32 = UINT32_MAX,
+ },
+ /* uint64 tests */
+ {
+ .description = "uint64_sanity1",
+ .type = PTYPE_U64,
+ .value.u64 = 1,
+ },
+ {
+ .description = "uint64_sanity2",
+ .type = PTYPE_U64,
+ .value.u64 = UINT64_MAX / 2 + 1,
+ },
+ {
+ .description = "uint64_min",
+ .type = PTYPE_U64,
+ .value.u64 = 0,
+ },
+ {
+ .description = "uint64_max",
+ .type = PTYPE_U64,
+ .value.u64 = UINT64_MAX,
+ },
+ /* int8 tests */
+ {
+ .description = "int8_sanity1",
+ .type = PTYPE_S8,
+ .value.s8 = -1,
+ },
+ {
+ .description = "int8_sanity2",
+ .type = PTYPE_S8,
+ .value.s8 = INT8_MAX / 2 + 1,
+ },
+ {
+ .description = "int8_min",
+ .type = PTYPE_S8,
+ .value.s8 = INT8_MIN,
+ },
+ {
+ .description = "int8_max",
+ .type = PTYPE_S8,
+ .value.s8 = INT8_MAX,
+ },
+ /* int16 tests */
+ {
+ .description = "int16_sanity1",
+ .type = PTYPE_S16,
+ .value.s16 = -1,
+ },
+ {
+ .description = "int16_sanity2",
+ .type = PTYPE_S16,
+ .value.s16 = INT16_MAX / 2 + 1,
+ },
+ {
+ .description = "int16_min",
+ .type = PTYPE_S16,
+ .value.s16 = INT16_MIN,
+ },
+ {
+ .description = "int16_max",
+ .type = PTYPE_S16,
+ .value.s16 = INT16_MAX,
+ },
+ /* int32 tests */
+ {
+ .description = "int32_sanity1",
+ .type = PTYPE_S32,
+ .value.s32 = -1,
+ },
+ {
+ .description = "int32_sanity2",
+ .type = PTYPE_S32,
+ .value.s32 = INT32_MAX / 2 + 1,
+ },
+ {
+ .description = "int32_min",
+ .type = PTYPE_S32,
+ .value.s32 = INT32_MIN,
+ },
+ {
+ .description = "int32_max",
+ .type = PTYPE_S32,
+ .value.s32 = INT32_MAX,
+ },
+ /* int64 tests */
+ {
+ .description = "int64_sanity1",
+ .type = PTYPE_S64,
+ .value.s64 = -1,
+ },
+ {
+ .description = "int64_sanity2",
+ .type = PTYPE_S64,
+ .value.s64 = INT64_MAX / 2 + 1,
+ },
+ {
+ .description = "int64_min",
+ .type = PTYPE_S64,
+ .value.s64 = INT64_MIN,
+ },
+ {
+ .description = "int64_max",
+ .type = PTYPE_S64,
+ .value.s64 = INT64_MAX,
+ },
+ { .type = PTYPE_EOL }
+};
+
+/* visitor-specific op implementations */
+
+typedef struct QmpSerializeData {
+ QmpOutputVisitor *qov;
+ QmpInputVisitor *qiv;
+} QmpSerializeData;
+
+static void qmp_serialize(void *native_in, void **datap,
+ VisitorFunc visit, Error **errp)
+{
+ QmpSerializeData *d = g_malloc0(sizeof(*d));
+
+ d->qov = qmp_output_visitor_new();
+ visit(qmp_output_get_visitor(d->qov), &native_in, errp);
+ *datap = d;
+}
+
+static void qmp_deserialize(void **native_out, void *datap,
+ VisitorFunc visit, Error **errp)
+{
+ QmpSerializeData *d = datap;
+ QString *output_json = qobject_to_json(qmp_output_get_qobject(d->qov));
+ QObject *obj = qobject_from_json(qstring_get_str(output_json));
+
+ QDECREF(output_json);
+ d->qiv = qmp_input_visitor_new(obj);
+ visit(qmp_input_get_visitor(d->qiv), native_out, errp);
+}
+
+static void qmp_cleanup(void *datap)
+{
+ QmpSerializeData *d = datap;
+ qmp_output_visitor_cleanup(d->qov);
+ qmp_input_visitor_cleanup(d->qiv);
+}
+
+/* visitor registration, test harness */
+
+/* note: to function interchangeably as a serialization mechanism your
+ * visitor test implementation should pass the test cases for all visitor
+ * capabilities: primitives, structures, and lists
+ */
+static const SerializeOps visitors[] = {
+ {
+ .type = "QMP",
+ .serialize = qmp_serialize,
+ .deserialize = qmp_deserialize,
+ .cleanup = qmp_cleanup,
+ .caps = VCAP_PRIMITIVES | VCAP_STRUCTURES | VCAP_LISTS
+ },
+ { NULL }
+};
+
+static void add_visitor_type(const SerializeOps *ops)
+{
+ char testname_prefix[128];
+ char testname[128];
+ TestArgs *args;
+ int i = 0;
+
+ sprintf(testname_prefix, "/visitor/serialization/%s", ops->type);
+
+ if (ops->caps & VCAP_PRIMITIVES) {
+ while (pt_values[i].type != PTYPE_EOL) {
+ sprintf(testname, "%s/primitives/%s", testname_prefix,
+ pt_values[i].description);
+ args = g_malloc0(sizeof(*args));
+ args->ops = ops;
+ args->test_data = &pt_values[i];
+ g_test_add_data_func(testname, args, test_primitives);
+ i++;
+ }
+ }
+
+ if (ops->caps & VCAP_STRUCTURES) {
+ sprintf(testname, "%s/struct", testname_prefix);
+ args = g_malloc0(sizeof(*args));
+ args->ops = ops;
+ args->test_data = NULL;
+ g_test_add_data_func(testname, args, test_struct);
+
+ sprintf(testname, "%s/nested_struct", testname_prefix);
+ args = g_malloc0(sizeof(*args));
+ args->ops = ops;
+ args->test_data = NULL;
+ g_test_add_data_func(testname, args, test_nested_struct);
+ }
+
+ if (ops->caps & VCAP_LISTS) {
+ sprintf(testname, "%s/nested_struct_list", testname_prefix);
+ args = g_malloc0(sizeof(*args));
+ args->ops = ops;
+ args->test_data = NULL;
+ g_test_add_data_func(testname, args, test_nested_struct_list);
+ }
+}
+
+int main(int argc, char **argv)
+{
+ int i = 0;
+
+ g_test_init(&argc, &argv, NULL);
+
+ while (visitors[i].type != NULL) {
+ add_visitor_type(&visitors[i]);
+ i++;
+ }
+
+ g_test_run();
+
+ return 0;
+}
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/8] qapi: String visitor, use %f representation for floats
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 2/8] qapi: Unit tests for visitor-based serialization Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 4/8] qapi: Add String visitor coverage to serialization unit tests Andreas Färber
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Anthony Liguori, Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Currently string-output-visitor formats floats as %g, which is nice in
that trailing 0's are automatically truncated, but otherwise this causes
some issues:
- it uses 6 significant figures instead of 6 decimal places, which
means something like 155777.5 (which even has an exact floating point
representation) will be rounded to 155778 when converted to a string.
- output will be presented in scientific notation when the normalized
form requires a 10^x multiplier. Not a huge deal, but arguably less
readable for command-line arguments.
- due to using scientific notation for numbers requiring more than 6
significant figures, instead of hard-defined decimal places, it
fails a lot of the test-visitor-serialization unit tests for floats.
Instead, let's just use %f, which is what the QJSON and the QMP visitors
use.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
qapi/string-output-visitor.c | 2 +-
tests/test-string-output-visitor.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 92b0305..34e525e 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, const char *name,
Error **errp)
{
StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
- string_output_set(sov, g_strdup_printf("%g", *obj));
+ string_output_set(sov, g_strdup_printf("%f", *obj));
}
char *string_output_get_string(StringOutputVisitor *sov)
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 22909b8..608f14a 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
str = string_output_get_string(data->sov);
g_assert(str != NULL);
- g_assert_cmpstr(str, ==, "3.14");
+ g_assert_cmpstr(str, ==, "3.140000");
g_free(str);
}
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/8] qapi: Add String visitor coverage to serialization unit tests
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (2 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 3/8] qapi: String visitor, use %f representation for floats Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 5/8] qdev: Use int32_t container for devfn property Andreas Färber
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Anthony Liguori, Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
tests/test-visitor-serialization.c | 40 ++++++++++++++++++++++++++++++++++++
1 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 6ef57d0..b8ad16f 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -19,6 +19,8 @@
#include "qemu-objects.h"
#include "qapi/qmp-input-visitor.h"
#include "qapi/qmp-output-visitor.h"
+#include "qapi/string-input-visitor.h"
+#include "qapi/string-output-visitor.h"
typedef struct PrimitiveType {
union {
@@ -666,6 +668,37 @@ static void qmp_cleanup(void *datap)
qmp_input_visitor_cleanup(d->qiv);
}
+typedef struct StringSerializeData {
+ StringOutputVisitor *sov;
+ StringInputVisitor *siv;
+} StringSerializeData;
+
+static void string_serialize(void *native_in, void **datap,
+ VisitorFunc visit, Error **errp)
+{
+ StringSerializeData *d = g_malloc0(sizeof(*d));
+
+ d->sov = string_output_visitor_new();
+ visit(string_output_get_visitor(d->sov), &native_in, errp);
+ *datap = d;
+}
+
+static void string_deserialize(void **native_out, void *datap,
+ VisitorFunc visit, Error **errp)
+{
+ StringSerializeData *d = datap;
+
+ d->siv = string_input_visitor_new(string_output_get_string(d->sov));
+ visit(string_input_get_visitor(d->siv), native_out, errp);
+}
+
+static void string_cleanup(void *datap)
+{
+ StringSerializeData *d = datap;
+ string_output_visitor_cleanup(d->sov);
+ string_input_visitor_cleanup(d->siv);
+}
+
/* visitor registration, test harness */
/* note: to function interchangeably as a serialization mechanism your
@@ -680,6 +713,13 @@ static const SerializeOps visitors[] = {
.cleanup = qmp_cleanup,
.caps = VCAP_PRIMITIVES | VCAP_STRUCTURES | VCAP_LISTS
},
+ {
+ .type = "String",
+ .serialize = string_serialize,
+ .deserialize = string_deserialize,
+ .cleanup = string_cleanup,
+ .caps = VCAP_PRIMITIVES
+ },
{ NULL }
};
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/8] qdev: Use int32_t container for devfn property
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (3 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 4/8] qapi: Add String visitor coverage to serialization unit tests Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 6/8] qdev: Switch property accessors to fixed-width visitor interfaces Andreas Färber
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Michael S. Tsirkin, Michael Roth, Anthony Liguori,
Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Valid range for devfn is -1 to 255 (-1 for automatic assignment). We do
not currently validate this due to devfn being stored as a uint32_t.
This can lead to segfaults and other strange behavior.
We could technically just cast it to int32_t to implement the checking,
but this will not work for visitor-based setting where we may do additional
bounds-checking based on target container type, which is int32_t for this
case.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
hw/pci.c | 2 +-
hw/pci.h | 2 +-
hw/qdev-properties.c | 11 ++++-------
hw/qdev.h | 2 +-
4 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index c1ebdde..127b7ac 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1537,7 +1537,7 @@ PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
DeviceState *dev;
dev = qdev_create(&bus->qbus, name);
- qdev_prop_set_uint32(dev, "addr", devfn);
+ qdev_prop_set_int32(dev, "addr", devfn);
qdev_prop_set_bit(dev, "multifunction", multifunction);
return PCI_DEVICE(dev);
}
diff --git a/hw/pci.h b/hw/pci.h
index c3cacce..7f223c0 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -197,7 +197,7 @@ struct PCIDevice {
/* the following fields are read only */
PCIBus *bus;
- uint32_t devfn;
+ int32_t devfn;
char name[64];
PCIIORegion io_regions[PCI_NUM_REGIONS];
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index b7b5597..d109f89 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -824,7 +824,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int32_t *ptr = qdev_get_prop_ptr(dev, prop);
unsigned int slot, fn, n;
Error *local_err = NULL;
char *str;
@@ -860,7 +860,7 @@ invalid:
static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len)
{
- uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int32_t *ptr = qdev_get_prop_ptr(dev, prop);
if (*ptr == -1) {
return snprintf(dest, len, "<unset>");
@@ -875,11 +875,8 @@ PropertyInfo qdev_prop_pci_devfn = {
.print = print_pci_devfn,
.get = get_int32,
.set = set_pci_devfn,
- /* FIXME: this should be -1...255, but the address is stored
- * into an uint32_t rather than int32_t.
- */
- .min = 0,
- .max = 0xFFFFFFFFULL,
+ .min = -1,
+ .max = 255,
};
/* --- blocksize --- */
diff --git a/hw/qdev.h b/hw/qdev.h
index 4e90119..d07da45 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -267,7 +267,7 @@ extern PropertyInfo qdev_prop_blocksize;
#define DEFINE_PROP_HEX64(_n, _s, _f, _d) \
DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, uint32_t)
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
#define DEFINE_PROP_PTR(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 6/8] qdev: Switch property accessors to fixed-width visitor interfaces
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (4 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 5/8] qdev: Use int32_t container for devfn property Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 7/8] qdev: Remove PropertyInfo range checking Andreas Färber
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Michael Roth, Anthony Liguori, Andreas Färber
From: Michael Roth <mdroth@linux.vnet.ibm.com>
This introduces {get,set}_uint{8,16,32,64}() functions for the
respective qdev types.
TADDR and VLAN are switched to explicit int64, BLOCKSIZE to uint16.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
hw/qdev-addr.c | 4 +-
hw/qdev-properties.c | 150 +++++++++++++++++++++++++++++--------------------
2 files changed, 91 insertions(+), 63 deletions(-)
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index 0bb16c7..b711b6b 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -27,7 +27,7 @@ static void get_taddr(Object *obj, Visitor *v, void *opaque,
int64_t value;
value = *ptr;
- visit_type_int(v, &value, name, errp);
+ visit_type_int64(v, &value, name, errp);
}
static void set_taddr(Object *obj, Visitor *v, void *opaque,
@@ -44,7 +44,7 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque,
return;
}
- visit_type_int(v, &value, name, &local_err);
+ visit_type_int64(v, &value, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index d109f89..553b0d1 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -76,33 +76,30 @@ PropertyInfo qdev_prop_bit = {
/* --- 8bit integer --- */
-static void get_int8(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void get_uint8(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int8_t *ptr = qdev_get_prop_ptr(dev, prop);
- int64_t value;
+ uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
- value = *ptr;
- visit_type_int(v, &value, name, errp);
+ visit_type_uint8(v, ptr, name, errp);
}
-static void set_int8(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void set_uint8(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int8_t *ptr = qdev_get_prop_ptr(dev, prop);
+ uint8_t value, *ptr = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
- int64_t value;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int(v, &value, name, &local_err);
+ visit_type_uint8(v, &value, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -111,15 +108,15 @@ static void set_int8(Object *obj, Visitor *v, void *opaque,
*ptr = value;
} else {
error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, value, prop->info->min,
+ dev->id?:"", name, (int64_t)value, prop->info->min,
prop->info->max);
}
}
PropertyInfo qdev_prop_uint8 = {
.name = "uint8",
- .get = get_int8,
- .set = set_int8,
+ .get = get_uint8,
+ .set = set_uint8,
.min = 0,
.max = 255,
};
@@ -154,41 +151,38 @@ PropertyInfo qdev_prop_hex8 = {
.legacy_name = "hex8",
.parse = parse_hex8,
.print = print_hex8,
- .get = get_int8,
- .set = set_int8,
+ .get = get_uint8,
+ .set = set_uint8,
.min = 0,
.max = 255,
};
/* --- 16bit integer --- */
-static void get_int16(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void get_uint16(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int16_t *ptr = qdev_get_prop_ptr(dev, prop);
- int64_t value;
+ uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
- value = *ptr;
- visit_type_int(v, &value, name, errp);
+ visit_type_uint16(v, ptr, name, errp);
}
-static void set_int16(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void set_uint16(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int16_t *ptr = qdev_get_prop_ptr(dev, prop);
+ uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
- int64_t value;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int(v, &value, name, &local_err);
+ visit_type_uint16(v, &value, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -197,31 +191,67 @@ static void set_int16(Object *obj, Visitor *v, void *opaque,
*ptr = value;
} else {
error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, value, prop->info->min,
+ dev->id?:"", name, (int64_t)value, prop->info->min,
prop->info->max);
}
}
PropertyInfo qdev_prop_uint16 = {
.name = "uint16",
- .get = get_int16,
- .set = set_int16,
+ .get = get_uint16,
+ .set = set_uint16,
.min = 0,
.max = 65535,
};
/* --- 32bit integer --- */
+static void get_uint32(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
+
+ value = *ptr;
+ visit_type_uint32(v, &value, name, errp);
+}
+
+static void set_uint32(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_uint32(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if (value >= prop->info->min && value <= prop->info->max) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, (int64_t)value, prop->info->min,
+ prop->info->max);
+ }
+}
+
static void get_int32(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
int32_t *ptr = qdev_get_prop_ptr(dev, prop);
- int64_t value;
- value = *ptr;
- visit_type_int(v, &value, name, errp);
+ visit_type_int32(v, ptr, name, errp);
}
static void set_int32(Object *obj, Visitor *v, void *opaque,
@@ -229,16 +259,15 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
- int64_t value;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int(v, &value, name, &local_err);
+ visit_type_int32(v, &value, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -247,15 +276,15 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
*ptr = value;
} else {
error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, value, prop->info->min,
+ dev->id?:"", name, (int64_t)value, prop->info->min,
prop->info->max);
}
}
PropertyInfo qdev_prop_uint32 = {
.name = "uint32",
- .get = get_int32,
- .set = set_int32,
+ .get = get_uint32,
+ .set = set_uint32,
.min = 0,
.max = 0xFFFFFFFFULL,
};
@@ -298,43 +327,43 @@ PropertyInfo qdev_prop_hex32 = {
.legacy_name = "hex32",
.parse = parse_hex32,
.print = print_hex32,
- .get = get_int32,
- .set = set_int32,
+ .get = get_uint32,
+ .set = set_uint32,
.min = 0,
.max = 0xFFFFFFFFULL,
};
/* --- 64bit integer --- */
-static void get_int64(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void get_uint64(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+ uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
- visit_type_int(v, ptr, name, errp);
+ visit_type_uint64(v, ptr, name, errp);
}
-static void set_int64(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+static void set_uint64(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+ uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int(v, ptr, name, errp);
+ visit_type_uint64(v, ptr, name, errp);
}
PropertyInfo qdev_prop_uint64 = {
.name = "uint64",
- .get = get_int64,
- .set = set_int64,
+ .get = get_uint64,
+ .set = set_uint64,
};
/* --- 64bit hex value --- */
@@ -367,8 +396,8 @@ PropertyInfo qdev_prop_hex64 = {
.legacy_name = "hex64",
.parse = parse_hex64,
.print = print_hex64,
- .get = get_int64,
- .set = set_int64,
+ .get = get_uint64,
+ .set = set_uint64,
};
/* --- string --- */
@@ -645,7 +674,7 @@ static void get_vlan(Object *obj, Visitor *v, void *opaque,
int64_t id;
id = *ptr ? (*ptr)->id : -1;
- visit_type_int(v, &id, name, errp);
+ visit_type_int64(v, &id, name, errp);
}
static void set_vlan(Object *obj, Visitor *v, void *opaque,
@@ -663,7 +692,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
return;
}
- visit_type_int(v, &id, name, &local_err);
+ visit_type_int64(v, &id, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -886,23 +915,22 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int16_t *ptr = qdev_get_prop_ptr(dev, prop);
+ uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
- int64_t value;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int(v, &value, name, &local_err);
+ visit_type_uint16(v, &value, name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
if (value < prop->info->min || value > prop->info->max) {
error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, value, prop->info->min,
+ dev->id?:"", name, (int64_t)value, prop->info->min,
prop->info->max);
return;
}
@@ -910,7 +938,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
/* We rely on power-of-2 blocksizes for bitmasks */
if ((value & (value - 1)) != 0) {
error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
- dev->id?:"", name, value);
+ dev->id?:"", name, (int64_t)value);
return;
}
@@ -919,7 +947,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
PropertyInfo qdev_prop_blocksize = {
.name = "blocksize",
- .get = get_int16,
+ .get = get_uint16,
.set = set_blocksize,
.min = 512,
.max = 65024,
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 7/8] qdev: Remove PropertyInfo range checking
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (5 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 6/8] qdev: Switch property accessors to fixed-width visitor interfaces Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 8/8] target-i386: Use uint32 visitor for [x]level properties Andreas Färber
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Andreas Färber, Anthony Liguori
From: Paolo Bonzini <pbonzini@redhat.com>
Range checking in PropertyInfo is now used only for pci_devfn
properties and some error reporting. Remove all code that implements
it in the various property types, and the now unused fields.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[AF: Fix blocksize min/max for 32-bit hosts by using const int64_t.]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
hw/qdev-properties.c | 106 ++++++++++++-------------------------------------
hw/qdev.h | 2 -
2 files changed, 26 insertions(+), 82 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 553b0d1..9ae3187 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -91,34 +91,20 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- uint8_t value, *ptr = qdev_get_prop_ptr(dev, prop);
- Error *local_err = NULL;
+ uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_uint8(v, &value, name, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- if (value >= prop->info->min && value <= prop->info->max) {
- *ptr = value;
- } else {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, (int64_t)value, prop->info->min,
- prop->info->max);
- }
+ visit_type_uint8(v, ptr, name, errp);
}
PropertyInfo qdev_prop_uint8 = {
.name = "uint8",
.get = get_uint8,
.set = set_uint8,
- .min = 0,
- .max = 255,
};
/* --- 8bit hex value --- */
@@ -153,8 +139,6 @@ PropertyInfo qdev_prop_hex8 = {
.print = print_hex8,
.get = get_uint8,
.set = set_uint8,
- .min = 0,
- .max = 255,
};
/* --- 16bit integer --- */
@@ -174,34 +158,20 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
- Error *local_err = NULL;
+ uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_uint16(v, &value, name, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- if (value >= prop->info->min && value <= prop->info->max) {
- *ptr = value;
- } else {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, (int64_t)value, prop->info->min,
- prop->info->max);
- }
+ visit_type_uint16(v, ptr, name, errp);
}
PropertyInfo qdev_prop_uint16 = {
.name = "uint16",
.get = get_uint16,
.set = set_uint16,
- .min = 0,
- .max = 65535,
};
/* --- 32bit integer --- */
@@ -211,10 +181,9 @@ static void get_uint32(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
+ uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
- value = *ptr;
- visit_type_uint32(v, &value, name, errp);
+ visit_type_uint32(v, ptr, name, errp);
}
static void set_uint32(Object *obj, Visitor *v, void *opaque,
@@ -222,26 +191,14 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
- Error *local_err = NULL;
+ uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_uint32(v, &value, name, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- if (value >= prop->info->min && value <= prop->info->max) {
- *ptr = value;
- } else {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, (int64_t)value, prop->info->min,
- prop->info->max);
- }
+ visit_type_uint32(v, ptr, name, errp);
}
static void get_int32(Object *obj, Visitor *v, void *opaque,
@@ -259,42 +216,26 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
- Error *local_err = NULL;
+ int32_t *ptr = qdev_get_prop_ptr(dev, prop);
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
- visit_type_int32(v, &value, name, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- if (value >= prop->info->min && value <= prop->info->max) {
- *ptr = value;
- } else {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, (int64_t)value, prop->info->min,
- prop->info->max);
- }
+ visit_type_int32(v, ptr, name, errp);
}
PropertyInfo qdev_prop_uint32 = {
.name = "uint32",
.get = get_uint32,
.set = set_uint32,
- .min = 0,
- .max = 0xFFFFFFFFULL,
};
PropertyInfo qdev_prop_int32 = {
.name = "int32",
.get = get_int32,
.set = set_int32,
- .min = -0x80000000LL,
- .max = 0x7FFFFFFFLL,
};
/* --- 32bit hex value --- */
@@ -329,8 +270,6 @@ PropertyInfo qdev_prop_hex32 = {
.print = print_hex32,
.get = get_uint32,
.set = set_uint32,
- .min = 0,
- .max = 0xFFFFFFFFULL,
};
/* --- 64bit integer --- */
@@ -853,7 +792,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
- int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
unsigned int slot, fn, n;
Error *local_err = NULL;
char *str;
@@ -866,7 +805,17 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
visit_type_str(v, &str, name, &local_err);
if (local_err) {
error_free(local_err);
- return set_int32(obj, v, opaque, name, errp);
+ local_err = NULL;
+ visit_type_int32(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ } else if (value < -1 || value > 255) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "pci_devfn");
+ } else {
+ *ptr = value;
+ }
+ return;
}
if (sscanf(str, "%x.%x%n", &slot, &fn, &n) != 2) {
@@ -904,8 +853,6 @@ PropertyInfo qdev_prop_pci_devfn = {
.print = print_pci_devfn,
.get = get_int32,
.set = set_pci_devfn,
- .min = -1,
- .max = 255,
};
/* --- blocksize --- */
@@ -917,6 +864,8 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
Property *prop = opaque;
uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
+ const int64_t min = 512;
+ const int64_t max = 32768;
if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
@@ -928,10 +877,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
error_propagate(errp, local_err);
return;
}
- if (value < prop->info->min || value > prop->info->max) {
+ if (value < min || value > max) {
error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- dev->id?:"", name, (int64_t)value, prop->info->min,
- prop->info->max);
+ dev->id?:"", name, (int64_t)value, min, max);
return;
}
@@ -949,8 +897,6 @@ PropertyInfo qdev_prop_blocksize = {
.name = "blocksize",
.get = get_uint16,
.set = set_blocksize,
- .min = 512,
- .max = 65024,
};
/* --- public helpers --- */
diff --git a/hw/qdev.h b/hw/qdev.h
index d07da45..5386b16 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -122,8 +122,6 @@ struct PropertyInfo {
const char *name;
const char *legacy_name;
const char **enum_table;
- int64_t min;
- int64_t max;
int (*parse)(DeviceState *dev, Property *prop, const char *str);
int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
ObjectPropertyAccessor *get;
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 8/8] target-i386: Use uint32 visitor for [x]level properties
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (6 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 7/8] qdev: Remove PropertyInfo range checking Andreas Färber
@ 2012-06-08 15:35 ` Andreas Färber
2012-06-08 21:39 ` [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 15:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Andreas Färber, Anthony Liguori
This simplifies the code and resolves TODOs.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
target-i386/cpu.c | 42 ++++--------------------------------------
1 files changed, 4 insertions(+), 38 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 388bc5c..0b61162 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -723,66 +723,32 @@ static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
- int64_t value;
- value = cpu->env.cpuid_level;
- /* TODO Use visit_type_uint32() once available */
- visit_type_int(v, &value, name, errp);
+ visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
}
static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
- const int64_t min = 0;
- const int64_t max = UINT32_MAX;
- int64_t value;
-
- /* TODO Use visit_type_uint32() once available */
- visit_type_int(v, &value, name, errp);
- if (error_is_set(errp)) {
- return;
- }
- if (value < min || value > max) {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
- name ? name : "null", value, min, max);
- return;
- }
- cpu->env.cpuid_level = value;
+ visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
}
static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
- int64_t value;
- value = cpu->env.cpuid_xlevel;
- /* TODO Use visit_type_uint32() once available */
- visit_type_int(v, &value, name, errp);
+ visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
}
static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
- const int64_t min = 0;
- const int64_t max = UINT32_MAX;
- int64_t value;
-
- /* TODO Use visit_type_uint32() once available */
- visit_type_int(v, &value, name, errp);
- if (error_is_set(errp)) {
- return;
- }
- if (value < min || value > max) {
- error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
- name ? name : "null", value, min, max);
- return;
- }
- cpu->env.cpuid_xlevel = value;
+ visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
}
static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
--
1.7.7
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (7 preceding siblings ...)
2012-06-08 15:35 ` [Qemu-devel] [PATCH 8/8] target-i386: Use uint32 visitor for [x]level properties Andreas Färber
@ 2012-06-08 21:39 ` Andreas Färber
2012-06-11 17:19 ` Luiz Capitulino
2012-06-11 18:28 ` Anthony Liguori
10 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2012-06-08 21:39 UTC (permalink / raw)
To: qemu-devel
Cc: Jan Kiszka, Michael Roth, Luiz Capitulino, Anthony Liguori,
Paolo Bonzini, Laszlo Ersek
Am 08.06.2012 17:35, schrieb Andreas Färber:
> Hello Anthony,
>
> To get moving with the merge of qom-next into qemu.git, now that the Makefile
> PULL is in, please pull a first small batch, grouping patches directly related
> to fixed-width visitors.
>
> Outlook:
>
> A second batch would include the QBus refactoring - still cherry-picking,
> reordering and re-testing.
Preview is ready:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next-2
exec.c | 4 +-
hw/acpi_piix4.c | 10 +-
hw/arm_l2x0.c | 2 +-
hw/i2c.c | 30 +++--
hw/ide/internal.h | 3 +
hw/ide/qdev.c | 31 +++--
hw/intel-hda.c | 37 +++--
hw/intel-hda.h | 3 +
hw/isa-bus.c | 23 +++-
hw/isa.h | 3 +
hw/lsi53c895a.c | 5 +-
hw/m48t59.c | 40 +++---
hw/pc_piix.c | 7 +-
hw/pci-hotplug.c | 6 +-
hw/pci.c | 51 +++++---
hw/pci_bridge.c | 2 +-
hw/pci_internals.h | 3 +-
hw/qdev-monitor.c | 101 ++++++++-------
hw/qdev-properties.c | 65 +++------
hw/qdev.c | 300
+++++++++++++++++++++++++++--------------
hw/qdev.h | 65 +++++----
hw/s390-virtio-bus.c | 37 +++---
hw/s390-virtio-bus.h | 4 +
hw/scsi-bus.c | 58 +++++---
hw/scsi.h | 3 +
hw/spapr_pci.c | 7 +-
hw/spapr_vio.c | 47 ++++---
hw/spapr_vio.h | 3 +
hw/spapr_vty.c | 6 +-
hw/ssi.c | 29 +++--
hw/sysbus.c | 45 ++++++-
hw/sysbus.h | 3 +
hw/usb.h | 3 +
hw/usb/bus.c | 45 ++++---
hw/usb/desc.c | 5 +-
hw/usb/dev-smartcard-reader.c | 25 +++--
hw/virtio-scsi.c | 6 +-
hw/virtio-serial-bus.c | 36 ++++--
include/qemu/object.h | 47 ++++++-
qom/object.c | 158 +++++++++++++++------
savevm.c | 12 +-
41 files changed, 868 insertions(+), 502 deletions(-)
> I'm preparing a two-patch realize series that could get appended to the second.
> Still open are the static property movements and their prerequisites.
>
> CPU-related patches are split off into their own pull.
>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
>
> Jan, this does not include the big file movement but the largest part of the
> qdev-properties.c changes and in particular the ones that affect PCI. I suggest
> that you rebase on this one and me/Paolo the remaining parts on yours.
>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>
> The following changes since commit fa79c914efd35cb60e0bc18512c03690c48b13e2:
>
> Merge remote-tracking branch 'bonzini/nested-makefiles-3' into staging (2012-06-07 17:21:40 +0800)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/afaerber.git qom-next-1
>
> Andreas Färber (1):
> target-i386: Use uint32 visitor for [x]level properties
>
> Michael Roth (6):
> qapi: Add Visitor interfaces for uint*_t and int*_t
> qapi: Unit tests for visitor-based serialization
> qapi: String visitor, use %f representation for floats
> qapi: Add String visitor coverage to serialization unit tests
> qdev: Use int32_t container for devfn property
> qdev: Switch property accessors to fixed-width visitor interfaces
>
> Paolo Bonzini (1):
> qdev: Remove PropertyInfo range checking
>
> hw/mc146818rtc.c | 7 -
> hw/pci.c | 2 +-
> hw/pci.h | 2 +-
> hw/qdev-addr.c | 4 +-
> hw/qdev-properties.c | 205 ++++------
> hw/qdev.h | 4 +-
> qapi/qapi-visit-core.c | 139 +++++++
> qapi/qapi-visit-core.h | 16 +
> qapi/string-output-visitor.c | 2 +-
> target-i386/cpu.c | 42 +--
> tests/Makefile | 4 +-
> tests/test-string-output-visitor.c | 2 +-
> tests/test-visitor-serialization.c | 784 ++++++++++++++++++++++++++++++++++++
> 13 files changed, 1041 insertions(+), 172 deletions(-)
> create mode 100644 tests/test-visitor-serialization.c
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t
2012-06-08 15:35 ` [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t Andreas Färber
@ 2012-06-09 15:03 ` Laszlo Ersek
2012-06-09 15:16 ` Andreas Färber
0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2012-06-09 15:03 UTC (permalink / raw)
To: Andreas Färber, Michael Roth
Cc: Paolo Bonzini, qemu-devel, Anthony Liguori
On 06/08/12 17:35, Andreas Färber wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> This adds visitor interfaces for fixed-width integers types.
> Implementing these in visitors is optional, otherwise we fall back to
> visit_type_int() (int64_t) with some additional bounds checking to avoid
> integer overflows for cases where the value fetched exceeds the bounds
> of our target C type.
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index e850746..a19d70c 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -52,6 +52,14 @@ struct Visitor
> void (*start_handle)(Visitor *v, void **obj, const char *kind,
> const char *name, Error **errp);
> void (*end_handle)(Visitor *v, Error **errp);
> + void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> + void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> + void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> + void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> + void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
> + void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
> + void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
> + void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
> };
>
> void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
> void visit_type_enum(Visitor *v, int *obj, const char *strings[],
> const char *kind, const char *name, Error **errp);
> void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
> +void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> +void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> +void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> +void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> +void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
> +void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
> +void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
> +void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
> void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
> void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
> void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
Shouldn't "scripts/qapi.py" be extended accordingly? (The c_type function.)
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t
2012-06-09 15:03 ` Laszlo Ersek
@ 2012-06-09 15:16 ` Andreas Färber
2012-06-09 15:33 ` Laszlo Ersek
0 siblings, 1 reply; 16+ messages in thread
From: Andreas Färber @ 2012-06-09 15:16 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Paolo Bonzini, Michael Roth, Anthony Liguori, qemu-devel
Am 09.06.2012 17:03, schrieb Laszlo Ersek:
> On 06/08/12 17:35, Andreas Färber wrote:
>> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>>
>> This adds visitor interfaces for fixed-width integers types.
>> Implementing these in visitors is optional, otherwise we fall back to
>> visit_type_int() (int64_t) with some additional bounds checking to avoid
>> integer overflows for cases where the value fetched exceeds the bounds
>> of our target C type.
>
>> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
>> index e850746..a19d70c 100644
>> --- a/qapi/qapi-visit-core.h
>> +++ b/qapi/qapi-visit-core.h
>> @@ -52,6 +52,14 @@ struct Visitor
>> void (*start_handle)(Visitor *v, void **obj, const char *kind,
>> const char *name, Error **errp);
>> void (*end_handle)(Visitor *v, Error **errp);
>> + void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
>> + void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
>> + void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
>> + void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>> + void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
>> + void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
>> + void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
>> + void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
>> };
>>
>> void visit_start_handle(Visitor *v, void **obj, const char *kind,
>> @@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
>> void visit_type_enum(Visitor *v, int *obj, const char *strings[],
>> const char *kind, const char *name, Error **errp);
>> void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
>> +void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
>> +void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
>> +void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
>> +void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>> +void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
>> +void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
>> +void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
>> +void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
>> void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>> void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>> void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
>
> Shouldn't "scripts/qapi.py" be extended accordingly? (The c_type function.)
What does that affect?
Is it a blocker for this PULL or an improvement for a follow-up?
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t
2012-06-09 15:16 ` Andreas Färber
@ 2012-06-09 15:33 ` Laszlo Ersek
2012-06-09 20:33 ` Michael Roth
0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2012-06-09 15:33 UTC (permalink / raw)
To: Andreas Färber
Cc: Paolo Bonzini, Michael Roth, Anthony Liguori, qemu-devel
On 06/09/12 17:16, Andreas Färber wrote:
> Am 09.06.2012 17:03, schrieb Laszlo Ersek:
>> On 06/08/12 17:35, Andreas Färber wrote:
>>> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>>>
>>> This adds visitor interfaces for fixed-width integers types.
>>> Implementing these in visitors is optional, otherwise we fall back to
>>> visit_type_int() (int64_t) with some additional bounds checking to avoid
>>> integer overflows for cases where the value fetched exceeds the bounds
>>> of our target C type.
>>
>>> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
>>> index e850746..a19d70c 100644
>>> --- a/qapi/qapi-visit-core.h
>>> +++ b/qapi/qapi-visit-core.h
>>> @@ -52,6 +52,14 @@ struct Visitor
>>> void (*start_handle)(Visitor *v, void **obj, const char *kind,
>>> const char *name, Error **errp);
>>> void (*end_handle)(Visitor *v, Error **errp);
>>> + void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
>>> + void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
>>> + void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
>>> + void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>>> + void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
>>> + void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
>>> + void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
>>> + void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
>>> };
>>>
>>> void visit_start_handle(Visitor *v, void **obj, const char *kind,
>>> @@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
>>> void visit_type_enum(Visitor *v, int *obj, const char *strings[],
>>> const char *kind, const char *name, Error **errp);
>>> void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
>>> +void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
>>> +void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
>>> +void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
>>> +void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>>> +void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
>>> +void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
>>> +void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
>>> +void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
>>> void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>>> void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>>> void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
>>
>> Shouldn't "scripts/qapi.py" be extended accordingly? (The c_type function.)
>
> What does that affect?
> Is it a blocker for this PULL or an improvement for a follow-up?
The latter. As long as nothing actually uses these types there's no problem.
Laszlo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t
2012-06-09 15:33 ` Laszlo Ersek
@ 2012-06-09 20:33 ` Michael Roth
0 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2012-06-09 20:33 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Paolo Bonzini, Andreas Färber, Anthony Liguori, qemu-devel
On Sat, Jun 09, 2012 at 05:33:14PM +0200, Laszlo Ersek wrote:
> On 06/09/12 17:16, Andreas Färber wrote:
> > Am 09.06.2012 17:03, schrieb Laszlo Ersek:
> >> On 06/08/12 17:35, Andreas Färber wrote:
> >>> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> >>>
> >>> This adds visitor interfaces for fixed-width integers types.
> >>> Implementing these in visitors is optional, otherwise we fall back to
> >>> visit_type_int() (int64_t) with some additional bounds checking to avoid
> >>> integer overflows for cases where the value fetched exceeds the bounds
> >>> of our target C type.
> >>
> >>> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> >>> index e850746..a19d70c 100644
> >>> --- a/qapi/qapi-visit-core.h
> >>> +++ b/qapi/qapi-visit-core.h
> >>> @@ -52,6 +52,14 @@ struct Visitor
> >>> void (*start_handle)(Visitor *v, void **obj, const char *kind,
> >>> const char *name, Error **errp);
> >>> void (*end_handle)(Visitor *v, Error **errp);
> >>> + void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> >>> + void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> >>> + void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> >>> + void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> >>> + void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
> >>> + void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
> >>> + void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
> >>> + void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
> >>> };
> >>>
> >>> void visit_start_handle(Visitor *v, void **obj, const char *kind,
> >>> @@ -69,6 +77,14 @@ void visit_end_optional(Visitor *v, Error **errp);
> >>> void visit_type_enum(Visitor *v, int *obj, const char *strings[],
> >>> const char *kind, const char *name, Error **errp);
> >>> void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp);
> >>> +void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp);
> >>> +void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
> >>> void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
> >>> void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
> >>> void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> >>
> >> Shouldn't "scripts/qapi.py" be extended accordingly? (The c_type function.)
> >
> > What does that affect?
> > Is it a blocker for this PULL or an improvement for a follow-up?
>
> The latter. As long as nothing actually uses these types there's no problem.
To clarify: as long as nobody uses these types in a QAPI schema it's
fine.
The current users don't rely on schema-generated fixed-width types so
we're okay as far as the pull.
There's an extension to c_type() to support fixed-width types floating around
in the QIDL series, but a standalone patch at some point would be warranted
as well.
>
> Laszlo
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (8 preceding siblings ...)
2012-06-08 21:39 ` [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
@ 2012-06-11 17:19 ` Luiz Capitulino
2012-06-11 18:28 ` Anthony Liguori
10 siblings, 0 replies; 16+ messages in thread
From: Luiz Capitulino @ 2012-06-11 17:19 UTC (permalink / raw)
To: Andreas Färber
Cc: Jan Kiszka, qemu-devel, Michael Roth, Anthony Liguori,
Paolo Bonzini, Laszlo Ersek
On Fri, 8 Jun 2012 17:35:05 +0200
Andreas Färber <afaerber@suse.de> wrote:
> Hello Anthony,
>
> To get moving with the merge of qom-next into qemu.git, now that the Makefile
> PULL is in, please pull a first small batch, grouping patches directly related
> to fixed-width visitors.
FWIW, the fixed-width visitors looked good to me last time I glanced at
its series.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
` (9 preceding siblings ...)
2012-06-11 17:19 ` Luiz Capitulino
@ 2012-06-11 18:28 ` Anthony Liguori
10 siblings, 0 replies; 16+ messages in thread
From: Anthony Liguori @ 2012-06-11 18:28 UTC (permalink / raw)
To: Andreas Färber
Cc: Michael Roth, Jan Kiszka, qemu-devel, Luiz Capitulino,
Paolo Bonzini, Laszlo Ersek
On 06/08/2012 10:35 AM, Andreas Färber wrote:
> Hello Anthony,
>
> To get moving with the merge of qom-next into qemu.git, now that the Makefile
> PULL is in, please pull a first small batch, grouping patches directly related
> to fixed-width visitors.
>
> Outlook:
>
> A second batch would include the QBus refactoring - still cherry-picking,
> reordering and re-testing.
>
> I'm preparing a two-patch realize series that could get appended to the second.
> Still open are the static property movements and their prerequisites.
>
> CPU-related patches are split off into their own pull.
Pulled. Thanks.
Regards,
Anthony Liguori
>
> Cc: Anthony Liguori<anthony@codemonkey.ws>
> Cc: Paolo Bonzini<pbonzini@redhat.com>
> Cc: Michael Roth<mdroth@linux.vnet.ibm.com>
> Cc: Laszlo Ersek<lersek@redhat.com>
> Cc: Luiz Capitulino<lcapitulino@redhat.com>
>
> Jan, this does not include the big file movement but the largest part of the
> qdev-properties.c changes and in particular the ones that affect PCI. I suggest
> that you rebase on this one and me/Paolo the remaining parts on yours.
>
> Cc: Jan Kiszka<jan.kiszka@siemens.com>
>
> The following changes since commit fa79c914efd35cb60e0bc18512c03690c48b13e2:
>
> Merge remote-tracking branch 'bonzini/nested-makefiles-3' into staging (2012-06-07 17:21:40 +0800)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/afaerber.git qom-next-1
>
> Andreas Färber (1):
> target-i386: Use uint32 visitor for [x]level properties
>
> Michael Roth (6):
> qapi: Add Visitor interfaces for uint*_t and int*_t
> qapi: Unit tests for visitor-based serialization
> qapi: String visitor, use %f representation for floats
> qapi: Add String visitor coverage to serialization unit tests
> qdev: Use int32_t container for devfn property
> qdev: Switch property accessors to fixed-width visitor interfaces
>
> Paolo Bonzini (1):
> qdev: Remove PropertyInfo range checking
>
> hw/mc146818rtc.c | 7 -
> hw/pci.c | 2 +-
> hw/pci.h | 2 +-
> hw/qdev-addr.c | 4 +-
> hw/qdev-properties.c | 205 ++++------
> hw/qdev.h | 4 +-
> qapi/qapi-visit-core.c | 139 +++++++
> qapi/qapi-visit-core.h | 16 +
> qapi/string-output-visitor.c | 2 +-
> target-i386/cpu.c | 42 +--
> tests/Makefile | 4 +-
> tests/test-string-output-visitor.c | 2 +-
> tests/test-visitor-serialization.c | 784 ++++++++++++++++++++++++++++++++++++
> 13 files changed, 1041 insertions(+), 172 deletions(-)
> create mode 100644 tests/test-visitor-serialization.c
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-06-11 18:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-08 15:35 [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 1/8] qapi: Add Visitor interfaces for uint*_t and int*_t Andreas Färber
2012-06-09 15:03 ` Laszlo Ersek
2012-06-09 15:16 ` Andreas Färber
2012-06-09 15:33 ` Laszlo Ersek
2012-06-09 20:33 ` Michael Roth
2012-06-08 15:35 ` [Qemu-devel] [PATCH 2/8] qapi: Unit tests for visitor-based serialization Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 3/8] qapi: String visitor, use %f representation for floats Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 4/8] qapi: Add String visitor coverage to serialization unit tests Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 5/8] qdev: Use int32_t container for devfn property Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 6/8] qdev: Switch property accessors to fixed-width visitor interfaces Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 7/8] qdev: Remove PropertyInfo range checking Andreas Färber
2012-06-08 15:35 ` [Qemu-devel] [PATCH 8/8] target-i386: Use uint32 visitor for [x]level properties Andreas Färber
2012-06-08 21:39 ` [Qemu-devel] [PULL] qom-next queue, first batch: fixed-width visitors Andreas Färber
2012-06-11 17:19 ` Luiz Capitulino
2012-06-11 18:28 ` Anthony Liguori
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).