* [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface
@ 2011-10-27 17:06 Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
` (9 more replies)
0 siblings, 10 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
These patches apply on top of origin/master, and can also be obtained from:
git://repo.or.cz/qemu/mdroth.git migration-visitor-v2
- rebased on 9f60639b848944200c3d33a89233d808de0b5a43. Juan: had a pretty
hard collision with your recent savevm/vmstate cleanups. I think most of
the issues were reconciled, but some of your checks such as bailing out
of vmstate_subsection_load() if qemu_peek_buffer() doesn't return the
length encoded in the stream (for idstr) and peeking and checking subsection
tags before progressing through the QEMUFile input buffer were not
compatible with the visitorized approach. The net effect of failing migration
if these assertions do not hold should still be intact, however.
- reworked QEMUFile<->Visitor lookup routines to support opaque objects and
register cleanup routines, mainly to allow cleanup of
QEMUFileInputVisitor/QEMUFileOutputVisitor/etc via qemu_fclose() without
being aware of the type of Visitor employed.
- fixed coding style issues as part of moving QEMUFile interfaces to
qemu-file.c
OVERVIEW
This patch series implements a QEMUFile Visitor class that's intended to
abstract away direct calls to qemu_put_*/qemu_get_* for save/load functions.
Currently this is done by always creating a
QEMUFileInputVisitor/QEMUFileOutputVisitor pair with each call to
qemu_fopen_ops() and maintaining a QEMUFile->I/O Visitor mapping. save/load
functions that are to be converted would them simply use lookup routines to get
a Visitor based on their QEMUFile arguments. Once these save/load functions are
all converted, we can then change the interfaces in bulk and switch over to
passing in the Visitor directly.
This series also converts all of vmstate over to using Visitors, leveraging the
existing vmstate hierachical structure to handle namespacing and structuring of
the fields that are fed to the visitor functions, and introduces trace
statements for qemu_put_*/qemu_get_* and visitor API calls to aid in testing.
Migration compatibility is not affected by these changes, as the
qemu_put_*/qemu_get_* calls have a trivial 1-to-1 mapping with the visit_type_*
calls, and the added field names/structure are not sent out over the wire when
using the QEMUFile visitors, but serve simply as placeholders for future
functionality.
Test scripts/procedure are documented on the wiki page:
http://wiki.qemu.org/Features/Migration/Visitor#Code.2FTesting
PLANS
With these patches in place non-vmstate save/load functions can be converted
over to using Visitors incrementally.
Short term (1.1 timeframe), the goal is to implement a new migration protocol
that is self-describing, such that incompatibilities or migration errors can be
more easilly detected. Currently, a simple change in data types for a
particular device can introduce subtle bugs that won't be detected by the
target, since the target interprets the data according to it's own expectation
of what those data types are. Currently the plan is to achieve this by using
ASN.1 BER in place of QEMUFile via a new BERVisitor.
Also planned is using the visitor interface to aid in things like device
introspection (for both vmstate and non-vmstate devices), likely via the
existing QMPOutputVisitor, as well as debugging/testing migration via a well
structured "PrettyPrintVisitor" type of visitor that also retains ordering,
field names, and type information (unlike with QMP/JSON or ASN.1). I've played
around with doing this indirectly to test the conversions by doing some
post-processing on the visit_* trace statements:
...
PIIX3.PCIDevice.config(256)(1).[255] = (uint8) 0x0
PIIX3.PCIDevice.irq_state(4)(4).[0] = (uint32) 0x0
PIIX3.PCIDevice.irq_state(4)(4).[1] = (uint32) 0x0
PIIX3.PCIDevice.irq_state(4)(4).[2] = (uint32) 0x0
PIIX3.PCIDevice.irq_state(4)(4).[3] = (uint32) 0x0
PIIX3.pci_irq_levels_vmstate(4)(4).[0] = (int32) 0x0
PIIX3.pci_irq_levels_vmstate(4)(4).[1] = (int32) 0x0
PIIX3.pci_irq_levels_vmstate(4)(4).[2] = (int32) 0x0
PIIX3.pci_irq_levels_vmstate(4)(4).[3] = (int32) 0x0
i8259.last_irr = (uint8) 0x1
i8259.irr = (uint8) 0x10
i8259.imr = (uint8) 0xb8
i8259.isr = (uint8) 0x0
i8259.priority_add = (uint8) 0x0
i8259.irq_base = (uint8) 0x8
i8259.read_reg_select = (uint8) 0x0
i8259.poll = (uint8) 0x0
i8259.special_mask = (uint8) 0x0
i8259.init_state = (uint8) 0x0
i8259.auto_eoi = (uint8) 0x0
...
slirp.ip_id = (uint16) 0x2
slirp.bootp.bootp_clients(16)(8).[0].allocated = (uint16) 0x1
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[0] = (uint8) 0x1
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[1] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[2] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[3] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[4] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[0].macaddr(6)(1).[5] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[1].allocated = (uint16) 0x0
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[0] = (uint8) 0x1
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[1] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[2] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[3] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[4] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[1].macaddr(6)(1).[5] = (uint8) 0x0
slirp.bootp.bootp_clients(16)(8).[2].allocated = (uint16) 0x0
...
It's not completely clear at this point how the latter will fall into place
with other plans such as QOM that would provide similar capabilities, but since
device serialization will likely be done via automatically-generated visitor
patterns, this conversion should at least serve as an incremental step toward
that transition.
Michael Roth (10):
qapi: add Visitor interfaces for uint*_t and int*_t
qapi: add QemuFileOutputVisitor
qapi: add QemuFileInputVisitor
savevm: move QEMUFile interfaces into qemu-file.c
qapi: test cases for QEMUFile input/output visitors
qemu-file: add QEMUFile<->visitor lookup routines
trace: qemu_(put|get)_(byte|buffer) events
trace: add trace statements for visitor interface
qapi: add trace statements to qapi-visit-core.c
vmstate: use visitors
Makefile | 2 +-
Makefile.objs | 22 +-
hw/eeprom93xx.c | 12 +-
hw/fw_cfg.c | 12 +-
hw/hw.h | 18 +-
hw/pci.c | 38 +-
hw/twl92230.c | 18 +-
qapi/qapi-visit-core.c | 119 +++++-
qapi/qapi-visit-core.h | 30 ++
qapi/qemu-file-input-visitor.c | 328 ++++++++++++
qapi/qemu-file-input-visitor.h | 26 +
qapi/qemu-file-output-visitor.c | 328 ++++++++++++
qapi/qemu-file-output-visitor.h | 26 +
qemu-file.c | 688 +++++++++++++++++++++++++
qemu-timer.h | 5 +
savevm.c | 1049 ++++++++++++---------------------------
target-alpha/machine.c | 13 +-
target-i386/machine.c | 49 ++-
test-visitor.c | 404 +++++++++++++++
trace-events | 43 ++
20 files changed, 2445 insertions(+), 785 deletions(-)
create mode 100644 qapi/qemu-file-input-visitor.c
create mode 100644 qapi/qemu-file-input-visitor.h
create mode 100644 qapi/qemu-file-output-visitor.c
create mode 100644 qapi/qemu-file-output-visitor.h
create mode 100644 qemu-file.c
--
1.7.4.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-12-20 11:12 ` Paolo Bonzini
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 02/10] qapi: add QemuFileOutputVisitor Michael Roth
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qapi/qapi-visit-core.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
qapi/qapi-visit-core.h | 30 ++++++++++++++++++
2 files changed, 108 insertions(+), 0 deletions(-)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index ddef3ed..58f11fe 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -66,6 +66,28 @@ void visit_end_list(Visitor *v, Error **errp)
}
}
+void visit_start_array(Visitor *v, void **obj, const char *name, size_t elem_count,
+ size_t elem_size, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->start_array(v, obj, name, elem_count, elem_size, errp);
+ }
+}
+
+void visit_next_array(Visitor *v, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->next_array(v, errp);
+ }
+}
+
+void visit_end_array(Visitor *v, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->end_array(v, errp);
+ }
+}
+
void visit_start_optional(Visitor *v, bool *present, const char *name,
Error **errp)
{
@@ -96,6 +118,62 @@ 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)
+{
+ if (!error_is_set(errp)) {
+ v->type_uint8(v, obj, name, errp);
+ }
+}
+
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_uint16(v, obj, name, errp);
+ }
+}
+
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_uint32(v, obj, name, errp);
+ }
+}
+
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_uint64(v, obj, name, errp);
+ }
+}
+
+void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_int8(v, obj, name, errp);
+ }
+}
+
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_int16(v, obj, name, errp);
+ }
+}
+
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_int32(v, obj, name, errp);
+ }
+}
+
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_int64(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..84f6c9e 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -22,6 +22,11 @@ typedef struct GenericList
struct GenericList *next;
} GenericList;
+typedef struct GenericItem
+{
+ void *value;
+} GenericItem;
+
typedef struct Visitor Visitor;
struct Visitor
@@ -35,10 +40,23 @@ struct Visitor
GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
void (*end_list)(Visitor *v, Error **errp);
+ void (*start_array)(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp);
+ void (*next_array)(Visitor *v, Error **errp);
+ void (*end_array)(Visitor *v, Error **errp);
+
void (*type_enum)(Visitor *v, int *obj, const char *strings[],
const char *kind, const char *name, Error **errp);
void (*type_int)(Visitor *v, int64_t *obj, const char *name, 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 (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
void (*type_number)(Visitor *v, double *obj, const char *name,
@@ -63,12 +81,24 @@ void visit_end_struct(Visitor *v, Error **errp);
void visit_start_list(Visitor *v, const char *name, Error **errp);
GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
void visit_end_list(Visitor *v, Error **errp);
+void visit_start_array(Visitor *v, void **obj, const char *name, size_t elem_count,
+ size_t elem_size, Error **errp);
+void visit_next_array(Visitor *v, Error **errp);
+void visit_end_array(Visitor *v, Error **errp);
void visit_start_optional(Visitor *v, bool *present, const char *name,
Error **errp);
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.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 02/10] qapi: add QemuFileOutputVisitor
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 03/10] qapi: add QemuFileInputVisitor Michael Roth
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Visitor interface to write values to a QEMUFile.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile.objs | 19 +--
qapi/qemu-file-output-visitor.c | 328 +++++++++++++++++++++++++++++++++++++++
qapi/qemu-file-output-visitor.h | 26 +++
3 files changed, 363 insertions(+), 10 deletions(-)
create mode 100644 qapi/qemu-file-output-visitor.c
create mode 100644 qapi/qemu-file-output-visitor.h
diff --git a/Makefile.objs b/Makefile.objs
index 01587c8..0229140 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -20,6 +20,12 @@ coroutine-obj-$(CONFIG_POSIX) += coroutine-gthread.o
endif
coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
+######################################################################
+# qapi
+qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-dealloc-visitor.o
+qapi-nested-y += qmp-registry.o qmp-dispatch.o
+qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
+
#######################################################################
# block-obj-y is code used by both qemu system emulation and qemu-img
@@ -73,6 +79,9 @@ fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y))
# CPUs and machines.
common-obj-y = $(block-obj-y) blockdev.o
+common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
+common-obj-y += qmp.o hmp.o
+common-obj-y += qapi/qemu-file-output-visitor.o
common-obj-y += $(net-obj-y)
common-obj-y += $(qobject-obj-y)
common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
@@ -401,16 +410,6 @@ $(trace-obj-y): $(GENERATED_HEADERS)
libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o
######################################################################
-# qapi
-
-qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-dealloc-visitor.o
-qapi-nested-y += qmp-registry.o qmp-dispatch.o
-qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
-
-common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
-common-obj-y += qmp.o hmp.o
-
-######################################################################
# guest agent
qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
diff --git a/qapi/qemu-file-output-visitor.c b/qapi/qemu-file-output-visitor.c
new file mode 100644
index 0000000..43a0b15
--- /dev/null
+++ b/qapi/qemu-file-output-visitor.c
@@ -0,0 +1,328 @@
+/*
+ * QEMUFile Output Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-file-output-visitor.h"
+#include "qemu-queue.h"
+#include "qemu-common.h"
+#include "qemu-objects.h"
+#include "hw/hw.h"
+#include "qerror.h"
+
+typedef struct {
+ size_t elem_count;
+ size_t elem_size;
+ size_t pos;
+} ArrayInfo;
+
+typedef struct StackEntry
+{
+ enum {
+ QFOV_ARRAY,
+ QFOV_LIST,
+ QFOV_STRUCT,
+ } type;
+ ArrayInfo array_info;
+ bool is_list_head;
+ QTAILQ_ENTRY(StackEntry) node;
+} StackEntry;
+
+struct QemuFileOutputVisitor
+{
+ Visitor visitor;
+ QTAILQ_HEAD(, StackEntry) stack;
+ QEMUFile *file;
+};
+
+static QemuFileOutputVisitor *to_ov(Visitor *v)
+{
+ return container_of(v, QemuFileOutputVisitor, visitor);
+}
+
+static void qemu_file_output_push(QemuFileOutputVisitor *ov, StackEntry *e)
+{
+ QTAILQ_INSERT_HEAD(&ov->stack, e, node);
+}
+
+static void qemu_file_output_push_array(QemuFileOutputVisitor *ov, ArrayInfo ai)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFOV_ARRAY;
+ e->array_info = ai;
+ qemu_file_output_push(ov, e);
+}
+
+static void qemu_file_output_push_list(QemuFileOutputVisitor *ov)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFOV_LIST;
+ e->is_list_head = true;
+ qemu_file_output_push(ov, e);
+}
+
+static void qemu_file_output_push_struct(QemuFileOutputVisitor *ov)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFOV_STRUCT;
+ qemu_file_output_push(ov, e);
+}
+
+static StackEntry *qemu_file_output_pop(QemuFileOutputVisitor *ov)
+{
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+ QTAILQ_REMOVE(&ov->stack, e, node);
+ return e;
+}
+
+static bool qemu_file_output_is_array(QemuFileOutputVisitor *ov)
+{
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+ return e && e->type == QFOV_ARRAY;
+}
+
+static bool qemu_file_output_is_list(QemuFileOutputVisitor *ov)
+{
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+ return e && e->type == QFOV_LIST;
+}
+
+static void qemu_file_output_start_struct(Visitor *v, void **obj,
+ const char *kind, const char *name,
+ size_t unused, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+
+ qemu_file_output_push_struct(ov);
+}
+
+static void qemu_file_output_end_struct(Visitor *v, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ StackEntry *e = qemu_file_output_pop(ov);
+
+ if (!e || e->type != QFOV_STRUCT) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ g_free(e);
+}
+
+static void qemu_file_output_start_list(Visitor *v, const char *name,
+ Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ qemu_file_output_push_list(ov);
+}
+
+static GenericList *qemu_file_output_next_list(Visitor *v, GenericList **list,
+ Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ GenericList *entry = *list;
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+
+ if (!entry || !qemu_file_output_is_list(ov)) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
+
+ /* The way the list iterator is currently used unfortunately clobbers
+ * **list by subseqently assigning our return value to the same container.
+ * This can cause an infinite loop, but we can get around this by tracking
+ * a bit of state to note when we should pass back the next entry rather
+ * than the current one.
+ */
+ if (e->is_list_head) {
+ e->is_list_head = false;
+ return entry;
+ }
+
+ *list = entry->next;
+ return entry->next;
+}
+
+static void qemu_file_output_end_list(Visitor *v, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ StackEntry *e = qemu_file_output_pop(ov);
+ if (!e || e->type != QFOV_LIST) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
+ g_free(e);
+}
+
+static void qemu_file_output_start_array(Visitor *v, void **obj,
+ const char *name,
+ size_t elem_count,
+ size_t elem_size, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ ArrayInfo ai = {
+ .elem_count = elem_count,
+ .elem_size = elem_size,
+ .pos = 0
+ };
+ qemu_file_output_push_array(ov, ai);
+}
+
+static void qemu_file_output_next_array(Visitor *v, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+ if (!qemu_file_output_is_array(ov) ||
+ e->array_info.pos >= e->array_info.elem_count) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
+
+ e->array_info.pos++;
+}
+
+static void qemu_file_output_end_array(Visitor *v, Error **errp)
+{
+ QemuFileOutputVisitor *ov = to_ov(v);
+ StackEntry *e = qemu_file_output_pop(ov);
+ if (!e || e->type != QFOV_ARRAY) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ g_free(e);
+}
+
+static void qemu_file_output_type_str(Visitor *v, char **obj, const char *name,
+ Error **errp)
+{
+ if (obj) {
+ g_free(*obj);
+ }
+}
+
+static void qemu_file_output_type_uint8(Visitor *v, uint8_t *obj,
+ const char *name,
+ Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_byte(ov->file, *obj);
+}
+
+static void qemu_file_output_type_uint16(Visitor *v, uint16_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_be16(ov->file, *obj);
+}
+
+static void qemu_file_output_type_uint32(Visitor *v, uint32_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_be32(ov->file, *obj);
+}
+
+static void qemu_file_output_type_uint64(Visitor *v, uint64_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_be64(ov->file, *obj);
+}
+
+static void qemu_file_output_type_int8(Visitor *v, int8_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_sbyte(ov->file, *obj);
+}
+
+static void qemu_file_output_type_int16(Visitor *v, int16_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_sbe16(ov->file, *obj);
+}
+
+static void qemu_file_output_type_int32(Visitor *v, int32_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_sbe32(ov->file, *obj);
+}
+
+static void qemu_file_output_type_int64(Visitor *v, int64_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileOutputVisitor *ov = container_of(v, QemuFileOutputVisitor, visitor);
+ qemu_put_be64(ov->file, *obj);
+}
+
+static void qemu_file_output_type_bool(Visitor *v, bool *obj, const char *name,
+ Error **errp)
+{
+ uint8_t val = *obj;
+ qemu_file_output_type_uint8(v, &val, name, errp);
+}
+
+static void qemu_file_output_type_number(Visitor *v, double *obj,
+ const char *name, Error **errp)
+{
+ uint64_t *val = (uint64_t *)obj;
+ qemu_file_output_type_uint64(v, val, name, errp);
+}
+
+static void qemu_file_output_type_enum(Visitor *v, int *obj,
+ const char *strings[],
+ const char *kind, const char *name,
+ Error **errp)
+{
+}
+
+Visitor *qemu_file_output_get_visitor(QemuFileOutputVisitor *v)
+{
+ return &v->visitor;
+}
+
+void qemu_file_output_visitor_cleanup(QemuFileOutputVisitor *ov)
+{
+ g_free(ov);
+}
+
+QemuFileOutputVisitor *qemu_file_output_visitor_new(QEMUFile *f)
+{
+ QemuFileOutputVisitor *v;
+
+ v = g_malloc0(sizeof(*v));
+
+ v->file = f;
+
+ v->visitor.start_struct = qemu_file_output_start_struct;
+ v->visitor.end_struct = qemu_file_output_end_struct;
+ v->visitor.start_list = qemu_file_output_start_list;
+ v->visitor.next_list = qemu_file_output_next_list;
+ v->visitor.end_list = qemu_file_output_end_list;
+ v->visitor.start_array = qemu_file_output_start_array;
+ v->visitor.next_array = qemu_file_output_next_array;
+ v->visitor.end_array = qemu_file_output_end_array;
+ v->visitor.type_enum = qemu_file_output_type_enum;
+ v->visitor.type_int = qemu_file_output_type_int64;
+ v->visitor.type_uint8 = qemu_file_output_type_uint8;
+ v->visitor.type_uint16 = qemu_file_output_type_uint16;
+ v->visitor.type_uint32 = qemu_file_output_type_uint32;
+ v->visitor.type_uint64 = qemu_file_output_type_uint64;
+ v->visitor.type_int8 = qemu_file_output_type_int8;
+ v->visitor.type_int16 = qemu_file_output_type_int16;
+ v->visitor.type_int32 = qemu_file_output_type_int32;
+ v->visitor.type_int64 = qemu_file_output_type_int64;
+ v->visitor.type_bool = qemu_file_output_type_bool;
+ v->visitor.type_str = qemu_file_output_type_str;
+ v->visitor.type_number = qemu_file_output_type_number;
+
+ QTAILQ_INIT(&v->stack);
+
+ return v;
+}
diff --git a/qapi/qemu-file-output-visitor.h b/qapi/qemu-file-output-visitor.h
new file mode 100644
index 0000000..e446f08
--- /dev/null
+++ b/qapi/qemu-file-output-visitor.h
@@ -0,0 +1,26 @@
+/*
+ * QEMUFile Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_FILE_OUTPUT_VISITOR_H
+#define QEMU_FILE_OUTPUT_VISITOR_H
+
+#include "qapi-visit-core.h"
+
+typedef struct QemuFileOutputVisitor QemuFileOutputVisitor;
+
+QemuFileOutputVisitor *qemu_file_output_visitor_new(QEMUFile *f);
+void qemu_file_output_visitor_cleanup(QemuFileOutputVisitor *d);
+
+Visitor *qemu_file_output_get_visitor(QemuFileOutputVisitor *v);
+
+#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 03/10] qapi: add QemuFileInputVisitor
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 02/10] qapi: add QemuFileOutputVisitor Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 04/10] savevm: move QEMUFile interfaces into qemu-file.c Michael Roth
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Visitor interfaces to read values from a QEMUFile
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile.objs | 1 +
qapi/qemu-file-input-visitor.c | 328 ++++++++++++++++++++++++++++++++++++++++
qapi/qemu-file-input-visitor.h | 26 +++
3 files changed, 355 insertions(+), 0 deletions(-)
create mode 100644 qapi/qemu-file-input-visitor.c
create mode 100644 qapi/qemu-file-input-visitor.h
diff --git a/Makefile.objs b/Makefile.objs
index 0229140..7733665 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -82,6 +82,7 @@ common-obj-y = $(block-obj-y) blockdev.o
common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
common-obj-y += qmp.o hmp.o
common-obj-y += qapi/qemu-file-output-visitor.o
+common-obj-y += qapi/qemu-file-input-visitor.o
common-obj-y += $(net-obj-y)
common-obj-y += $(qobject-obj-y)
common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
diff --git a/qapi/qemu-file-input-visitor.c b/qapi/qemu-file-input-visitor.c
new file mode 100644
index 0000000..ba966de
--- /dev/null
+++ b/qapi/qemu-file-input-visitor.c
@@ -0,0 +1,328 @@
+/*
+ * QEMUFile Output Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-file-input-visitor.h"
+#include "qemu-queue.h"
+#include "qemu-common.h"
+#include "qemu-objects.h"
+#include "hw/hw.h"
+#include "qerror.h"
+
+typedef struct {
+ size_t elem_count;
+ size_t elem_size;
+ size_t pos;
+} ArrayInfo;
+
+typedef struct StackEntry
+{
+ enum {
+ QFIV_ARRAY,
+ QFIV_LIST,
+ QFIV_STRUCT,
+ } type;
+ ArrayInfo array_info;
+ QTAILQ_ENTRY(StackEntry) node;
+} StackEntry;
+
+struct QemuFileInputVisitor
+{
+ Visitor visitor;
+ QTAILQ_HEAD(, StackEntry) stack;
+ QEMUFile *file;
+};
+
+static QemuFileInputVisitor *to_iv(Visitor *v)
+{
+ return container_of(v, QemuFileInputVisitor, visitor);
+}
+
+static void qemu_file_input_push(QemuFileInputVisitor *iv, StackEntry *e)
+{
+ QTAILQ_INSERT_HEAD(&iv->stack, e, node);
+}
+
+static void qemu_file_input_push_array(QemuFileInputVisitor *iv, ArrayInfo ai)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFIV_ARRAY;
+ e->array_info = ai;
+ qemu_file_input_push(iv, e);
+}
+
+static void qemu_file_input_push_list(QemuFileInputVisitor *iv)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFIV_LIST;
+ qemu_file_input_push(iv, e);
+}
+
+static void qemu_file_input_push_struct(QemuFileInputVisitor *iv)
+{
+ StackEntry *e = g_malloc0(sizeof(*e));
+ e->type = QFIV_STRUCT;
+ qemu_file_input_push(iv, e);
+}
+
+static void *qemu_file_input_pop(QemuFileInputVisitor *iv)
+{
+ StackEntry *e = QTAILQ_FIRST(&iv->stack);
+ QTAILQ_REMOVE(&iv->stack, e, node);
+ return e;
+}
+
+static bool qemu_file_input_is_array(QemuFileInputVisitor *iv)
+{
+ StackEntry *e = QTAILQ_FIRST(&iv->stack);
+ return e->type == QFIV_ARRAY;
+}
+
+static bool qemu_file_input_is_list(QemuFileInputVisitor *ov)
+{
+ StackEntry *e = QTAILQ_FIRST(&ov->stack);
+ return e && e->type == QFIV_LIST;
+}
+
+static void qemu_file_input_start_struct(Visitor *v, void **obj,
+ const char *kind,
+ const char *name, size_t size,
+ Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+
+ if (obj && *obj == NULL) {
+ *obj = g_malloc0(size);
+ }
+ qemu_file_input_push_struct(iv);
+}
+
+static void qemu_file_input_end_struct(Visitor *v, Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ StackEntry *e = qemu_file_input_pop(iv);
+
+ if (!e || e->type != QFIV_STRUCT) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ g_free(e);
+}
+
+static void qemu_file_input_start_list(Visitor *v, const char *name,
+ Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ qemu_file_input_push_list(iv);
+}
+
+static GenericList *qemu_file_input_next_list(Visitor *v, GenericList **list,
+ Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ GenericList *entry;
+
+ if (!qemu_file_input_is_list(iv)) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
+
+ entry = g_malloc0(sizeof(*entry));
+ if (*list) {
+ (*list)->next = entry;
+ }
+
+ *list = entry;
+ return entry;
+}
+
+static void qemu_file_input_end_list(Visitor *v, Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ StackEntry *e = qemu_file_input_pop(iv);
+ if (!e || e->type != QFIV_LIST) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ g_free(e);
+}
+
+static void qemu_file_input_start_array(Visitor *v, void **obj,
+ const char *name,
+ size_t elem_count,
+ size_t elem_size,
+ Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ ArrayInfo ai = {
+ .elem_count = elem_count,
+ .elem_size = elem_size,
+ .pos = 0
+ };
+ if (obj && *obj == NULL) {
+ *obj = g_malloc0(elem_count * elem_size);
+ }
+ qemu_file_input_push_array(iv, ai);
+}
+
+static void qemu_file_input_next_array(Visitor *v, Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ StackEntry *e = QTAILQ_FIRST(&iv->stack);
+
+ if (!qemu_file_input_is_array(iv) ||
+ e->array_info.pos >= e->array_info.elem_count) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
+
+ e->array_info.pos++;
+}
+
+static void qemu_file_input_end_array(Visitor *v, Error **errp)
+{
+ QemuFileInputVisitor *iv = to_iv(v);
+ StackEntry *e = qemu_file_input_pop(iv);
+ if (!e || e->type != QFIV_ARRAY) {
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ return;
+ }
+ g_free(e);
+}
+
+static void qemu_file_input_type_str(Visitor *v, char **obj, const char *name,
+ Error **errp)
+{
+ if (obj) {
+ g_free(*obj);
+ }
+}
+
+static void qemu_file_input_type_uint8(Visitor *v, uint8_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_byte(ov->file);
+}
+
+static void qemu_file_input_type_uint16(Visitor *v, uint16_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_be16(ov->file);
+}
+
+static void qemu_file_input_type_uint32(Visitor *v, uint32_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_be32(ov->file);
+}
+
+static void qemu_file_input_type_uint64(Visitor *v, uint64_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_be64(ov->file);
+}
+
+static void qemu_file_input_type_int8(Visitor *v, int8_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_sbyte(ov->file);
+}
+
+static void qemu_file_input_type_int16(Visitor *v, int16_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_sbe16(ov->file);
+}
+
+static void qemu_file_input_type_int32(Visitor *v, int32_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_sbe32(ov->file);
+}
+
+static void qemu_file_input_type_int64(Visitor *v, int64_t *obj,
+ const char *name, Error **errp)
+{
+ QemuFileInputVisitor *ov = container_of(v, QemuFileInputVisitor, visitor);
+ *obj = qemu_get_sbe64(ov->file);
+}
+
+static void qemu_file_input_type_bool(Visitor *v, bool *obj, const char *name,
+ Error **errp)
+{
+ uint8_t val;
+ qemu_file_input_type_uint8(v, &val, name, errp);
+ *obj = val;
+}
+
+static void qemu_file_input_type_number(Visitor *v, double *obj,
+ const char *name, Error **errp)
+{
+ uint64_t *val = (uint64_t *)obj;
+ qemu_file_input_type_uint64(v, val, name, errp);
+}
+
+static void qemu_file_input_type_enum(Visitor *v, int *obj,
+ const char *strings[], const char *kind,
+ const char *name, Error **errp)
+{
+}
+
+Visitor *qemu_file_input_get_visitor(QemuFileInputVisitor *ov)
+{
+ return &ov->visitor;
+}
+
+void qemu_file_input_visitor_cleanup(QemuFileInputVisitor *ov)
+{
+ g_free(ov);
+}
+
+QemuFileInputVisitor *qemu_file_input_visitor_new(QEMUFile *f)
+{
+ QemuFileInputVisitor *v;
+
+ v = g_malloc0(sizeof(*v));
+
+ v->file = f;
+
+ v->visitor.start_struct = qemu_file_input_start_struct;
+ v->visitor.end_struct = qemu_file_input_end_struct;
+ v->visitor.start_list = qemu_file_input_start_list;
+ v->visitor.next_list = qemu_file_input_next_list;
+ v->visitor.end_list = qemu_file_input_end_list;
+ v->visitor.start_array = qemu_file_input_start_array;
+ v->visitor.next_array = qemu_file_input_next_array;
+ v->visitor.end_array = qemu_file_input_end_array;
+ v->visitor.type_enum = qemu_file_input_type_enum;
+ v->visitor.type_int = qemu_file_input_type_int64;
+ v->visitor.type_uint8 = qemu_file_input_type_uint8;
+ v->visitor.type_uint16 = qemu_file_input_type_uint16;
+ v->visitor.type_uint32 = qemu_file_input_type_uint32;
+ v->visitor.type_uint64 = qemu_file_input_type_uint64;
+ v->visitor.type_int8 = qemu_file_input_type_int8;
+ v->visitor.type_int16 = qemu_file_input_type_int16;
+ v->visitor.type_int32 = qemu_file_input_type_int32;
+ v->visitor.type_int64 = qemu_file_input_type_int64;
+ v->visitor.type_bool = qemu_file_input_type_bool;
+ v->visitor.type_str = qemu_file_input_type_str;
+ v->visitor.type_number = qemu_file_input_type_number;
+
+ QTAILQ_INIT(&v->stack);
+
+ return v;
+}
diff --git a/qapi/qemu-file-input-visitor.h b/qapi/qemu-file-input-visitor.h
new file mode 100644
index 0000000..67b5554
--- /dev/null
+++ b/qapi/qemu-file-input-visitor.h
@@ -0,0 +1,26 @@
+/*
+ * QEMUFile Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_FILE_INPUT_VISITOR_H
+#define QEMU_FILE_INPUT_VISITOR_H
+
+#include "qapi-visit-core.h"
+
+typedef struct QemuFileInputVisitor QemuFileInputVisitor;
+
+QemuFileInputVisitor *qemu_file_input_visitor_new(QEMUFile *f);
+void qemu_file_input_visitor_cleanup(QemuFileInputVisitor *d);
+
+Visitor *qemu_file_input_get_visitor(QemuFileInputVisitor *v);
+
+#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 04/10] savevm: move QEMUFile interfaces into qemu-file.c
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (2 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 03/10] qapi: add QemuFileInputVisitor Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 05/10] qapi: test cases for QEMUFile input/output visitors Michael Roth
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile.objs | 2 +-
hw/hw.h | 3 +
qemu-file.c | 573 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
savevm.c | 534 -----------------------------------------------------
4 files changed, 577 insertions(+), 535 deletions(-)
create mode 100644 qemu-file.c
diff --git a/Makefile.objs b/Makefile.objs
index 7733665..631174f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -119,7 +119,7 @@ common-obj-$(CONFIG_SD) += sd.o
common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o usb-bt.o
common-obj-y += bt-hci-csr.o
common-obj-y += buffered_file.o migration.o migration-tcp.o
-common-obj-y += qemu-char.o savevm.o #aio.o
+common-obj-y += qemu-char.o savevm.o qemu-file.o #aio.o
common-obj-y += msmouse.o ps2.o
common-obj-y += qdev.o qdev-properties.o
common-obj-y += block-migration.o iohandler.o
diff --git a/hw/hw.h b/hw/hw.h
index ed20f5a..a793974 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -58,6 +58,9 @@ void qemu_fflush(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
void qemu_put_byte(QEMUFile *f, int v);
+int qemu_peek_byte(QEMUFile *f, int offset);
+int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
+void qemu_file_skip(QEMUFile *f, int size);
static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
{
diff --git a/qemu-file.c b/qemu-file.c
new file mode 100644
index 0000000..a4aee9a
--- /dev/null
+++ b/qemu-file.c
@@ -0,0 +1,573 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "qemu_socket.h"
+#include "hw/hw.h"
+
+#define IO_BUF_SIZE 32768
+
+struct QEMUFile {
+ QEMUFilePutBufferFunc *put_buffer;
+ QEMUFileGetBufferFunc *get_buffer;
+ QEMUFileCloseFunc *close;
+ QEMUFileRateLimit *rate_limit;
+ QEMUFileSetRateLimit *set_rate_limit;
+ QEMUFileGetRateLimit *get_rate_limit;
+ void *opaque;
+ int is_write;
+
+ int64_t buf_offset; /* start of buffer when writing, end of buffer
+ when reading */
+ int buf_index;
+ int buf_size; /* 0 when writing */
+ uint8_t buf[IO_BUF_SIZE];
+
+ int last_error;
+};
+
+typedef struct QEMUFileStdio
+{
+ FILE *stdio_file;
+ QEMUFile *file;
+} QEMUFileStdio;
+
+typedef struct QEMUFileSocket
+{
+ int fd;
+ QEMUFile *file;
+} QEMUFileSocket;
+
+static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+ QEMUFileSocket *s = opaque;
+ ssize_t len;
+
+ do {
+ len = qemu_recv(s->fd, buf, size, 0);
+ } while (len == -1 && socket_error() == EINTR);
+
+ if (len == -1) {
+ len = -socket_error();
+ }
+
+ return len;
+}
+
+static int socket_close(void *opaque)
+{
+ QEMUFileSocket *s = opaque;
+ g_free(s);
+ return 0;
+}
+
+static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
+{
+ QEMUFileStdio *s = opaque;
+ return fwrite(buf, 1, size, s->stdio_file);
+}
+
+static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+ QEMUFileStdio *s = opaque;
+ FILE *fp = s->stdio_file;
+ int bytes;
+
+ do {
+ clearerr(fp);
+ bytes = fread(buf, 1, size, fp);
+ } while ((bytes == 0) && ferror(fp) && (errno == EINTR));
+ return bytes;
+}
+
+static int stdio_pclose(void *opaque)
+{
+ QEMUFileStdio *s = opaque;
+ int ret;
+ ret = pclose(s->stdio_file);
+ g_free(s);
+ return ret;
+}
+
+static int stdio_fclose(void *opaque)
+{
+ QEMUFileStdio *s = opaque;
+ fclose(s->stdio_file);
+ g_free(s);
+ return 0;
+}
+
+QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
+{
+ QEMUFileStdio *s;
+
+ if (stdio_file == NULL || mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
+ fprintf(stderr, "qemu_popen: Argument validity check failed\n");
+ return NULL;
+ }
+
+ s = g_malloc0(sizeof(QEMUFileStdio));
+
+ s->stdio_file = stdio_file;
+
+ if (mode[0] == 'r') {
+ s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
+ NULL, NULL, NULL);
+ } else {
+ s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
+ NULL, NULL, NULL);
+ }
+ return s->file;
+}
+
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
+{
+ FILE *popen_file;
+
+ popen_file = popen(command, mode);
+ if (popen_file == NULL) {
+ return NULL;
+ }
+
+ return qemu_popen(popen_file, mode);
+}
+
+int qemu_stdio_fd(QEMUFile *f)
+{
+ QEMUFileStdio *p;
+ int fd;
+
+ p = (QEMUFileStdio *)f->opaque;
+ fd = fileno(p->stdio_file);
+
+ return fd;
+}
+
+QEMUFile *qemu_fdopen(int fd, const char *mode)
+{
+ QEMUFileStdio *s;
+
+ if (mode == NULL ||
+ (mode[0] != 'r' && mode[0] != 'w') ||
+ mode[1] != 'b' || mode[2] != 0) {
+ fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
+ return NULL;
+ }
+
+ s = g_malloc0(sizeof(QEMUFileStdio));
+ s->stdio_file = fdopen(fd, mode);
+ if (!s->stdio_file)
+ goto fail;
+
+ if (mode[0] == 'r') {
+ s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
+ NULL, NULL, NULL);
+ } else {
+ s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
+ NULL, NULL, NULL);
+ }
+ return s->file;
+
+fail:
+ g_free(s);
+ return NULL;
+}
+
+QEMUFile *qemu_fopen_socket(int fd)
+{
+ QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
+
+ s->fd = fd;
+ s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
+ NULL, NULL, NULL);
+ return s->file;
+}
+
+static int file_put_buffer(void *opaque, const uint8_t *buf,
+ int64_t pos, int size)
+{
+ QEMUFileStdio *s = opaque;
+ fseek(s->stdio_file, pos, SEEK_SET);
+ return fwrite(buf, 1, size, s->stdio_file);
+}
+
+static int file_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+ QEMUFileStdio *s = opaque;
+ fseek(s->stdio_file, pos, SEEK_SET);
+ return fread(buf, 1, size, s->stdio_file);
+}
+
+QEMUFile *qemu_fopen(const char *filename, const char *mode)
+{
+ QEMUFileStdio *s;
+
+ if (mode == NULL ||
+ (mode[0] != 'r' && mode[0] != 'w') ||
+ mode[1] != 'b' || mode[2] != 0) {
+ fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+ return NULL;
+ }
+
+ s = g_malloc0(sizeof(QEMUFileStdio));
+
+ s->stdio_file = fopen(filename, mode);
+ if (!s->stdio_file) {
+ goto fail;
+ }
+
+ if (mode[0] == 'w') {
+ s->file = qemu_fopen_ops(s, file_put_buffer, NULL, stdio_fclose,
+ NULL, NULL, NULL);
+ } else {
+ s->file = qemu_fopen_ops(s, NULL, file_get_buffer, stdio_fclose,
+ NULL, NULL, NULL);
+ }
+ return s->file;
+fail:
+ g_free(s);
+ return NULL;
+}
+
+QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
+ QEMUFileGetBufferFunc *get_buffer,
+ QEMUFileCloseFunc *close,
+ QEMUFileRateLimit *rate_limit,
+ QEMUFileSetRateLimit *set_rate_limit,
+ QEMUFileGetRateLimit *get_rate_limit)
+{
+ QEMUFile *f;
+
+ f = g_malloc0(sizeof(QEMUFile));
+
+ f->opaque = opaque;
+ f->put_buffer = put_buffer;
+ f->get_buffer = get_buffer;
+ f->close = close;
+ f->rate_limit = rate_limit;
+ f->set_rate_limit = set_rate_limit;
+ f->get_rate_limit = get_rate_limit;
+ f->is_write = 0;
+
+ return f;
+}
+
+int qemu_file_get_error(QEMUFile *f)
+{
+ return f->last_error;
+}
+
+void qemu_file_set_error(QEMUFile *f, int ret)
+{
+ f->last_error = ret;
+}
+
+void qemu_fflush(QEMUFile *f)
+{
+ if (!f->put_buffer) {
+ return;
+ }
+
+ if (f->is_write && f->buf_index > 0) {
+ int len;
+
+ len = f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
+ if (len > 0) {
+ f->buf_offset += f->buf_index;
+ } else {
+ f->last_error = -EINVAL;
+ }
+ f->buf_index = 0;
+ }
+}
+
+static void qemu_fill_buffer(QEMUFile *f)
+{
+ int len;
+ int pending;
+
+ if (!f->get_buffer) {
+ return;
+ }
+
+ if (f->is_write) {
+ abort();
+ }
+
+ pending = f->buf_size - f->buf_index;
+ if (pending > 0) {
+ memmove(f->buf, f->buf + f->buf_index, pending);
+ }
+ f->buf_index = 0;
+ f->buf_size = pending;
+
+ len = f->get_buffer(f->opaque, f->buf + pending, f->buf_offset,
+ IO_BUF_SIZE - pending);
+ if (len > 0) {
+ f->buf_size += len;
+ f->buf_offset += len;
+ } else if (len != -EAGAIN) {
+ f->last_error = len;
+ }
+}
+
+int qemu_fclose(QEMUFile *f)
+{
+ int ret = 0;
+ qemu_fflush(f);
+ if (f->close) {
+ ret = f->close(f->opaque);
+ }
+ g_free(f);
+ return ret;
+}
+
+void qemu_file_put_notify(QEMUFile *f)
+{
+ f->put_buffer(f->opaque, NULL, 0, 0);
+}
+
+void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
+{
+ int l;
+
+ if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+ fprintf(stderr,
+ "Attempted to write to buffer while read buffer is not empty\n");
+ abort();
+ }
+
+ while (!f->last_error && size > 0) {
+ l = IO_BUF_SIZE - f->buf_index;
+ if (l > size) {
+ l = size;
+ }
+ memcpy(f->buf + f->buf_index, buf, l);
+ f->is_write = 1;
+ f->buf_index += l;
+ buf += l;
+ size -= l;
+ if (f->buf_index >= IO_BUF_SIZE) {
+ qemu_fflush(f);
+ }
+ }
+}
+
+void qemu_put_byte(QEMUFile *f, int v)
+{
+ if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+ fprintf(stderr,
+ "Attempted to write to buffer while read buffer is not empty\n");
+ abort();
+ }
+
+ f->buf[f->buf_index++] = v;
+ f->is_write = 1;
+ if (f->buf_index >= IO_BUF_SIZE) {
+ qemu_fflush(f);
+ }
+}
+
+void qemu_file_skip(QEMUFile *f, int size)
+{
+ if (f->buf_index + size <= f->buf_size) {
+ f->buf_index += size;
+ }
+}
+
+int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+ int pending;
+ int index;
+
+ if (f->is_write) {
+ abort();
+ }
+
+ index = f->buf_index + offset;
+ pending = f->buf_size - index;
+ if (pending < size) {
+ qemu_fill_buffer(f);
+ index = f->buf_index + offset;
+ pending = f->buf_size - index;
+ }
+
+ if (pending <= 0) {
+ return 0;
+ }
+ if (size > pending) {
+ size = pending;
+ }
+
+ memcpy(buf, f->buf + index, size);
+ return size;
+}
+
+int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+{
+ int pending = size;
+ int done = 0;
+
+ while (pending > 0) {
+ int res;
+
+ res = qemu_peek_buffer(f, buf, pending, 0);
+ if (res == 0) {
+ return done;
+ }
+ qemu_file_skip(f, res);
+ buf += res;
+ pending -= res;
+ done += res;
+ }
+ return done;
+}
+
+int qemu_peek_byte(QEMUFile *f, int offset)
+{
+ int index = f->buf_index + offset;
+
+ if (f->is_write) {
+ abort();
+ }
+
+ if (index >= f->buf_size) {
+ qemu_fill_buffer(f);
+ index = f->buf_index + offset;
+ if (index >= f->buf_size) {
+ return 0;
+ }
+ }
+ return f->buf[index];
+}
+
+int qemu_get_byte(QEMUFile *f)
+{
+ int result;
+
+ result = qemu_peek_byte(f, 0);
+ qemu_file_skip(f, 1);
+ return result;
+}
+
+int64_t qemu_ftell(QEMUFile *f)
+{
+ return f->buf_offset - f->buf_size + f->buf_index;
+}
+
+int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence)
+{
+ if (whence == SEEK_SET) {
+ /* nothing to do */
+ } else if (whence == SEEK_CUR) {
+ pos += qemu_ftell(f);
+ } else {
+ /* SEEK_END not supported */
+ return -1;
+ }
+ if (f->put_buffer) {
+ qemu_fflush(f);
+ f->buf_offset = pos;
+ } else {
+ f->buf_offset = pos;
+ f->buf_index = 0;
+ f->buf_size = 0;
+ }
+ return pos;
+}
+
+int qemu_file_rate_limit(QEMUFile *f)
+{
+ if (f->rate_limit) {
+ return f->rate_limit(f->opaque);
+ }
+
+ return 0;
+}
+
+int64_t qemu_file_get_rate_limit(QEMUFile *f)
+{
+ if (f->get_rate_limit) {
+ return f->get_rate_limit(f->opaque);
+ }
+
+ return 0;
+}
+
+int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate)
+{
+ /* any failed or completed migration keeps its state to allow probing of
+ * migration data, but has no associated file anymore */
+ if (f && f->set_rate_limit) {
+ return f->set_rate_limit(f->opaque, new_rate);
+ }
+
+ return 0;
+}
+
+void qemu_put_be16(QEMUFile *f, unsigned int v)
+{
+ qemu_put_byte(f, v >> 8);
+ qemu_put_byte(f, v);
+}
+
+void qemu_put_be32(QEMUFile *f, unsigned int v)
+{
+ qemu_put_byte(f, v >> 24);
+ qemu_put_byte(f, v >> 16);
+ qemu_put_byte(f, v >> 8);
+ qemu_put_byte(f, v);
+}
+
+void qemu_put_be64(QEMUFile *f, uint64_t v)
+{
+ qemu_put_be32(f, v >> 32);
+ qemu_put_be32(f, v);
+}
+
+unsigned int qemu_get_be16(QEMUFile *f)
+{
+ unsigned int v;
+ v = qemu_get_byte(f) << 8;
+ v |= qemu_get_byte(f);
+ return v;
+}
+
+unsigned int qemu_get_be32(QEMUFile *f)
+{
+ unsigned int v;
+ v = qemu_get_byte(f) << 24;
+ v |= qemu_get_byte(f) << 16;
+ v |= qemu_get_byte(f) << 8;
+ v |= qemu_get_byte(f);
+ return v;
+}
+
+uint64_t qemu_get_be64(QEMUFile *f)
+{
+ uint64_t v;
+ v = (uint64_t)qemu_get_be32(f) << 32;
+ v |= qemu_get_be32(f);
+ return v;
+}
diff --git a/savevm.c b/savevm.c
index f01838f..0b8e46f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -156,228 +156,6 @@ void qemu_announce_self(void)
/***********************************************************/
/* savevm/loadvm support */
-#define IO_BUF_SIZE 32768
-
-struct QEMUFile {
- QEMUFilePutBufferFunc *put_buffer;
- QEMUFileGetBufferFunc *get_buffer;
- QEMUFileCloseFunc *close;
- QEMUFileRateLimit *rate_limit;
- QEMUFileSetRateLimit *set_rate_limit;
- QEMUFileGetRateLimit *get_rate_limit;
- void *opaque;
- int is_write;
-
- int64_t buf_offset; /* start of buffer when writing, end of buffer
- when reading */
- int buf_index;
- int buf_size; /* 0 when writing */
- uint8_t buf[IO_BUF_SIZE];
-
- int last_error;
-};
-
-typedef struct QEMUFileStdio
-{
- FILE *stdio_file;
- QEMUFile *file;
-} QEMUFileStdio;
-
-typedef struct QEMUFileSocket
-{
- int fd;
- QEMUFile *file;
-} QEMUFileSocket;
-
-static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
- QEMUFileSocket *s = opaque;
- ssize_t len;
-
- do {
- len = qemu_recv(s->fd, buf, size, 0);
- } while (len == -1 && socket_error() == EINTR);
-
- if (len == -1)
- len = -socket_error();
-
- return len;
-}
-
-static int socket_close(void *opaque)
-{
- QEMUFileSocket *s = opaque;
- g_free(s);
- return 0;
-}
-
-static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
-{
- QEMUFileStdio *s = opaque;
- return fwrite(buf, 1, size, s->stdio_file);
-}
-
-static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
- QEMUFileStdio *s = opaque;
- FILE *fp = s->stdio_file;
- int bytes;
-
- do {
- clearerr(fp);
- bytes = fread(buf, 1, size, fp);
- } while ((bytes == 0) && ferror(fp) && (errno == EINTR));
- return bytes;
-}
-
-static int stdio_pclose(void *opaque)
-{
- QEMUFileStdio *s = opaque;
- int ret;
- ret = pclose(s->stdio_file);
- g_free(s);
- return ret;
-}
-
-static int stdio_fclose(void *opaque)
-{
- QEMUFileStdio *s = opaque;
- fclose(s->stdio_file);
- g_free(s);
- return 0;
-}
-
-QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
-{
- QEMUFileStdio *s;
-
- if (stdio_file == NULL || mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
- fprintf(stderr, "qemu_popen: Argument validity check failed\n");
- return NULL;
- }
-
- s = g_malloc0(sizeof(QEMUFileStdio));
-
- s->stdio_file = stdio_file;
-
- if(mode[0] == 'r') {
- s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
- NULL, NULL, NULL);
- } else {
- s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
- NULL, NULL, NULL);
- }
- return s->file;
-}
-
-QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
-{
- FILE *popen_file;
-
- popen_file = popen(command, mode);
- if(popen_file == NULL) {
- return NULL;
- }
-
- return qemu_popen(popen_file, mode);
-}
-
-int qemu_stdio_fd(QEMUFile *f)
-{
- QEMUFileStdio *p;
- int fd;
-
- p = (QEMUFileStdio *)f->opaque;
- fd = fileno(p->stdio_file);
-
- return fd;
-}
-
-QEMUFile *qemu_fdopen(int fd, const char *mode)
-{
- QEMUFileStdio *s;
-
- if (mode == NULL ||
- (mode[0] != 'r' && mode[0] != 'w') ||
- mode[1] != 'b' || mode[2] != 0) {
- fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
- return NULL;
- }
-
- s = g_malloc0(sizeof(QEMUFileStdio));
- s->stdio_file = fdopen(fd, mode);
- if (!s->stdio_file)
- goto fail;
-
- if(mode[0] == 'r') {
- s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
- } else {
- s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
- }
- return s->file;
-
-fail:
- g_free(s);
- return NULL;
-}
-
-QEMUFile *qemu_fopen_socket(int fd)
-{
- QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
-
- s->fd = fd;
- s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
- NULL, NULL, NULL);
- return s->file;
-}
-
-static int file_put_buffer(void *opaque, const uint8_t *buf,
- int64_t pos, int size)
-{
- QEMUFileStdio *s = opaque;
- fseek(s->stdio_file, pos, SEEK_SET);
- return fwrite(buf, 1, size, s->stdio_file);
-}
-
-static int file_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
-{
- QEMUFileStdio *s = opaque;
- fseek(s->stdio_file, pos, SEEK_SET);
- return fread(buf, 1, size, s->stdio_file);
-}
-
-QEMUFile *qemu_fopen(const char *filename, const char *mode)
-{
- QEMUFileStdio *s;
-
- if (mode == NULL ||
- (mode[0] != 'r' && mode[0] != 'w') ||
- mode[1] != 'b' || mode[2] != 0) {
- fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
- return NULL;
- }
-
- s = g_malloc0(sizeof(QEMUFileStdio));
-
- s->stdio_file = fopen(filename, mode);
- if (!s->stdio_file)
- goto fail;
-
- if(mode[0] == 'w') {
- s->file = qemu_fopen_ops(s, file_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
- } else {
- s->file = qemu_fopen_ops(s, NULL, file_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
- }
- return s->file;
-fail:
- g_free(s);
- return NULL;
-}
-
static int block_put_buffer(void *opaque, const uint8_t *buf,
int64_t pos, int size)
{
@@ -403,317 +181,6 @@ static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL, NULL);
}
-QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
- QEMUFileGetBufferFunc *get_buffer,
- QEMUFileCloseFunc *close,
- QEMUFileRateLimit *rate_limit,
- QEMUFileSetRateLimit *set_rate_limit,
- QEMUFileGetRateLimit *get_rate_limit)
-{
- QEMUFile *f;
-
- f = g_malloc0(sizeof(QEMUFile));
-
- f->opaque = opaque;
- f->put_buffer = put_buffer;
- f->get_buffer = get_buffer;
- f->close = close;
- f->rate_limit = rate_limit;
- f->set_rate_limit = set_rate_limit;
- f->get_rate_limit = get_rate_limit;
- f->is_write = 0;
-
- return f;
-}
-
-int qemu_file_get_error(QEMUFile *f)
-{
- return f->last_error;
-}
-
-void qemu_file_set_error(QEMUFile *f, int ret)
-{
- f->last_error = ret;
-}
-
-void qemu_fflush(QEMUFile *f)
-{
- if (!f->put_buffer)
- return;
-
- if (f->is_write && f->buf_index > 0) {
- int len;
-
- len = f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
- if (len > 0)
- f->buf_offset += f->buf_index;
- else
- f->last_error = -EINVAL;
- f->buf_index = 0;
- }
-}
-
-static void qemu_fill_buffer(QEMUFile *f)
-{
- int len;
- int pending;
-
- if (!f->get_buffer)
- return;
-
- if (f->is_write)
- abort();
-
- pending = f->buf_size - f->buf_index;
- if (pending > 0) {
- memmove(f->buf, f->buf + f->buf_index, pending);
- }
- f->buf_index = 0;
- f->buf_size = pending;
-
- len = f->get_buffer(f->opaque, f->buf + pending, f->buf_offset,
- IO_BUF_SIZE - pending);
- if (len > 0) {
- f->buf_size += len;
- f->buf_offset += len;
- } else if (len != -EAGAIN)
- f->last_error = len;
-}
-
-int qemu_fclose(QEMUFile *f)
-{
- int ret = 0;
- qemu_fflush(f);
- if (f->close)
- ret = f->close(f->opaque);
- g_free(f);
- return ret;
-}
-
-void qemu_file_put_notify(QEMUFile *f)
-{
- f->put_buffer(f->opaque, NULL, 0, 0);
-}
-
-void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
-{
- int l;
-
- if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
- fprintf(stderr,
- "Attempted to write to buffer while read buffer is not empty\n");
- abort();
- }
-
- while (!f->last_error && size > 0) {
- l = IO_BUF_SIZE - f->buf_index;
- if (l > size)
- l = size;
- memcpy(f->buf + f->buf_index, buf, l);
- f->is_write = 1;
- f->buf_index += l;
- buf += l;
- size -= l;
- if (f->buf_index >= IO_BUF_SIZE)
- qemu_fflush(f);
- }
-}
-
-void qemu_put_byte(QEMUFile *f, int v)
-{
- if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
- fprintf(stderr,
- "Attempted to write to buffer while read buffer is not empty\n");
- abort();
- }
-
- f->buf[f->buf_index++] = v;
- f->is_write = 1;
- if (f->buf_index >= IO_BUF_SIZE)
- qemu_fflush(f);
-}
-
-static void qemu_file_skip(QEMUFile *f, int size)
-{
- if (f->buf_index + size <= f->buf_size) {
- f->buf_index += size;
- }
-}
-
-static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
-{
- int pending;
- int index;
-
- if (f->is_write) {
- abort();
- }
-
- index = f->buf_index + offset;
- pending = f->buf_size - index;
- if (pending < size) {
- qemu_fill_buffer(f);
- index = f->buf_index + offset;
- pending = f->buf_size - index;
- }
-
- if (pending <= 0) {
- return 0;
- }
- if (size > pending) {
- size = pending;
- }
-
- memcpy(buf, f->buf + index, size);
- return size;
-}
-
-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
-{
- int pending = size;
- int done = 0;
-
- while (pending > 0) {
- int res;
-
- res = qemu_peek_buffer(f, buf, pending, 0);
- if (res == 0) {
- return done;
- }
- qemu_file_skip(f, res);
- buf += res;
- pending -= res;
- done += res;
- }
- return done;
-}
-
-static int qemu_peek_byte(QEMUFile *f, int offset)
-{
- int index = f->buf_index + offset;
-
- if (f->is_write) {
- abort();
- }
-
- if (index >= f->buf_size) {
- qemu_fill_buffer(f);
- index = f->buf_index + offset;
- if (index >= f->buf_size) {
- return 0;
- }
- }
- return f->buf[index];
-}
-
-int qemu_get_byte(QEMUFile *f)
-{
- int result;
-
- result = qemu_peek_byte(f, 0);
- qemu_file_skip(f, 1);
- return result;
-}
-
-int64_t qemu_ftell(QEMUFile *f)
-{
- return f->buf_offset - f->buf_size + f->buf_index;
-}
-
-int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence)
-{
- if (whence == SEEK_SET) {
- /* nothing to do */
- } else if (whence == SEEK_CUR) {
- pos += qemu_ftell(f);
- } else {
- /* SEEK_END not supported */
- return -1;
- }
- if (f->put_buffer) {
- qemu_fflush(f);
- f->buf_offset = pos;
- } else {
- f->buf_offset = pos;
- f->buf_index = 0;
- f->buf_size = 0;
- }
- return pos;
-}
-
-int qemu_file_rate_limit(QEMUFile *f)
-{
- if (f->rate_limit)
- return f->rate_limit(f->opaque);
-
- return 0;
-}
-
-int64_t qemu_file_get_rate_limit(QEMUFile *f)
-{
- if (f->get_rate_limit)
- return f->get_rate_limit(f->opaque);
-
- return 0;
-}
-
-int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate)
-{
- /* any failed or completed migration keeps its state to allow probing of
- * migration data, but has no associated file anymore */
- if (f && f->set_rate_limit)
- return f->set_rate_limit(f->opaque, new_rate);
-
- return 0;
-}
-
-void qemu_put_be16(QEMUFile *f, unsigned int v)
-{
- qemu_put_byte(f, v >> 8);
- qemu_put_byte(f, v);
-}
-
-void qemu_put_be32(QEMUFile *f, unsigned int v)
-{
- qemu_put_byte(f, v >> 24);
- qemu_put_byte(f, v >> 16);
- qemu_put_byte(f, v >> 8);
- qemu_put_byte(f, v);
-}
-
-void qemu_put_be64(QEMUFile *f, uint64_t v)
-{
- qemu_put_be32(f, v >> 32);
- qemu_put_be32(f, v);
-}
-
-unsigned int qemu_get_be16(QEMUFile *f)
-{
- unsigned int v;
- v = qemu_get_byte(f) << 8;
- v |= qemu_get_byte(f);
- return v;
-}
-
-unsigned int qemu_get_be32(QEMUFile *f)
-{
- unsigned int v;
- v = qemu_get_byte(f) << 24;
- v |= qemu_get_byte(f) << 16;
- v |= qemu_get_byte(f) << 8;
- v |= qemu_get_byte(f);
- return v;
-}
-
-uint64_t qemu_get_be64(QEMUFile *f)
-{
- uint64_t v;
- v = (uint64_t)qemu_get_be32(f) << 32;
- v |= qemu_get_be32(f);
- return v;
-}
-
-
/* timer */
void qemu_put_timer(QEMUFile *f, QEMUTimer *ts)
@@ -736,7 +203,6 @@ void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
}
}
-
/* bool */
static int get_bool(QEMUFile *f, void *pv, size_t size)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 05/10] qapi: test cases for QEMUFile input/output visitors
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (3 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 04/10] savevm: move QEMUFile interfaces into qemu-file.c Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 06/10] qemu-file: add QEMUFile<->visitor lookup routines Michael Roth
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 2 +-
test-visitor.c | 404 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 405 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index f63fc02..50803b0 100644
--- a/Makefile
+++ b/Makefile
@@ -204,7 +204,7 @@ qmp-marshal.c: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -m -o "." < $<, " GEN $@")
test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
-test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o qapi/qemu-file-input-visitor.o qapi/qemu-file-output-visitor.o qemu-file.o
test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c test-qmp-commands.h) $(qapi-obj-y)
test-qmp-commands: test-qmp-commands.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o
diff --git a/test-visitor.c b/test-visitor.c
index 847ce14..15a9ce9 100644
--- a/test-visitor.c
+++ b/test-visitor.c
@@ -4,6 +4,10 @@
#include "test-qapi-types.h"
#include "test-qapi-visit.h"
#include "qemu-objects.h"
+#include "qapi/qemu-file-output-visitor.h"
+#include "qapi/qemu-file-input-visitor.h"
+#include "hw/hw.h"
+#include "qemu-common.h"
typedef struct TestStruct
{
@@ -323,6 +327,404 @@ static void test_nested_enums(void)
qapi_free_NestedEnumsOne(nested_enums_cpy);
}
+#define TEST_QEMU_FILE_PATH "/tmp/test_qemu_file_visitors"
+
+typedef struct QEMUFileValue {
+ union {
+ bool boolean;
+ double number;
+ uint8_t u8;
+ uint16_t u16;
+ uint32_t u32;
+ uint64_t u64;
+ int8_t s8;
+ int16_t s16;
+ int32_t s32;
+ int64_t s64;
+ uintmax_t umax;
+ } value;
+ union {
+ uint8_t u8[32];
+ uint16_t u16[32];
+ uint32_t u32[32];
+ uint64_t u64[32];
+ int8_t s8[32];
+ int16_t s16[32];
+ int32_t s32[32];
+ int64_t s64[32];
+ uintmax_t umax[32];
+ } array;
+ size_t array_len;
+ enum {
+ QFV_BOOL = 0,
+ QFV_NUMBER,
+ QFV_U8,
+ QFV_U16,
+ QFV_U32,
+ QFV_U64,
+ QFV_S8,
+ QFV_S16,
+ QFV_S32,
+ QFV_S64,
+ QFV_U8_ARRAY,
+ QFV_EOL,
+ } type;
+} QEMUFileValue;
+
+QEMUFileValue qfvalues[] = {
+ { .value.boolean = true, .type = QFV_BOOL },
+ { .value.boolean = false, .type = QFV_BOOL },
+ { .value.number = 3.14159265, .type = QFV_NUMBER },
+ { .value.u8 = 0, .type = QFV_U8 },
+ { .value.u8 = 1, .type = QFV_U8 },
+ { .value.u8 = 128, .type = QFV_U8 },
+ { .value.u8 = 255u, .type = QFV_U8 },
+ { .value.u16 = 0, .type = QFV_U16 },
+ { .value.u16 = 1, .type = QFV_U16 },
+ { .value.u16 = 32768, .type = QFV_U16 },
+ { .value.u16 = 65535u, .type = QFV_U16 },
+ { .value.u32 = 0, .type = QFV_U32 },
+ { .value.u32 = 1, .type = QFV_U32 },
+ { .value.u32 = 2147483648, .type = QFV_U32 },
+ { .value.u32 = 4294967295u, .type = QFV_U32 },
+ { .value.u64 = 0, .type = QFV_U64 },
+ { .value.u64 = 1, .type = QFV_U64 },
+ { .value.u64 = 9223372036854775808u, .type = QFV_U64 },
+ { .value.u64 = 18446744073709551615u, .type = QFV_U64 },
+ { .value.s8 = 0, .type = QFV_S8 },
+ { .value.s8 = 1, .type = QFV_S8 },
+ { .value.s8 = 128, .type = QFV_S8 },
+ { .value.s8 = -1, .type = QFV_S8 },
+ { .value.s16 = 0, .type = QFV_S16 },
+ { .value.s16 = 1, .type = QFV_S16 },
+ { .value.s16 = 32768, .type = QFV_S16 },
+ { .value.s16 = -1, .type = QFV_S16 },
+ { .value.s32 = 0, .type = QFV_S32 },
+ { .value.s32 = 1, .type = QFV_S32 },
+ { .value.s32 = 2147483648, .type = QFV_S32 },
+ { .value.s32 = -1, .type = QFV_S32 },
+ { .value.s64 = 0, .type = QFV_S64 },
+ { .value.s64 = 1, .type = QFV_S64 },
+ { .value.s64 = 9223372036854775808u, .type = QFV_S64 },
+ { .value.s64 = -1, .type = QFV_S64 },
+ { .array.u8 = { }, .array_len = 0, .type = QFV_U8_ARRAY },
+ { .array.u8 = { 1, 2, 3, 4 }, .array_len = 4, .type = QFV_U8_ARRAY },
+ { .type = QFV_EOL }
+};
+
+static void qfv_process(QEMUFileValue *qfv, bool visitor, bool write,
+ void *opaque)
+{
+ int i;
+ void *ptr;
+
+ switch (qfv->type) {
+ case QFV_BOOL:
+ if (visitor) {
+ visit_type_bool(opaque, &qfv->value.boolean, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_byte(opaque, qfv->value.boolean);
+ } else {
+ qfv->value.boolean = qemu_get_byte(opaque);
+ }
+ }
+ break;
+ case QFV_NUMBER:
+ if (visitor) {
+ visit_type_number(opaque, &qfv->value.number, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be64s(opaque, (uint64_t *)&qfv->value.number);
+ } else {
+ qemu_get_be64s(opaque, (uint64_t *)&qfv->value.number);
+ }
+ }
+ break;
+ case QFV_U8:
+ if (visitor) {
+ visit_type_uint8(opaque, &qfv->value.u8, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_byte(opaque, qfv->value.u8);
+ } else {
+ qfv->value.u8 = qemu_get_byte(opaque);
+ }
+ }
+ break;
+ case QFV_U16:
+ if (visitor) {
+ visit_type_uint16(opaque, &qfv->value.u16, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be16(opaque, qfv->value.u16);
+ } else {
+ qfv->value.u16 = qemu_get_be16(opaque);
+ }
+ }
+ break;
+ case QFV_U32:
+ if (visitor) {
+ visit_type_uint32(opaque, &qfv->value.u32, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be32(opaque, qfv->value.u32);
+ } else {
+ qfv->value.u32 = qemu_get_be32(opaque);
+ }
+ }
+ break;
+ case QFV_U64:
+ if (visitor) {
+ visit_type_uint64(opaque, &qfv->value.u64, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be64(opaque, qfv->value.u64);
+ } else {
+ qfv->value.u64 = qemu_get_be64(opaque);
+ }
+ }
+ break;
+ case QFV_S8:
+ if (visitor) {
+ visit_type_int8(opaque, &qfv->value.s8, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_byte(opaque, qfv->value.s8);
+ } else {
+ qfv->value.s8 = qemu_get_byte(opaque);
+ }
+ }
+ break;
+ case QFV_S16:
+ if (visitor) {
+ visit_type_int16(opaque, &qfv->value.s16, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be16(opaque, qfv->value.s16);
+ } else {
+ qfv->value.s16 = qemu_get_be16(opaque);
+ }
+ }
+ break;
+ case QFV_S32:
+ if (visitor) {
+ visit_type_int32(opaque, &qfv->value.s32, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be32(opaque, qfv->value.s32);
+ } else {
+ qfv->value.s32 = qemu_get_be32(opaque);
+ }
+ }
+ break;
+ case QFV_S64:
+ if (visitor) {
+ visit_type_int64(opaque, &qfv->value.s64, NULL, NULL);
+ } else {
+ if (write) {
+ qemu_put_be64(opaque, qfv->value.s64);
+ } else {
+ qfv->value.s64 = qemu_get_be64(opaque);
+ }
+ }
+ break;
+ case QFV_U8_ARRAY:
+ if (visitor) {
+ ptr = qfv->array.u8;
+ visit_start_array(opaque, (void **)&ptr, NULL,
+ qfv->array_len, sizeof(uint8_t), NULL);
+ for (i = 0; i < qfv->array_len; i++, visit_next_array(opaque, NULL)) {
+ visit_type_uint8(opaque, &((uint8_t *)ptr)[i], NULL, NULL);
+ }
+ visit_end_array(opaque, NULL);
+ } else {
+ for (i = 0; i < qfv->array_len; i++) {
+ if (write) {
+ qemu_put_byte(opaque, qfv->array.u8[i]);
+ } else {
+ qfv->array.u8[i] = qemu_get_byte(opaque);
+ }
+ }
+ }
+ break;
+ default:
+ return;
+ }
+}
+
+static void qfv_visitor_write(QEMUFileValue *qfv, Visitor *v)
+{
+ qfv_process(qfv, true, true, v);
+}
+
+static void qfv_visitor_read(QEMUFileValue *qfv, Visitor *v)
+{
+ qfv_process(qfv, true, false, v);
+}
+
+static void qfv_write(QEMUFileValue *qfv, QEMUFile *f)
+{
+ qfv_process(qfv, false, true, f);
+}
+
+static void qfv_read(QEMUFileValue *qfv, QEMUFile *f)
+{
+ qfv_process(qfv, false, false, f);
+}
+
+static void test_qemu_file_in_visitor(void)
+{
+ QEMUFile *f1, *f2;
+ QemuFileInputVisitor *qfi;
+ QemuFileOutputVisitor *qfo;
+ Visitor *v;
+ QEMUFileValue qfval1, qfval2;
+ int i, j;
+ TestStruct ts, *pts;
+ TestStructList *lts;
+
+ /* write our test scalars/arrays */
+ f1 = qemu_fopen(TEST_QEMU_FILE_PATH, "wb");
+ g_assert(f1);
+ qfo = qemu_file_output_visitor_new(f1);
+ v = qemu_file_output_get_visitor(qfo);
+ for (i = 0; qfvalues[i].type != QFV_EOL; i++) {
+ qfv_write(&qfvalues[i], f1);
+ }
+ /* write our test struct/list. qemu_put_* interfaces have
+ * no analogue for this and instead rely on byte arrays,
+ * so we'll write this using a visitor and simply test
+ * visitor input/output compatibility
+ */
+ /* write a simple struct */
+ ts.x = 42;
+ ts.y = 43;
+ pts = &ts;
+ visit_type_TestStruct(v, &pts, NULL, NULL);
+ /* throw in a linked list as well */
+ lts = g_malloc0(sizeof(*lts));
+ lts->value = g_malloc0(sizeof(TestStruct));
+ lts->value->x = 44;
+ lts->value->y = 45;
+ lts->next = g_malloc0(sizeof(*lts));
+ lts->next->value = g_malloc0(sizeof(TestStruct));
+ lts->next->value->x = 46;
+ lts->next->value->y = 47;
+ visit_type_TestStructList(v, <s, NULL, NULL);
+ g_free(lts->next->value);
+ g_free(lts->next);
+ g_free(lts->value);
+ g_free(lts);
+
+ qemu_file_output_visitor_cleanup(qfo);
+ qemu_fclose(f1);
+
+ /* make sure qemu_get_be* and input visitor read same/correct input */
+ f1 = qemu_fopen(TEST_QEMU_FILE_PATH, "rb");
+ f2 = qemu_fopen(TEST_QEMU_FILE_PATH, "rb");
+ qfi = qemu_file_input_visitor_new(f2);
+ g_assert(qfi);
+ v = qemu_file_input_get_visitor(qfi);
+ g_assert(v);
+ for (i = 0; qfvalues[i].type != QFV_EOL; i++) {
+ qfval1.value.umax = qfval2.value.umax = 0;
+ memset(qfval1.array.umax, 0, sizeof(qfval1.array.umax));
+ memset(qfval2.array.umax, 0, sizeof(qfval2.array.umax));
+ qfval1.type = qfval2.type = qfvalues[i].type;
+ qfval1.array_len = qfval2.array_len = qfvalues[i].array_len;
+ qfv_read(&qfval1, f1);
+ qfv_visitor_read(&qfval2, v);
+ if (qfvalues[i].type >= QFV_U8_ARRAY) {
+ for (j = 0; j < qfvalues[i].array_len; j++) {
+ g_assert(qfval1.array.u8[j] == qfval2.array.u8[j]);
+ g_assert(qfval2.array.u8[j] == qfvalues[i].array.u8[j]);
+ }
+ } else {
+ g_assert(qfval1.value.umax == qfval2.value.umax);
+ g_assert(qfval2.value.umax == qfvalues[i].value.umax);
+ }
+ }
+ qemu_file_input_visitor_cleanup(qfi);
+ qemu_fclose(f1);
+ qemu_fclose(f2);
+ unlink(TEST_QEMU_FILE_PATH);
+}
+
+static void test_qemu_file_out_visitor(void)
+{
+ QEMUFile *f;
+ QemuFileOutputVisitor *qfo;
+ Visitor *v;
+ QEMUFileValue qfval1;
+ int i, j;
+ TestStruct ts, *pts;
+ TestStructList *lts;
+
+ /* write test scalars/arrays using an output visitor */
+ f = qemu_fopen(TEST_QEMU_FILE_PATH, "wb");
+ g_assert(f);
+ qfo = qemu_file_output_visitor_new(f);
+ g_assert(qfo);
+ v = qemu_file_output_get_visitor(qfo);
+ g_assert(v);
+ for (i = 0; qfvalues[i].type != QFV_EOL; i++) {
+ qfv_visitor_write(&qfvalues[i], v);
+ }
+ /* write a simple struct */
+ ts.x = 42;
+ ts.y = 43;
+ pts = &ts;
+ visit_type_TestStruct(v, &pts, NULL, NULL);
+ /* throw in a linked list as well */
+ lts = g_malloc0(sizeof(*lts));
+ lts->value = g_malloc0(sizeof(TestStruct));
+ lts->value->x = 44;
+ lts->value->y = 45;
+ lts->next = g_malloc0(sizeof(*lts));
+ lts->next->value = g_malloc0(sizeof(TestStruct));
+ lts->next->value->x = 46;
+ lts->next->value->y = 47;
+ visit_type_TestStructList(v, <s, NULL, NULL);
+ g_free(lts->next->value);
+ g_free(lts->next);
+ g_free(lts->value);
+ g_free(lts);
+
+ qemu_file_output_visitor_cleanup(qfo);
+ qemu_fclose(f);
+
+ /* make sure output visitor wrote the expected values */
+ f = qemu_fopen(TEST_QEMU_FILE_PATH, "rb");
+ g_assert(f);
+ for (i = 0; qfvalues[i].type != QFV_EOL; i++) {
+ qfval1.type = qfvalues[i].type;
+ qfval1.value.umax = 0;
+ memset(qfval1.array.umax, 0, sizeof(qfval1.array.umax));
+ qfval1.array_len = qfvalues[i].array_len;
+
+ qfv_read(&qfval1, f);
+ if (qfvalues[i].type >= QFV_U8_ARRAY) {
+ for (j = 0; j < qfvalues[i].array_len; j++) {
+ g_assert(qfval1.array.u8[j] == qfvalues[i].array.u8[j]);
+ }
+ } else {
+ g_assert(qfval1.value.umax == qfvalues[i].value.umax);
+ }
+ }
+ /* test the struct */
+ g_assert(qemu_get_be64(f) == ts.x);
+ g_assert(qemu_get_be64(f) == ts.y);
+ /* test the linked list */
+ g_assert(qemu_get_be64(f) == 44);
+ g_assert(qemu_get_be64(f) == 45);
+ g_assert(qemu_get_be64(f) == 46);
+ g_assert(qemu_get_be64(f) == 47);
+
+ qemu_fclose(f);
+ unlink(TEST_QEMU_FILE_PATH);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -331,6 +733,8 @@ int main(int argc, char **argv)
g_test_add_func("/0.15/nested_structs", test_nested_structs);
g_test_add_func("/0.15/enums", test_enums);
g_test_add_func("/0.15/nested_enums", test_nested_enums);
+ g_test_add_func("/1.0/qemu_file_input_visitor", test_qemu_file_in_visitor);
+ g_test_add_func("/1.0/qemu_file_output_visitor", test_qemu_file_out_visitor);
g_test_run();
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 06/10] qemu-file: add QEMUFile<->visitor lookup routines
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (4 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 05/10] qapi: test cases for QEMUFile input/output visitors Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 07/10] trace: qemu_(put|get)_(byte|buffer) events Michael Roth
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
This interface is to allow switching between Visitor-based and direct
QEMUFile usage to serialize/de-serialize fields. Once we're passed the
transitionary stages and all requisite interfaces are converted to
accepting Visitor objects, we can drop this dual approach and begin
utilizing non-QEMUFile-based Visitors.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/hw.h | 7 ++++
qemu-file.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 110 insertions(+), 1 deletions(-)
diff --git a/hw/hw.h b/hw/hw.h
index a793974..5c0eb65 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -10,6 +10,13 @@
#include "ioport.h"
#include "irq.h"
+#include "qemu-queue.h"
+#include "qapi/qapi-visit-core.h"
+#include "qapi/qemu-file-output-visitor.h"
+#include "qapi/qemu-file-input-visitor.h"
+
+Visitor *qemu_file_get_visitor(QEMUFile *f);
+QEMUFile *qemu_file_from_visitor(Visitor *v);
/* VM Load/Save */
diff --git a/qemu-file.c b/qemu-file.c
index a4aee9a..cdfe8a3 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -25,6 +25,8 @@
#include "qemu-common.h"
#include "qemu_socket.h"
#include "hw/hw.h"
+#include "qapi/qemu-file-output-visitor.h"
+#include "qapi/qemu-file-input-visitor.h"
#define IO_BUF_SIZE 32768
@@ -59,6 +61,94 @@ typedef struct QEMUFileSocket
QEMUFile *file;
} QEMUFileSocket;
+/* TODO: temporary mechanism to support existing function signatures by
+ * creating a 1-to-1 mapping between QEMUFile's and the actual Visitor type.
+ * In the case of QemuFileOutputVisitor/QemuFileInputVisitor, the QEMUFile
+ * arg corresponds to the handle used by the visitor for reads/writes. For
+ * other visitors, the QEMUFile will serve purely as a key.
+ *
+ * Once all interfaces are converted to using Visitor directly, we will
+ * remove this lookup logic and pass the Visitor to the registered save/load
+ * functions directly.
+ */
+
+/* Clean up a *Visitor object associated with the QEMUFile */
+typedef int (QEMUFileVisitorCleanupFunc)(void *opaque);
+
+typedef struct VisitorNode {
+ void *opaque;
+ Visitor *visitor;
+ QEMUFile *file;
+ QEMUFileVisitorCleanupFunc *cleanup;
+ QTAILQ_ENTRY(VisitorNode) entry;
+} VisitorNode;
+
+static QTAILQ_HEAD(, VisitorNode) qemu_file_visitors =
+ QTAILQ_HEAD_INITIALIZER(qemu_file_visitors);
+
+Visitor *qemu_file_get_visitor(QEMUFile *f)
+{
+ VisitorNode *vnode;
+ QTAILQ_FOREACH(vnode, &qemu_file_visitors, entry) {
+ if (vnode->file == f) {
+ return vnode->visitor;
+ }
+ }
+ /* all QEMUFile instances should have an associated visitor */
+ assert(false);
+}
+
+QEMUFile *qemu_file_from_visitor(Visitor *v)
+{
+ VisitorNode *vnode;
+ QTAILQ_FOREACH(vnode, &qemu_file_visitors, entry) {
+ if (vnode->visitor == v) {
+ return vnode->file;
+ }
+ }
+ return NULL;
+}
+
+static void qemu_file_put_visitor(QEMUFile *f, Visitor *v, void *opaque,
+ QEMUFileVisitorCleanupFunc *cleanup)
+{
+ VisitorNode *vnode = g_malloc0(sizeof(*vnode));
+ vnode->file = f;
+ vnode->visitor = v;
+ vnode->opaque = opaque;
+ vnode->cleanup = cleanup;
+ QTAILQ_INSERT_TAIL(&qemu_file_visitors, vnode, entry);
+}
+
+static void qemu_file_remove_visitor(QEMUFile *f)
+{
+ VisitorNode *vnode;
+ QTAILQ_FOREACH(vnode, &qemu_file_visitors, entry) {
+ if (vnode->file == f) {
+ QTAILQ_REMOVE(&qemu_file_visitors, vnode, entry);
+ if (vnode->cleanup) {
+ vnode->cleanup(vnode->opaque);
+ }
+ g_free(vnode);
+ }
+ }
+}
+
+static int qemu_file_cleanup_output_visitor(void *opaque)
+{
+ QemuFileOutputVisitor *v = opaque;
+ qemu_file_output_visitor_cleanup(v);
+ return 0;
+}
+
+static int qemu_file_cleanup_input_visitor(void *opaque)
+{
+ QemuFileInputVisitor *v = opaque;
+ qemu_file_input_visitor_cleanup(v);
+ return 0;
+}
+
+
static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
{
QEMUFileSocket *s = opaque;
@@ -177,8 +267,9 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
s = g_malloc0(sizeof(QEMUFileStdio));
s->stdio_file = fdopen(fd, mode);
- if (!s->stdio_file)
+ if (!s->stdio_file) {
goto fail;
+ }
if (mode[0] == 'r') {
s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
@@ -270,6 +361,16 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
f->get_rate_limit = get_rate_limit;
f->is_write = 0;
+ if (put_buffer) {
+ QemuFileOutputVisitor *ov = qemu_file_output_visitor_new(f);
+ qemu_file_put_visitor(f, qemu_file_output_get_visitor(ov), ov,
+ qemu_file_cleanup_output_visitor);
+ } else {
+ QemuFileInputVisitor *iv = qemu_file_input_visitor_new(f);
+ qemu_file_put_visitor(f, qemu_file_input_get_visitor(iv), iv,
+ qemu_file_cleanup_input_visitor);
+ }
+
return f;
}
@@ -336,6 +437,7 @@ int qemu_fclose(QEMUFile *f)
{
int ret = 0;
qemu_fflush(f);
+ qemu_file_remove_visitor(f);
if (f->close) {
ret = f->close(f->opaque);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 07/10] trace: qemu_(put|get)_(byte|buffer) events
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (5 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 06/10] qemu-file: add QEMUFile<->visitor lookup routines Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 08/10] trace: add trace statements for visitor interface Michael Roth
` (2 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qemu-file.c | 13 +++++++++++++
trace-events | 18 ++++++++++++++++++
2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/qemu-file.c b/qemu-file.c
index cdfe8a3..89b58be 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -27,6 +27,7 @@
#include "hw/hw.h"
#include "qapi/qemu-file-output-visitor.h"
#include "qapi/qemu-file-input-visitor.h"
+#include "trace.h"
#define IO_BUF_SIZE 32768
@@ -454,6 +455,8 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
+ trace_qemu_put_buffer(f, buf, size);
+
if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
fprintf(stderr,
"Attempted to write to buffer while read buffer is not empty\n");
@@ -484,6 +487,7 @@ void qemu_put_byte(QEMUFile *f, int v)
abort();
}
+ trace_qemu_put_byte(f, v & 0xff);
f->buf[f->buf_index++] = v;
f->is_write = 1;
if (f->buf_index >= IO_BUF_SIZE) {
@@ -531,6 +535,8 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
int pending = size;
int done = 0;
+ trace_qemu_get_buffer(f, buf, size);
+
while (pending > 0) {
int res;
@@ -570,6 +576,7 @@ int qemu_get_byte(QEMUFile *f)
result = qemu_peek_byte(f, 0);
qemu_file_skip(f, 1);
+ trace_qemu_get_byte(f, result);
return result;
}
@@ -632,6 +639,7 @@ void qemu_put_be16(QEMUFile *f, unsigned int v)
{
qemu_put_byte(f, v >> 8);
qemu_put_byte(f, v);
+ trace_qemu_put_be16(f, v);
}
void qemu_put_be32(QEMUFile *f, unsigned int v)
@@ -640,12 +648,14 @@ void qemu_put_be32(QEMUFile *f, unsigned int v)
qemu_put_byte(f, v >> 16);
qemu_put_byte(f, v >> 8);
qemu_put_byte(f, v);
+ trace_qemu_put_be32(f, v);
}
void qemu_put_be64(QEMUFile *f, uint64_t v)
{
qemu_put_be32(f, v >> 32);
qemu_put_be32(f, v);
+ trace_qemu_put_be64(f, v);
}
unsigned int qemu_get_be16(QEMUFile *f)
@@ -653,6 +663,7 @@ unsigned int qemu_get_be16(QEMUFile *f)
unsigned int v;
v = qemu_get_byte(f) << 8;
v |= qemu_get_byte(f);
+ trace_qemu_get_be16(f, v);
return v;
}
@@ -663,6 +674,7 @@ unsigned int qemu_get_be32(QEMUFile *f)
v |= qemu_get_byte(f) << 16;
v |= qemu_get_byte(f) << 8;
v |= qemu_get_byte(f);
+ trace_qemu_get_be32(f, v);
return v;
}
@@ -671,5 +683,6 @@ uint64_t qemu_get_be64(QEMUFile *f)
uint64_t v;
v = (uint64_t)qemu_get_be32(f) << 32;
v |= qemu_get_be32(f);
+ trace_qemu_get_be64(f, v);
return v;
}
diff --git a/trace-events b/trace-events
index 820b1d6..6e36266 100644
--- a/trace-events
+++ b/trace-events
@@ -624,3 +624,21 @@ win_helper_no_switch_pstate(uint32_t new_pstate_regs) "change_pstate: regs new=%
win_helper_wrpil(uint32_t psrpil, uint32_t new_pil) "old=%x new=%x"
win_helper_done(uint32_t tl) "tl=%d"
win_helper_retry(uint32_t tl) "tl=%d"
+
+# qemu-file.c
+qemu_put_buffer(void *f, const uint8_t *buf, int size) "file=%p, buf=%p, size=%d"
+qemu_get_buffer(void *f, const uint8_t *buf, int size) "file=%p, buf=%p, size=%d"
+qemu_put_byte(void *f, int val) "file=%p, val=0x%x"
+qemu_get_byte(void *f, int val) "file=%p, val=0x%x"
+qemu_put_be16(void *f, unsigned int v) "file=%p, val=0x%x"
+qemu_get_be16(void *f, unsigned int v) "file=%p, val=0x%x"
+qemu_put_be32(void *f, unsigned int v) "file=%p, val=0x%x"
+qemu_get_be32(void *f, unsigned int v) "file=%p, val=0x%x"
+qemu_put_be64(void *f, uint64_t v) "file=%p, val=0x%"PRIx64
+qemu_get_be64(void *f, uint64_t v) "file=%p, val=0x%"PRIx64
+qemu_put_sbe16(void *f, int v) "file=%p, val=0x%x"
+qemu_get_sbe16(void *f, int v) "file=%p, val=0x%x"
+qemu_put_sbe32(void *f, int v) "file=%p, val=0x%x"
+qemu_get_sbe32(void *f, int v) "file=%p, val=0x%x"
+qemu_put_sbe64(void *f, int64_t v) "file=%p, val=0x%"PRIx64
+qemu_get_sbe64(void *f, int64_t v) "file=%p, val=0x%"PRIx64
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 08/10] trace: add trace statements for visitor interface
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (6 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 07/10] trace: qemu_(put|get)_(byte|buffer) events Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 09/10] qapi: add trace statements to qapi-visit-core.c Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 10/10] vmstate: use visitors Michael Roth
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
trace-events | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/trace-events b/trace-events
index 6e36266..ec5c772 100644
--- a/trace-events
+++ b/trace-events
@@ -642,3 +642,28 @@ qemu_put_sbe32(void *f, int v) "file=%p, val=0x%x"
qemu_get_sbe32(void *f, int v) "file=%p, val=0x%x"
qemu_put_sbe64(void *f, int64_t v) "file=%p, val=0x%"PRIx64
qemu_get_sbe64(void *f, int64_t v) "file=%p, val=0x%"PRIx64
+
+# qapi/qapi-visit-core.c
+qapi_visit_type_uint8(void *v, const char *field, uint8_t *value_ptr, uint8_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_uint16(void *v, const char *field, uint16_t *value_ptr, uint16_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_uint32(void *v, const char *field, uint32_t *value_ptr, uint32_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_uint64(void *v, const char *field, uint64_t *value_ptr, uint64_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%"PRIx64", err=%p"
+qapi_visit_type_int8(void *v, const char *field, int8_t *value_ptr, int8_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_int16(void *v, const char *field, int16_t *value_ptr, int16_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_int32(void *v, const char *field, int32_t *value_ptr, int32_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_int64(void *v, const char *field, int64_t *value_ptr, int64_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%"PRIx64", err=%p"
+qapi_visit_type_int(void *v, const char *field, int64_t *value_ptr, int64_t value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%"PRIx64", err=%p"
+qapi_visit_type_bool(void *v, const char *field, bool *value_ptr, bool value, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, err=%p"
+qapi_visit_type_str(void *v, const char *field, void *value_ptr, char *value, void *e) "visitor=%p, field=%s, ptr=%p, val=%s, err=%p"
+qapi_visit_type_double(void *v, const char *field, double *value_ptr, double value, void *e) "visitor=%p, field=%s, ptr=%p, val=%f, err=%p"
+qapi_visit_type_enum(void *v, const char *field, int *value_ptr, int value, const void *strings_ptr, void *e) "visitor=%p, field=%s, ptr=%p, val=0x%x, strings_ptr=%p, err=%p"
+qapi_start_struct(void *v, const char *field, void *ptr, int size, const char *kind, void *err) "visitor=%p, field=%s, ptr=%p, size=%d, kind=%s, err=%p"
+qapi_end_struct(void *v, void *err) "visitor=%p, err=%p"
+qapi_start_list(void *v, const char *field, void *e) "visitor=%p, field=%s, err=%p"
+qapi_visit_next_list(void *v, void *list_entry, void *e) "visitor=%p, list_entry=%p, err=%p"
+qapi_end_list(void *v, void *e) "visitor=%p, err=%p"
+qapi_start_array(void *v, const char *field, void *ptr, int elem_count, int elem_size, void *e) "visitor=%p, field=%s, ptr=%p, elem_count=%d, elem_size=%d, err=%p"
+qapi_visit_next_array(void *v, void *e) "visitor=%p, err=%p"
+qapi_end_array(void *v, void *e) "visitor=%p, err=%p"
+qapi_start_optional(void *v, const char *field, bool *value_ptr, bool value, void *e) "visitor=%p, field=%s, value_ptr=%p, val=0x%x, err=%p"
+qapi_end_optional(void *v, void *e) "visitor=%p, err=%p"
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 09/10] qapi: add trace statements to qapi-visit-core.c
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (7 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 08/10] trace: add trace statements for visitor interface Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 10/10] vmstate: use visitors Michael Roth
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile.objs | 2 +-
qapi/qapi-visit-core.c | 41 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index 631174f..802b85d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -24,7 +24,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
# qapi
qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-dealloc-visitor.o
qapi-nested-y += qmp-registry.o qmp-dispatch.o
-qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
+qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y)) $(trace-obj-y)
#######################################################################
# block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 58f11fe..7b70d6e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -12,6 +12,7 @@
*/
#include "qapi/qapi-visit-core.h"
+#include "trace.h"
void visit_start_handle(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp)
@@ -34,6 +35,7 @@ void visit_start_struct(Visitor *v, void **obj, const char *kind,
if (!error_is_set(errp)) {
v->start_struct(v, obj, kind, name, size, errp);
}
+ trace_qapi_start_struct(v, name, obj, size, kind, errp ? *errp : NULL);
}
void visit_end_struct(Visitor *v, Error **errp)
@@ -41,6 +43,7 @@ void visit_end_struct(Visitor *v, Error **errp)
if (!error_is_set(errp)) {
v->end_struct(v, errp);
}
+ trace_qapi_end_struct(v, errp ? *errp : NULL);
}
void visit_start_list(Visitor *v, const char *name, Error **errp)
@@ -48,15 +51,18 @@ void visit_start_list(Visitor *v, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->start_list(v, name, errp);
}
+ trace_qapi_start_list(v, name, errp ? *errp : NULL);
}
GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
{
+ GenericList *next = NULL;
if (!error_is_set(errp)) {
- return v->next_list(v, list, errp);
+ next = v->next_list(v, list, errp);
}
+ trace_qapi_visit_next_list(v, list, errp ? *errp : NULL);
- return 0;
+ return next;
}
void visit_end_list(Visitor *v, Error **errp)
@@ -64,6 +70,7 @@ void visit_end_list(Visitor *v, Error **errp)
if (!error_is_set(errp)) {
v->end_list(v, errp);
}
+ trace_qapi_end_list(v, errp ? *errp : NULL);
}
void visit_start_array(Visitor *v, void **obj, const char *name, size_t elem_count,
@@ -72,6 +79,7 @@ void visit_start_array(Visitor *v, void **obj, const char *name, size_t elem_cou
if (!error_is_set(errp)) {
v->start_array(v, obj, name, elem_count, elem_size, errp);
}
+ trace_qapi_start_array(v, name, obj, elem_count, elem_size, errp ? *errp : NULL);
}
void visit_next_array(Visitor *v, Error **errp)
@@ -79,6 +87,7 @@ void visit_next_array(Visitor *v, Error **errp)
if (!error_is_set(errp)) {
v->next_array(v, errp);
}
+ trace_qapi_visit_next_array(v, errp ? *errp : NULL);
}
void visit_end_array(Visitor *v, Error **errp)
@@ -86,6 +95,7 @@ void visit_end_array(Visitor *v, Error **errp)
if (!error_is_set(errp)) {
v->end_array(v, errp);
}
+ trace_qapi_end_array(v, errp ? *errp : NULL);
}
void visit_start_optional(Visitor *v, bool *present, const char *name,
@@ -94,6 +104,7 @@ void visit_start_optional(Visitor *v, bool *present, const char *name,
if (!error_is_set(errp) && v->start_optional) {
v->start_optional(v, present, name, errp);
}
+ trace_qapi_start_optional(v, name, present, *present, errp ? *errp : NULL);
}
void visit_end_optional(Visitor *v, Error **errp)
@@ -101,6 +112,7 @@ void visit_end_optional(Visitor *v, Error **errp)
if (!error_is_set(errp) && v->end_optional) {
v->end_optional(v, errp);
}
+ trace_qapi_end_optional(v, errp ? *errp : NULL);
}
void visit_type_enum(Visitor *v, int *obj, const char *strings[],
@@ -109,6 +121,7 @@ void visit_type_enum(Visitor *v, int *obj, const char *strings[],
if (!error_is_set(errp)) {
v->type_enum(v, obj, strings, kind, name, errp);
}
+ trace_qapi_visit_type_enum(v, name, obj, *obj, strings, errp ? *errp : NULL);
}
void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
@@ -116,6 +129,8 @@ void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_int(v, obj, name, errp);
}
+ trace_qapi_visit_type_int64(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
@@ -123,6 +138,8 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_uint8(v, obj, name, errp);
}
+ trace_qapi_visit_type_uint8(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
@@ -130,6 +147,8 @@ void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp
if (!error_is_set(errp)) {
v->type_uint16(v, obj, name, errp);
}
+ trace_qapi_visit_type_uint16(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
@@ -137,6 +156,8 @@ void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp
if (!error_is_set(errp)) {
v->type_uint32(v, obj, name, errp);
}
+ trace_qapi_visit_type_uint32(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
@@ -144,6 +165,8 @@ void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp
if (!error_is_set(errp)) {
v->type_uint64(v, obj, name, errp);
}
+ trace_qapi_visit_type_uint64(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
@@ -151,6 +174,8 @@ void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_int8(v, obj, name, errp);
}
+ trace_qapi_visit_type_int8(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
@@ -158,6 +183,8 @@ void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_int16(v, obj, name, errp);
}
+ trace_qapi_visit_type_int16(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
@@ -165,6 +192,8 @@ void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_int32(v, obj, name, errp);
}
+ trace_qapi_visit_type_int32(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
@@ -172,6 +201,8 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_int64(v, obj, name, errp);
}
+ trace_qapi_visit_type_int64(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
@@ -179,6 +210,8 @@ void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_bool(v, obj, name, errp);
}
+ trace_qapi_visit_type_bool(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
@@ -186,6 +219,8 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_str(v, obj, name, errp);
}
+ trace_qapi_visit_type_str(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
@@ -193,4 +228,6 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
if (!error_is_set(errp)) {
v->type_number(v, obj, name, errp);
}
+ trace_qapi_visit_type_double(v, name, obj, obj ? *obj : 0,
+ errp ? *errp : NULL);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 10/10] vmstate: use visitors
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
` (8 preceding siblings ...)
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 09/10] qapi: add trace statements to qapi-visit-core.c Michael Roth
@ 2011-10-27 17:06 ` Michael Roth
9 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2011-10-27 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, quintela
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/eeprom93xx.c | 12 +-
hw/fw_cfg.c | 12 +-
hw/hw.h | 8 +-
hw/pci.c | 38 +++-
hw/twl92230.c | 18 +-
qemu-timer.h | 5 +
savevm.c | 521 ++++++++++++++++++++++++++++++------------------
target-alpha/machine.c | 13 +-
target-i386/machine.c | 49 +++--
9 files changed, 436 insertions(+), 240 deletions(-)
diff --git a/hw/eeprom93xx.c b/hw/eeprom93xx.c
index 4c7158d..2d6cbe7 100644
--- a/hw/eeprom93xx.c
+++ b/hw/eeprom93xx.c
@@ -93,14 +93,18 @@ struct _eeprom_t {
This is a Big hack, but it is how the old state did it.
*/
-static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size)
+static int get_uint16_from_uint8(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- uint16_t *v = pv;
- *v = qemu_get_ubyte(f);
+ uint16_t *v1 = pv;
+ uint8_t v2;
+ visit_type_uint8(v, &v2, NULL, err);
+ *v1 = v2;
return 0;
}
-static void put_unused(QEMUFile *f, void *pv, size_t size)
+static void put_unused(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
fprintf(stderr, "uint16_from_uint8 is used only for backwards compatibility.\n");
fprintf(stderr, "Never should be used to write a new state.\n");
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 8df265c..8a8033e 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -326,14 +326,18 @@ static void fw_cfg_reset(DeviceState *d)
Or we broke compatibility in the state, or we can't use struct tm
*/
-static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_uint32_as_uint16(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- uint32_t *v = pv;
- *v = qemu_get_be16(f);
+ uint32_t *val = pv;
+ uint16_t val2;
+ visit_type_uint16(v, &val2, name, err);
+ *val = val2;
return 0;
}
-static void put_unused(QEMUFile *f, void *pv, size_t size)
+static void put_unused(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
fprintf(stderr, "uint32_as_uint16 is only used for backward compatibility.\n");
fprintf(stderr, "This functions shouldn't be called.\n");
diff --git a/hw/hw.h b/hw/hw.h
index 5c0eb65..0e189da 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -293,8 +293,8 @@ typedef struct VMStateDescription VMStateDescription;
struct VMStateInfo {
const char *name;
- int (*get)(QEMUFile *f, void *pv, size_t size);
- void (*put)(QEMUFile *f, void *pv, size_t size);
+ int (*get)(Visitor *v, const char *name, void *pv, size_t size, Error **err);
+ void (*put)(Visitor *v, const char *name, void *pv, size_t size, Error **err);
};
enum VMStateFlags {
@@ -941,10 +941,14 @@ extern const VMStateDescription vmstate_hid_ptr_device;
#define VMSTATE_END_OF_LIST() \
{}
+#define VMS_LOAD true
+#define VMS_SAVE false
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id);
void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque);
+int vmstate_process(Visitor *v, const VMStateDescription *vmsd,
+ void *opaque, int version_id, bool load,Error **errp);
int vmstate_register(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd, void *base);
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
diff --git a/hw/pci.c b/hw/pci.c
index e8cc1b0..a0ab0e0 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -334,23 +334,25 @@ int pci_bus_num(PCIBus *s)
return s->parent_dev->config[PCI_SECONDARY_BUS];
}
-static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
+static int get_pci_config_device(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
PCIDevice *s = container_of(pv, PCIDevice, config);
- uint8_t *config;
+ uint8_t *config = NULL;
int i;
assert(size == pci_config_size(s));
- config = g_malloc(size);
- qemu_get_buffer(f, config, size);
+ visit_start_array(v, (void **)&config, name, size, 1, err);
for (i = 0; i < size; ++i) {
+ visit_type_uint8(v, &config[i], NULL, err);
if ((config[i] ^ s->config[i]) &
s->cmask[i] & ~s->wmask[i] & ~s->w1cmask[i]) {
g_free(config);
return -EINVAL;
}
}
+ visit_end_array(v, err);
memcpy(s->config, config, size);
pci_update_mappings(s);
@@ -360,11 +362,17 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
}
/* just put buffer */
-static void put_pci_config_device(QEMUFile *f, void *pv, size_t size)
+static void put_pci_config_device(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- const uint8_t **v = pv;
+ uint8_t *config = *(uint8_t **)pv;
+ int i;
assert(size == pci_config_size(container_of(pv, PCIDevice, config)));
- qemu_put_buffer(f, *v, size);
+ visit_start_array(v, (void **)&config, name, size, 1, err);
+ for (i = 0; i < size; i++) {
+ visit_type_uint8(v, &config[i], NULL, err);
+ }
+ visit_end_array(v, err);
}
static VMStateInfo vmstate_info_pci_config = {
@@ -373,19 +381,22 @@ static VMStateInfo vmstate_info_pci_config = {
.put = put_pci_config_device,
};
-static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size)
+static int get_pci_irq_state(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
PCIDevice *s = container_of(pv, PCIDevice, irq_state);
uint32_t irq_state[PCI_NUM_PINS];
int i;
+ visit_start_array(v, NULL, name, PCI_NUM_PINS, 4, err);
for (i = 0; i < PCI_NUM_PINS; ++i) {
- irq_state[i] = qemu_get_be32(f);
+ visit_type_uint32(v, &irq_state[i], NULL, err);
if (irq_state[i] != 0x1 && irq_state[i] != 0) {
fprintf(stderr, "irq state %d: must be 0 or 1.\n",
irq_state[i]);
return -EINVAL;
}
}
+ visit_end_array(v, err);
for (i = 0; i < PCI_NUM_PINS; ++i) {
pci_set_irq_state(s, i, irq_state[i]);
@@ -394,14 +405,19 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size)
return 0;
}
-static void put_pci_irq_state(QEMUFile *f, void *pv, size_t size)
+static void put_pci_irq_state(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
int i;
PCIDevice *s = container_of(pv, PCIDevice, irq_state);
+ uint32_t irq_state;
+ visit_start_array(v, NULL, name, PCI_NUM_PINS, 4, err);
for (i = 0; i < PCI_NUM_PINS; ++i) {
- qemu_put_be32(f, pci_irq_state(s, i));
+ irq_state = pci_irq_state(s, i);
+ visit_type_uint32(v, &irq_state, NULL, err);
}
+ visit_end_array(v, err);
}
static VMStateInfo vmstate_info_pci_irq_state = {
diff --git a/hw/twl92230.c b/hw/twl92230.c
index a75448f..732f2d6 100644
--- a/hw/twl92230.c
+++ b/hw/twl92230.c
@@ -742,17 +742,23 @@ static int menelaus_rx(i2c_slave *i2c)
Or we broke compatibility in the state, or we can't use struct tm
*/
-static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_int32_as_uint16(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- int *v = pv;
- *v = qemu_get_be16(f);
+ uint32_t *val = pv;
+ uint16_t val2;
+ visit_type_uint16(v, &val2, name, err);
+ *val = val2;
return 0;
}
-static void put_int32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static void put_int32_as_uint16(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- int *v = pv;
- qemu_put_be16(f, *v);
+ uint32_t *val = pv;
+ uint16_t val2;
+ visit_type_uint16(v, &val2, name, err);
+ *val = val2;
}
static const VMStateInfo vmstate_hack_int32_as_uint16 = {
diff --git a/qemu-timer.h b/qemu-timer.h
index 67ca72e..043b1e1 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -6,6 +6,7 @@
#include "notify.h"
#include <time.h>
#include <sys/time.h>
+#include "qapi/qapi-visit-core.h"
#ifdef _WIN32
#include <windows.h>
@@ -137,7 +138,11 @@ static inline int64_t get_clock(void)
#endif
void qemu_get_timer(QEMUFile *f, QEMUTimer *ts);
+void qemu_get_timer_visitor(Visitor *v, const char *name, QEMUTimer *ts,
+ Error **err);
void qemu_put_timer(QEMUFile *f, QEMUTimer *ts);
+void qemu_put_timer_visitor(Visitor *v, const char *name, QEMUTimer *ts,
+ Error **err);
/* ptimer.c */
typedef struct ptimer_state ptimer_state;
diff --git a/savevm.c b/savevm.c
index 0b8e46f..3ce7fd3 100644
--- a/savevm.c
+++ b/savevm.c
@@ -183,19 +183,28 @@ static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
/* timer */
-void qemu_put_timer(QEMUFile *f, QEMUTimer *ts)
+void qemu_put_timer_visitor(Visitor *v, const char *name, QEMUTimer *ts,
+ Error **err)
{
uint64_t expire_time;
-
expire_time = qemu_timer_expire_time_ns(ts);
- qemu_put_be64(f, expire_time);
+
+ visit_type_uint64(v, &expire_time, name, err);
}
-void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
+void qemu_put_timer(QEMUFile *f, QEMUTimer *ts)
+{
+ Visitor *v = qemu_file_get_visitor(f);
+ assert(v);
+ qemu_put_timer_visitor(v, "timer", ts, NULL);
+}
+
+void qemu_get_timer_visitor(Visitor *v, const char *name, QEMUTimer *ts,
+ Error **err)
{
uint64_t expire_time;
- expire_time = qemu_get_be64(f);
+ visit_type_uint64(v, &expire_time, name, err);
if (expire_time != -1) {
qemu_mod_timer_ns(ts, expire_time);
} else {
@@ -203,19 +212,28 @@ void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
}
}
+void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
+{
+ Visitor *v = qemu_file_get_visitor(f);
+ assert(v);
+ qemu_get_timer_visitor(v, "timer", ts, NULL);
+}
+
/* bool */
-static int get_bool(QEMUFile *f, void *pv, size_t size)
+static int get_bool(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- bool *v = pv;
- *v = qemu_get_byte(f);
+ bool *val = pv;
+ visit_type_bool(v, val, name, err);
return 0;
}
-static void put_bool(QEMUFile *f, void *pv, size_t size)
+static void put_bool(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- bool *v = pv;
- qemu_put_byte(f, *v);
+ bool *val = pv;
+ visit_type_bool(v, val, name, err);
}
const VMStateInfo vmstate_info_bool = {
@@ -226,17 +244,19 @@ const VMStateInfo vmstate_info_bool = {
/* 8 bit int */
-static int get_int8(QEMUFile *f, void *pv, size_t size)
+static int get_int8(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int8_t *v = pv;
- qemu_get_s8s(f, v);
+ int8_t *val = pv;
+ visit_type_int8(v, val, name, err);
return 0;
}
-static void put_int8(QEMUFile *f, void *pv, size_t size)
+static void put_int8(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int8_t *v = pv;
- qemu_put_s8s(f, v);
+ int8_t *val = pv;
+ visit_type_int8(v, val, name, err);
}
const VMStateInfo vmstate_info_int8 = {
@@ -247,17 +267,19 @@ const VMStateInfo vmstate_info_int8 = {
/* 16 bit int */
-static int get_int16(QEMUFile *f, void *pv, size_t size)
+static int get_int16(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int16_t *v = pv;
- qemu_get_sbe16s(f, v);
+ int16_t *val = pv;
+ visit_type_int16(v, val, name, err);
return 0;
}
-static void put_int16(QEMUFile *f, void *pv, size_t size)
+static void put_int16(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int16_t *v = pv;
- qemu_put_sbe16s(f, v);
+ int16_t *val = pv;
+ visit_type_int16(v, val, name, err);
}
const VMStateInfo vmstate_info_int16 = {
@@ -268,17 +290,19 @@ const VMStateInfo vmstate_info_int16 = {
/* 32 bit int */
-static int get_int32(QEMUFile *f, void *pv, size_t size)
+static int get_int32(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int32_t *v = pv;
- qemu_get_sbe32s(f, v);
+ int32_t *val = pv;
+ visit_type_int32(v, val, name, err);
return 0;
}
-static void put_int32(QEMUFile *f, void *pv, size_t size)
+static void put_int32(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int32_t *v = pv;
- qemu_put_sbe32s(f, v);
+ int32_t *val = pv;
+ visit_type_int32(v, val, name, err);
}
const VMStateInfo vmstate_info_int32 = {
@@ -290,13 +314,14 @@ const VMStateInfo vmstate_info_int32 = {
/* 32 bit int. See that the received value is the same than the one
in the field */
-static int get_int32_equal(QEMUFile *f, void *pv, size_t size)
+static int get_int32_equal(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int32_t *v = pv;
+ int32_t *v1 = pv;
int32_t v2;
- qemu_get_sbe32s(f, &v2);
+ visit_type_int32(v, &v2, name, err);
- if (*v == v2)
+ if (*v1 == v2)
return 0;
return -EINVAL;
}
@@ -310,11 +335,12 @@ const VMStateInfo vmstate_info_int32_equal = {
/* 32 bit int. See that the received value is the less or the same
than the one in the field */
-static int get_int32_le(QEMUFile *f, void *pv, size_t size)
+static int get_int32_le(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
int32_t *old = pv;
int32_t new;
- qemu_get_sbe32s(f, &new);
+ visit_type_int32(v, &new, name, err);
if (*old <= new)
return 0;
@@ -329,17 +355,19 @@ const VMStateInfo vmstate_info_int32_le = {
/* 64 bit int */
-static int get_int64(QEMUFile *f, void *pv, size_t size)
+static int get_int64(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int64_t *v = pv;
- qemu_get_sbe64s(f, v);
+ int64_t *val = pv;
+ visit_type_int64(v, val, name, err);
return 0;
}
-static void put_int64(QEMUFile *f, void *pv, size_t size)
+static void put_int64(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- int64_t *v = pv;
- qemu_put_sbe64s(f, v);
+ int64_t *val = pv;
+ visit_type_int64(v, val, name, err);
}
const VMStateInfo vmstate_info_int64 = {
@@ -350,17 +378,19 @@ const VMStateInfo vmstate_info_int64 = {
/* 8 bit unsigned int */
-static int get_uint8(QEMUFile *f, void *pv, size_t size)
+static int get_uint8(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint8_t *v = pv;
- qemu_get_8s(f, v);
+ uint8_t *val = pv;
+ visit_type_uint8(v, val, name, err);
return 0;
}
-static void put_uint8(QEMUFile *f, void *pv, size_t size)
+static void put_uint8(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint8_t *v = pv;
- qemu_put_8s(f, v);
+ uint8_t *val = pv;
+ visit_type_uint8(v, val, name, err);
}
const VMStateInfo vmstate_info_uint8 = {
@@ -371,17 +401,19 @@ const VMStateInfo vmstate_info_uint8 = {
/* 16 bit unsigned int */
-static int get_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_uint16(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint16_t *v = pv;
- qemu_get_be16s(f, v);
+ uint16_t *val = pv;
+ visit_type_uint16(v, val, name, err);
return 0;
}
-static void put_uint16(QEMUFile *f, void *pv, size_t size)
+static void put_uint16(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint16_t *v = pv;
- qemu_put_be16s(f, v);
+ uint16_t *val = pv;
+ visit_type_uint16(v, val, name, err);
}
const VMStateInfo vmstate_info_uint16 = {
@@ -392,17 +424,19 @@ const VMStateInfo vmstate_info_uint16 = {
/* 32 bit unsigned int */
-static int get_uint32(QEMUFile *f, void *pv, size_t size)
+static int get_uint32(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint32_t *v = pv;
- qemu_get_be32s(f, v);
+ uint32_t *val = pv;
+ visit_type_uint32(v, val, name, err);
return 0;
}
-static void put_uint32(QEMUFile *f, void *pv, size_t size)
+static void put_uint32(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint32_t *v = pv;
- qemu_put_be32s(f, v);
+ uint32_t *val = pv;
+ visit_type_uint32(v, val, name, err);
}
const VMStateInfo vmstate_info_uint32 = {
@@ -414,13 +448,14 @@ const VMStateInfo vmstate_info_uint32 = {
/* 32 bit uint. See that the received value is the same than the one
in the field */
-static int get_uint32_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint32_equal(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint32_t *v = pv;
+ uint32_t *v1 = pv;
uint32_t v2;
- qemu_get_be32s(f, &v2);
+ visit_type_uint32(v, &v2, name, err);
- if (*v == v2) {
+ if (*v1 == v2) {
return 0;
}
return -EINVAL;
@@ -434,17 +469,19 @@ const VMStateInfo vmstate_info_uint32_equal = {
/* 64 bit unsigned int */
-static int get_uint64(QEMUFile *f, void *pv, size_t size)
+static int get_uint64(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint64_t *v = pv;
- qemu_get_be64s(f, v);
+ uint64_t *val = pv;
+ visit_type_uint64(v, val, name, err);
return 0;
}
-static void put_uint64(QEMUFile *f, void *pv, size_t size)
+static void put_uint64(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint64_t *v = pv;
- qemu_put_be64s(f, v);
+ uint64_t *val = pv;
+ visit_type_uint64(v, val, name, err);
}
const VMStateInfo vmstate_info_uint64 = {
@@ -456,13 +493,14 @@ const VMStateInfo vmstate_info_uint64 = {
/* 8 bit int. See that the received value is the same than the one
in the field */
-static int get_uint8_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint8_equal(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint8_t *v = pv;
+ uint8_t *v1 = pv;
uint8_t v2;
- qemu_get_8s(f, &v2);
+ visit_type_uint8(v, &v2, name, err);
- if (*v == v2)
+ if (*v1 == v2)
return 0;
return -EINVAL;
}
@@ -476,13 +514,14 @@ const VMStateInfo vmstate_info_uint8_equal = {
/* 16 bit unsigned int int. See that the received value is the same than the one
in the field */
-static int get_uint16_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint16_equal(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint16_t *v = pv;
+ uint16_t *v1 = pv;
uint16_t v2;
- qemu_get_be16s(f, &v2);
+ visit_type_uint16(v, &v2, name, err);
- if (*v == v2)
+ if (*v1 == v2)
return 0;
return -EINVAL;
}
@@ -495,17 +534,19 @@ const VMStateInfo vmstate_info_uint16_equal = {
/* timers */
-static int get_timer(QEMUFile *f, void *pv, size_t size)
+static int get_timer(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- QEMUTimer *v = pv;
- qemu_get_timer(f, v);
+ QEMUTimer *t = pv;
+ qemu_get_timer_visitor(v, name, t, err);
return 0;
}
-static void put_timer(QEMUFile *f, void *pv, size_t size)
+static void put_timer(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- QEMUTimer *v = pv;
- qemu_put_timer(f, v);
+ QEMUTimer *t = pv;
+ qemu_put_timer_visitor(v, name, t, err);
}
const VMStateInfo vmstate_info_timer = {
@@ -516,17 +557,29 @@ const VMStateInfo vmstate_info_timer = {
/* uint8_t buffers */
-static int get_buffer(QEMUFile *f, void *pv, size_t size)
+static int get_buffer(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint8_t *v = pv;
- qemu_get_buffer(f, v, size);
+ uint8_t *val = pv;
+ int i;
+ visit_start_array(v, NULL, name, size, 1, err);
+ for (i = 0; i < size; i++) {
+ visit_type_uint8(v, &val[i], NULL, err);
+ }
+ visit_end_array(v, err);
return 0;
}
-static void put_buffer(QEMUFile *f, void *pv, size_t size)
+static void put_buffer(Visitor *v, const char *name, void *pv, size_t size,
+ Error **err)
{
- uint8_t *v = pv;
- qemu_put_buffer(f, v, size);
+ uint8_t *val = pv;
+ int i;
+ visit_start_array(v, NULL, name, size, 1, err);
+ for (i = 0; i < size; i++) {
+ visit_type_uint8(v, &val[i], NULL, err);
+ }
+ visit_end_array(v, err);
}
const VMStateInfo vmstate_info_buffer = {
@@ -538,29 +591,39 @@ const VMStateInfo vmstate_info_buffer = {
/* unused buffers: space that was used for some fields that are
not useful anymore */
-static int get_unused_buffer(QEMUFile *f, void *pv, size_t size)
+static int get_unused_buffer(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
uint8_t buf[1024];
- int block_len;
+ int block_len, i;
+ visit_start_array(v, NULL, name, size, 1, err);
while (size > 0) {
block_len = MIN(sizeof(buf), size);
size -= block_len;
- qemu_get_buffer(f, buf, block_len);
+ for (i = 0; i < block_len; i++) {
+ visit_type_uint8(v, &buf[i], NULL, err);
+ }
}
+ visit_end_array(v, err);
return 0;
}
-static void put_unused_buffer(QEMUFile *f, void *pv, size_t size)
+static void put_unused_buffer(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- static const uint8_t buf[1024];
- int block_len;
+ static uint8_t buf[1024];
+ int block_len, i;
+ visit_start_array(v, NULL, name, size, 1, err);
while (size > 0) {
block_len = MIN(sizeof(buf), size);
size -= block_len;
- qemu_put_buffer(f, buf, block_len);
+ for (i = 0; i < block_len; i++) {
+ visit_type_uint8(v, &buf[i], NULL, err);
+ }
}
+ visit_end_array(v, err);
}
const VMStateInfo vmstate_info_unused_buffer = {
@@ -820,34 +883,57 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque);
-int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, int version_id)
+static bool vmstate_field_needed(VMStateField *field,
+ const VMStateDescription *vmsd,
+ void *opaque, int version_id, bool load)
+{
+ if (load) {
+ return ((field->field_exists &&
+ field->field_exists(opaque, version_id)) ||
+ (!field->field_exists && field->version_id <= version_id));
+ }
+ return (!field->field_exists ||
+ field->field_exists(opaque, vmsd->version_id));
+}
+
+static int vmstate_process_qf(QEMUFile *f, const VMStateDescription *vmsd,
+ void *opaque, int version_id, bool load, Error **errp)
{
VMStateField *field = vmsd->fields;
int ret;
+ Visitor *v = qemu_file_get_visitor(f);
+ Error *err = NULL;
- if (version_id > vmsd->version_id) {
- return -EINVAL;
- }
- if (version_id < vmsd->minimum_version_id_old) {
- return -EINVAL;
- }
- if (version_id < vmsd->minimum_version_id) {
- return vmsd->load_state_old(f, opaque, version_id);
- }
- if (vmsd->pre_load) {
- int ret = vmsd->pre_load(opaque);
- if (ret)
- return ret;
+ assert(v);
+ if (load) {
+ if (version_id > vmsd->version_id) {
+ return -EINVAL;
+ }
+ if (version_id < vmsd->minimum_version_id_old) {
+ return -EINVAL;
+ }
+ if (version_id < vmsd->minimum_version_id) {
+ return vmsd->load_state_old(f, opaque, version_id);
+ }
+ if (vmsd->pre_load) {
+ int ret = vmsd->pre_load(opaque);
+ if (ret) {
+ return ret;
+ }
+ }
+ } else {
+ if (vmsd->pre_save) {
+ vmsd->pre_save(opaque);
+ }
}
+
+ visit_start_struct(v, NULL, NULL, vmsd->name, 0, &err);
while(field->name) {
- if ((field->field_exists &&
- field->field_exists(opaque, version_id)) ||
- (!field->field_exists &&
- field->version_id <= version_id)) {
+ if (vmstate_field_needed(field, vmsd, opaque, version_id, load)) {
void *base_addr = opaque + field->offset;
int i, n_elems = 1;
int size = field->size;
+ bool is_array = false;
if (field->flags & VMS_VBUFFER) {
size = *(int32_t *)(opaque+field->size_offset);
@@ -857,14 +943,28 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
}
if (field->flags & VMS_ARRAY) {
n_elems = field->num;
+ visit_start_array(v, NULL, field->name, n_elems, size, &err);
+ is_array = true;
} else if (field->flags & VMS_VARRAY_INT32) {
n_elems = *(int32_t *)(opaque+field->num_offset);
+ visit_start_array(v, NULL, field->name, n_elems,
+ sizeof(int32_t), &err);
+ is_array = true;
} else if (field->flags & VMS_VARRAY_UINT32) {
n_elems = *(uint32_t *)(opaque+field->num_offset);
+ visit_start_array(v, NULL, field->name, n_elems,
+ sizeof(uint32_t), &err);
+ is_array = true;
} else if (field->flags & VMS_VARRAY_UINT16) {
n_elems = *(uint16_t *)(opaque+field->num_offset);
+ visit_start_array(v, NULL, field->name, n_elems,
+ sizeof(uint16_t), &err);
+ is_array = true;
} else if (field->flags & VMS_VARRAY_UINT8) {
n_elems = *(uint8_t *)(opaque+field->num_offset);
+ visit_start_array(v, NULL, field->name, n_elems,
+ sizeof(uint8_t), &err);
+ is_array = true;
}
if (field->flags & VMS_POINTER) {
base_addr = *(void **)base_addr + field->start;
@@ -876,77 +976,63 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
addr = *(void **)addr;
}
if (field->flags & VMS_STRUCT) {
- ret = vmstate_load_state(f, field->vmsd, addr, field->vmsd->version_id);
+ if (load) {
+ ret = vmstate_load_state(f, field->vmsd, addr,
+ field->vmsd->version_id);
+ } else {
+ vmstate_save_state(f, field->vmsd, addr);
+ }
} else {
- ret = field->info->get(f, addr, size);
-
+ if (load) {
+ ret = field->info->get(v, field->name, addr, size, &err);
+ } else {
+ field->info->put(v, field->name, addr, size, &err);
+ }
}
- if (ret < 0) {
+ if (load && ret < 0) {
return ret;
}
}
+ if (is_array) {
+ visit_end_array(v, &err);
+ }
}
field++;
}
- ret = vmstate_subsection_load(f, vmsd, opaque);
- if (ret != 0) {
- return ret;
+
+ if (error_is_set(&err)) {
+ error_report("error %s state: %s", load ? "loading" : "saving",
+ error_get_pretty(err));
+ error_propagate(errp, err);
}
- if (vmsd->post_load) {
+
+ if (load) {
+ ret = vmstate_subsection_load(f, vmsd, opaque);
+ if (ret != 0) {
+ return ret;
+ }
+ } else {
+ vmstate_subsection_save(f, vmsd, opaque);
+ }
+
+ visit_end_struct(v, &err);
+
+ if (load && vmsd->post_load) {
return vmsd->post_load(opaque, version_id);
}
+
return 0;
}
-void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque)
+int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
+ void *opaque, int version_id)
{
- VMStateField *field = vmsd->fields;
-
- if (vmsd->pre_save) {
- vmsd->pre_save(opaque);
- }
- while(field->name) {
- if (!field->field_exists ||
- field->field_exists(opaque, vmsd->version_id)) {
- void *base_addr = opaque + field->offset;
- int i, n_elems = 1;
- int size = field->size;
-
- if (field->flags & VMS_VBUFFER) {
- size = *(int32_t *)(opaque+field->size_offset);
- if (field->flags & VMS_MULTIPLY) {
- size *= field->size;
- }
- }
- if (field->flags & VMS_ARRAY) {
- n_elems = field->num;
- } else if (field->flags & VMS_VARRAY_INT32) {
- n_elems = *(int32_t *)(opaque+field->num_offset);
- } else if (field->flags & VMS_VARRAY_UINT16) {
- n_elems = *(uint16_t *)(opaque+field->num_offset);
- } else if (field->flags & VMS_VARRAY_UINT8) {
- n_elems = *(uint8_t *)(opaque+field->num_offset);
- }
- if (field->flags & VMS_POINTER) {
- base_addr = *(void **)base_addr + field->start;
- }
- for (i = 0; i < n_elems; i++) {
- void *addr = base_addr + size * i;
+ return vmstate_process_qf(f, vmsd, opaque, version_id, VMS_LOAD, NULL);
+}
- if (field->flags & VMS_ARRAY_OF_POINTER) {
- addr = *(void **)addr;
- }
- if (field->flags & VMS_STRUCT) {
- vmstate_save_state(f, field->vmsd, addr);
- } else {
- field->info->put(f, addr, size);
- }
- }
- }
- field++;
- }
- vmstate_subsection_save(f, vmsd, opaque);
+void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, void *opaque)
+{
+ vmstate_process_qf(f, vmsd, opaque, 0, VMS_SAVE, NULL);
}
static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
@@ -963,7 +1049,22 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
se->save_state(f, se->opaque);
return;
}
- vmstate_save_state(f,se->vmsd, se->opaque);
+ vmstate_save_state(f, se->vmsd, se->opaque);
+}
+
+/* This wrapper allows direct callers of vmstate_load_state/vmstate_save_state
+ * to be migrated to Visitors even though internally we still use QEMUFile
+ * for old-style LoadStateHandler/SaveStateHandler users. Once the latter users
+ * are converted we can modify the interfaces accordingly, allowing us to drop
+ * references to QEMUFile in the vmstate path. Thus, both old-style users and
+ * vmstate users are decoupled from QEMUFile, leaving only live save/load users
+ * and savevm.c, which can then be reworked without touching device code.
+ */
+int vmstate_process(Visitor *v, const VMStateDescription *vmsd,
+ void *opaque, int version_id, bool load, Error **errp)
+{
+ return vmstate_process_qf(qemu_file_from_visitor(v), vmsd, opaque,
+ version_id, load, errp);
}
#define QEMU_VM_FILE_MAGIC 0x5145564d
@@ -1199,40 +1300,60 @@ static const VMStateDescription *vmstate_get_subsection(const VMStateSubsection
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque)
{
+ Visitor *v = qemu_file_get_visitor(f);
+ Error *err = NULL;
+
+ assert(v);
+
+ visit_start_list(v, "subsections", &err);
+ /* FIXME: unforunately there's no way around using a peek here, since
+ * when using an input visitor we *must* read to know whether or not to
+ * continue, which will misalign the rest of the stream.
+ * When we switch interfaces to be Visitor-centric and move to
+ * and non-QEMUFile-based Visitor this will need to be one of the first
+ * migration compatibility breaks.
+ */
while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
char idstr[256];
- int ret;
- uint8_t version_id, len, size;
+ int ret, i;
+ uint32_t version_id, len;
const VMStateDescription *sub_vmsd;
+ uint8_t tag;
- len = qemu_peek_byte(f, 1);
- if (len < strlen(vmsd->name) + 1) {
- /* subsection name has be be "section_name/a" */
- return 0;
- }
- size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
- if (size != len) {
- return 0;
- }
- idstr[size] = 0;
+ visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+ visit_type_uint8(v, &tag, "__SUBSECTION__", &err);
+ visit_type_uint8(v, (uint8_t *)&len, "len", &err);
+ visit_start_array(v, NULL, "name", len, 1, &err);
+ for (i = 0; i < len; i++) {
+ visit_type_uint8(v, (uint8_t *)&idstr[i], NULL, &err);
+ }
+ visit_end_array(v, &err);
+ idstr[len] = 0;
if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
- /* it don't have a valid subsection name */
+ /* doesn't have a valid subsection name */
return 0;
}
+ visit_type_uint32(v, &version_id, "version_id", &err);
+
sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
if (sub_vmsd == NULL) {
return -ENOENT;
}
- qemu_file_skip(f, 1); /* subsection */
- qemu_file_skip(f, 1); /* len */
- qemu_file_skip(f, len); /* idstr */
- version_id = qemu_get_be32(f);
ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
if (ret) {
return ret;
}
+
+ visit_end_struct(v, &err);
+ }
+ visit_end_list(v, &err);
+
+ if (error_is_set(&err)) {
+ error_report("error loading subsections: %s", error_get_pretty(err));
+ error_free(err);
+ return -EINVAL;
}
return 0;
}
@@ -1241,21 +1362,37 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque)
{
const VMStateSubsection *sub = vmsd->subsections;
+ uint8_t tag = QEMU_VM_SUBSECTION;
+ int i;
+ Visitor *v = qemu_file_get_visitor(f);
+ Error *err = NULL;
+ assert(v);
+ visit_start_list(v, "subsections", &err);
while (sub && sub->needed) {
if (sub->needed(opaque)) {
const VMStateDescription *vmsd = sub->vmsd;
uint8_t len;
- qemu_put_byte(f, QEMU_VM_SUBSECTION);
+ visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+
+ visit_type_uint8(v, &tag, "__SUBSECTION__", &err);
len = strlen(vmsd->name);
- qemu_put_byte(f, len);
- qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
- qemu_put_be32(f, vmsd->version_id);
+ visit_type_uint8(v, &len, "len", &err);
+ visit_start_array(v, (void **)&vmsd->name, "name", len, 1, &err);
+ for (i = 0; i < len; i++) {
+ visit_type_uint8(v, (uint8_t *)&vmsd->name[i], NULL, &err);
+ }
+ visit_end_array(v, &err);
+ visit_type_uint32(v, (uint32_t *)&vmsd->version_id, "version_id", &err);
+ assert(!vmsd->subsections);
vmstate_save_state(f, vmsd, opaque);
+
+ visit_end_struct(v, &err);
}
sub++;
}
+ visit_end_list(v, &err);
}
typedef struct LoadStateEntry {
diff --git a/target-alpha/machine.c b/target-alpha/machine.c
index 76d70d9..4e6c549 100644
--- a/target-alpha/machine.c
+++ b/target-alpha/machine.c
@@ -1,17 +1,22 @@
#include "hw/hw.h"
#include "hw/boards.h"
-static int get_fpcr(QEMUFile *f, void *opaque, size_t size)
+static int get_fpcr(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
CPUAlphaState *env = opaque;
- cpu_alpha_store_fpcr(env, qemu_get_be64(f));
+ uint64_t fpcr;
+ visit_type_uint64(v, &fpcr, name, err);
+ cpu_alpha_store_fpcr(env, fpcr);
return 0;
}
-static void put_fpcr(QEMUFile *f, void *opaque, size_t size)
+static void put_fpcr(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
CPUAlphaState *env = opaque;
- qemu_put_be64(f, cpu_alpha_load_fpcr(env));
+ uint64_t fpcr = cpu_alpha_load_fpcr(env);
+ visit_type_uint64(v, &fpcr, name, err);
}
static const VMStateInfo vmstate_fpcr = {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 25fa97d..1a4281c 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -78,7 +78,8 @@ static const VMStateDescription vmstate_mtrr_var = {
#define VMSTATE_MTRR_VARS(_field, _state, _n, _v) \
VMSTATE_STRUCT_ARRAY(_field, _state, _n, _v, vmstate_mtrr_var, MTRRVar)
-static void put_fpreg_error(QEMUFile *f, void *opaque, size_t size)
+static void put_fpreg_error(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
fprintf(stderr, "call put_fpreg() with invalid arguments\n");
exit(0);
@@ -106,19 +107,23 @@ static void fp64_to_fp80(union x86_longdouble *p, uint64_t temp)
p->exp = e;
}
-static int get_fpreg(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
FPReg *fp_reg = opaque;
uint64_t mant;
uint16_t exp;
- qemu_get_be64s(f, &mant);
- qemu_get_be16s(f, &exp);
+ visit_start_struct(v, NULL, NULL, name, 0, err);
+ visit_type_uint64(v, &mant, "mant", err);
+ visit_type_uint16(v, &exp, "exp", err);
+ visit_end_struct(v, err);
fp_reg->d = cpu_set_fp80(mant, exp);
return 0;
}
-static void put_fpreg(QEMUFile *f, void *opaque, size_t size)
+static void put_fpreg(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
FPReg *fp_reg = opaque;
uint64_t mant;
@@ -126,8 +131,10 @@ static void put_fpreg(QEMUFile *f, void *opaque, size_t size)
/* we save the real CPU data (in case of MMX usage only 'mant'
contains the MMX register */
cpu_get_fp80(&mant, &exp, fp_reg->d);
- qemu_put_be64s(f, &mant);
- qemu_put_be16s(f, &exp);
+ visit_start_struct(v, NULL, NULL, name, 0, err);
+ visit_type_uint64(v, &mant, "mant", err);
+ visit_type_uint16(v, &exp, "exp", err);
+ visit_end_struct(v, err);
}
static const VMStateInfo vmstate_fpreg = {
@@ -136,12 +143,13 @@ static const VMStateInfo vmstate_fpreg = {
.put = put_fpreg,
};
-static int get_fpreg_1_mmx(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg_1_mmx(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
union x86_longdouble *p = opaque;
uint64_t mant;
- qemu_get_be64s(f, &mant);
+ visit_type_uint64(v, &mant, name, err);
p->mant = mant;
p->exp = 0xffff;
return 0;
@@ -153,12 +161,13 @@ static const VMStateInfo vmstate_fpreg_1_mmx = {
.put = put_fpreg_error,
};
-static int get_fpreg_1_no_mmx(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg_1_no_mmx(Visitor *v, const char *name, void *opaque,
+ size_t size, Error **err)
{
union x86_longdouble *p = opaque;
uint64_t mant;
- qemu_get_be64s(f, &mant);
+ visit_type_uint64(v, &mant, name, err);
fp64_to_fp80(p, mant);
return 0;
}
@@ -212,17 +221,23 @@ static bool less_than_7(void *opaque, int version_id)
return version_id < 7;
}
-static int get_uint64_as_uint32(QEMUFile *f, void *pv, size_t size)
+static int get_uint64_as_uint32(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- uint64_t *v = pv;
- *v = qemu_get_be32(f);
+ uint64_t *val1 = pv;
+ uint32_t val2;
+ visit_type_uint32(v, &val2, name, err);
+ *val1 = val2;
return 0;
}
-static void put_uint64_as_uint32(QEMUFile *f, void *pv, size_t size)
+static void put_uint64_as_uint32(Visitor *v, const char *name, void *pv,
+ size_t size, Error **err)
{
- uint64_t *v = pv;
- qemu_put_be32(f, *v);
+ uint64_t *val1 = pv;
+ uint32_t val2;
+ visit_type_uint32(v, &val2, name, err);
+ *val1 = val2;
}
static const VMStateInfo vmstate_hack_uint64_as_uint32 = {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
@ 2011-12-20 11:12 ` Paolo Bonzini
2011-12-20 11:43 ` Paolo Bonzini
2011-12-20 13:50 ` Anthony Liguori
0 siblings, 2 replies; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-20 11:12 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Anthony Liguori, Juan Quintela
> +void visit_start_array(Visitor *v, void **obj, const char *name, size_t elem_count,
> + size_t elem_size, Error **errp);
> +void visit_next_array(Visitor *v, Error **errp);
> +void visit_end_array(Visitor *v, Error **errp);
> void visit_start_optional(Visitor *v, bool *present, const char *name,
> Error **errp);
> 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);
I think this approach is wrong. We're mashing the design of vmstate
with that of visitors and getting something that is not a visitor and
not vmstate.
Instead, I think you should have something like this:
struct VMStateInfo {
const char *name;
// takes a QMPOutputVisitor and a QEMUFile open for reading
int (*load)(QEMUFile *f, const char *name, Visitor *v,
size_t size, Error **err);
// takes a QMPInputVisitor and a QEMUFile open for writing
void (*save)(QEMUFile *f, const char *name, Visitor *v,
size_t size, Error **err);
// takes a QMPOutputVisitor and reads from *pv
int (*get)(Visitor *v, const char *name, void *pv,
size_t size, Error **err);
// takes a QMPInputVisitor and writes into *pv
void (*set)(Visitor *v, const char *name, void *pv,
size_t size, Error **err);
};
that splits the existing callbacks in two steps.
For saving, you would adapt your visitor-based vmstate "put" routines so
that they put things in a dictionary with no regard for integer types (a
bit ugly for uint64, but perfectly fine for everything else). You take
the dictionary from the output visitor and (with an input visitor) you
feed it back to the "save" routines, which convert the dictionary to a
QEMUFile. Both steps keep the types internal to vmstate.
For loading, it's the other way round: you interpret the vmstate with
the QEMUFile reading routines, and create a dictionary. Then make an
input visitor and use the vmstate "set" interpreter to fill in the
struct fields.
I'm sorry for noticing this just now, I was waiting for Anthony's QOM
plans to be committed so that I could understand better how vmstate and
QOM properties would interact. In fact, it would be great and not hard
if the struct<->visitor step (get/set) was also exposed as a QOM property.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 11:12 ` Paolo Bonzini
@ 2011-12-20 11:43 ` Paolo Bonzini
2011-12-20 12:00 ` Paolo Bonzini
2011-12-20 13:50 ` Anthony Liguori
1 sibling, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-20 11:43 UTC (permalink / raw)
Cc: Juan Quintela, qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2729 bytes --]
On 12/20/2011 12:12 PM, Paolo Bonzini wrote:
>
> I think this approach is wrong. We're mashing the design of vmstate
> with that of visitors and getting something that is not a visitor and
> not vmstate.
>
> Instead, I think you should have something like this:
>
> struct VMStateInfo {
> const char *name;
> // takes a QMPOutputVisitor and a QEMUFile open for reading
> int (*load)(QEMUFile *f, const char *name, Visitor *v,
> size_t size, Error **err);
>
> // takes a QMPInputVisitor and a QEMUFile open for writing
> void (*save)(QEMUFile *f, const char *name, Visitor *v,
> size_t size, Error **err);
>
> // takes a QMPOutputVisitor and reads from *pv
> int (*get)(Visitor *v, const char *name, void *pv,
> size_t size, Error **err);
>
> // takes a QMPInputVisitor and writes into *pv
> void (*set)(Visitor *v, const char *name, void *pv,
> size_t size, Error **err);
> };
>
> that splits the existing callbacks in two steps.
>
> For saving, you would adapt your visitor-based vmstate "put" routines so
> that they put things in a dictionary with no regard for integer types (a
> bit ugly for uint64, but perfectly fine for everything else). You take
> the dictionary from the output visitor and (with an input visitor) you
> feed it back to the "save" routines, which convert the dictionary to a
> QEMUFile. Both steps keep the types internal to vmstate.
>
> For loading, it's the other way round: you interpret the vmstate with
> the QEMUFile reading routines, and create a dictionary. Then make an
> input visitor and use the vmstate "set" interpreter to fill in the
> struct fields.
>
> I'm sorry for noticing this just now, I was waiting for Anthony's QOM
> plans to be committed so that I could understand better how vmstate and
> QOM properties would interact. In fact, it would be great and not hard
> if the struct<->visitor step (get/set) was also exposed as a QOM property.
Note that this doesn't prevent sharing the code for loading and saving.
1) You can still add a vtable to QEMUFile for "visit_type_int*" and
"visit_type_uint*". But this vtable doesn't need start/end callbacks.
2) Similarly, I don't object to adding visit_type_int* and
visit_type_uint* to Visitor. However, these should be exclusively a
convenience for the callers, so that they do not have to convert between
int64 and other types. There should be exactly two implementations of
these callbacks, one for input visitors (including e.g. the dealloc
visitor) and one for output visitors. I attach a sample patch that does
this for int16 only.
Paolo
[-- Attachment #2: qapi-visitor-types.patch --]
[-- Type: text/x-patch, Size: 4682 bytes --]
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index a154523..17964ad 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -154,6 +154,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
v = g_malloc0(sizeof(*v));
+ qapi_init_input_visitor(&v->visitor);
v->visitor.start_struct = qapi_dealloc_start_struct;
v->visitor.end_struct = qapi_dealloc_end_struct;
v->visitor.start_list = qapi_dealloc_start_list;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index ddef3ed..5c1881d 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -116,3 +116,37 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
v->type_number(v, obj, name, errp);
}
}
+
+static void visit_type_int16_in(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ int64_t value;
+ v->type_int(v, &value, name, errp);
+ *obj = value;
+ }
+}
+
+static void visit_type_int16_out(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ int64_t value = *obj;
+ v->type_int(v, &value, name, errp);
+ }
+}
+
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
+{
+ if (!error_is_set(errp)) {
+ v->type_int16(v, obj, name, errp);
+ }
+}
+
+void qapi_init_input_visitor(Visitor *v)
+{
+ v->type_int16 = visit_type_int16_in;
+}
+
+void qapi_init_output_visitor(Visitor *v)
+{
+ v->type_int16 = visit_type_int16_out;
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index e850746..c0395b3 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -52,6 +52,9 @@ struct Visitor
void (*start_handle)(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp);
void (*end_handle)(Visitor *v, Error **errp);
+
+ /* Internal only. */
+ void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
};
void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -69,8 +72,24 @@ 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_int16(Visitor *v, int16_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);
+static inline void visit_type_uint(Visitor *v, uint64_t *obj,
+ const char *name, Error **errp)
+{
+ visit_type_int(v, (int64_t *) obj, name, errp);
+}
+
+static inline void visit_type_uint16(Visitor *v, uint16_t *obj,
+ const char *name, Error **errp)
+{
+ visit_type_int16(v, (int16_t *) obj, name, errp);
+}
+
+void qapi_init_input_visitor(Visitor *v);
+void qapi_init_output_visitor(Visitor *v);
+
#endif
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index c78022b..67da359 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -283,6 +283,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
v = g_malloc0(sizeof(*v));
+ qapi_init_input_visitor(&v->visitor);
v->visitor.start_struct = qmp_input_start_struct;
v->visitor.end_struct = qmp_input_end_struct;
v->visitor.start_list = qmp_input_start_list;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index f76d015..65f8e3e 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -234,6 +234,7 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
v = g_malloc0(sizeof(*v));
+ qapi_init_output_visitor(&v->visitor);
v->visitor.start_struct = qmp_output_start_struct;
v->visitor.end_struct = qmp_output_end_struct;
v->visitor.start_list = qmp_output_start_list;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 663c2a0..77bc529 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -229,10 +229,8 @@ static void get_int16(DeviceState *dev, Visitor *v, void *opaque,
{
Property *prop = opaque;
int16_t *ptr = qdev_get_prop_ptr(dev, prop);
- int64_t value;
- value = *ptr;
- visit_type_int(v, &value, name, errp);
+ visit_type_int16(v, ptr, name, errp);
}
static void set_int16(DeviceState *dev, Visitor *v, void *opaque,
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 11:43 ` Paolo Bonzini
@ 2011-12-20 12:00 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-20 12:00 UTC (permalink / raw)
Cc: Michael Roth, qemu-devel, Juan Quintela
On 12/20/2011 12:43 PM, Paolo Bonzini wrote:
>
> 1) You can still add a vtable to QEMUFile for "visit_type_int*" and
> "visit_type_uint*". But this vtable doesn't need start/end callbacks.
Here I meant something simple like:
void (*visit_type_int16) (QEMUFile *f, int16_t *x);
that is really the same as qemu_{get,put}_be16s and friends.
It may even be not a vtable, but a function that dispatches based on the
existing get_buffer/put_buffer callbacks: note that one of them is
always NULL. Which also means that the is_write field is superfluous.
So much cleanup to do. :(
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 11:12 ` Paolo Bonzini
2011-12-20 11:43 ` Paolo Bonzini
@ 2011-12-20 13:50 ` Anthony Liguori
2011-12-20 14:30 ` Paolo Bonzini
1 sibling, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2011-12-20 13:50 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Juan Quintela, qemu-devel, Michael Roth
On 12/20/2011 05:12 AM, Paolo Bonzini wrote:
>> +void visit_start_array(Visitor *v, void **obj, const char *name, size_t
>> elem_count,
>> + size_t elem_size, Error **errp);
>> +void visit_next_array(Visitor *v, Error **errp);
>> +void visit_end_array(Visitor *v, Error **errp);
>> void visit_start_optional(Visitor *v, bool *present, const char *name,
>> Error **errp);
>> 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);
>
> I think this approach is wrong. We're mashing the design of vmstate with that of
> visitors and getting something that is not a visitor and not vmstate.
Thanks for taking a look at this.
>
> Instead, I think you should have something like this:
>
> struct VMStateInfo {
> const char *name;
> // takes a QMPOutputVisitor and a QEMUFile open for reading
> int (*load)(QEMUFile *f, const char *name, Visitor *v,
> size_t size, Error **err);
>
> // takes a QMPInputVisitor and a QEMUFile open for writing
> void (*save)(QEMUFile *f, const char *name, Visitor *v,
> size_t size, Error **err);
>
> // takes a QMPOutputVisitor and reads from *pv
> int (*get)(Visitor *v, const char *name, void *pv,
> size_t size, Error **err);
>
> // takes a QMPInputVisitor and writes into *pv
> void (*set)(Visitor *v, const char *name, void *pv,
> size_t size, Error **err);
> };
>
> that splits the existing callbacks in two steps.
>
> For saving, you would adapt your visitor-based vmstate "put" routines so that
> they put things in a dictionary with no regard for integer types (a bit ugly for
> uint64, but perfectly fine for everything else).
I don't understand this. The visitor interface should expose the C level
primitives so that we can maintain fidelity when visiting something. The fact
that it only knows about "ints" today is a short cut.
> You take the dictionary from
> the output visitor and (with an input visitor) you feed it back to the "save"
> routines, which convert the dictionary to a QEMUFile. Both steps keep the types
> internal to vmstate.
That doesn't make effective use of visitors. Visitors should preserve as much
type information as possible. I'm not really sure I understand the whole
QEMUFile tie in either. This series:
1) Makes a fully compatible QEMUFile input and output Visitor
2) Makes VMState no longer know about QEMUFile by using (1)
(2) is really the end goal. If we have an interface that still uses QEMUFile,
we're doing something wrong IMHO.
> For loading, it's the other way round: you interpret the vmstate with the
> QEMUFile reading routines, and create a dictionary. Then make an input visitor
> and use the vmstate "set" interpreter to fill in the struct fields.
>
> I'm sorry for noticing this just now, I was waiting for Anthony's QOM plans to
> be committed so that I could understand better how vmstate and QOM properties
> would interact. In fact, it would be great and not hard if the struct<->visitor
> step (get/set) was also exposed as a QOM property.
That's exactly why I'm so anxious to get this merged :-)
Regards,
Anthony Liguori
> Paolo
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 13:50 ` Anthony Liguori
@ 2011-12-20 14:30 ` Paolo Bonzini
2011-12-20 20:22 ` Michael Roth
2011-12-20 20:56 ` Anthony Liguori
0 siblings, 2 replies; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-20 14:30 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Juan Quintela, qemu-devel, Michael Roth
On 12/20/2011 02:50 PM, Anthony Liguori wrote:
>> For saving, you would adapt your visitor-based vmstate "put"
>> routines so that they put things in a dictionary with no regard for
>> integer types (a bit ugly for uint64, but perfectly fine for
>> everything else).
>
> I don't understand this. The visitor interface should expose the C
> level primitives so that we can maintain fidelity when visiting
> something. The fact that it only knows about "ints" today is a short
> cut.
Why does this need to be in Visitor? You can always embed C knowledge
in an adaptor or decorator. Visitors only need to know about names and
JSON types (well, they also distinguish int from double).
We already have such an adaptor: QOM static properties know about
names, JSON types, C type and struct offset.
VMState fields know about all that plus QEMUFile encoding. QEMUFile
encoding can be hidden in the decorator, it does not need to become
visible to the concrete visitors.
As always, you can implement that in many ways. However, I think the
point of using Visitors is not to remove QEMUFile. It is to provide a
backend-independent representation that backends can transform and that
secondarily can be exposed by QOM.
This is only half-true in Michael's code, because he relies on
primitives that QMPInputVisitor and QMPOutputVisitor do not implement.
Fixing this is quite easy, you only need to add a base-class
implementation of the int8/int16/... primitives.
On top of this the representation he passes to visitors is somewhat
redundant. For example, VMState has "equal" fields; they are fields
that are serialized but are really fixed at compile- or realize-time.
Such fields should not be part of the backend-independent
representation. With Michael's approach they are, and that's quite deep
in the implementation.
>> You take the dictionary from the output visitor and (with an input
>> visitor) you feed it back to the "save" routines, which convert the
>> dictionary to a QEMUFile. Both steps keep the types internal to
>> vmstate.
>
> That doesn't make effective use of visitors. Visitors should preserve
> as much type information as possible. I'm not really sure I
> understand the whole QEMUFile tie in either. This series:
>
> 1) Makes a fully compatible QEMUFile input and output Visitor
>
> 2) Makes VMState no longer know about QEMUFile by using (1)
>
> (2) is really the end goal. If we have an interface that still uses
> QEMUFile, we're doing something wrong IMHO.
Yes, this is accurate, but I see the goals differently. We should:
(1) First and foremost, provide a backend-independent representation of
device state so that we can add other backends later.
(2) Serialize this with QEMUFile, both for backwards-compatibility and
to ensure that the whole thing works.
Whether you do (2) directly with QEMUFile or, like Michael does, with
QEMUFile*Visitors is secondary. I don't have big objections to either
approach. However, the series is missing (1).
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 14:30 ` Paolo Bonzini
@ 2011-12-20 20:22 ` Michael Roth
2011-12-21 12:29 ` Paolo Bonzini
2011-12-20 20:56 ` Anthony Liguori
1 sibling, 1 reply; 24+ messages in thread
From: Michael Roth @ 2011-12-20 20:22 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Juan Quintela
On 12/20/2011 08:30 AM, Paolo Bonzini wrote:
> On 12/20/2011 02:50 PM, Anthony Liguori wrote:
>>> For saving, you would adapt your visitor-based vmstate "put"
>>> routines so that they put things in a dictionary with no regard for
>>> integer types (a bit ugly for uint64, but perfectly fine for
>>> everything else).
>>
>> I don't understand this. The visitor interface should expose the C
>> level primitives so that we can maintain fidelity when visiting
>> something. The fact that it only knows about "ints" today is a short
>> cut.
>
> Why does this need to be in Visitor? You can always embed C knowledge in
> an adaptor or decorator. Visitors only need to know about names and JSON
> types (well, they also distinguish int from double).
The main goal is to abstract away data serialization schemes
(QObject->JSON, C->QEMUFile, etc). In the case of a JSON-based
serialization, the visitor interface for fixed-with types would end up
serializing everything as int64_t/double, but QEMUFile requires
byte-length affinity to remain backward-compatible, so that information
must be passed on to the Visitor interface when we call it.
And beyond QEMUFile, we'd like to eventually move to a serialization
scheme that is self-describing in the types of the fields it stores so
that we can do stricter checking in the deserialization/input visitor
routines, or just plain be able to make sense of the serialized data
without any outside information, since those schemes would eventually be
used directly in implementing a new migration wire protocol and/or
device state introspection.
>
> We already have such an adaptor: QOM static properties know about names,
> JSON types, C type and struct offset.
>
> VMState fields know about all that plus QEMUFile encoding. QEMUFile
> encoding can be hidden in the decorator, it does not need to become
> visible to the concrete visitors.
And these are both requirements to implementing a robust, flexible
serialization/Visitor interface with pluggable back-ends, but if those
interface throw away the type/field names then the only way to get them
back is to deserialize, which isn't useful for introspection, and
volatile for migration (since type errors can be silently missed in a
lot of cases)
>
> As always, you can implement that in many ways. However, I think the
> point of using Visitors is not to remove QEMUFile. It is to provide a
> backend-independent representation that backends can transform and that
> secondarily can be exposed by QOM.
Agreed, it's just a matter of wanting to maintain that information from
start to finish.
>
> This is only half-true in Michael's code, because he relies on
> primitives that QMPInputVisitor and QMPOutputVisitor do not implement.
> Fixing this is quite easy, you only need to add a base-class
> implementation of the int8/int16/... primitives.
Yup, that's the plan. These patches are a bit lazy in that regard. I
agree that if we get into the habit of adding interfaces for a specific
back-end without mapping these to the base-class implementations in
other backends things will get out of hand quickly. Fortunately we
haven't yet hit a situation where one backend ends up adding an
interface that other backends cant' handle in some form.
>
> On top of this the representation he passes to visitors is somewhat
> redundant. For example, VMState has "equal" fields; they are fields that
> are serialized but are really fixed at compile- or realize-time. Such
> fields should not be part of the backend-independent representation.
> With Michael's approach they are, and that's quite deep in the
> implementation.
You mean, for instance, put_int32()/get_int32_equal()? If so, I'm not
sure I follow. In that case we use a Visitor purely to
serialize/deserialize an int32, vmstate adds the *_equal() interface as
helper function on top of that, but it's not part of the Visitor interfaces.
>
>>> You take the dictionary from the output visitor and (with an input
>>> visitor) you feed it back to the "save" routines, which convert the
>>> dictionary to a QEMUFile. Both steps keep the types internal to
>>> vmstate.
>>
>> That doesn't make effective use of visitors. Visitors should preserve
>> as much type information as possible. I'm not really sure I
>> understand the whole QEMUFile tie in either. This series:
>>
>> 1) Makes a fully compatible QEMUFile input and output Visitor
>>
>> 2) Makes VMState no longer know about QEMUFile by using (1)
>>
>> (2) is really the end goal. If we have an interface that still uses
>> QEMUFile, we're doing something wrong IMHO.
>
> Yes, this is accurate, but I see the goals differently. We should:
>
> (1) First and foremost, provide a backend-independent representation of
> device state so that we can add other backends later.
>
> (2) Serialize this with QEMUFile, both for backwards-compatibility and
> to ensure that the whole thing works.
>
> Whether you do (2) directly with QEMUFile or, like Michael does, with
> QEMUFile*Visitors is secondary. I don't have big objections to either
> approach. However, the series is missing (1).
I'll fix up the existing non-QEMUFile Visitor backends with base-class
implementations for all the new interfaces. Beyond that, is there
anything else missing to achieve 1)?
>
> Paolo
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 14:30 ` Paolo Bonzini
2011-12-20 20:22 ` Michael Roth
@ 2011-12-20 20:56 ` Anthony Liguori
2011-12-21 12:35 ` Paolo Bonzini
1 sibling, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2011-12-20 20:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Michael Roth, qemu-devel, Juan Quintela
On 12/20/2011 08:30 AM, Paolo Bonzini wrote:
> On 12/20/2011 02:50 PM, Anthony Liguori wrote:
>>> For saving, you would adapt your visitor-based vmstate "put"
>>> routines so that they put things in a dictionary with no regard for
>>> integer types (a bit ugly for uint64, but perfectly fine for
>>> everything else).
>>
>> I don't understand this. The visitor interface should expose the C
>> level primitives so that we can maintain fidelity when visiting
>> something. The fact that it only knows about "ints" today is a short
>> cut.
>
> Why does this need to be in Visitor? You can always embed C knowledge in an
> adaptor or decorator. Visitors only need to know about names and JSON types
> (well, they also distinguish int from double).
You are tying Visitors too closely to JSON. We should be able to write a
Visitor that can output a serialization format that has more interesting integer
types and maintains better fidelity with standard C types.
> We already have such an adaptor: QOM static properties know about names, JSON
> types, C type and struct offset.
Yes... But I don't see the relevance here.
>
> VMState fields know about all that plus QEMUFile encoding. QEMUFile encoding can
> be hidden in the decorator, it does not need to become visible to the concrete
> visitors.
This is mixing up too many concepts.
A visit function -> knows only how to walk a C data structure. It's just
saying, I have an int, it's name is X, i have a double, it's name is Y.
The Visitor is the thing that plugs into the visit function and decides what to
do with this information.
Having a "QEMUFile" decorator just doesn't fit the model. I'm not even sure
what it means.
> As always, you can implement that in many ways. However, I think the point of
> using Visitors is not to remove QEMUFile.
Yes, it is.
> It is to provide a backend-independent
> representation that backends can transform and that secondarily can be exposed
> by QOM.
The point of Visitors is to make up for the fact that C lacks introspection.
It's meant to be a standard way to introspect a C data structure (or type).
>
> This is only half-true in Michael's code, because he relies on primitives that
> QMPInputVisitor and QMPOutputVisitor do not implement. Fixing this is quite
> easy, you only need to add a base-class implementation of the int8/int16/...
> primitives.
>
> On top of this the representation he passes to visitors is somewhat redundant.
> For example, VMState has "equal" fields; they are fields that are serialized but
> are really fixed at compile- or realize-time. Such fields should not be part of
> the backend-independent representation. With Michael's approach they are, and
> that's quite deep in the implementation.
Yes, but there's no way to do this today without breaking the format. There's
just too much magic in VMState right now. We need something like a migration
filter capability where we can encapsulate this kind of logic such that we can
ween VMState away from these things (and ultimately switch to an IDL compiler).
We can't do a migration filter until we have something like Michael's series.
>
>>> You take the dictionary from the output visitor and (with an input
>>> visitor) you feed it back to the "save" routines, which convert the
>>> dictionary to a QEMUFile. Both steps keep the types internal to
>>> vmstate.
>>
>> That doesn't make effective use of visitors. Visitors should preserve
>> as much type information as possible. I'm not really sure I
>> understand the whole QEMUFile tie in either. This series:
>>
>> 1) Makes a fully compatible QEMUFile input and output Visitor
>>
>> 2) Makes VMState no longer know about QEMUFile by using (1)
>>
>> (2) is really the end goal. If we have an interface that still uses
>> QEMUFile, we're doing something wrong IMHO.
>
> Yes, this is accurate, but I see the goals differently. We should:
>
> (1) First and foremost, provide a backend-independent representation of device
> state so that we can add other backends later.
And Mike's series does this, no?
> (2) Serialize this with QEMUFile, both for backwards-compatibility and to ensure
> that the whole thing works.
Mike's series also does this, no?
> Whether you do (2) directly with QEMUFile or, like Michael does, with
> QEMUFile*Visitors is secondary. I don't have big objections to either approach.
> However, the series is missing (1).
I don't see how.
Regards,
Anthony Liguori
>
> Paolo
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 20:22 ` Michael Roth
@ 2011-12-21 12:29 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-21 12:29 UTC (permalink / raw)
To: Michael Roth; +Cc: qemu-devel, Juan Quintela
On 12/20/2011 09:22 PM, Michael Roth wrote:
> The main goal is to abstract away data serialization schemes
> (QObject->JSON, C->QEMUFile, etc).
Right. And the simplest way to abstract the scheme is to provide a
backend-independent representation of device state.
As a convention, I'll call:
- "device state" the fields in the struct
- "QEMUFile" the serialized output that VMState save produces
- "device state representation" a representation of device state that is
independent of the actual serialization backend.
- "device configuration" is the parameters that are used to create the
device (coming from compile-time constants or the command-line)
> In the case of a JSON-based serialization, the visitor interface for
> fixed-with types would end up serializing everything as
> int64_t/double, but QEMUFile requires byte-length affinity to remain
> backward-compatible, so that information must be passed on to the
> Visitor interface when we call it.
When creating the device state representation from either device state
or QEMUFile, you need the byte length to fetch fields correctly.
When recreating device state from its representation, or saving to
QEMUFile state, you need the byte length to store fields correctly.
However, you do not need the byte length in the device state
representation. In all four cases (from/to device state, from/to
QEMUFile) the byte length can be fetched from the VMState.
>> As always, you can implement that in many ways. However, I think the
>> point of using Visitors is not to remove QEMUFile. It is to provide a
>> backend-independent representation that backends can transform and that
>> secondarily can be exposed by QOM.
>
> Agreed, it's just a matter of wanting to maintain that information from
> start to finish.
I don't see the point of maintaining the type hints from start to
finish, as long as you can reconstruct it any time you want it.
>> On top of this the representation he passes to visitors is somewhat
>> redundant. For example, VMState has "equal" fields; they are fields that
>> are serialized but are really fixed at compile- or realize-time. Such
>> fields should not be part of the backend-independent representation.
>> With Michael's approach they are, and that's quite deep in the
>> implementation.
>
> You mean, for instance, put_int32()/get_int32_equal()? If so, I'm not
> sure I follow. In that case we use a Visitor purely to
> serialize/deserialize an int32, vmstate adds the *_equal() interface as
> helper function on top of that, but it's not part of the Visitor
> interfaces.
It should be only in QEMUFile, because it's one of its quirks. It is a
separate property that is fixed at compile-time or realize-time, so it's
part of device configuration; it's not part of device state representation.
>> Yes, this is accurate, but I see the goals differently. We should:
>>
>> (1) First and foremost, provide a backend-independent representation of
>> device state so that we can add other backends later.
>>
>> (2) Serialize this with QEMUFile, both for backwards-compatibility and
>> to ensure that the whole thing works.
>>
>> Whether you do (2) directly with QEMUFile or, like Michael does, with
>> QEMUFile*Visitors is secondary. I don't have big objections to either
>> approach. However, the series is missing (1).
>
> I'll fix up the existing non-QEMUFile Visitor backends with base-class
> implementations for all the new interfaces. Beyond that, is there
> anything else missing to achieve 1)?
I think almost nothing (you need to integrate into the QOM properties,
and to handle a few special cases such as VMSTATE_EQUAL and
VMSTATE_UNUSED). That's quite good.
However, once you do this, you will still be serializing QEMUFile
directly from device state rather than from device state representation.
This is fine, but not what we should do when working on BER
serialization or similar.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-20 20:56 ` Anthony Liguori
@ 2011-12-21 12:35 ` Paolo Bonzini
2011-12-21 14:45 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-21 12:35 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Juan Quintela, Michael Roth, qemu-devel
On 12/20/2011 09:56 PM, Anthony Liguori wrote:
>> As always, you can implement that in many ways. However, I think the
>> point of using Visitors is not to remove QEMUFile.
>
> Yes, it is.
The point of using Visitors is to provide a standard representation of
device state. QEMUFile is but one consumer of that representation,
along with any other migration filter.
> We need something like a migration filter capability where we can
> encapsulate this kind of logic such that we can ween VMState away
> from these things (and ultimately switch to an IDL compiler).
>
> We can't do a migration filter until we have something like Michael's
> series.
Agreed. I'm saying that the QEMUFile serialization should be the first
example of a migration filter. Mike's patch add a layer of indirection
in the code, but not in the data. We can't do a migration filter until
we have device state represented as introspectable data.
>> Yes, this is accurate, but I see the goals differently. We should:
>>
>> (1) First and foremost, provide a backend-independent representation
>> of device state so that we can add other backends later.
>
> And Mike's series does this, no?
No.
>> (2) Serialize this with QEMUFile, both for backwards-compatibility and
>> to ensure that the whole thing works.
>
> Mike's series also does this, no?
It serializes device state directly, without going through a
backend-independent representation. It doesn't provide a place in which
you can plug a filter.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-21 12:35 ` Paolo Bonzini
@ 2011-12-21 14:45 ` Anthony Liguori
2011-12-21 15:39 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2011-12-21 14:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Michael Roth, Juan Quintela
On 12/21/2011 06:35 AM, Paolo Bonzini wrote:
> On 12/20/2011 09:56 PM, Anthony Liguori wrote:
>>> As always, you can implement that in many ways. However, I think the
>>> point of using Visitors is not to remove QEMUFile.
>>
>> Yes, it is.
>
> The point of using Visitors is to provide a standard representation of device
> state. QEMUFile is but one consumer of that representation, along with any other
> migration filter.
Can you do a quick code mock up of how you'd expect this to work? I'm not sure
if I'm just being dense, but I'm having a really hard time understanding what
you're suggesting. How I see this evolving with Mike's series is:
struct DeviceStateClass {
ObjectClass parent;
void (*load_state)(DeviceState *obj, Visitor *v, const char *name,
Error **errp);
void (*save_state)(DeviceState *obj, Visitor *v, const char *name,
Error **errp);
};
The PCIDevice base class would expose:
/* protected */
void pci_load_state(PCIDevice *obj, Visitor *v, const char *name,
Error **errp)
{
visit_start_struct(v, "PCIDevice", name, errp);
visit_type_int8(v, &obj->reg[0], "reg[0]", errp);
..
visit_end_struct(v, errp);
}
Or:
static VMStateDescription pci_device_desc = {
.name = "PCIDevice",
.fields = {
VMSTATE_UINT8(PCIDevice, reg[0]),
...
}
};
void pci_load_state(PCIDevice *obj, Visitor *v, const char *name,
Error **errp)
{
visit_with_vmstate(v, obj, &pci_device_desc, name, errp);
}
A subclass would do:
static void my_device_load_state(DeviceState *obj, Visitor *v, const char *name,
Error **errp)
{
MyDevice *s = MY_DEVICE(obj);
visit_start_struct(v, "MyDevice", name, errp);
pci_load_state(PCI_DEVICE(obj), v, "super", errp);
visit_end_struct(v, errp);
}
static void my_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
dc->load_state = my_device_load_state;
...
}
There's no reference at all to QEMUFile. The load_state/save_state methods can
be exposed as a "state" property too.
Once the series 2/4 lands and Mike's series, implementing this should be very
straight forward.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-21 14:45 ` Anthony Liguori
@ 2011-12-21 15:39 ` Paolo Bonzini
2011-12-21 16:24 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-21 15:39 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Michael Roth, Juan Quintela
On 12/21/2011 03:45 PM, Anthony Liguori wrote:
> On 12/21/2011 06:35 AM, Paolo Bonzini wrote:
>> On 12/20/2011 09:56 PM, Anthony Liguori wrote:
>>>> As always, you can implement that in many ways. However, I think the
>>>> point of using Visitors is not to remove QEMUFile.
>>>
>>> Yes, it is.
>>
>> The point of using Visitors is to provide a standard representation of
>> device
>> state. QEMUFile is but one consumer of that representation, along with
>> any other
>> migration filter.
>
> Can you do a quick code mock up of how you'd expect this to work?
void
vmstate_get(Device *obj, Visitor *v, void *opaque, const char *name)
{
/* Use the VMStateDescription in opaque to add fields to Visitor
from the struct. */
}
void
vmstate_set(Device *obj, Visitor *v, void *opaque, const char *name)
{
/* Use the VMStateDescription in opaque to retrieve fields from
Visitor and store them in the struct. */
}
void
vmstate_load_state(Device *obj, Visitor *v, QEMUFile )
{
VMStateDescription *vmsd = ???; /* something from the DeviceClass */
/* Use the VMStateDescription in opaque to retrieve fields from
Visitor and store them in the struct. This is basically the
VMState interpreter currently in savevm.c, only fetching fields
from v rather than the file. */
}
void
vmstate_save_state(Device *obj, Visitor *v, QEMUFile *out)
{
VMStateDescription *vmsd = ???; /* something from the DeviceClass */
/* Use the VMStateDescription in opaque to fetch fields from
Visitor and store them in the file. This is basically the
other VMState interpreter currently in savevm.c, but fetching
fields from v rather than the struct. */
}
void
qdev_add_vmstate(Device *obj, VMStateDescription *vmsd)
{
qdev_add_property(obj, "vmstate", vmstate_get, vmstate_set, vmsd);
qdev_add_interface(obj, "QEMUFileSerializable", vmstate_load_state,
vmstate_save_state);
}
savevm.c:
Visitor *v = qmp_output_visitor_new();
qdev_property_get(obj, "vmstate", v);
QObject *qobj = qmp_visitor_get_obj(v);
qmp_visitor_free(v);
Visitor *v = qmp_input_visitor_new(qobj);
QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
s->save_state(obj, v, outfile);
qmp_visitor_free(v);
...
Visitor *v = qmp_output_visitor_new();
QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
s->load_state(obj, v, outfile);
QObject *qobj = qmp_visitor_get_obj(v);
qmp_visitor_free(v);
Visitor *v = qmp_input_visitor_new(qobj);
QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
qdev_property_set(obj, "vmstate", v);
qmp_visitor_free(v);
---------------------
Yes, this makes QEMUFile serialization special. But that's because it's
legacy, and it needs to do strange things and store things beyond the
contents of the vmstate.
Take for example "unused" fields. In Mike's implementation, if you try
to pass a QMPOutputVisitor to save_state you might get a "unused" entry
that is an array of zeros. I have no idea what happens if a single
VMStateDescription has more than one VMSTATE_UNUSED field.
QEMUFileOutputVisitor completely ignores the field names, so it probably
works but would break with QMPOutputVisitor.
Other serialization backends should not need any hooks in the devices
beyond save_state and load_state.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-21 15:39 ` Paolo Bonzini
@ 2011-12-21 16:24 ` Anthony Liguori
2011-12-21 16:52 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2011-12-21 16:24 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Juan Quintela, qemu-devel, Michael Roth
On 12/21/2011 09:39 AM, Paolo Bonzini wrote:
> On 12/21/2011 03:45 PM, Anthony Liguori wrote:
>> On 12/21/2011 06:35 AM, Paolo Bonzini wrote:
>>> On 12/20/2011 09:56 PM, Anthony Liguori wrote:
>>>>> As always, you can implement that in many ways. However, I think the
>>>>> point of using Visitors is not to remove QEMUFile.
>>>>
>>>> Yes, it is.
>>>
>>> The point of using Visitors is to provide a standard representation of
>>> device
>>> state. QEMUFile is but one consumer of that representation, along with
>>> any other
>>> migration filter.
>>
>> Can you do a quick code mock up of how you'd expect this to work?
>
> void
> vmstate_get(Device *obj, Visitor *v, void *opaque, const char *name)
> {
> /* Use the VMStateDescription in opaque to add fields to Visitor
> from the struct. */
> }
>
> void
> vmstate_set(Device *obj, Visitor *v, void *opaque, const char *name)
> {
> /* Use the VMStateDescription in opaque to retrieve fields from
> Visitor and store them in the struct. */
> }
>
> void
> vmstate_load_state(Device *obj, Visitor *v, QEMUFile )
> {
> VMStateDescription *vmsd = ???; /* something from the DeviceClass */
>
> /* Use the VMStateDescription in opaque to retrieve fields from
> Visitor and store them in the struct. This is basically the
> VMState interpreter currently in savevm.c, only fetching fields
> from v rather than the file. */
> }
>
> void
> vmstate_save_state(Device *obj, Visitor *v, QEMUFile *out)
> {
> VMStateDescription *vmsd = ???; /* something from the DeviceClass */
>
> /* Use the VMStateDescription in opaque to fetch fields from
> Visitor and store them in the file. This is basically the
> other VMState interpreter currently in savevm.c, but fetching
> fields from v rather than the struct. */
> }
>
> void
> qdev_add_vmstate(Device *obj, VMStateDescription *vmsd)
> {
> qdev_add_property(obj, "vmstate", vmstate_get, vmstate_set, vmsd);
> qdev_add_interface(obj, "QEMUFileSerializable", vmstate_load_state,
> vmstate_save_state);
Ok, I get what you're suggesting. You would like to continue to keep
VMStateDescription describing both the QEMUFile format and the fields.
I'm suggesting something fundamentally different. What I see us doing is
breaking VMStateDescription apart into two different things. One would be a
pure description of current fields. The other one would be the description of
the old protocol.
The later would be fed to a migration filter and the former would live off of
DeviceState.
By doing Mike's series, we can do this incrementally by splitting the
description up, and invoking the filter post_load/pre_save.
One of the reasons for this split up is because I would like to generate the
first table by IDL and make the second table not dependent on structure members
(so we don't end up with all the hacks we have with dummy struct fields).
Regards,
Anthony Liguori
> }
>
>
> savevm.c:
>
> Visitor *v = qmp_output_visitor_new();
> qdev_property_get(obj, "vmstate", v);
> QObject *qobj = qmp_visitor_get_obj(v);
> qmp_visitor_free(v);
>
> Visitor *v = qmp_input_visitor_new(qobj);
> QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
> s->save_state(obj, v, outfile);
> qmp_visitor_free(v);
>
> ...
>
> Visitor *v = qmp_output_visitor_new();
> QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
> s->load_state(obj, v, outfile);
> QObject *qobj = qmp_visitor_get_obj(v);
> qmp_visitor_free(v);
>
> Visitor *v = qmp_input_visitor_new(qobj);
> QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj);
> qdev_property_set(obj, "vmstate", v);
> qmp_visitor_free(v);
>
> ---------------------
>
> Yes, this makes QEMUFile serialization special. But that's because it's legacy,
> and it needs to do strange things and store things beyond the contents of the
> vmstate.
>
> Take for example "unused" fields. In Mike's implementation, if you try to pass a
> QMPOutputVisitor to save_state you might get a "unused" entry that is an array
> of zeros. I have no idea what happens if a single VMStateDescription has more
> than one VMSTATE_UNUSED field. QEMUFileOutputVisitor completely ignores the
> field names, so it probably works but would break with QMPOutputVisitor.
>
> Other serialization backends should not need any hooks in the devices beyond
> save_state and load_state.
>
> Paolo
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
2011-12-21 16:24 ` Anthony Liguori
@ 2011-12-21 16:52 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2011-12-21 16:52 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Juan Quintela, qemu-devel, Michael Roth
On 12/21/2011 05:24 PM, Anthony Liguori wrote:
> Ok, I get what you're suggesting. You would like to continue to keep
> VMStateDescription describing both the QEMUFile format and the fields.
I don't "like" to do that, but yes, that's what I'm suggesting. :)
You envision having the front-end (state->introspectable state
representation) and back-end (state representation->serialization)
completely decoupled in the future, with migration filters in the middle
if necessary.
I actually agree this is as the final direction. For this reason, the
first step should be to decouple the front-end and backend and actually
have this introspectable state representation. Then you can also break
VMStateDescription apart into a front-end and backend part.
Mike's patches change the way you write to QEMUFile, so the new code
_does_ go in the right direction. However, they fail to provide an
introspectable, backend-independent state representation. Let's put it
in a few diagrams:
where we are now: struct ---> QEMUFile
^
|
VMStateDescription
what Mike's patches do: struct ---> QEMUFileVisitor ---> QEMUFile
^
|
VMStateDescription
what I proposed: struct ---> QMPOutputVisitor ---> QObject
^
|
VMStateDescription
QObject ---> QEMUFile
^
|
VMStateDescription
where you want to go: struct ---> QMPOutputVisitor ---> QObject
^
|
DeviceStateDescription
QObject ---> QEMUFileOutputVisitor ---> QEMUFile
^
|
VMStateDescription
As Mike's patches stand, I'm worried that they would make further steps
harder rather than easier. This is because I'm not convinced that the
QEMUFileOutputVisitor is actually useful.
However, the new code from Mike's patches is very close to the
backend-independent representation from the VMStateDescription. So,
Mike's patches could be morphed into this pretty easily:
struct ---> QMPOutputVisitor ---> QObject
^
|
VMStateDescription
struct ---> QEMUFile \
^ \ that's savevm.c,
| / unchanged
VMStateDescription /
This is an intermediate state that lets us do the next steps:
- serialize to QEMUFile from a QObject;
- reintroduce Mike's QEMUFileOutputVisitor [I don't really see the
benefit of this];
- split the DeviceStateDescription and the VMStateDescription;
None of these three steps are a prerequisite for introducing a new
migration format.
> One of the reasons for this split up is because I would like to generate
> the first table by IDL and make the second table not dependent on
> structure members (so we don't end up with all the hacks we have with
> dummy struct fields).
That would be a few years away. Let's reason in incremental steps.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2011-12-21 16:52 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-27 17:06 [Qemu-devel] [PATCH v2 00/10] do savevm/migration save/load via Visitor interface Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t Michael Roth
2011-12-20 11:12 ` Paolo Bonzini
2011-12-20 11:43 ` Paolo Bonzini
2011-12-20 12:00 ` Paolo Bonzini
2011-12-20 13:50 ` Anthony Liguori
2011-12-20 14:30 ` Paolo Bonzini
2011-12-20 20:22 ` Michael Roth
2011-12-21 12:29 ` Paolo Bonzini
2011-12-20 20:56 ` Anthony Liguori
2011-12-21 12:35 ` Paolo Bonzini
2011-12-21 14:45 ` Anthony Liguori
2011-12-21 15:39 ` Paolo Bonzini
2011-12-21 16:24 ` Anthony Liguori
2011-12-21 16:52 ` Paolo Bonzini
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 02/10] qapi: add QemuFileOutputVisitor Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 03/10] qapi: add QemuFileInputVisitor Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 04/10] savevm: move QEMUFile interfaces into qemu-file.c Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 05/10] qapi: test cases for QEMUFile input/output visitors Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 06/10] qemu-file: add QEMUFile<->visitor lookup routines Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 07/10] trace: qemu_(put|get)_(byte|buffer) events Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 08/10] trace: add trace statements for visitor interface Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 09/10] qapi: add trace statements to qapi-visit-core.c Michael Roth
2011-10-27 17:06 ` [Qemu-devel] [PATCH v2 10/10] vmstate: use visitors Michael Roth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.