* [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-13 13:29 ` Dr. David Alan Gilbert
` (2 more replies)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id Marc-André Lureau
` (5 subsequent siblings)
6 siblings, 3 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
migration/qjson.h | 2 ++
migration/savevm.c | 3 +--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/migration/qjson.h b/migration/qjson.h
index 41664f2d71..1786bb5864 100644
--- a/migration/qjson.h
+++ b/migration/qjson.h
@@ -24,4 +24,6 @@ void json_start_object(QJSON *json, const char *name);
const char *qjson_get_str(QJSON *json);
void qjson_finish(QJSON *json);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy)
+
#endif /* QEMU_QJSON_H */
diff --git a/migration/savevm.c b/migration/savevm.c
index 4a86128ac4..6caa35a679 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1290,7 +1290,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
bool in_postcopy,
bool inactivate_disks)
{
- QJSON *vmdesc;
+ g_autoptr(QJSON) vmdesc = NULL;
int vmdesc_len;
SaveStateEntry *se;
int ret;
@@ -1351,7 +1351,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
qemu_put_be32(f, vmdesc_len);
qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
}
- qjson_destroy(vmdesc);
return 0;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error Marc-André Lureau
@ 2019-09-13 13:29 ` Dr. David Alan Gilbert
2019-09-17 12:31 ` Daniel P. Berrangé
2019-09-25 9:49 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-13 13:29 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Could have put that as a separate patch.
> ---
> migration/qjson.h | 2 ++
> migration/savevm.c | 3 +--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/qjson.h b/migration/qjson.h
> index 41664f2d71..1786bb5864 100644
> --- a/migration/qjson.h
> +++ b/migration/qjson.h
> @@ -24,4 +24,6 @@ void json_start_object(QJSON *json, const char *name);
> const char *qjson_get_str(QJSON *json);
> void qjson_finish(QJSON *json);
>
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy)
> +
> #endif /* QEMU_QJSON_H */
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a86128ac4..6caa35a679 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1290,7 +1290,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> bool in_postcopy,
> bool inactivate_disks)
> {
> - QJSON *vmdesc;
> + g_autoptr(QJSON) vmdesc = NULL;
> int vmdesc_len;
> SaveStateEntry *se;
> int ret;
> @@ -1351,7 +1351,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> qemu_put_be32(f, vmdesc_len);
> qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
> }
> - qjson_destroy(vmdesc);
>
> return 0;
> }
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error Marc-André Lureau
2019-09-13 13:29 ` Dr. David Alan Gilbert
@ 2019-09-17 12:31 ` Daniel P. Berrangé
2019-09-25 9:49 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 12:31 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:09PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> migration/qjson.h | 2 ++
> migration/savevm.c | 3 +--
> 2 files changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error Marc-André Lureau
2019-09-13 13:29 ` Dr. David Alan Gilbert
2019-09-17 12:31 ` Daniel P. Berrangé
@ 2019-09-25 9:49 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-25 9:49 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
I've queued this 1/6 only.
> ---
> migration/qjson.h | 2 ++
> migration/savevm.c | 3 +--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/qjson.h b/migration/qjson.h
> index 41664f2d71..1786bb5864 100644
> --- a/migration/qjson.h
> +++ b/migration/qjson.h
> @@ -24,4 +24,6 @@ void json_start_object(QJSON *json, const char *name);
> const char *qjson_get_str(QJSON *json);
> void qjson_finish(QJSON *json);
>
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QJSON, qjson_destroy)
> +
> #endif /* QEMU_QJSON_H */
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a86128ac4..6caa35a679 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1290,7 +1290,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> bool in_postcopy,
> bool inactivate_disks)
> {
> - QJSON *vmdesc;
> + g_autoptr(QJSON) vmdesc = NULL;
> int vmdesc_len;
> SaveStateEntry *se;
> int ret;
> @@ -1351,7 +1351,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> qemu_put_be32(f, vmdesc_len);
> qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
> }
> - qjson_destroy(vmdesc);
>
> return 0;
> }
> --
> 2.23.0
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-16 9:54 ` Dr. David Alan Gilbert
2019-09-17 12:33 ` Daniel P. Berrangé
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf Marc-André Lureau
` (4 subsequent siblings)
6 siblings, 2 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
Add an interface to get the instance id, instead of depending on
Device and qdev_get_dev_path().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/core/Makefile.objs | 1 +
hw/core/qdev.c | 14 ++++++++++++++
hw/core/vmstate-if.c | 23 +++++++++++++++++++++++
include/hw/vmstate-if.h | 32 ++++++++++++++++++++++++++++++++
include/migration/register.h | 2 ++
include/migration/vmstate.h | 2 ++
tests/Makefile.include | 1 +
7 files changed, 75 insertions(+)
create mode 100644 hw/core/vmstate-if.c
create mode 100644 include/hw/vmstate-if.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index fd0550d1d9..0edd9e635d 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -9,6 +9,7 @@ common-obj-y += hotplug.o
common-obj-$(CONFIG_SOFTMMU) += nmi.o
common-obj-$(CONFIG_SOFTMMU) += vm-change-state-handler.o
common-obj-y += cpu.o
+common-obj-y += vmstate-if.o
common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
common-obj-$(CONFIG_XILINX_AXI) += stream.o
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 60d66c2f39..7e083dfcae 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1047,9 +1047,18 @@ static void device_unparent(Object *obj)
}
}
+static char *
+device_vmstate_if_get_id(VMStateIf *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ return qdev_get_dev_path(dev);
+}
+
static void device_class_init(ObjectClass *class, void *data)
{
DeviceClass *dc = DEVICE_CLASS(class);
+ VMStateIfClass *vc = VMSTATE_IF_CLASS(class);
class->unparent = device_unparent;
@@ -1061,6 +1070,7 @@ static void device_class_init(ObjectClass *class, void *data)
*/
dc->hotpluggable = true;
dc->user_creatable = true;
+ vc->get_id = device_vmstate_if_get_id;
}
void device_class_set_parent_reset(DeviceClass *dc,
@@ -1118,6 +1128,10 @@ static const TypeInfo device_type_info = {
.class_init = device_class_init,
.abstract = true,
.class_size = sizeof(DeviceClass),
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_VMSTATE_IF },
+ { }
+ }
};
static void qdev_register_types(void)
diff --git a/hw/core/vmstate-if.c b/hw/core/vmstate-if.c
new file mode 100644
index 0000000000..d0b2392b0a
--- /dev/null
+++ b/hw/core/vmstate-if.c
@@ -0,0 +1,23 @@
+/*
+ * VMState interface
+ *
+ * Copyright (c) 2009-2017 Red Hat Inc
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/vmstate-if.h"
+
+static const TypeInfo vmstate_if_info = {
+ .name = TYPE_VMSTATE_IF,
+ .parent = TYPE_INTERFACE,
+ .class_size = sizeof(VMStateIfClass),
+};
+
+static void vmstate_register_types(void)
+{
+ type_register_static(&vmstate_if_info);
+}
+
+type_init(vmstate_register_types);
diff --git a/include/hw/vmstate-if.h b/include/hw/vmstate-if.h
new file mode 100644
index 0000000000..92682f5bc2
--- /dev/null
+++ b/include/hw/vmstate-if.h
@@ -0,0 +1,32 @@
+#ifndef VMSTATE_IF_H
+#define VMSTATE_IF_H
+
+#include "qom/object.h"
+
+#define TYPE_VMSTATE_IF "vmstate-if"
+
+#define VMSTATE_IF_CLASS(klass) \
+ OBJECT_CLASS_CHECK(VMStateIfClass, (klass), TYPE_VMSTATE_IF)
+#define VMSTATE_IF_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(VMStateIfClass, (obj), TYPE_VMSTATE_IF)
+#define VMSTATE_IF(obj) \
+ INTERFACE_CHECK(VMStateIf, (obj), TYPE_VMSTATE_IF)
+
+typedef struct VMStateIf VMStateIf;
+
+typedef struct VMStateIfClass {
+ InterfaceClass parent_class;
+
+ char * (*get_id)(VMStateIf *obj);
+} VMStateIfClass;
+
+static inline char *vmstate_if_get_id(VMStateIf *vmif)
+{
+ if (!vmif) {
+ return NULL;
+ }
+
+ return VMSTATE_IF_GET_CLASS(vmif)->get_id(vmif);
+}
+
+#endif /* VMSTATE_IF_H */
diff --git a/include/migration/register.h b/include/migration/register.h
index 3d0b9833c6..74f1578b29 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -14,6 +14,8 @@
#ifndef MIGRATION_REGISTER_H
#define MIGRATION_REGISTER_H
+#include "hw/vmstate-if.h"
+
typedef struct SaveVMHandlers {
/* This runs inside the iothread lock. */
SaveStateHandler *save_state;
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1fbfd099dd..bdcf8a1652 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -27,6 +27,8 @@
#ifndef QEMU_VMSTATE_H
#define QEMU_VMSTATE_H
+#include "hw/vmstate-if.h"
+
typedef struct VMStateInfo VMStateInfo;
typedef struct VMStateField VMStateField;
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f5ac09549c..d4502a30eb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -566,6 +566,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
hw/core/irq.o \
hw/core/fw-path-provider.o \
hw/core/reset.o \
+ hw/core/vmstate-if.o \
$(test-qapi-obj-y)
tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
migration/vmstate.o migration/vmstate-types.o migration/qemu-file.o \
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id Marc-André Lureau
@ 2019-09-16 9:54 ` Dr. David Alan Gilbert
2019-09-17 12:33 ` Daniel P. Berrangé
1 sibling, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-16 9:54 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Add an interface to get the instance id, instead of depending on
> Device and qdev_get_dev_path().
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/core/Makefile.objs | 1 +
> hw/core/qdev.c | 14 ++++++++++++++
> hw/core/vmstate-if.c | 23 +++++++++++++++++++++++
> include/hw/vmstate-if.h | 32 ++++++++++++++++++++++++++++++++
> include/migration/register.h | 2 ++
> include/migration/vmstate.h | 2 ++
> tests/Makefile.include | 1 +
> 7 files changed, 75 insertions(+)
> create mode 100644 hw/core/vmstate-if.c
> create mode 100644 include/hw/vmstate-if.h
>
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index fd0550d1d9..0edd9e635d 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -9,6 +9,7 @@ common-obj-y += hotplug.o
> common-obj-$(CONFIG_SOFTMMU) += nmi.o
> common-obj-$(CONFIG_SOFTMMU) += vm-change-state-handler.o
> common-obj-y += cpu.o
> +common-obj-y += vmstate-if.o
>
> common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> common-obj-$(CONFIG_XILINX_AXI) += stream.o
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 60d66c2f39..7e083dfcae 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1047,9 +1047,18 @@ static void device_unparent(Object *obj)
> }
> }
>
> +static char *
> +device_vmstate_if_get_id(VMStateIf *obj)
> +{
> + DeviceState *dev = DEVICE(obj);
> +
> + return qdev_get_dev_path(dev);
> +}
> +
> static void device_class_init(ObjectClass *class, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(class);
> + VMStateIfClass *vc = VMSTATE_IF_CLASS(class);
>
> class->unparent = device_unparent;
>
> @@ -1061,6 +1070,7 @@ static void device_class_init(ObjectClass *class, void *data)
> */
> dc->hotpluggable = true;
> dc->user_creatable = true;
> + vc->get_id = device_vmstate_if_get_id;
> }
>
> void device_class_set_parent_reset(DeviceClass *dc,
> @@ -1118,6 +1128,10 @@ static const TypeInfo device_type_info = {
> .class_init = device_class_init,
> .abstract = true,
> .class_size = sizeof(DeviceClass),
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_VMSTATE_IF },
> + { }
> + }
> };
>
> static void qdev_register_types(void)
> diff --git a/hw/core/vmstate-if.c b/hw/core/vmstate-if.c
> new file mode 100644
> index 0000000000..d0b2392b0a
> --- /dev/null
> +++ b/hw/core/vmstate-if.c
> @@ -0,0 +1,23 @@
> +/*
> + * VMState interface
> + *
> + * Copyright (c) 2009-2017 Red Hat Inc
You might want to update the years; but other than from the migration
side this looks OK to me, but it needs checking by a QOM person.
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/vmstate-if.h"
> +
> +static const TypeInfo vmstate_if_info = {
> + .name = TYPE_VMSTATE_IF,
> + .parent = TYPE_INTERFACE,
> + .class_size = sizeof(VMStateIfClass),
> +};
> +
> +static void vmstate_register_types(void)
> +{
> + type_register_static(&vmstate_if_info);
> +}
> +
> +type_init(vmstate_register_types);
> diff --git a/include/hw/vmstate-if.h b/include/hw/vmstate-if.h
> new file mode 100644
> index 0000000000..92682f5bc2
> --- /dev/null
> +++ b/include/hw/vmstate-if.h
> @@ -0,0 +1,32 @@
> +#ifndef VMSTATE_IF_H
> +#define VMSTATE_IF_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_VMSTATE_IF "vmstate-if"
> +
> +#define VMSTATE_IF_CLASS(klass) \
> + OBJECT_CLASS_CHECK(VMStateIfClass, (klass), TYPE_VMSTATE_IF)
> +#define VMSTATE_IF_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(VMStateIfClass, (obj), TYPE_VMSTATE_IF)
> +#define VMSTATE_IF(obj) \
> + INTERFACE_CHECK(VMStateIf, (obj), TYPE_VMSTATE_IF)
> +
> +typedef struct VMStateIf VMStateIf;
> +
> +typedef struct VMStateIfClass {
> + InterfaceClass parent_class;
> +
> + char * (*get_id)(VMStateIf *obj);
> +} VMStateIfClass;
> +
> +static inline char *vmstate_if_get_id(VMStateIf *vmif)
> +{
> + if (!vmif) {
> + return NULL;
> + }
> +
> + return VMSTATE_IF_GET_CLASS(vmif)->get_id(vmif);
> +}
> +
> +#endif /* VMSTATE_IF_H */
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 3d0b9833c6..74f1578b29 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -14,6 +14,8 @@
> #ifndef MIGRATION_REGISTER_H
> #define MIGRATION_REGISTER_H
>
> +#include "hw/vmstate-if.h"
> +
> typedef struct SaveVMHandlers {
> /* This runs inside the iothread lock. */
> SaveStateHandler *save_state;
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 1fbfd099dd..bdcf8a1652 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -27,6 +27,8 @@
> #ifndef QEMU_VMSTATE_H
> #define QEMU_VMSTATE_H
>
> +#include "hw/vmstate-if.h"
> +
> typedef struct VMStateInfo VMStateInfo;
> typedef struct VMStateField VMStateField;
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f5ac09549c..d4502a30eb 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -566,6 +566,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
> hw/core/irq.o \
> hw/core/fw-path-provider.o \
> hw/core/reset.o \
> + hw/core/vmstate-if.o \
> $(test-qapi-obj-y)
> tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
> migration/vmstate.o migration/vmstate-types.o migration/qemu-file.o \
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id Marc-André Lureau
2019-09-16 9:54 ` Dr. David Alan Gilbert
@ 2019-09-17 12:33 ` Daniel P. Berrangé
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 12:33 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:10PM +0400, Marc-André Lureau wrote:
> Add an interface to get the instance id, instead of depending on
> Device and qdev_get_dev_path().
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/core/Makefile.objs | 1 +
> hw/core/qdev.c | 14 ++++++++++++++
> hw/core/vmstate-if.c | 23 +++++++++++++++++++++++
> include/hw/vmstate-if.h | 32 ++++++++++++++++++++++++++++++++
These two new files will want adding to some section of MAINTAINERS, since
there's no wildcard rule matching for these dirs currently.
> include/migration/register.h | 2 ++
> include/migration/vmstate.h | 2 ++
> tests/Makefile.include | 1 +
> 7 files changed, 75 insertions(+)
> create mode 100644 hw/core/vmstate-if.c
> create mode 100644 include/hw/vmstate-if.h
> diff --git a/include/hw/vmstate-if.h b/include/hw/vmstate-if.h
> new file mode 100644
> index 0000000000..92682f5bc2
> --- /dev/null
> +++ b/include/hw/vmstate-if.h
> @@ -0,0 +1,32 @@
License header
> +#ifndef VMSTATE_IF_H
> +#define VMSTATE_IF_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_VMSTATE_IF "vmstate-if"
> +
> +#define VMSTATE_IF_CLASS(klass) \
> + OBJECT_CLASS_CHECK(VMStateIfClass, (klass), TYPE_VMSTATE_IF)
> +#define VMSTATE_IF_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(VMStateIfClass, (obj), TYPE_VMSTATE_IF)
> +#define VMSTATE_IF(obj) \
> + INTERFACE_CHECK(VMStateIf, (obj), TYPE_VMSTATE_IF)
> +
> +typedef struct VMStateIf VMStateIf;
> +
> +typedef struct VMStateIfClass {
> + InterfaceClass parent_class;
> +
> + char * (*get_id)(VMStateIf *obj);
> +} VMStateIfClass;
> +
> +static inline char *vmstate_if_get_id(VMStateIf *vmif)
> +{
> + if (!vmif) {
> + return NULL;
> + }
> +
> + return VMSTATE_IF_GET_CLASS(vmif)->get_id(vmif);
> +}
With license header fixes
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 1/6] migration: fix vmdesc leak on vmstate_save() error Marc-André Lureau
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 2/6] vmstate: add qom interface to get id Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-12 16:18 ` Halil Pasic
` (2 more replies)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status() Marc-André Lureau
` (3 subsequent siblings)
6 siblings, 3 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
Replace DeviceState dependency with VMStateIf on vmstate API.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/devel/migration.rst | 2 +-
hw/block/onenand.c | 2 +-
hw/core/qdev.c | 7 ++++---
hw/ide/cmd646.c | 2 +-
hw/ide/isa.c | 2 +-
hw/ide/piix.c | 2 +-
hw/ide/via.c | 2 +-
hw/misc/max111x.c | 2 +-
hw/net/eepro100.c | 4 ++--
hw/net/vmxnet3.c | 2 +-
hw/nvram/eeprom93xx.c | 4 ++--
hw/ppc/spapr_drc.c | 9 +++++----
hw/ppc/spapr_iommu.c | 4 ++--
hw/s390x/s390-skeys.c | 2 +-
include/migration/register.h | 4 ++--
include/migration/vmstate.h | 8 ++++----
migration/savevm.c | 26 +++++++++++++-------------
stubs/vmstate.c | 4 ++--
18 files changed, 45 insertions(+), 43 deletions(-)
diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index f7668ae389..c9932194d9 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -183,7 +183,7 @@ another to load the state back.
.. code:: c
- int register_savevm_live(DeviceState *dev,
+ int register_savevm_live(VMStateIf *obj,
const char *idstr,
int instance_id,
int version_id,
diff --git a/hw/block/onenand.c b/hw/block/onenand.c
index fcc5a69b90..9c233c12e4 100644
--- a/hw/block/onenand.c
+++ b/hw/block/onenand.c
@@ -822,7 +822,7 @@ static void onenand_realize(DeviceState *dev, Error **errp)
onenand_mem_setup(s);
sysbus_init_irq(sbd, &s->intr);
sysbus_init_mmio(sbd, &s->container);
- vmstate_register(dev,
+ vmstate_register(VMSTATE_IF(dev),
((s->shift & 0x7f) << 24)
| ((s->id.man & 0xff) << 16)
| ((s->id.dev & 0xff) << 8)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 7e083dfcae..ad18c0383d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -849,7 +849,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
dev->canonical_path = object_get_canonical_path(OBJECT(dev));
if (qdev_get_vmsd(dev)) {
- if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
+ if (vmstate_register_with_alias_id(VMSTATE_IF(dev),
+ -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
dev->alias_required_for_version,
&local_err) < 0) {
@@ -884,7 +885,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
local_errp);
}
if (qdev_get_vmsd(dev)) {
- vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
+ vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
}
if (dc->unrealize) {
local_errp = local_err ? NULL : &local_err;
@@ -908,7 +909,7 @@ child_realize_fail:
}
if (qdev_get_vmsd(dev)) {
- vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
+ vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
}
post_realize_fail:
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index f3ccd11c79..01a982acbc 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -301,7 +301,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
ide_register_restart_cb(&d->bus[i]);
}
- vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
+ vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
qemu_register_reset(cmd646_reset, d);
}
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 7b6e283679..9c7f88b2d5 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -75,7 +75,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
isa_init_irq(isadev, &s->irq, s->isairq);
ide_init2(&s->bus, s->irq);
- vmstate_register(dev, 0, &vmstate_ide_isa, s);
+ vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
ide_register_restart_cb(&s->bus);
}
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index fba6bc8bff..cbaee9e2cc 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -159,7 +159,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
bmdma_setup_bar(d);
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
- vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
+ vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
pci_piix_init_ports(d);
}
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 7087dc676e..6b9d4c6d78 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -193,7 +193,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
bmdma_setup_bar(d);
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
- vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
+ vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
for (i = 0; i < 2; i++) {
ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index a713149f16..211008ce02 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -146,7 +146,7 @@ static int max111x_init(SSISlave *d, int inputs)
s->input[7] = 0x80;
s->com = 0;
- vmstate_register(dev, -1, &vmstate_max111x, s);
+ vmstate_register(VMSTATE_IF(dev), -1, &vmstate_max111x, s);
return 0;
}
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index cc2dd8b1c9..cc71a7a036 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1815,7 +1815,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
{
EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
- vmstate_unregister(&pci_dev->qdev, s->vmstate, s);
+ vmstate_unregister(VMSTATE_IF(&pci_dev->qdev), s->vmstate, s);
g_free(s->vmstate);
eeprom93xx_free(&pci_dev->qdev, s->eeprom);
qemu_del_nic(s->nic);
@@ -1874,7 +1874,7 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
s->vmstate = g_memdup(&vmstate_eepro100, sizeof(vmstate_eepro100));
s->vmstate->name = qemu_get_queue(s->nic)->model;
- vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
+ vmstate_register(VMSTATE_IF(&pci_dev->qdev), -1, s->vmstate, s);
}
static void eepro100_instance_init(Object *obj)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index b07adeed9c..8625e93dc9 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2247,7 +2247,7 @@ static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
VMW_CBPRN("Starting uninit...");
- unregister_savevm(dev, "vmxnet3-msix", s);
+ unregister_savevm(VMSTATE_IF(dev), "vmxnet3-msix", s);
vmxnet3_net_uninit(s);
diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
index 5b01b9b03f..07f09549ed 100644
--- a/hw/nvram/eeprom93xx.c
+++ b/hw/nvram/eeprom93xx.c
@@ -321,7 +321,7 @@ eeprom_t *eeprom93xx_new(DeviceState *dev, uint16_t nwords)
/* Output DO is tristate, read results in 1. */
eeprom->eedo = 1;
logout("eeprom = 0x%p, nwords = %u\n", eeprom, nwords);
- vmstate_register(dev, 0, &vmstate_eeprom, eeprom);
+ vmstate_register(VMSTATE_IF(dev), 0, &vmstate_eeprom, eeprom);
return eeprom;
}
@@ -329,7 +329,7 @@ void eeprom93xx_free(DeviceState *dev, eeprom_t *eeprom)
{
/* Destroy EEPROM. */
logout("eeprom = 0x%p\n", eeprom);
- vmstate_unregister(dev, &vmstate_eeprom, eeprom);
+ vmstate_unregister(VMSTATE_IF(dev), &vmstate_eeprom, eeprom);
g_free(eeprom);
}
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 62f1a42592..17aeac3801 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -511,7 +511,7 @@ static void realize(DeviceState *d, Error **errp)
error_propagate(errp, err);
return;
}
- vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
+ vmstate_register(VMSTATE_IF(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
drc);
trace_spapr_drc_realize_complete(spapr_drc_index(drc));
}
@@ -523,7 +523,7 @@ static void unrealize(DeviceState *d, Error **errp)
gchar *name;
trace_spapr_drc_unrealize(spapr_drc_index(drc));
- vmstate_unregister(DEVICE(drc), &vmstate_spapr_drc, drc);
+ vmstate_unregister(VMSTATE_IF(drc), &vmstate_spapr_drc, drc);
root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
name = g_strdup_printf("%x", spapr_drc_index(drc));
object_property_del(root_container, name, errp);
@@ -619,7 +619,8 @@ static void realize_physical(DeviceState *d, Error **errp)
return;
}
- vmstate_register(DEVICE(drcp), spapr_drc_index(SPAPR_DR_CONNECTOR(drcp)),
+ vmstate_register(VMSTATE_IF(drcp),
+ spapr_drc_index(SPAPR_DR_CONNECTOR(drcp)),
&vmstate_spapr_drc_physical, drcp);
qemu_register_reset(drc_physical_reset, drcp);
}
@@ -635,7 +636,7 @@ static void unrealize_physical(DeviceState *d, Error **errp)
return;
}
- vmstate_unregister(DEVICE(drcp), &vmstate_spapr_drc_physical, drcp);
+ vmstate_unregister(VMSTATE_IF(drcp), &vmstate_spapr_drc_physical, drcp);
qemu_unregister_reset(drc_physical_reset, drcp);
}
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index e87b3d50f7..b9e7854da9 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -315,7 +315,7 @@ static void spapr_tce_table_realize(DeviceState *dev, Error **errp)
QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
- vmstate_register(DEVICE(tcet), tcet->liobn, &vmstate_spapr_tce_table,
+ vmstate_register(VMSTATE_IF(tcet), tcet->liobn, &vmstate_spapr_tce_table,
tcet);
}
@@ -418,7 +418,7 @@ static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
{
SpaprTceTable *tcet = SPAPR_TCE_TABLE(dev);
- vmstate_unregister(DEVICE(tcet), &vmstate_spapr_tce_table, tcet);
+ vmstate_unregister(VMSTATE_IF(tcet), &vmstate_spapr_tce_table, tcet);
QLIST_REMOVE(tcet, list);
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index d4807f7777..16b9bbf04d 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -392,7 +392,7 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
&savevm_s390_storage_keys, ss);
} else {
- unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
+ unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
}
}
diff --git a/include/migration/register.h b/include/migration/register.h
index 74f1578b29..f22f180286 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -70,13 +70,13 @@ typedef struct SaveVMHandlers {
int (*resume_prepare)(MigrationState *s, void *opaque);
} SaveVMHandlers;
-int register_savevm_live(DeviceState *dev,
+int register_savevm_live(VMStateIf *obj,
const char *idstr,
int instance_id,
int version_id,
const SaveVMHandlers *ops,
void *opaque);
-void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque);
+void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
#endif
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index bdcf8a1652..1911e37d72 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1116,22 +1116,22 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
/* Returns: 0 on success, -1 on failure */
-int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
+int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
int required_for_version,
Error **errp);
/* Returns: 0 on success, -1 on failure */
-static inline int vmstate_register(DeviceState *dev, int instance_id,
+static inline int vmstate_register(VMStateIf *obj, int instance_id,
const VMStateDescription *vmsd,
void *opaque)
{
- return vmstate_register_with_alias_id(dev, instance_id, vmsd,
+ return vmstate_register_with_alias_id(obj, instance_id, vmsd,
opaque, -1, 0, NULL);
}
-void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
+void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
void *opaque);
struct MemoryRegion;
diff --git a/migration/savevm.c b/migration/savevm.c
index 6caa35a679..7902da16e6 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -684,7 +684,7 @@ static void savevm_state_handler_insert(SaveStateEntry *nse)
of the system, so instance_id should be removed/replaced.
Meanwhile pass -1 as instance_id if you do not already have a clearly
distinguishing id for all instances of your device class. */
-int register_savevm_live(DeviceState *dev,
+int register_savevm_live(VMStateIf *obj,
const char *idstr,
int instance_id,
int version_id,
@@ -704,8 +704,8 @@ int register_savevm_live(DeviceState *dev,
se->is_ram = 1;
}
- if (dev) {
- char *id = qdev_get_dev_path(dev);
+ if (obj) {
+ char *id = vmstate_if_get_id(obj);
if (id) {
if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
sizeof(se->idstr)) {
@@ -736,17 +736,17 @@ int register_savevm_live(DeviceState *dev,
return 0;
}
-void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
+void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque)
{
SaveStateEntry *se, *new_se;
char id[256] = "";
- if (dev) {
- char *path = qdev_get_dev_path(dev);
- if (path) {
- pstrcpy(id, sizeof(id), path);
+ if (obj) {
+ char *oid = vmstate_if_get_id(obj);
+ if (oid) {
+ pstrcpy(id, sizeof(id), oid);
pstrcat(id, sizeof(id), "/");
- g_free(path);
+ g_free(oid);
}
}
pstrcat(id, sizeof(id), idstr);
@@ -760,7 +760,7 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
}
}
-int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
+int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
const VMStateDescription *vmsd,
void *opaque, int alias_id,
int required_for_version,
@@ -778,8 +778,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
se->vmsd = vmsd;
se->alias_id = alias_id;
- if (dev) {
- char *id = qdev_get_dev_path(dev);
+ if (obj) {
+ char *id = vmstate_if_get_id(obj);
if (id) {
if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
sizeof(se->idstr)) {
@@ -810,7 +810,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
return 0;
}
-void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
+void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
void *opaque)
{
SaveStateEntry *se, *new_se;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index e1e89b87f0..6951d9fdc5 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -3,7 +3,7 @@
const VMStateDescription vmstate_dummy = {};
-int vmstate_register_with_alias_id(DeviceState *dev,
+int vmstate_register_with_alias_id(VMStateIf *obj,
int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
@@ -13,7 +13,7 @@ int vmstate_register_with_alias_id(DeviceState *dev,
return 0;
}
-void vmstate_unregister(DeviceState *dev,
+void vmstate_unregister(VMStateIf *obj,
const VMStateDescription *vmsd,
void *opaque)
{
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf Marc-André Lureau
@ 2019-09-12 16:18 ` Halil Pasic
2019-09-13 7:12 ` Marc-André Lureau
2019-09-16 9:06 ` Dr. David Alan Gilbert
2019-09-17 12:35 ` Daniel P. Berrangé
2 siblings, 1 reply; 32+ messages in thread
From: Halil Pasic @ 2019-09-12 16:18 UTC (permalink / raw)
To: Marc-André Lureau
Cc: berrange, quintela, mprivozn, qemu-devel, dgilbert, pbonzini
On Thu, 12 Sep 2019 16:25:11 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index d4807f7777..16b9bbf04d 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -392,7 +392,7 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
> register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
> &savevm_s390_storage_keys, ss);
> } else {
> - unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
> + unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
> }
> }
Acked-by: Halil Pasic <pasic@linux.ibm.com>
BTW what does the 'f' in VMStateIf stand for? (I've had a look
at 2/6 but didn't figure out the answer).
Regards,
Halil
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf
2019-09-12 16:18 ` Halil Pasic
@ 2019-09-13 7:12 ` Marc-André Lureau
0 siblings, 0 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-13 7:12 UTC (permalink / raw)
To: Halil Pasic
Cc: P. Berrange, Daniel, Juan Quintela, Privoznik, Michal, qemu-devel,
Dr. David Alan Gilbert, Bonzini, Paolo
Hi
On Thu, Sep 12, 2019 at 8:19 PM Halil Pasic <pasic@linux.ibm.com> wrote:
>
> On Thu, 12 Sep 2019 16:25:11 +0400
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>
> > diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> > index d4807f7777..16b9bbf04d 100644
> > --- a/hw/s390x/s390-skeys.c
> > +++ b/hw/s390x/s390-skeys.c
> > @@ -392,7 +392,7 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
> > register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
> > &savevm_s390_storage_keys, ss);
> > } else {
> > - unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
> > + unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
> > }
> > }
>
> Acked-by: Halil Pasic <pasic@linux.ibm.com>
>
thanks
> BTW what does the 'f' in VMStateIf stand for? (I've had a look
> at 2/6 but didn't figure out the answer).
"If" stands for interface, and seems to be more common in qemu.
gobject code usually uses "Iface", and other prefer "I" prefix.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf Marc-André Lureau
2019-09-12 16:18 ` Halil Pasic
@ 2019-09-16 9:06 ` Dr. David Alan Gilbert
2019-09-17 12:35 ` Daniel P. Berrangé
2 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-16 9:06 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Replace DeviceState dependency with VMStateIf on vmstate API.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/devel/migration.rst | 2 +-
> hw/block/onenand.c | 2 +-
> hw/core/qdev.c | 7 ++++---
> hw/ide/cmd646.c | 2 +-
> hw/ide/isa.c | 2 +-
> hw/ide/piix.c | 2 +-
> hw/ide/via.c | 2 +-
> hw/misc/max111x.c | 2 +-
> hw/net/eepro100.c | 4 ++--
> hw/net/vmxnet3.c | 2 +-
> hw/nvram/eeprom93xx.c | 4 ++--
> hw/ppc/spapr_drc.c | 9 +++++----
> hw/ppc/spapr_iommu.c | 4 ++--
> hw/s390x/s390-skeys.c | 2 +-
> include/migration/register.h | 4 ++--
> include/migration/vmstate.h | 8 ++++----
> migration/savevm.c | 26 +++++++++++++-------------
> stubs/vmstate.c | 4 ++--
> 18 files changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
> index f7668ae389..c9932194d9 100644
> --- a/docs/devel/migration.rst
> +++ b/docs/devel/migration.rst
> @@ -183,7 +183,7 @@ another to load the state back.
>
> .. code:: c
>
> - int register_savevm_live(DeviceState *dev,
> + int register_savevm_live(VMStateIf *obj,
> const char *idstr,
> int instance_id,
> int version_id,
> diff --git a/hw/block/onenand.c b/hw/block/onenand.c
> index fcc5a69b90..9c233c12e4 100644
> --- a/hw/block/onenand.c
> +++ b/hw/block/onenand.c
> @@ -822,7 +822,7 @@ static void onenand_realize(DeviceState *dev, Error **errp)
> onenand_mem_setup(s);
> sysbus_init_irq(sbd, &s->intr);
> sysbus_init_mmio(sbd, &s->container);
> - vmstate_register(dev,
> + vmstate_register(VMSTATE_IF(dev),
> ((s->shift & 0x7f) << 24)
> | ((s->id.man & 0xff) << 16)
> | ((s->id.dev & 0xff) << 8)
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 7e083dfcae..ad18c0383d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -849,7 +849,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> dev->canonical_path = object_get_canonical_path(OBJECT(dev));
>
> if (qdev_get_vmsd(dev)) {
> - if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
> + if (vmstate_register_with_alias_id(VMSTATE_IF(dev),
> + -1, qdev_get_vmsd(dev), dev,
> dev->instance_id_alias,
> dev->alias_required_for_version,
> &local_err) < 0) {
> @@ -884,7 +885,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> local_errp);
> }
> if (qdev_get_vmsd(dev)) {
> - vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
> + vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
> }
> if (dc->unrealize) {
> local_errp = local_err ? NULL : &local_err;
> @@ -908,7 +909,7 @@ child_realize_fail:
> }
>
> if (qdev_get_vmsd(dev)) {
> - vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
> + vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
> }
>
> post_realize_fail:
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index f3ccd11c79..01a982acbc 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -301,7 +301,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
> ide_register_restart_cb(&d->bus[i]);
> }
>
> - vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
> + vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
> qemu_register_reset(cmd646_reset, d);
> }
>
> diff --git a/hw/ide/isa.c b/hw/ide/isa.c
> index 7b6e283679..9c7f88b2d5 100644
> --- a/hw/ide/isa.c
> +++ b/hw/ide/isa.c
> @@ -75,7 +75,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
> ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
> isa_init_irq(isadev, &s->irq, s->isairq);
> ide_init2(&s->bus, s->irq);
> - vmstate_register(dev, 0, &vmstate_ide_isa, s);
> + vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
> ide_register_restart_cb(&s->bus);
> }
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index fba6bc8bff..cbaee9e2cc 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -159,7 +159,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
> bmdma_setup_bar(d);
> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>
> - vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
> + vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
>
> pci_piix_init_ports(d);
> }
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 7087dc676e..6b9d4c6d78 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -193,7 +193,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
> bmdma_setup_bar(d);
> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>
> - vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
> + vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
>
> for (i = 0; i < 2; i++) {
> ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index a713149f16..211008ce02 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -146,7 +146,7 @@ static int max111x_init(SSISlave *d, int inputs)
> s->input[7] = 0x80;
> s->com = 0;
>
> - vmstate_register(dev, -1, &vmstate_max111x, s);
> + vmstate_register(VMSTATE_IF(dev), -1, &vmstate_max111x, s);
> return 0;
> }
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index cc2dd8b1c9..cc71a7a036 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -1815,7 +1815,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
> {
> EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
>
> - vmstate_unregister(&pci_dev->qdev, s->vmstate, s);
> + vmstate_unregister(VMSTATE_IF(&pci_dev->qdev), s->vmstate, s);
> g_free(s->vmstate);
> eeprom93xx_free(&pci_dev->qdev, s->eeprom);
> qemu_del_nic(s->nic);
> @@ -1874,7 +1874,7 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
>
> s->vmstate = g_memdup(&vmstate_eepro100, sizeof(vmstate_eepro100));
> s->vmstate->name = qemu_get_queue(s->nic)->model;
> - vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
> + vmstate_register(VMSTATE_IF(&pci_dev->qdev), -1, s->vmstate, s);
> }
>
> static void eepro100_instance_init(Object *obj)
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index b07adeed9c..8625e93dc9 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2247,7 +2247,7 @@ static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
>
> VMW_CBPRN("Starting uninit...");
>
> - unregister_savevm(dev, "vmxnet3-msix", s);
> + unregister_savevm(VMSTATE_IF(dev), "vmxnet3-msix", s);
>
> vmxnet3_net_uninit(s);
>
> diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
> index 5b01b9b03f..07f09549ed 100644
> --- a/hw/nvram/eeprom93xx.c
> +++ b/hw/nvram/eeprom93xx.c
> @@ -321,7 +321,7 @@ eeprom_t *eeprom93xx_new(DeviceState *dev, uint16_t nwords)
> /* Output DO is tristate, read results in 1. */
> eeprom->eedo = 1;
> logout("eeprom = 0x%p, nwords = %u\n", eeprom, nwords);
> - vmstate_register(dev, 0, &vmstate_eeprom, eeprom);
> + vmstate_register(VMSTATE_IF(dev), 0, &vmstate_eeprom, eeprom);
> return eeprom;
> }
>
> @@ -329,7 +329,7 @@ void eeprom93xx_free(DeviceState *dev, eeprom_t *eeprom)
> {
> /* Destroy EEPROM. */
> logout("eeprom = 0x%p\n", eeprom);
> - vmstate_unregister(dev, &vmstate_eeprom, eeprom);
> + vmstate_unregister(VMSTATE_IF(dev), &vmstate_eeprom, eeprom);
> g_free(eeprom);
> }
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 62f1a42592..17aeac3801 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -511,7 +511,7 @@ static void realize(DeviceState *d, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
> + vmstate_register(VMSTATE_IF(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
> drc);
> trace_spapr_drc_realize_complete(spapr_drc_index(drc));
> }
> @@ -523,7 +523,7 @@ static void unrealize(DeviceState *d, Error **errp)
> gchar *name;
>
> trace_spapr_drc_unrealize(spapr_drc_index(drc));
> - vmstate_unregister(DEVICE(drc), &vmstate_spapr_drc, drc);
> + vmstate_unregister(VMSTATE_IF(drc), &vmstate_spapr_drc, drc);
> root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> name = g_strdup_printf("%x", spapr_drc_index(drc));
> object_property_del(root_container, name, errp);
> @@ -619,7 +619,8 @@ static void realize_physical(DeviceState *d, Error **errp)
> return;
> }
>
> - vmstate_register(DEVICE(drcp), spapr_drc_index(SPAPR_DR_CONNECTOR(drcp)),
> + vmstate_register(VMSTATE_IF(drcp),
> + spapr_drc_index(SPAPR_DR_CONNECTOR(drcp)),
> &vmstate_spapr_drc_physical, drcp);
> qemu_register_reset(drc_physical_reset, drcp);
> }
> @@ -635,7 +636,7 @@ static void unrealize_physical(DeviceState *d, Error **errp)
> return;
> }
>
> - vmstate_unregister(DEVICE(drcp), &vmstate_spapr_drc_physical, drcp);
> + vmstate_unregister(VMSTATE_IF(drcp), &vmstate_spapr_drc_physical, drcp);
> qemu_unregister_reset(drc_physical_reset, drcp);
> }
>
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index e87b3d50f7..b9e7854da9 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -315,7 +315,7 @@ static void spapr_tce_table_realize(DeviceState *dev, Error **errp)
>
> QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>
> - vmstate_register(DEVICE(tcet), tcet->liobn, &vmstate_spapr_tce_table,
> + vmstate_register(VMSTATE_IF(tcet), tcet->liobn, &vmstate_spapr_tce_table,
> tcet);
> }
>
> @@ -418,7 +418,7 @@ static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
> {
> SpaprTceTable *tcet = SPAPR_TCE_TABLE(dev);
>
> - vmstate_unregister(DEVICE(tcet), &vmstate_spapr_tce_table, tcet);
> + vmstate_unregister(VMSTATE_IF(tcet), &vmstate_spapr_tce_table, tcet);
>
> QLIST_REMOVE(tcet, list);
>
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index d4807f7777..16b9bbf04d 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -392,7 +392,7 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
> register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
> &savevm_s390_storage_keys, ss);
> } else {
> - unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
> + unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
> }
> }
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 74f1578b29..f22f180286 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -70,13 +70,13 @@ typedef struct SaveVMHandlers {
> int (*resume_prepare)(MigrationState *s, void *opaque);
> } SaveVMHandlers;
>
> -int register_savevm_live(DeviceState *dev,
> +int register_savevm_live(VMStateIf *obj,
> const char *idstr,
> int instance_id,
> int version_id,
> const SaveVMHandlers *ops,
> void *opaque);
>
> -void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque);
> +void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque);
>
> #endif
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index bdcf8a1652..1911e37d72 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1116,22 +1116,22 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
>
> /* Returns: 0 on success, -1 on failure */
> -int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> +int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
> const VMStateDescription *vmsd,
> void *base, int alias_id,
> int required_for_version,
> Error **errp);
>
> /* Returns: 0 on success, -1 on failure */
> -static inline int vmstate_register(DeviceState *dev, int instance_id,
> +static inline int vmstate_register(VMStateIf *obj, int instance_id,
> const VMStateDescription *vmsd,
> void *opaque)
> {
> - return vmstate_register_with_alias_id(dev, instance_id, vmsd,
> + return vmstate_register_with_alias_id(obj, instance_id, vmsd,
> opaque, -1, 0, NULL);
> }
>
> -void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
> +void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
> void *opaque);
>
> struct MemoryRegion;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 6caa35a679..7902da16e6 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -684,7 +684,7 @@ static void savevm_state_handler_insert(SaveStateEntry *nse)
> of the system, so instance_id should be removed/replaced.
> Meanwhile pass -1 as instance_id if you do not already have a clearly
> distinguishing id for all instances of your device class. */
> -int register_savevm_live(DeviceState *dev,
> +int register_savevm_live(VMStateIf *obj,
> const char *idstr,
> int instance_id,
> int version_id,
> @@ -704,8 +704,8 @@ int register_savevm_live(DeviceState *dev,
> se->is_ram = 1;
> }
>
> - if (dev) {
> - char *id = qdev_get_dev_path(dev);
> + if (obj) {
> + char *id = vmstate_if_get_id(obj);
> if (id) {
> if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
> sizeof(se->idstr)) {
> @@ -736,17 +736,17 @@ int register_savevm_live(DeviceState *dev,
> return 0;
> }
>
> -void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
> +void unregister_savevm(VMStateIf *obj, const char *idstr, void *opaque)
> {
> SaveStateEntry *se, *new_se;
> char id[256] = "";
>
> - if (dev) {
> - char *path = qdev_get_dev_path(dev);
> - if (path) {
> - pstrcpy(id, sizeof(id), path);
> + if (obj) {
> + char *oid = vmstate_if_get_id(obj);
> + if (oid) {
> + pstrcpy(id, sizeof(id), oid);
> pstrcat(id, sizeof(id), "/");
> - g_free(path);
> + g_free(oid);
> }
And since your previous patch defaults vmstate_if_get_id to return
qdev_get_dev_path that should mean there's no change.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> }
> pstrcat(id, sizeof(id), idstr);
> @@ -760,7 +760,7 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
> }
> }
>
> -int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> +int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
> const VMStateDescription *vmsd,
> void *opaque, int alias_id,
> int required_for_version,
> @@ -778,8 +778,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> se->vmsd = vmsd;
> se->alias_id = alias_id;
>
> - if (dev) {
> - char *id = qdev_get_dev_path(dev);
> + if (obj) {
> + char *id = vmstate_if_get_id(obj);
> if (id) {
> if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
> sizeof(se->idstr)) {
> @@ -810,7 +810,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> return 0;
> }
>
> -void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
> +void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
> void *opaque)
> {
> SaveStateEntry *se, *new_se;
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index e1e89b87f0..6951d9fdc5 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -3,7 +3,7 @@
>
> const VMStateDescription vmstate_dummy = {};
>
> -int vmstate_register_with_alias_id(DeviceState *dev,
> +int vmstate_register_with_alias_id(VMStateIf *obj,
> int instance_id,
> const VMStateDescription *vmsd,
> void *base, int alias_id,
> @@ -13,7 +13,7 @@ int vmstate_register_with_alias_id(DeviceState *dev,
> return 0;
> }
>
> -void vmstate_unregister(DeviceState *dev,
> +void vmstate_unregister(VMStateIf *obj,
> const VMStateDescription *vmsd,
> void *opaque)
> {
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf Marc-André Lureau
2019-09-12 16:18 ` Halil Pasic
2019-09-16 9:06 ` Dr. David Alan Gilbert
@ 2019-09-17 12:35 ` Daniel P. Berrangé
2 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 12:35 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:11PM +0400, Marc-André Lureau wrote:
> Replace DeviceState dependency with VMStateIf on vmstate API.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/devel/migration.rst | 2 +-
> hw/block/onenand.c | 2 +-
> hw/core/qdev.c | 7 ++++---
> hw/ide/cmd646.c | 2 +-
> hw/ide/isa.c | 2 +-
> hw/ide/piix.c | 2 +-
> hw/ide/via.c | 2 +-
> hw/misc/max111x.c | 2 +-
> hw/net/eepro100.c | 4 ++--
> hw/net/vmxnet3.c | 2 +-
> hw/nvram/eeprom93xx.c | 4 ++--
> hw/ppc/spapr_drc.c | 9 +++++----
> hw/ppc/spapr_iommu.c | 4 ++--
> hw/s390x/s390-skeys.c | 2 +-
> include/migration/register.h | 4 ++--
> include/migration/vmstate.h | 8 ++++----
> migration/savevm.c | 26 +++++++++++++-------------
> stubs/vmstate.c | 4 ++--
> 18 files changed, 45 insertions(+), 43 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status()
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
` (2 preceding siblings ...)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 3/6] vmstate: replace DeviceState with VMStateIf Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-13 13:33 ` Dr. David Alan Gilbert
2019-09-17 12:36 ` Daniel P. Berrangé
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage Marc-André Lureau
` (2 subsequent siblings)
6 siblings, 2 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
Modify the behaviour of qtest_quit() to check against the expected
exit status value. The default remains 0.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/libqtest.c | 41 ++++++++++++++++++++++-------------------
tests/libqtest.h | 9 +++++++++
2 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 0a6b91737e..1f7910e583 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -44,6 +44,7 @@ struct QTestState
bool big_endian;
bool irq_level[MAX_IRQ];
GString *rx;
+ int exit_status;
};
static GHookList abrt_hooks;
@@ -123,27 +124,29 @@ static void kill_qemu(QTestState *s)
assert(pid == s->qemu_pid);
}
- /*
- * We expect qemu to exit with status 0; anything else is
- * fishy and should be logged with as much detail as possible.
- */
wstatus = s->wstatus;
- if (wstatus) {
- if (WIFEXITED(wstatus)) {
- fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
- "process but encountered exit status %d\n",
- __FILE__, __LINE__, WEXITSTATUS(wstatus));
- } else if (WIFSIGNALED(wstatus)) {
- int sig = WTERMSIG(wstatus);
- const char *signame = strsignal(sig) ?: "unknown ???";
- const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
-
- fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
- "from signal %d (%s)%s\n",
- __FILE__, __LINE__, sig, signame, dump);
+ if (WIFEXITED(wstatus)) {
+ if (WEXITSTATUS(wstatus) == s->exit_status) {
+ return;
}
- abort();
+ fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
+ "process but encountered exit status %d\n",
+ __FILE__, __LINE__, WEXITSTATUS(wstatus));
+ } else if (WIFSIGNALED(wstatus)) {
+ int sig = WTERMSIG(wstatus);
+ const char *signame = strsignal(sig) ?: "unknown ???";
+ const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
+
+ fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
+ "from signal %d (%s)%s\n",
+ __FILE__, __LINE__, sig, signame, dump);
}
+ abort();
+}
+
+void qtest_expect_exit_status(QTestState *s, int status)
+{
+ s->exit_status = status;
}
static void kill_qemu_hook_func(void *s)
@@ -213,7 +216,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
gchar *command;
const char *qemu_binary = qtest_qemu_binary();
- s = g_new(QTestState, 1);
+ s = g_new0(QTestState, 1);
socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
diff --git a/tests/libqtest.h b/tests/libqtest.h
index c8cffe5d68..d41229d7fd 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -82,6 +82,15 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
*/
void qtest_quit(QTestState *s);
+/**
+ * qtest_expect_exit_status:
+ * @s: #QTestState instance to operate on.
+ * @status: the expected exit status
+ *
+ * Set the expected exit status when calling qtest_quit().
+ */
+void qtest_expect_exit_status(QTestState *s, int status);
+
/**
* qtest_qmp_fds:
* @s: #QTestState instance to operate on.
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status()
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status() Marc-André Lureau
@ 2019-09-13 13:33 ` Dr. David Alan Gilbert
2019-09-17 12:36 ` Daniel P. Berrangé
1 sibling, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-13 13:33 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Modify the behaviour of qtest_quit() to check against the expected
> exit status value. The default remains 0.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This looks similar to the one in the current pending migration pull
by Yury Kotov:
'tests/libqtest: Allow setting expected exit status'
Dave
> ---
> tests/libqtest.c | 41 ++++++++++++++++++++++-------------------
> tests/libqtest.h | 9 +++++++++
> 2 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 0a6b91737e..1f7910e583 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -44,6 +44,7 @@ struct QTestState
> bool big_endian;
> bool irq_level[MAX_IRQ];
> GString *rx;
> + int exit_status;
> };
>
> static GHookList abrt_hooks;
> @@ -123,27 +124,29 @@ static void kill_qemu(QTestState *s)
> assert(pid == s->qemu_pid);
> }
>
> - /*
> - * We expect qemu to exit with status 0; anything else is
> - * fishy and should be logged with as much detail as possible.
> - */
> wstatus = s->wstatus;
> - if (wstatus) {
> - if (WIFEXITED(wstatus)) {
> - fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> - "process but encountered exit status %d\n",
> - __FILE__, __LINE__, WEXITSTATUS(wstatus));
> - } else if (WIFSIGNALED(wstatus)) {
> - int sig = WTERMSIG(wstatus);
> - const char *signame = strsignal(sig) ?: "unknown ???";
> - const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> -
> - fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> - "from signal %d (%s)%s\n",
> - __FILE__, __LINE__, sig, signame, dump);
> + if (WIFEXITED(wstatus)) {
> + if (WEXITSTATUS(wstatus) == s->exit_status) {
> + return;
> }
> - abort();
> + fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> + "process but encountered exit status %d\n",
> + __FILE__, __LINE__, WEXITSTATUS(wstatus));
> + } else if (WIFSIGNALED(wstatus)) {
> + int sig = WTERMSIG(wstatus);
> + const char *signame = strsignal(sig) ?: "unknown ???";
> + const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> +
> + fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> + "from signal %d (%s)%s\n",
> + __FILE__, __LINE__, sig, signame, dump);
> }
> + abort();
> +}
> +
> +void qtest_expect_exit_status(QTestState *s, int status)
> +{
> + s->exit_status = status;
> }
>
> static void kill_qemu_hook_func(void *s)
> @@ -213,7 +216,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
> gchar *command;
> const char *qemu_binary = qtest_qemu_binary();
>
> - s = g_new(QTestState, 1);
> + s = g_new0(QTestState, 1);
>
> socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index c8cffe5d68..d41229d7fd 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -82,6 +82,15 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
> */
> void qtest_quit(QTestState *s);
>
> +/**
> + * qtest_expect_exit_status:
> + * @s: #QTestState instance to operate on.
> + * @status: the expected exit status
> + *
> + * Set the expected exit status when calling qtest_quit().
> + */
> +void qtest_expect_exit_status(QTestState *s, int status);
> +
> /**
> * qtest_qmp_fds:
> * @s: #QTestState instance to operate on.
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status()
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status() Marc-André Lureau
2019-09-13 13:33 ` Dr. David Alan Gilbert
@ 2019-09-17 12:36 ` Daniel P. Berrangé
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 12:36 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:12PM +0400, Marc-André Lureau wrote:
> Modify the behaviour of qtest_quit() to check against the expected
> exit status value. The default remains 0.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/libqtest.c | 41 ++++++++++++++++++++++-------------------
> tests/libqtest.h | 9 +++++++++
> 2 files changed, 31 insertions(+), 19 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
` (3 preceding siblings ...)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 4/6] tests: add qtest_expect_exit_status() Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-16 10:00 ` Dr. David Alan Gilbert
2019-09-17 13:07 ` Daniel P. Berrangé
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object Marc-André Lureau
2019-09-12 13:50 ` [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate no-reply
6 siblings, 2 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
docs/interop/index.rst | 1 +
2 files changed, 74 insertions(+)
create mode 100644 docs/interop/dbus.rst
diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
new file mode 100644
index 0000000000..c08f026edc
--- /dev/null
+++ b/docs/interop/dbus.rst
@@ -0,0 +1,73 @@
+=====
+D-Bus
+=====
+
+Introduction
+============
+
+QEMU may be running with various helper processes involved:
+ - vhost-user* processes (gpu, virtfs, input, etc...)
+ - TPM emulation (or other devices)
+ - user networking (slirp)
+ - network services (DHCP/DNS, samba/ftp etc)
+ - background tasks (compression, streaming etc)
+ - client UI
+ - admin & cli
+
+Having several processes allows stricter security rules, as well as
+greater modularity.
+
+While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
+display), D-Bus is the de facto IPC of choice on Unix systems. The
+wire format is machine friendly, good bindings exist for various
+languages, and there are various tools available.
+
+Using a bus, helper processes can discover and communicate with each
+other easily, without going through QEMU. The bus topology is also
+easier to apprehend and debug than a mesh. However, it is wise to
+consider the security aspects of it.
+
+Security
+========
+
+A QEMU D-Bus bus should be private to a single VM. Thus, only
+cooperative tasks are running on the same bus to serve the VM.
+
+D-Bus, the protocol and standard, doesn't have mechanisms to enforce
+security between peers once the connection is established. Peers may
+have additional mechanisms to enforce security rules, based for
+example on UNIX credentials.
+
+dbus-daemon can enforce various policies based on the UID/GID of the
+processes that are connected to it. It is thus a good idea to run
+helpers as different UID from QEMU and set appropriate policies (so
+helper processes are only allowed to talk to qemu for example).
+
+For example, this allows only ``qemu`` user to talk to ``qemu-helper``
+``org.qemu.Helper1`` service:
+
+.. code:: xml
+
+ <policy user="qemu">
+ <allow send_destination="org.qemu.Helper1"/>
+ <allow receive_sender="org.qemu.Helper1"/>
+ </policy>
+
+ <policy user="qemu-helper">
+ <allow own="org.qemu.Helper1"/>
+ </policy>
+
+
+dbus-daemon can also perfom SELinux checks based on the security
+context of the source and the target. For example, ``virtiofs_t``
+could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
+wouldn't be allowed to send a message to ``virtiofs_t``.
+
+Guidelines
+==========
+
+When implementing new D-Bus interfaces, it is recommended to follow
+the "D-Bus API Design Guidelines":
+https://dbus.freedesktop.org/doc/dbus-api-design.html
+
+The "org.qemu*" prefix is reserved for the QEMU project.
diff --git a/docs/interop/index.rst b/docs/interop/index.rst
index b4bfcab417..fa4478ce2e 100644
--- a/docs/interop/index.rst
+++ b/docs/interop/index.rst
@@ -13,6 +13,7 @@ Contents:
:maxdepth: 2
bitmaps
+ dbus
live-block-operations
pr-helper
vhost-user
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage Marc-André Lureau
@ 2019-09-16 10:00 ` Dr. David Alan Gilbert
2019-09-16 10:57 ` Marc-André Lureau
` (2 more replies)
2019-09-17 13:07 ` Daniel P. Berrangé
1 sibling, 3 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-16 10:00 UTC (permalink / raw)
To: Marc-André Lureau, stefanha
Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
(Copying in Stefan since he was looking at DBus for virtiofs)
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> docs/interop/index.rst | 1 +
> 2 files changed, 74 insertions(+)
> create mode 100644 docs/interop/dbus.rst
>
> diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> new file mode 100644
> index 0000000000..c08f026edc
> --- /dev/null
> +++ b/docs/interop/dbus.rst
> @@ -0,0 +1,73 @@
> +=====
> +D-Bus
> +=====
> +
> +Introduction
> +============
> +
> +QEMU may be running with various helper processes involved:
> + - vhost-user* processes (gpu, virtfs, input, etc...)
> + - TPM emulation (or other devices)
> + - user networking (slirp)
> + - network services (DHCP/DNS, samba/ftp etc)
> + - background tasks (compression, streaming etc)
> + - client UI
> + - admin & cli
> +
> +Having several processes allows stricter security rules, as well as
> +greater modularity.
> +
> +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> +display), D-Bus is the de facto IPC of choice on Unix systems. The
> +wire format is machine friendly, good bindings exist for various
> +languages, and there are various tools available.
> +
> +Using a bus, helper processes can discover and communicate with each
> +other easily, without going through QEMU. The bus topology is also
> +easier to apprehend and debug than a mesh. However, it is wise to
> +consider the security aspects of it.
> +
> +Security
> +========
> +
> +A QEMU D-Bus bus should be private to a single VM. Thus, only
> +cooperative tasks are running on the same bus to serve the VM.
> +
> +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> +security between peers once the connection is established. Peers may
> +have additional mechanisms to enforce security rules, based for
> +example on UNIX credentials.
> +
> +dbus-daemon can enforce various policies based on the UID/GID of the
> +processes that are connected to it. It is thus a good idea to run
> +helpers as different UID from QEMU and set appropriate policies (so
> +helper processes are only allowed to talk to qemu for example).
> +
> +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> +``org.qemu.Helper1`` service:
> +
> +.. code:: xml
> +
> + <policy user="qemu">
> + <allow send_destination="org.qemu.Helper1"/>
> + <allow receive_sender="org.qemu.Helper1"/>
> + </policy>
> +
> + <policy user="qemu-helper">
> + <allow own="org.qemu.Helper1"/>
> + </policy>
> +
> +
> +dbus-daemon can also perfom SELinux checks based on the security
> +context of the source and the target. For example, ``virtiofs_t``
> +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> +wouldn't be allowed to send a message to ``virtiofs_t``.
I think we need to start thinking about this more now rather than
'can'. .
Dave
> +Guidelines
> +==========
> +
> +When implementing new D-Bus interfaces, it is recommended to follow
> +the "D-Bus API Design Guidelines":
> +https://dbus.freedesktop.org/doc/dbus-api-design.html
> +
> +The "org.qemu*" prefix is reserved for the QEMU project.
> diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> index b4bfcab417..fa4478ce2e 100644
> --- a/docs/interop/index.rst
> +++ b/docs/interop/index.rst
> @@ -13,6 +13,7 @@ Contents:
> :maxdepth: 2
>
> bitmaps
> + dbus
> live-block-operations
> pr-helper
> vhost-user
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 10:00 ` Dr. David Alan Gilbert
@ 2019-09-16 10:57 ` Marc-André Lureau
2019-09-16 13:15 ` Dr. David Alan Gilbert
2019-09-17 12:47 ` Daniel P. Berrangé
2019-09-19 9:23 ` Stefan Hajnoczi
2 siblings, 1 reply; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-16 10:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Daniel P. Berrange, Juan Quintela, Michal Privoznik, QEMU,
Stefan Hajnoczi, Paolo Bonzini
Hi
On Mon, Sep 16, 2019 at 2:02 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> (Copying in Stefan since he was looking at DBus for virtiofs)
>
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > docs/interop/index.rst | 1 +
> > 2 files changed, 74 insertions(+)
> > create mode 100644 docs/interop/dbus.rst
> >
> > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > new file mode 100644
> > index 0000000000..c08f026edc
> > --- /dev/null
> > +++ b/docs/interop/dbus.rst
> > @@ -0,0 +1,73 @@
> > +=====
> > +D-Bus
> > +=====
> > +
> > +Introduction
> > +============
> > +
> > +QEMU may be running with various helper processes involved:
> > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > + - TPM emulation (or other devices)
> > + - user networking (slirp)
> > + - network services (DHCP/DNS, samba/ftp etc)
> > + - background tasks (compression, streaming etc)
> > + - client UI
> > + - admin & cli
> > +
> > +Having several processes allows stricter security rules, as well as
> > +greater modularity.
> > +
> > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > +wire format is machine friendly, good bindings exist for various
> > +languages, and there are various tools available.
> > +
> > +Using a bus, helper processes can discover and communicate with each
> > +other easily, without going through QEMU. The bus topology is also
> > +easier to apprehend and debug than a mesh. However, it is wise to
> > +consider the security aspects of it.
> > +
> > +Security
> > +========
> > +
> > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > +cooperative tasks are running on the same bus to serve the VM.
> > +
> > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > +security between peers once the connection is established. Peers may
> > +have additional mechanisms to enforce security rules, based for
> > +example on UNIX credentials.
> > +
> > +dbus-daemon can enforce various policies based on the UID/GID of the
> > +processes that are connected to it. It is thus a good idea to run
> > +helpers as different UID from QEMU and set appropriate policies (so
> > +helper processes are only allowed to talk to qemu for example).
> > +
> > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > +``org.qemu.Helper1`` service:
> > +
> > +.. code:: xml
> > +
> > + <policy user="qemu">
> > + <allow send_destination="org.qemu.Helper1"/>
> > + <allow receive_sender="org.qemu.Helper1"/>
> > + </policy>
> > +
> > + <policy user="qemu-helper">
> > + <allow own="org.qemu.Helper1"/>
> > + </policy>
> > +
> > +
> > +dbus-daemon can also perfom SELinux checks based on the security
> > +context of the source and the target. For example, ``virtiofs_t``
> > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > +wouldn't be allowed to send a message to ``virtiofs_t``.
>
> I think we need to start thinking about this more now rather than
> 'can'. .
Do you have a specific question we can answer or guide for qemu? Is
there something we have to document or implement?
Since qemu is not managing the extra processes or applying policies, I
don't know what else could be done. From qemu pov, it can rely on
management layer to trust the bus and the helpers, similar to trusting
the system in general.
> Dave
>
> > +Guidelines
> > +==========
> > +
> > +When implementing new D-Bus interfaces, it is recommended to follow
> > +the "D-Bus API Design Guidelines":
> > +https://dbus.freedesktop.org/doc/dbus-api-design.html
> > +
> > +The "org.qemu*" prefix is reserved for the QEMU project.
> > diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> > index b4bfcab417..fa4478ce2e 100644
> > --- a/docs/interop/index.rst
> > +++ b/docs/interop/index.rst
> > @@ -13,6 +13,7 @@ Contents:
> > :maxdepth: 2
> >
> > bitmaps
> > + dbus
> > live-block-operations
> > pr-helper
> > vhost-user
> > --
> > 2.23.0
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 10:57 ` Marc-André Lureau
@ 2019-09-16 13:15 ` Dr. David Alan Gilbert
2019-09-16 19:13 ` Marc-André Lureau
0 siblings, 1 reply; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-16 13:15 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Daniel P. Berrange, Juan Quintela, Michal Privoznik, QEMU,
Stefan Hajnoczi, Paolo Bonzini
* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
>
> On Mon, Sep 16, 2019 at 2:02 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > (Copying in Stefan since he was looking at DBus for virtiofs)
> >
> > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > > docs/interop/index.rst | 1 +
> > > 2 files changed, 74 insertions(+)
> > > create mode 100644 docs/interop/dbus.rst
> > >
> > > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > > new file mode 100644
> > > index 0000000000..c08f026edc
> > > --- /dev/null
> > > +++ b/docs/interop/dbus.rst
> > > @@ -0,0 +1,73 @@
> > > +=====
> > > +D-Bus
> > > +=====
> > > +
> > > +Introduction
> > > +============
> > > +
> > > +QEMU may be running with various helper processes involved:
> > > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > > + - TPM emulation (or other devices)
> > > + - user networking (slirp)
> > > + - network services (DHCP/DNS, samba/ftp etc)
> > > + - background tasks (compression, streaming etc)
> > > + - client UI
> > > + - admin & cli
> > > +
> > > +Having several processes allows stricter security rules, as well as
> > > +greater modularity.
> > > +
> > > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > > +wire format is machine friendly, good bindings exist for various
> > > +languages, and there are various tools available.
> > > +
> > > +Using a bus, helper processes can discover and communicate with each
> > > +other easily, without going through QEMU. The bus topology is also
> > > +easier to apprehend and debug than a mesh. However, it is wise to
> > > +consider the security aspects of it.
> > > +
> > > +Security
> > > +========
> > > +
> > > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > > +cooperative tasks are running on the same bus to serve the VM.
> > > +
> > > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > > +security between peers once the connection is established. Peers may
> > > +have additional mechanisms to enforce security rules, based for
> > > +example on UNIX credentials.
> > > +
> > > +dbus-daemon can enforce various policies based on the UID/GID of the
> > > +processes that are connected to it. It is thus a good idea to run
> > > +helpers as different UID from QEMU and set appropriate policies (so
> > > +helper processes are only allowed to talk to qemu for example).
> > > +
> > > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > > +``org.qemu.Helper1`` service:
> > > +
> > > +.. code:: xml
> > > +
> > > + <policy user="qemu">
> > > + <allow send_destination="org.qemu.Helper1"/>
> > > + <allow receive_sender="org.qemu.Helper1"/>
> > > + </policy>
> > > +
> > > + <policy user="qemu-helper">
> > > + <allow own="org.qemu.Helper1"/>
> > > + </policy>
> > > +
> > > +
> > > +dbus-daemon can also perfom SELinux checks based on the security
> > > +context of the source and the target. For example, ``virtiofs_t``
> > > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > > +wouldn't be allowed to send a message to ``virtiofs_t``.
> >
> > I think we need to start thinking about this more now rather than
> > 'can'. .
>
> Do you have a specific question we can answer or guide for qemu? Is
> there something we have to document or implement?
>
> Since qemu is not managing the extra processes or applying policies, I
> don't know what else could be done. From qemu pov, it can rely on
> management layer to trust the bus and the helpers, similar to trusting
> the system in general.
Well pretty much the same questions I asked in the discussion on v2;
what is the supported configuration to ensure that one helper that's
been compromised can't attack the others and qemu?
Dave
> > Dave
> >
> > > +Guidelines
> > > +==========
> > > +
> > > +When implementing new D-Bus interfaces, it is recommended to follow
> > > +the "D-Bus API Design Guidelines":
> > > +https://dbus.freedesktop.org/doc/dbus-api-design.html
> > > +
> > > +The "org.qemu*" prefix is reserved for the QEMU project.
> > > diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> > > index b4bfcab417..fa4478ce2e 100644
> > > --- a/docs/interop/index.rst
> > > +++ b/docs/interop/index.rst
> > > @@ -13,6 +13,7 @@ Contents:
> > > :maxdepth: 2
> > >
> > > bitmaps
> > > + dbus
> > > live-block-operations
> > > pr-helper
> > > vhost-user
> > > --
> > > 2.23.0
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
>
> --
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 13:15 ` Dr. David Alan Gilbert
@ 2019-09-16 19:13 ` Marc-André Lureau
2019-09-17 8:12 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-16 19:13 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Daniel P. Berrange, Juan Quintela, Michal Privoznik, QEMU,
Stefan Hajnoczi, Paolo Bonzini
Hi
On Mon, Sep 16, 2019 at 5:15 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> > Hi
> >
> > On Mon, Sep 16, 2019 at 2:02 PM Dr. David Alan Gilbert
> > <dgilbert@redhat.com> wrote:
> > >
> > > (Copying in Stefan since he was looking at DBus for virtiofs)
> > >
> > > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > > > docs/interop/index.rst | 1 +
> > > > 2 files changed, 74 insertions(+)
> > > > create mode 100644 docs/interop/dbus.rst
> > > >
> > > > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > > > new file mode 100644
> > > > index 0000000000..c08f026edc
> > > > --- /dev/null
> > > > +++ b/docs/interop/dbus.rst
> > > > @@ -0,0 +1,73 @@
> > > > +=====
> > > > +D-Bus
> > > > +=====
> > > > +
> > > > +Introduction
> > > > +============
> > > > +
> > > > +QEMU may be running with various helper processes involved:
> > > > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > > > + - TPM emulation (or other devices)
> > > > + - user networking (slirp)
> > > > + - network services (DHCP/DNS, samba/ftp etc)
> > > > + - background tasks (compression, streaming etc)
> > > > + - client UI
> > > > + - admin & cli
> > > > +
> > > > +Having several processes allows stricter security rules, as well as
> > > > +greater modularity.
> > > > +
> > > > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > > > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > > > +wire format is machine friendly, good bindings exist for various
> > > > +languages, and there are various tools available.
> > > > +
> > > > +Using a bus, helper processes can discover and communicate with each
> > > > +other easily, without going through QEMU. The bus topology is also
> > > > +easier to apprehend and debug than a mesh. However, it is wise to
> > > > +consider the security aspects of it.
> > > > +
> > > > +Security
> > > > +========
> > > > +
> > > > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > > > +cooperative tasks are running on the same bus to serve the VM.
> > > > +
> > > > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > > > +security between peers once the connection is established. Peers may
> > > > +have additional mechanisms to enforce security rules, based for
> > > > +example on UNIX credentials.
> > > > +
> > > > +dbus-daemon can enforce various policies based on the UID/GID of the
> > > > +processes that are connected to it. It is thus a good idea to run
> > > > +helpers as different UID from QEMU and set appropriate policies (so
> > > > +helper processes are only allowed to talk to qemu for example).
> > > > +
> > > > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > > > +``org.qemu.Helper1`` service:
> > > > +
> > > > +.. code:: xml
> > > > +
> > > > + <policy user="qemu">
> > > > + <allow send_destination="org.qemu.Helper1"/>
> > > > + <allow receive_sender="org.qemu.Helper1"/>
> > > > + </policy>
> > > > +
> > > > + <policy user="qemu-helper">
> > > > + <allow own="org.qemu.Helper1"/>
> > > > + </policy>
> > > > +
> > > > +
> > > > +dbus-daemon can also perfom SELinux checks based on the security
> > > > +context of the source and the target. For example, ``virtiofs_t``
> > > > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > > > +wouldn't be allowed to send a message to ``virtiofs_t``.
> > >
> > > I think we need to start thinking about this more now rather than
> > > 'can'. .
> >
> > Do you have a specific question we can answer or guide for qemu? Is
> > there something we have to document or implement?
> >
> > Since qemu is not managing the extra processes or applying policies, I
> > don't know what else could be done. From qemu pov, it can rely on
> > management layer to trust the bus and the helpers, similar to trusting
> > the system in general.
>
> Well pretty much the same questions I asked in the discussion on v2;
> what is the supported configuration to ensure that one helper that's
> been compromised can't attack the others and qemu?
I thought I gave the answer to that question above. What is missing? I
don't think one can generalize it here, it will be a case by case for
each helper, how they interact with each other and qemu.
>
> Dave
>
> > > Dave
> > >
> > > > +Guidelines
> > > > +==========
> > > > +
> > > > +When implementing new D-Bus interfaces, it is recommended to follow
> > > > +the "D-Bus API Design Guidelines":
> > > > +https://dbus.freedesktop.org/doc/dbus-api-design.html
> > > > +
> > > > +The "org.qemu*" prefix is reserved for the QEMU project.
> > > > diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> > > > index b4bfcab417..fa4478ce2e 100644
> > > > --- a/docs/interop/index.rst
> > > > +++ b/docs/interop/index.rst
> > > > @@ -13,6 +13,7 @@ Contents:
> > > > :maxdepth: 2
> > > >
> > > > bitmaps
> > > > + dbus
> > > > live-block-operations
> > > > pr-helper
> > > > vhost-user
> > > > --
> > > > 2.23.0
> > > >
> > > --
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > >
> >
> >
> > --
> > Marc-André Lureau
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 19:13 ` Marc-André Lureau
@ 2019-09-17 8:12 ` Dr. David Alan Gilbert
2019-09-17 8:23 ` Marc-André Lureau
0 siblings, 1 reply; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-17 8:12 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Daniel P. Berrange, Juan Quintela, Michal Privoznik, QEMU,
Stefan Hajnoczi, Paolo Bonzini
* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
>
> On Mon, Sep 16, 2019 at 5:15 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> > > Hi
> > >
> > > On Mon, Sep 16, 2019 at 2:02 PM Dr. David Alan Gilbert
> > > <dgilbert@redhat.com> wrote:
> > > >
> > > > (Copying in Stefan since he was looking at DBus for virtiofs)
> > > >
> > > > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
<snip>
> > > Do you have a specific question we can answer or guide for qemu? Is
> > > there something we have to document or implement?
> > >
> > > Since qemu is not managing the extra processes or applying policies, I
> > > don't know what else could be done. From qemu pov, it can rely on
> > > management layer to trust the bus and the helpers, similar to trusting
> > > the system in general.
> >
> > Well pretty much the same questions I asked in the discussion on v2;
> > what is the supported configuration to ensure that one helper that's
> > been compromised can't attack the others and qemu?
>
> I thought I gave the answer to that question above. What is missing? I
> don't think one can generalize it here, it will be a case by case for
> each helper, how they interact with each other and qemu.
I think we need an example of how to lock it down; i.e. to allow a
helper to provide migration data but not to be able to speak to other
helpers.
Dave
> >
> > Dave
> >
> > > > Dave
> > > >
> > > > > +Guidelines
> > > > > +==========
> > > > > +
> > > > > +When implementing new D-Bus interfaces, it is recommended to follow
> > > > > +the "D-Bus API Design Guidelines":
> > > > > +https://dbus.freedesktop.org/doc/dbus-api-design.html
> > > > > +
> > > > > +The "org.qemu*" prefix is reserved for the QEMU project.
> > > > > diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> > > > > index b4bfcab417..fa4478ce2e 100644
> > > > > --- a/docs/interop/index.rst
> > > > > +++ b/docs/interop/index.rst
> > > > > @@ -13,6 +13,7 @@ Contents:
> > > > > :maxdepth: 2
> > > > >
> > > > > bitmaps
> > > > > + dbus
> > > > > live-block-operations
> > > > > pr-helper
> > > > > vhost-user
> > > > > --
> > > > > 2.23.0
> > > > >
> > > > --
> > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > >
> > >
> > >
> > > --
> > > Marc-André Lureau
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>
>
> --
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-17 8:12 ` Dr. David Alan Gilbert
@ 2019-09-17 8:23 ` Marc-André Lureau
0 siblings, 0 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-17 8:23 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Daniel P. Berrange, Juan Quintela, Michal Privoznik, QEMU,
Stefan Hajnoczi, Paolo Bonzini
Hi
On Tue, Sep 17, 2019 at 12:12 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> > Hi
> >
> > On Mon, Sep 16, 2019 at 5:15 PM Dr. David Alan Gilbert
> > <dgilbert@redhat.com> wrote:
> > >
> > > * Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> > > > Hi
> > > >
> > > > On Mon, Sep 16, 2019 at 2:02 PM Dr. David Alan Gilbert
> > > > <dgilbert@redhat.com> wrote:
> > > > >
> > > > > (Copying in Stefan since he was looking at DBus for virtiofs)
> > > > >
> > > > > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> <snip>
>
> > > > Do you have a specific question we can answer or guide for qemu? Is
> > > > there something we have to document or implement?
> > > >
> > > > Since qemu is not managing the extra processes or applying policies, I
> > > > don't know what else could be done. From qemu pov, it can rely on
> > > > management layer to trust the bus and the helpers, similar to trusting
> > > > the system in general.
> > >
> > > Well pretty much the same questions I asked in the discussion on v2;
> > > what is the supported configuration to ensure that one helper that's
> > > been compromised can't attack the others and qemu?
> >
> > I thought I gave the answer to that question above. What is missing? I
> > don't think one can generalize it here, it will be a case by case for
> > each helper, how they interact with each other and qemu.
>
> I think we need an example of how to lock it down; i.e. to allow a
> helper to provide migration data but not to be able to speak to other
> helpers.
>
That's the example policy I gave for dbus-dameon. Only qemu user can
talk to Helper1 (for ex, a helper migration interface). Only
qemu-helper user can claim Helper1.
You could have additionally other ways: selinux policy, p2p helpers
SCM credentials checks, or other methods.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 10:00 ` Dr. David Alan Gilbert
2019-09-16 10:57 ` Marc-André Lureau
@ 2019-09-17 12:47 ` Daniel P. Berrangé
2019-09-17 13:03 ` Dr. David Alan Gilbert
2019-09-19 9:23 ` Stefan Hajnoczi
2 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 12:47 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: quintela, mprivozn, qemu-devel, stefanha, pbonzini,
Marc-André Lureau
On Mon, Sep 16, 2019 at 11:00:35AM +0100, Dr. David Alan Gilbert wrote:
> (Copying in Stefan since he was looking at DBus for virtiofs)
>
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > docs/interop/index.rst | 1 +
> > 2 files changed, 74 insertions(+)
> > create mode 100644 docs/interop/dbus.rst
> >
> > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > new file mode 100644
> > index 0000000000..c08f026edc
> > --- /dev/null
> > +++ b/docs/interop/dbus.rst
> > @@ -0,0 +1,73 @@
> > +=====
> > +D-Bus
> > +=====
> > +
> > +Introduction
> > +============
> > +
> > +QEMU may be running with various helper processes involved:
> > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > + - TPM emulation (or other devices)
> > + - user networking (slirp)
> > + - network services (DHCP/DNS, samba/ftp etc)
> > + - background tasks (compression, streaming etc)
> > + - client UI
> > + - admin & cli
> > +
> > +Having several processes allows stricter security rules, as well as
> > +greater modularity.
> > +
> > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > +wire format is machine friendly, good bindings exist for various
> > +languages, and there are various tools available.
> > +
> > +Using a bus, helper processes can discover and communicate with each
> > +other easily, without going through QEMU. The bus topology is also
> > +easier to apprehend and debug than a mesh. However, it is wise to
> > +consider the security aspects of it.
> > +
> > +Security
> > +========
> > +
> > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > +cooperative tasks are running on the same bus to serve the VM.
> > +
> > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > +security between peers once the connection is established. Peers may
> > +have additional mechanisms to enforce security rules, based for
> > +example on UNIX credentials.
> > +
> > +dbus-daemon can enforce various policies based on the UID/GID of the
> > +processes that are connected to it. It is thus a good idea to run
> > +helpers as different UID from QEMU and set appropriate policies (so
> > +helper processes are only allowed to talk to qemu for example).
> > +
> > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > +``org.qemu.Helper1`` service:
> > +
> > +.. code:: xml
> > +
> > + <policy user="qemu">
> > + <allow send_destination="org.qemu.Helper1"/>
> > + <allow receive_sender="org.qemu.Helper1"/>
> > + </policy>
> > +
> > + <policy user="qemu-helper">
> > + <allow own="org.qemu.Helper1"/>
> > + </policy>
> > +
> > +
> > +dbus-daemon can also perfom SELinux checks based on the security
> > +context of the source and the target. For example, ``virtiofs_t``
> > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > +wouldn't be allowed to send a message to ``virtiofs_t``.
>
> I think we need to start thinking about this more now rather than
> 'can'. .
Thinking about DBus usage with helpers, as compared to the current state
with monolithic QEMU, the top priority is to ensure no degradation in
security vs current practice.
That is fairly easy from libvirt's POV - we simply need to make sure
that the dbus daemon and all helpers get given the same SELinux svirt_t
content as used for QEMU, so each QEMU is still siloed to the same
extent.
If SELinux is not enabled, then currently an out of the box libvirt
config only protects the host from QEMU, it doesn't protect QEMU
from other QEMUs, since they all run the same user ID.
It is possible to tell libvirt to run each QEMU as a separate user
ID if the mgmt app has a range of user IDs avalable. In this case,
we would simply run the helpers/dbus as the same per-QEMU user ID
to ensure we don't regress.
Getting an improved security model is obviously the ultimate goal,
as this modularity needs to offer some benefit to outweight its
costs.
In terms of SELinux, this will involve creating distinct SElinux
contexts for each helper process. (svirt_slirp_t, svirt_swtpm_t,
etc, etc).
In terms of DAC, in the per QEMU user ID scenario, we would need
to allocate at least 2 UIDs for each QEMU process, so that helpers
would be separate from the QEMU. To be honest it would be better
if we had 3 UIDs, to the dbus daemon was separated from both the
helpers and QEMU.
This starts to sound like alot of UIDs which is tedious to manage.
Libvirt already puts QEMU in a separate mount namespace. From a
DAC POV, to get meaninguful separation will probably want libvirt
to consider the "user" namespace too. This is quite a bit of work
to get everything labelled right for different user namespace,
but it may well simplify mgmt thereafter. We still have the same
problem though, of needing to assign a range of user IDs for each
user namespace.
Overall, I can see the possible technical options for securing
this use of DBus, so I'm not too concerned here.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-17 12:47 ` Daniel P. Berrangé
@ 2019-09-17 13:03 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-17 13:03 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: quintela, mprivozn, qemu-devel, stefanha, pbonzini,
Marc-André Lureau
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Mon, Sep 16, 2019 at 11:00:35AM +0100, Dr. David Alan Gilbert wrote:
> > (Copying in Stefan since he was looking at DBus for virtiofs)
> >
> > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > > docs/interop/index.rst | 1 +
> > > 2 files changed, 74 insertions(+)
> > > create mode 100644 docs/interop/dbus.rst
> > >
> > > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > > new file mode 100644
> > > index 0000000000..c08f026edc
> > > --- /dev/null
> > > +++ b/docs/interop/dbus.rst
> > > @@ -0,0 +1,73 @@
> > > +=====
> > > +D-Bus
> > > +=====
> > > +
> > > +Introduction
> > > +============
> > > +
> > > +QEMU may be running with various helper processes involved:
> > > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > > + - TPM emulation (or other devices)
> > > + - user networking (slirp)
> > > + - network services (DHCP/DNS, samba/ftp etc)
> > > + - background tasks (compression, streaming etc)
> > > + - client UI
> > > + - admin & cli
> > > +
> > > +Having several processes allows stricter security rules, as well as
> > > +greater modularity.
> > > +
> > > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > > +wire format is machine friendly, good bindings exist for various
> > > +languages, and there are various tools available.
> > > +
> > > +Using a bus, helper processes can discover and communicate with each
> > > +other easily, without going through QEMU. The bus topology is also
> > > +easier to apprehend and debug than a mesh. However, it is wise to
> > > +consider the security aspects of it.
> > > +
> > > +Security
> > > +========
> > > +
> > > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > > +cooperative tasks are running on the same bus to serve the VM.
> > > +
> > > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > > +security between peers once the connection is established. Peers may
> > > +have additional mechanisms to enforce security rules, based for
> > > +example on UNIX credentials.
> > > +
> > > +dbus-daemon can enforce various policies based on the UID/GID of the
> > > +processes that are connected to it. It is thus a good idea to run
> > > +helpers as different UID from QEMU and set appropriate policies (so
> > > +helper processes are only allowed to talk to qemu for example).
> > > +
> > > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > > +``org.qemu.Helper1`` service:
> > > +
> > > +.. code:: xml
> > > +
> > > + <policy user="qemu">
> > > + <allow send_destination="org.qemu.Helper1"/>
> > > + <allow receive_sender="org.qemu.Helper1"/>
> > > + </policy>
> > > +
> > > + <policy user="qemu-helper">
> > > + <allow own="org.qemu.Helper1"/>
> > > + </policy>
> > > +
> > > +
> > > +dbus-daemon can also perfom SELinux checks based on the security
> > > +context of the source and the target. For example, ``virtiofs_t``
> > > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > > +wouldn't be allowed to send a message to ``virtiofs_t``.
> >
> > I think we need to start thinking about this more now rather than
> > 'can'. .
>
> Thinking about DBus usage with helpers, as compared to the current state
> with monolithic QEMU, the top priority is to ensure no degradation in
> security vs current practice.
>
> That is fairly easy from libvirt's POV - we simply need to make sure
> that the dbus daemon and all helpers get given the same SELinux svirt_t
> content as used for QEMU, so each QEMU is still siloed to the same
> extent.
>
> If SELinux is not enabled, then currently an out of the box libvirt
> config only protects the host from QEMU, it doesn't protect QEMU
> from other QEMUs, since they all run the same user ID.
>
> It is possible to tell libvirt to run each QEMU as a separate user
> ID if the mgmt app has a range of user IDs avalable. In this case,
> we would simply run the helpers/dbus as the same per-QEMU user ID
> to ensure we don't regress.
>
>
> Getting an improved security model is obviously the ultimate goal,
> as this modularity needs to offer some benefit to outweight its
> costs.
>
> In terms of SELinux, this will involve creating distinct SElinux
> contexts for each helper process. (svirt_slirp_t, svirt_swtpm_t,
> etc, etc).
>
> In terms of DAC, in the per QEMU user ID scenario, we would need
> to allocate at least 2 UIDs for each QEMU process, so that helpers
> would be separate from the QEMU. To be honest it would be better
> if we had 3 UIDs, to the dbus daemon was separated from both the
> helpers and QEMU.
>
> This starts to sound like alot of UIDs which is tedious to manage.
> Libvirt already puts QEMU in a separate mount namespace. From a
> DAC POV, to get meaninguful separation will probably want libvirt
> to consider the "user" namespace too. This is quite a bit of work
> to get everything labelled right for different user namespace,
> but it may well simplify mgmt thereafter. We still have the same
> problem though, of needing to assign a range of user IDs for each
> user namespace.
A separate user namespace might cause problems for things like
virtiofs where it's trying to access the files with particular perms,
or with say a GPU where it needs access to a display.
Dave
> Overall, I can see the possible technical options for securing
> this use of DBus, so I'm not too concerned here.
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-16 10:00 ` Dr. David Alan Gilbert
2019-09-16 10:57 ` Marc-André Lureau
2019-09-17 12:47 ` Daniel P. Berrangé
@ 2019-09-19 9:23 ` Stefan Hajnoczi
2 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2019-09-19 9:23 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: berrange, quintela, mprivozn, qemu-devel, pbonzini,
Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 3660 bytes --]
On Mon, Sep 16, 2019 at 11:00:35AM +0100, Dr. David Alan Gilbert wrote:
> (Copying in Stefan since he was looking at DBus for virtiofs)
>
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> > docs/interop/index.rst | 1 +
> > 2 files changed, 74 insertions(+)
> > create mode 100644 docs/interop/dbus.rst
> >
> > diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> > new file mode 100644
> > index 0000000000..c08f026edc
> > --- /dev/null
> > +++ b/docs/interop/dbus.rst
> > @@ -0,0 +1,73 @@
> > +=====
> > +D-Bus
> > +=====
> > +
> > +Introduction
> > +============
> > +
> > +QEMU may be running with various helper processes involved:
> > + - vhost-user* processes (gpu, virtfs, input, etc...)
> > + - TPM emulation (or other devices)
> > + - user networking (slirp)
> > + - network services (DHCP/DNS, samba/ftp etc)
> > + - background tasks (compression, streaming etc)
> > + - client UI
> > + - admin & cli
> > +
> > +Having several processes allows stricter security rules, as well as
> > +greater modularity.
> > +
> > +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> > +display), D-Bus is the de facto IPC of choice on Unix systems. The
> > +wire format is machine friendly, good bindings exist for various
> > +languages, and there are various tools available.
> > +
> > +Using a bus, helper processes can discover and communicate with each
> > +other easily, without going through QEMU. The bus topology is also
> > +easier to apprehend and debug than a mesh. However, it is wise to
> > +consider the security aspects of it.
> > +
> > +Security
> > +========
> > +
> > +A QEMU D-Bus bus should be private to a single VM. Thus, only
> > +cooperative tasks are running on the same bus to serve the VM.
> > +
> > +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> > +security between peers once the connection is established. Peers may
> > +have additional mechanisms to enforce security rules, based for
> > +example on UNIX credentials.
> > +
> > +dbus-daemon can enforce various policies based on the UID/GID of the
> > +processes that are connected to it. It is thus a good idea to run
> > +helpers as different UID from QEMU and set appropriate policies (so
> > +helper processes are only allowed to talk to qemu for example).
> > +
> > +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> > +``org.qemu.Helper1`` service:
> > +
> > +.. code:: xml
> > +
> > + <policy user="qemu">
> > + <allow send_destination="org.qemu.Helper1"/>
> > + <allow receive_sender="org.qemu.Helper1"/>
> > + </policy>
> > +
> > + <policy user="qemu-helper">
> > + <allow own="org.qemu.Helper1"/>
> > + </policy>
> > +
> > +
> > +dbus-daemon can also perfom SELinux checks based on the security
> > +context of the source and the target. For example, ``virtiofs_t``
> > +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> > +wouldn't be allowed to send a message to ``virtiofs_t``.
>
> I think we need to start thinking about this more now rather than
> 'can'. .
virtiofsd has two DBus interfaces:
1. org.qemu.Virtiofsd - the management interface
We don't expect QEMU to communicate with this. Administrators or
management tools will connect to this.
2. dbus-vmstate - we'll probably need this for live migration
This is for QEMU<->vhost-user device backend communication.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage Marc-André Lureau
2019-09-16 10:00 ` Dr. David Alan Gilbert
@ 2019-09-17 13:07 ` Daniel P. Berrangé
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 13:07 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:13PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> docs/interop/dbus.rst | 73 ++++++++++++++++++++++++++++++++++++++++++
> docs/interop/index.rst | 1 +
> 2 files changed, 74 insertions(+)
> create mode 100644 docs/interop/dbus.rst
>
> diff --git a/docs/interop/dbus.rst b/docs/interop/dbus.rst
> new file mode 100644
> index 0000000000..c08f026edc
> --- /dev/null
> +++ b/docs/interop/dbus.rst
> @@ -0,0 +1,73 @@
> +=====
> +D-Bus
> +=====
> +
> +Introduction
> +============
> +
> +QEMU may be running with various helper processes involved:
> + - vhost-user* processes (gpu, virtfs, input, etc...)
> + - TPM emulation (or other devices)
> + - user networking (slirp)
> + - network services (DHCP/DNS, samba/ftp etc)
> + - background tasks (compression, streaming etc)
> + - client UI
> + - admin & cli
> +
> +Having several processes allows stricter security rules, as well as
> +greater modularity.
> +
> +While QEMU itself uses QMP as primary IPC (and Spice/VNC for remote
> +display), D-Bus is the de facto IPC of choice on Unix systems. The
> +wire format is machine friendly, good bindings exist for various
> +languages, and there are various tools available.
> +
> +Using a bus, helper processes can discover and communicate with each
> +other easily, without going through QEMU. The bus topology is also
> +easier to apprehend and debug than a mesh. However, it is wise to
> +consider the security aspects of it.
> +
> +Security
> +========
> +
> +A QEMU D-Bus bus should be private to a single VM. Thus, only
> +cooperative tasks are running on the same bus to serve the VM.
> +
> +D-Bus, the protocol and standard, doesn't have mechanisms to enforce
> +security between peers once the connection is established. Peers may
> +have additional mechanisms to enforce security rules, based for
> +example on UNIX credentials.
> +
> +dbus-daemon can enforce various policies based on the UID/GID of the
> +processes that are connected to it. It is thus a good idea to run
> +helpers as different UID from QEMU and set appropriate policies (so
> +helper processes are only allowed to talk to qemu for example).
We should also recommend that the daemon itself be run as separate
UID from QEMU, otherwise a compromised QEMU can trivially compromise
the dbus daemon too.
I'd say three scenarios are reasonable to document
- Everything the same UID.
- Convenient for developers
- Improved reliability - crash of one part doens't take
out entire VM
- No security benefit over traditional QEMU
- Two UIDs, one for QEMU, one for dbus & helpers
- Moderately improved security isolation
- Many UIDs, one for QEMU one for dbus and one for each helpers
- Best security isolation
- Complex to manager distinct UIDs needed for each VM
Documenting SELinux scenarios is probably a bit out of scope for
this.
We probably need to mention about the trust semantics associated
with messages sent over the bus.
ie, just because the daemon has controlled who can send/recv
messages to who, doesn't magically make this secure.
The semantics of the actual methods implemented using dbus
are just as critical. Peers need to carefully validate any
information they received from a peer with a different trust
level.
> +For example, this allows only ``qemu`` user to talk to ``qemu-helper``
> +``org.qemu.Helper1`` service:
> +
> +.. code:: xml
> +
> + <policy user="qemu">
> + <allow send_destination="org.qemu.Helper1"/>
> + <allow receive_sender="org.qemu.Helper1"/>
> + </policy>
> +
> + <policy user="qemu-helper">
> + <allow own="org.qemu.Helper1"/>
> + </policy>
> +
> +
> +dbus-daemon can also perfom SELinux checks based on the security
> +context of the source and the target. For example, ``virtiofs_t``
> +could be allowed to send a message to ``svirt_t``, but ``virtiofs_t``
> +wouldn't be allowed to send a message to ``virtiofs_t``.
> +Guidelines
> +==========
> +
> +When implementing new D-Bus interfaces, it is recommended to follow
> +the "D-Bus API Design Guidelines":
> +https://dbus.freedesktop.org/doc/dbus-api-design.html
> +
> +The "org.qemu*" prefix is reserved for the QEMU project.
s/org.qemu*/org.qemu.*/ - we can't claim ownership of every
possible domain name with 'qemu' as a prefix.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
` (4 preceding siblings ...)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 5/6] docs: start a document to describe D-Bus usage Marc-André Lureau
@ 2019-09-12 12:25 ` Marc-André Lureau
2019-09-12 14:29 ` Eric Blake
` (2 more replies)
2019-09-12 13:50 ` [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate no-reply
6 siblings, 3 replies; 32+ messages in thread
From: Marc-André Lureau @ 2019-09-12 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: berrange, quintela, mprivozn, dgilbert, Marc-André Lureau,
pbonzini
When instanciated, this object will connect to the given D-Bus
bus. During migration, it will take the data from org.qemu.VMState1
instances.
See documentation for further details.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
MAINTAINERS | 6 +
backends/Makefile.objs | 4 +
backends/dbus-vmstate.c | 530 ++++++++++++++++++++++++++++++++++
configure | 7 +
docs/interop/dbus-vmstate.rst | 68 +++++
docs/interop/index.rst | 1 +
tests/Makefile.include | 19 +-
tests/dbus-vmstate-daemon.sh | 95 ++++++
tests/dbus-vmstate-test.c | 386 +++++++++++++++++++++++++
tests/dbus-vmstate1.xml | 12 +
10 files changed, 1127 insertions(+), 1 deletion(-)
create mode 100644 backends/dbus-vmstate.c
create mode 100644 docs/interop/dbus-vmstate.rst
create mode 100755 tests/dbus-vmstate-daemon.sh
create mode 100644 tests/dbus-vmstate-test.c
create mode 100644 tests/dbus-vmstate1.xml
diff --git a/MAINTAINERS b/MAINTAINERS
index 50eaf005f4..f641870c40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2153,6 +2153,12 @@ F: tests/migration-test.c
F: docs/devel/migration.rst
F: qapi/migration.json
+DBus VMState
+M: Marc-André Lureau <marcandre.lureau@redhat.com>
+S: Maintained
+F: backends/dbus-vmstate.c
+F: tests/dbus-vmstate*
+
Seccomp
M: Eduardo Otubo <otubo@redhat.com>
S: Supported
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index f0691116e8..28a847cd57 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -17,3 +17,7 @@ endif
common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_VIRTIO)) += vhost-user.o
common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
+
+common-obj-$(CONFIG_GIO) += dbus-vmstate.o
+dbus-vmstate.o-cflags = $(GIO_CFLAGS)
+dbus-vmstate.o-libs = $(GIO_LIBS)
diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
new file mode 100644
index 0000000000..559f5430c5
--- /dev/null
+++ b/backends/dbus-vmstate.c
@@ -0,0 +1,530 @@
+/*
+ * QEMU dbus-vmstate
+ *
+ * Copyright (C) 2019 Red Hat Inc
+ *
+ * Authors:
+ * Marc-André Lureau <marcandre.lureau@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "qapi/qmp/qerror.h"
+#include "migration/vmstate.h"
+#include <gio/gio.h>
+
+typedef struct DBusVMState DBusVMState;
+typedef struct DBusVMStateClass DBusVMStateClass;
+
+#define TYPE_DBUS_VMSTATE "dbus-vmstate"
+#define DBUS_VMSTATE(obj) \
+ OBJECT_CHECK(DBusVMState, (obj), TYPE_DBUS_VMSTATE)
+#define DBUS_VMSTATE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(DBusVMStateClass, (obj), TYPE_DBUS_VMSTATE)
+#define DBUS_VMSTATE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(DBusVMStateClass, (klass), TYPE_DBUS_VMSTATE)
+
+struct DBusVMStateClass {
+ ObjectClass parent_class;
+};
+
+struct DBusVMState {
+ Object parent;
+
+ GDBusConnection *bus;
+ char *dbus_addr;
+ char *id_list;
+
+ uint32_t data_size;
+ uint8_t *data;
+};
+
+static const GDBusPropertyInfo vmstate_property_info[] = {
+ { -1, (char *) "Id", (char *) "s",
+ G_DBUS_PROPERTY_INFO_FLAGS_READABLE, NULL },
+};
+
+static const GDBusPropertyInfo * const vmstate_property_info_pointers[] = {
+ &vmstate_property_info[0],
+ NULL
+};
+
+static const GDBusInterfaceInfo vmstate1_interface_info = {
+ -1,
+ (char *) "org.qemu.VMState1",
+ (GDBusMethodInfo **) NULL,
+ (GDBusSignalInfo **) NULL,
+ (GDBusPropertyInfo **) &vmstate_property_info_pointers,
+ NULL,
+};
+
+#define DBUS_VMSTATE_SIZE_LIMIT (1 * MiB)
+
+static char **
+dbus_get_vmstate1_names(DBusVMState *self, GError **err)
+{
+ g_autoptr(GDBusProxy) proxy = NULL;
+ g_autoptr(GVariant) result = NULL;
+ g_autoptr(GVariant) child = NULL;
+
+ proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE, NULL,
+ "org.freedesktop.DBus",
+ "/org/freedesktop/DBus",
+ "org.freedesktop.DBus",
+ NULL, err);
+ if (!proxy) {
+ return NULL;
+ }
+
+ result = g_dbus_proxy_call_sync(proxy, "ListQueuedOwners",
+ g_variant_new("(s)", "org.qemu.VMState1"),
+ G_DBUS_CALL_FLAGS_NO_AUTO_START,
+ -1, NULL, err);
+ if (!result) {
+ return NULL;
+ }
+
+ child = g_variant_get_child_value(result, 0);
+ return g_variant_dup_strv(child, NULL);
+}
+
+static GHashTable *
+get_id_list_set(DBusVMState *self)
+{
+ g_auto(GStrv) ids = NULL;
+ g_autoptr(GHashTable) set = NULL;
+ int i;
+
+ if (!self->id_list) {
+ return NULL;
+ }
+
+ ids = g_strsplit(self->id_list, ",", -1);
+ set = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
+ for (i = 0; ids[i]; i++) {
+ g_hash_table_add(set, ids[i]);
+ ids[i] = NULL;
+ }
+
+ return g_steal_pointer(&set);
+}
+
+static GHashTable *
+dbus_get_proxies(DBusVMState *self, GError **err)
+{
+ g_autoptr(GError) local_err = NULL;
+ g_autoptr(GHashTable) proxies = NULL;
+ g_autoptr(GHashTable) ids = NULL;
+ g_auto(GStrv) names = NULL;
+ size_t i;
+
+ ids = get_id_list_set(self);
+ proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
+ g_free, g_object_unref);
+
+ names = dbus_get_vmstate1_names(self, &local_err);
+ if (!names) {
+ if (g_error_matches(local_err,
+ G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER)) {
+ return proxies;
+ }
+ g_propagate_error(err, local_err);
+ return NULL;
+ }
+
+ for (i = 0; names[i]; i++) {
+ g_autoptr(GDBusProxy) proxy = NULL;
+ g_autoptr(GVariant) result = NULL;
+ g_autofree char *id = NULL;
+ size_t size;
+
+ proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE,
+ (GDBusInterfaceInfo *) &vmstate1_interface_info,
+ names[i],
+ "/org/qemu/VMState1",
+ "org.qemu.VMState1",
+ NULL, err);
+ if (!proxy) {
+ return NULL;
+ }
+
+ result = g_dbus_proxy_get_cached_property(proxy, "Id");
+ if (!result) {
+ g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "VMState Id property is missing.");
+ return NULL;
+ }
+
+ id = g_variant_dup_string(result, &size);
+ if (ids && !g_hash_table_remove(ids, id)) {
+ g_clear_pointer(&id, g_free);
+ g_clear_object(&proxy);
+ continue;
+ }
+ if (size == 0 || size >= 256) {
+ g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "VMState Id '%s' is invalid.", id);
+ return NULL;
+ }
+
+ if (!g_hash_table_insert(proxies, id, proxy)) {
+ g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "Duplicated VMState Id '%s'", id);
+ return NULL;
+ }
+ id = NULL;
+ proxy = NULL;
+
+ g_clear_pointer(&result, g_variant_unref);
+ }
+
+ if (ids) {
+ g_autofree char **left = NULL;
+
+ left = (char **)g_hash_table_get_keys_as_array(ids, NULL);
+ if (*left) {
+ g_autofree char *leftids = g_strjoinv(",", left);
+ g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "Required VMState Id are missing: %s", leftids);
+ return NULL;
+ }
+ }
+
+ return g_steal_pointer(&proxies);
+}
+
+static int
+dbus_load_state_proxy(GDBusProxy *proxy, const uint8_t *data, size_t size)
+{
+ g_autoptr(GError) err = NULL;
+ g_autoptr(GVariant) result = NULL;
+ g_autoptr(GVariant) value = NULL;
+
+ value = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
+ data, size, sizeof(char));
+ result = g_dbus_proxy_call_sync(proxy, "Load",
+ g_variant_new("(@ay)",
+ g_steal_pointer(&value)),
+ G_DBUS_CALL_FLAGS_NO_AUTO_START,
+ -1, NULL, &err);
+ if (!result) {
+ error_report("Failed to Load: %s", err->message);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int dbus_vmstate_post_load(void *opaque, int version_id)
+{
+ DBusVMState *self = DBUS_VMSTATE(opaque);
+ g_autoptr(GInputStream) m = NULL;
+ g_autoptr(GDataInputStream) s = NULL;
+ g_autoptr(GError) err = NULL;
+ g_autoptr(GHashTable) proxies = NULL;
+ uint32_t nelem;
+
+ proxies = dbus_get_proxies(self, &err);
+ if (!proxies) {
+ error_report("Failed to get proxies: %s", err->message);
+ return -1;
+ }
+
+ m = g_memory_input_stream_new_from_data(self->data, self->data_size, NULL);
+ s = g_data_input_stream_new(m);
+ g_data_input_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
+
+ nelem = g_data_input_stream_read_uint32(s, NULL, &err);
+ if (err) {
+ goto error;
+ }
+
+ while (nelem > 0) {
+ GDBusProxy *proxy = NULL;
+ uint32_t len;
+ gsize bytes_read, avail;
+ char id[256];
+
+ len = g_data_input_stream_read_uint32(s, NULL, &err);
+ if (err) {
+ goto error;
+ }
+ if (len >= 256) {
+ error_report("Invalid DBus vmstate proxy name %u", len);
+ return -1;
+ }
+ if (!g_input_stream_read_all(G_INPUT_STREAM(s), id, len,
+ &bytes_read, NULL, &err)) {
+ goto error;
+ }
+ g_return_val_if_fail(bytes_read == len, -1);
+ id[len] = 0;
+
+ proxy = g_hash_table_lookup(proxies, id);
+ if (!proxy) {
+ error_report("Failed to find proxy Id '%s'", id);
+ return -1;
+ }
+
+ len = g_data_input_stream_read_uint32(s, NULL, &err);
+ avail = g_buffered_input_stream_get_available(
+ G_BUFFERED_INPUT_STREAM(s));
+
+ if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
+ error_report("Invalid vmstate size: %u", len);
+ return -1;
+ }
+
+ if (dbus_load_state_proxy(proxy,
+ g_buffered_input_stream_peek_buffer(G_BUFFERED_INPUT_STREAM(s),
+ NULL),
+ len) < 0) {
+ error_report("Failed to restore Id '%s'", id);
+ return -1;
+ }
+
+ if (!g_seekable_seek(G_SEEKABLE(s), len, G_SEEK_CUR, NULL, &err)) {
+ goto error;
+ }
+
+ nelem -= 1;
+ }
+
+ return 0;
+
+error:
+ error_report("Failed to read from stream: %s", err->message);
+ return -1;
+}
+
+static void
+dbus_save_state_proxy(gpointer key,
+ gpointer value,
+ gpointer user_data)
+{
+ GDataOutputStream *s = user_data;
+ const char *id = key;
+ GDBusProxy *proxy = value;
+ g_autoptr(GVariant) result = NULL;
+ g_autoptr(GVariant) child = NULL;
+ g_autoptr(GError) err = NULL;
+ const uint8_t *data;
+ gsize size;
+
+ result = g_dbus_proxy_call_sync(proxy, "Save",
+ NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START,
+ -1, NULL, &err);
+ if (!result) {
+ error_report("Failed to Save: %s", err->message);
+ return;
+ }
+
+ child = g_variant_get_child_value(result, 0);
+ data = g_variant_get_fixed_array(child, &size, sizeof(char));
+ if (!data) {
+ error_report("Failed to Save: not a byte array");
+ return;
+ }
+ if (size > DBUS_VMSTATE_SIZE_LIMIT) {
+ error_report("Too large vmstate data to save: %lu", size);
+ return;
+ }
+
+ if (!g_data_output_stream_put_uint32(s, strlen(id), NULL, &err) ||
+ !g_data_output_stream_put_string(s, id, NULL, &err) ||
+ !g_data_output_stream_put_uint32(s, size, NULL, &err) ||
+ !g_output_stream_write_all(G_OUTPUT_STREAM(s),
+ data, size, NULL, NULL, &err)) {
+ error_report("Failed to write to stream: %s", err->message);
+ }
+}
+
+static int dbus_vmstate_pre_save(void *opaque)
+{
+ DBusVMState *self = DBUS_VMSTATE(opaque);
+ g_autoptr(GOutputStream) m = NULL;
+ g_autoptr(GDataOutputStream) s = NULL;
+ g_autoptr(GHashTable) proxies = NULL;
+ g_autoptr(GError) err = NULL;
+
+ proxies = dbus_get_proxies(self, &err);
+ if (!proxies) {
+ error_report("Failed to get proxies: %s", err->message);
+ return -1;
+ }
+
+ m = g_memory_output_stream_new_resizable();
+ s = g_data_output_stream_new(m);
+ g_data_output_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
+
+ if (!g_data_output_stream_put_uint32(s, g_hash_table_size(proxies),
+ NULL, &err)) {
+ error_report("Failed to write to stream: %s", err->message);
+ return -1;
+ }
+
+ g_hash_table_foreach(proxies, dbus_save_state_proxy, s);
+
+ if (g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m))
+ > UINT32_MAX) {
+ error_report("DBus vmstate buffer is too large");
+ return -1;
+ }
+
+ if (!g_output_stream_close(G_OUTPUT_STREAM(m), NULL, &err)) {
+ error_report("Failed to close stream: %s", err->message);
+ return -1;
+ }
+
+ g_free(self->data);
+ self->data_size =
+ g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m));
+ self->data =
+ g_memory_output_stream_steal_data(G_MEMORY_OUTPUT_STREAM(m));
+
+ return 0;
+}
+
+static const VMStateDescription dbus_vmstate = {
+ .name = TYPE_DBUS_VMSTATE,
+ .version_id = 0,
+ .pre_save = dbus_vmstate_pre_save,
+ .post_load = dbus_vmstate_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(data_size, DBusVMState),
+ VMSTATE_VBUFFER_ALLOC_UINT32(data, DBusVMState, 0, 0, data_size),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void
+dbus_vmstate_complete(UserCreatable *uc, Error **errp)
+{
+ DBusVMState *self = DBUS_VMSTATE(uc);
+ GError *err = NULL;
+ GDBusConnection *bus;
+
+ if (!object_resolve_path_type("", TYPE_DBUS_VMSTATE, NULL)) {
+ error_setg(errp, "There is already an instance of %s",
+ TYPE_DBUS_VMSTATE);
+ return;
+ }
+
+ if (!self->dbus_addr) {
+ error_setg(errp, QERR_MISSING_PARAMETER, "addr");
+ return;
+ }
+
+ bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
+ G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
+ G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
+ NULL, NULL, &err);
+ if (err) {
+ error_setg(errp, "failed to connect to DBus: '%s'", err->message);
+ g_clear_error(&err);
+ }
+
+ self->bus = bus;
+
+ if (vmstate_register(VMSTATE_IF(self), -1, &dbus_vmstate, self) < 0) {
+ error_setg(errp, "Failed to register vmstate");
+ }
+}
+
+static void
+dbus_vmstate_finalize(Object *o)
+{
+ DBusVMState *self = DBUS_VMSTATE(o);
+
+ vmstate_unregister(VMSTATE_IF(self), &dbus_vmstate, self);
+
+ g_clear_object(&self->bus);
+ g_free(self->dbus_addr);
+ g_free(self->id_list);
+ g_free(self->data);
+}
+
+static char *
+get_dbus_addr(Object *o, Error **errp)
+{
+ DBusVMState *self = DBUS_VMSTATE(o);
+
+ return g_strdup(self->dbus_addr);
+}
+
+static void
+set_dbus_addr(Object *o, const char *str, Error **errp)
+{
+ DBusVMState *self = DBUS_VMSTATE(o);
+
+ g_free(self->dbus_addr);
+ self->dbus_addr = g_strdup(str);
+}
+
+static char *
+get_id_list(Object *o, Error **errp)
+{
+ DBusVMState *self = DBUS_VMSTATE(o);
+
+ return g_strdup(self->id_list);
+}
+
+static void
+set_id_list(Object *o, const char *str, Error **errp)
+{
+ DBusVMState *self = DBUS_VMSTATE(o);
+
+ g_free(self->id_list);
+ self->id_list = g_strdup(str);
+}
+
+static char *
+dbus_vmstate_get_id(VMStateIf *vmif)
+{
+ return g_strdup(TYPE_DBUS_VMSTATE);
+}
+
+static void
+dbus_vmstate_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);
+
+ ucc->complete = dbus_vmstate_complete;
+ vc->get_id = dbus_vmstate_get_id;
+
+ object_class_property_add_str(oc, "addr",
+ get_dbus_addr, set_dbus_addr,
+ &error_abort);
+ object_class_property_add_str(oc, "id-list",
+ get_id_list, set_id_list,
+ &error_abort);
+}
+
+static const TypeInfo dbus_vmstate_info = {
+ .name = TYPE_DBUS_VMSTATE,
+ .parent = TYPE_OBJECT,
+ .instance_size = sizeof(DBusVMState),
+ .instance_finalize = dbus_vmstate_finalize,
+ .class_size = sizeof(DBusVMStateClass),
+ .class_init = dbus_vmstate_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { TYPE_VMSTATE_IF },
+ { }
+ }
+};
+
+static void
+register_types(void)
+{
+ type_register_static(&dbus_vmstate_info);
+}
+
+type_init(register_types);
diff --git a/configure b/configure
index 95134c0180..0a6eb2ebcd 100755
--- a/configure
+++ b/configure
@@ -3665,10 +3665,16 @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then
gio=yes
gio_cflags=$($pkg_config --cflags gio-2.0)
gio_libs=$($pkg_config --libs gio-2.0)
+ gdbus_codegen=$($pkg_config --variable=gdbus_codegen gio-2.0)
else
gio=no
fi
+if $pkg_config --atleast-version=$glib_req_ver gio-unix-2.0; then
+ gio_cflags="$gio_cflags $($pkg_config --cflags gio-unix-2.0)"
+ gio_libs="$gio_libs $($pkg_config --libs gio-unix-2.0)"
+fi
+
# Sanity check that the current size_t matches the
# size that glib thinks it should be. This catches
# problems on multi-arch where people try to build
@@ -6831,6 +6837,7 @@ if test "$gio" = "yes" ; then
echo "CONFIG_GIO=y" >> $config_host_mak
echo "GIO_CFLAGS=$gio_cflags" >> $config_host_mak
echo "GIO_LIBS=$gio_libs" >> $config_host_mak
+ echo "GDBUS_CODEGEN=$gdbus_codegen" >> $config_host_mak
fi
echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
if test "$gnutls" = "yes" ; then
diff --git a/docs/interop/dbus-vmstate.rst b/docs/interop/dbus-vmstate.rst
new file mode 100644
index 0000000000..78070be1bd
--- /dev/null
+++ b/docs/interop/dbus-vmstate.rst
@@ -0,0 +1,68 @@
+=============
+D-Bus VMState
+=============
+
+Introduction
+============
+
+The QEMU dbus-vmstate object aim is to migrate helpers data running on
+a QEMU D-Bus bus. (refer to the :doc:`dbus` document for
+recommendation on D-Bus usage)
+
+Upon migration, QEMU will go through the queue of
+``org.qemu.VMState1`` D-Bus name owners and query their ``Id``. It
+must be unique among the helpers.
+
+It will then save arbitrary data of each Id to be transferred in the
+migration stream and restored/loaded at the corresponding destination
+helper.
+
+The data amount to be transferred is limited to 1Mb. The state must be
+saved quickly (a few seconds maximum). (D-Bus imposes a time limit on
+reply anyway, and migration would fail if data isn't given quickly
+enough)
+
+dbus-vmstate object can be configured with the expected list of
+helpers by setting its ``id-list`` property, with a coma-separated
+``Id`` list.
+
+Interface
+=========
+
+On object path ``/org/qemu/VMState1``, the following
+``org.qemu.VMState1`` interface should be implemented:
+
+.. code:: xml
+
+ <interface name="org.qemu.VMState1">
+ <property name="Id" type="s" access="read"/>
+ <method name="Load">
+ <arg type="ay" name="data" direction="in"/>
+ </method>
+ <method name="Save">
+ <arg type="ay" name="data" direction="out"/>
+ </method>
+ </interface>
+
+"Id" property
+-------------
+
+A string that identifies the helper uniquely.
+
+Load(in u8[] bytes) method
+--------------------------
+
+The method called on destination with the state to restore.
+
+The helper may be initially started in a waiting state (with
+an --incoming argument for example), and it may resume on success.
+
+An error may be returned to the caller.
+
+Save(out u8[] bytes) method
+---------------------------
+
+The method called on the source to get the current state to be
+migrated. The helper should continue to run normally.
+
+An error may be returned to the caller.
diff --git a/docs/interop/index.rst b/docs/interop/index.rst
index fa4478ce2e..75832f44ac 100644
--- a/docs/interop/index.rst
+++ b/docs/interop/index.rst
@@ -14,6 +14,7 @@ Contents:
bitmaps
dbus
+ dbus-vmstate
live-block-operations
pr-helper
vhost-user
diff --git a/tests/Makefile.include b/tests/Makefile.include
index d4502a30eb..eb40e8a5e7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -158,7 +158,9 @@ check-qtest-pci-$(CONFIG_RTL8139_PCI) += tests/rtl8139-test$(EXESUF)
check-qtest-pci-$(CONFIG_VGA) += tests/display-vga-test$(EXESUF)
check-qtest-pci-$(CONFIG_HDA) += tests/intel-hda-test$(EXESUF)
check-qtest-pci-$(CONFIG_IVSHMEM_DEVICE) += tests/ivshmem-test$(EXESUF)
-
+ifneq ($(GDBUS_CODEGEN),)
+check-qtest-pci-$(CONFIG_GIO) += tests/dbus-vmstate-test$(EXESUF)
+endif
check-qtest-i386-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF)
check-qtest-i386-y += tests/fdc-test$(EXESUF)
check-qtest-i386-y += tests/ide-test$(EXESUF)
@@ -620,6 +622,19 @@ tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.jso
@mv tests/qapi-schema/doc-good-qapi-doc.texi $@
@rm -f tests/qapi-schema/doc-good-qapi-*.[ch] tests/qapi-schema/doc-good-qmp-*.[ch]
+tests/dbus-vmstate1.h tests/dbus-vmstate1.c: tests/dbus-vmstate1-gen-timestamp ;
+tests/dbus-vmstate1-gen-timestamp: $(SRC_PATH)/tests/dbus-vmstate1.xml
+ $(call quiet-command,$(GDBUS_CODEGEN) $< \
+ --interface-prefix org.qemu --generate-c-code tests/dbus-vmstate1, \
+ "GEN","$(@:%-timestamp=%)")
+ @>$@
+
+tests/dbus-vmstate-test.o-cflags := -DSRCDIR="$(SRC_PATH)"
+tests/dbus-vmstate1.o-cflags := $(GIO_CFLAGS)
+tests/dbus-vmstate1.o-libs := $(GIO_LIBS)
+
+tests/dbus-vmstate-test.o: tests/dbus-vmstate1.h
+
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y)
tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y)
tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/test-qapi-events.o
@@ -822,6 +837,7 @@ tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
tests/test-x86-cpuid-compat$(EXESUF): tests/test-x86-cpuid-compat.o $(qtest-obj-y)
tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+tests/dbus-vmstate-test$(EXESUF): tests/dbus-vmstate-test.o tests/dbus-vmstate1.o $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a
tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
@@ -1176,6 +1192,7 @@ check-clean:
rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
rm -f tests/test-qapi-gen-timestamp
+ rm -f tests/dbus-vmstate1-gen-timestamp
rm -rf $(TESTS_VENV_DIR) $(TESTS_RESULTS_DIR)
clean: check-clean
diff --git a/tests/dbus-vmstate-daemon.sh b/tests/dbus-vmstate-daemon.sh
new file mode 100755
index 0000000000..c4394ac918
--- /dev/null
+++ b/tests/dbus-vmstate-daemon.sh
@@ -0,0 +1,95 @@
+#!/bin/sh
+
+# dbus-daemon wrapper script for dbus-vmstate testing
+#
+# This script allows to tweak the dbus-daemon policy during the test
+# to test different configurations.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses/>.
+#
+# Copyright (C) 2019 Red Hat, Inc.
+
+write_config()
+{
+ CONF="$1"
+ cat > "$CONF" <<EOF
+<busconfig>
+ <type>session</type>
+ <listen>unix:tmpdir=/tmp</listen>
+
+ <policy context="default">
+ <!-- Holes must be punched in service configuration files for
+ name ownership and sending method calls -->
+ <deny own="*"/>
+ <deny send_type="method_call"/>
+
+ <!-- Signals and reply messages (method returns, errors) are allowed
+ by default -->
+ <allow send_type="signal"/>
+ <allow send_requested_reply="true" send_type="method_return"/>
+ <allow send_requested_reply="true" send_type="error"/>
+
+ <!-- All messages may be received by default -->
+ <allow receive_type="method_call"/>
+ <allow receive_type="method_return"/>
+ <allow receive_type="error"/>
+ <allow receive_type="signal"/>
+
+ <!-- Allow anyone to talk to the message bus -->
+ <allow send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.DBus" />
+ <allow send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.DBus.Introspectable"/>
+ <allow send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.DBus.Properties"/>
+ <!-- But disallow some specific bus services -->
+ <deny send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.DBus"
+ send_member="UpdateActivationEnvironment"/>
+ <deny send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.DBus.Debug.Stats"/>
+ <deny send_destination="org.freedesktop.DBus"
+ send_interface="org.freedesktop.systemd1.Activator"/>
+
+ <allow own="org.qemu.VMState1"/>
+ <allow send_destination="org.qemu.VMState1"/>
+ <allow receive_sender="org.qemu.VMState1"/>
+
+ </policy>
+
+ <include if_selinux_enabled="yes"
+ selinux_root_relative="yes">contexts/dbus_contexts</include>
+
+</busconfig>
+EOF
+}
+
+ARGS=
+for arg in "$@"
+do
+ case $arg in
+ --config-file=*)
+ CONF="${arg#*=}"
+ write_config "$CONF"
+ ARGS="$ARGS $1"
+ shift
+ ;;
+ *)
+ ARGS="$ARGS $1"
+ shift
+ ;;
+ esac
+done
+
+exec dbus-daemon $ARGS
diff --git a/tests/dbus-vmstate-test.c b/tests/dbus-vmstate-test.c
new file mode 100644
index 0000000000..bcf2372e15
--- /dev/null
+++ b/tests/dbus-vmstate-test.c
@@ -0,0 +1,386 @@
+#include "qemu/osdep.h"
+#include <glib/gstdio.h>
+#include <gio/gio.h>
+#include "libqtest.h"
+#include "qemu-common.h"
+#include "dbus-vmstate1.h"
+
+static char *workdir;
+
+typedef struct TestServerId {
+ const char *name;
+ const char *data;
+ size_t size;
+} TestServerId;
+
+static const TestServerId idA = {
+ "idA", "I'am\0idA!", sizeof("I'am\0idA!")
+};
+
+static const TestServerId idB = {
+ "idB", "I'am\0idB!", sizeof("I'am\0idB!")
+};
+
+typedef struct TestServer {
+ const TestServerId *id;
+ bool save_called;
+ bool load_called;
+} TestServer;
+
+typedef struct Test {
+ const char *id_list;
+ bool migrate_fail;
+ bool without_dst_b;
+ TestServer srcA;
+ TestServer dstA;
+ TestServer srcB;
+ TestServer dstB;
+ GMainLoop *loop;
+ QTestState *src_qemu;
+} Test;
+
+static gboolean
+vmstate_load(VMState1 *object, GDBusMethodInvocation *invocation,
+ const gchar *arg_data, gpointer user_data)
+{
+ TestServer *h = user_data;
+ g_autoptr(GVariant) var = NULL;
+ GVariant *args;
+ const uint8_t *data;
+ size_t size;
+
+ args = g_dbus_method_invocation_get_parameters(invocation);
+ var = g_variant_get_child_value(args, 0);
+ data = g_variant_get_fixed_array(var, &size, sizeof(char));
+ g_assert_cmpuint(size, ==, h->id->size);
+ g_assert(!memcmp(data, h->id->data, h->id->size));
+ h->load_called = true;
+
+ g_dbus_method_invocation_return_value(invocation, g_variant_new("()"));
+ return TRUE;
+}
+
+static gboolean
+vmstate_save(VMState1 *object, GDBusMethodInvocation *invocation,
+ gpointer user_data)
+{
+ TestServer *h = user_data;
+ GVariant *var;
+
+ var = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
+ h->id->data, h->id->size, sizeof(char));
+ g_dbus_method_invocation_return_value(invocation,
+ g_variant_new("(@ay)", var));
+ h->save_called = true;
+
+ return TRUE;
+}
+
+static gboolean
+wait_for_migration_complete(gpointer user_data)
+{
+ Test *test = user_data;
+ QDict *rsp_return;
+ bool stop = false;
+ const char *status;
+
+ qtest_qmp_send(test->src_qemu, "{ 'execute': 'query-migrate' }");
+ rsp_return = qtest_qmp_receive_success(test->src_qemu, NULL, NULL);
+ status = qdict_get_str(rsp_return, "status");
+ if (g_str_equal(status, "completed") || g_str_equal(status, "failed")) {
+ stop = true;
+ g_assert_cmpstr(status, ==,
+ test->migrate_fail ? "failed" : "completed");
+ }
+ qobject_unref(rsp_return);
+
+ if (stop) {
+ g_main_loop_quit(test->loop);
+ }
+ return stop ? G_SOURCE_REMOVE : G_SOURCE_CONTINUE;
+}
+
+static void migrate(QTestState *who, const char *uri)
+{
+ QDict *args, *rsp;
+
+ args = qdict_new();
+ qdict_put_str(args, "uri", uri);
+
+ rsp = qtest_qmp(who, "{ 'execute': 'migrate', 'arguments': %p }", args);
+
+ g_assert(qdict_haskey(rsp, "return"));
+ qobject_unref(rsp);
+}
+
+typedef struct WaitNamed {
+ GMainLoop *loop;
+ bool named;
+} WaitNamed;
+
+static void
+named_cb(GDBusConnection *connection,
+ const gchar *name,
+ gpointer user_data)
+{
+ WaitNamed *t = user_data;
+
+ t->named = true;
+ g_main_loop_quit(t->loop);
+}
+
+static GDBusConnection *
+get_connection(Test *test, guint *ownid)
+{
+ g_autofree gchar *addr = NULL;
+ WaitNamed *wait;
+ GError *err = NULL;
+ GDBusConnection *c;
+
+ wait = g_new0(WaitNamed, 1);
+ wait->loop = test->loop;
+ addr = g_dbus_address_get_for_bus_sync(G_BUS_TYPE_SESSION, NULL, &err);
+ g_assert_no_error(err);
+
+ c = g_dbus_connection_new_for_address_sync(
+ addr,
+ G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION |
+ G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT,
+ NULL, NULL, &err);
+ g_assert_no_error(err);
+ *ownid = g_bus_own_name_on_connection(c, "org.qemu.VMState1",
+ G_BUS_NAME_OWNER_FLAGS_NONE,
+ named_cb, named_cb, wait, g_free);
+ if (!wait->named) {
+ g_main_loop_run(wait->loop);
+ }
+
+ return c;
+}
+
+static GDBusObjectManagerServer *
+get_server(GDBusConnection *conn, TestServer *s, const TestServerId *id)
+{
+ g_autoptr(GDBusObjectSkeleton) sk = NULL;
+ g_autoptr(VMState1Skeleton) v = NULL;
+ GDBusObjectManagerServer *os;
+
+ s->id = id;
+ os = g_dbus_object_manager_server_new("/org/qemu");
+ sk = g_dbus_object_skeleton_new("/org/qemu/VMState1");
+
+ v = VMSTATE1_SKELETON(vmstate1_skeleton_new());
+ g_object_set(v, "id", id->name, NULL);
+
+ g_signal_connect(v, "handle-load", G_CALLBACK(vmstate_load), s);
+ g_signal_connect(v, "handle-save", G_CALLBACK(vmstate_save), s);
+
+ g_dbus_object_skeleton_add_interface(sk, G_DBUS_INTERFACE_SKELETON(v));
+ g_dbus_object_manager_server_export(os, sk);
+ g_dbus_object_manager_server_set_connection(os, conn);
+
+ return os;
+}
+
+static void
+set_id_list(Test *test, QTestState *s)
+{
+ if (!test->id_list) {
+ return;
+ }
+
+ g_assert(!qmp_rsp_is_err(qtest_qmp(s,
+ "{ 'execute': 'qom-set', 'arguments': "
+ "{ 'path': '/objects/dv', 'property': 'id-list', 'value': %s } }",
+ test->id_list)));
+}
+static void
+test_dbus_vmstate(Test *test)
+{
+ g_autofree char *src_qemu_args = NULL;
+ g_autofree char *dst_qemu_args = NULL;
+ g_autoptr(GTestDBus) srcbus = NULL;
+ g_autoptr(GTestDBus) dstbus = NULL;
+ g_autoptr(GDBusConnection) srcconnA = NULL;
+ g_autoptr(GDBusConnection) srcconnB = NULL;
+ g_autoptr(GDBusConnection) dstconnA = NULL;
+ g_autoptr(GDBusConnection) dstconnB = NULL;
+ g_autoptr(GDBusObjectManagerServer) srcserverA = NULL;
+ g_autoptr(GDBusObjectManagerServer) srcserverB = NULL;
+ g_autoptr(GDBusObjectManagerServer) dstserverA = NULL;
+ g_autoptr(GDBusObjectManagerServer) dstserverB = NULL;
+ g_auto(GStrv) srcaddr = NULL;
+ g_auto(GStrv) dstaddr = NULL;
+ g_autofree char *uri = NULL;
+ QTestState *src_qemu = NULL, *dst_qemu = NULL;
+ guint ownsrcA, ownsrcB, owndstA, owndstB;
+
+ uri = g_strdup_printf("unix:%s/migsocket", workdir);
+
+ test->loop = g_main_loop_new(NULL, TRUE);
+
+ srcbus = g_test_dbus_new(G_TEST_DBUS_NONE);
+ g_test_dbus_up(srcbus);
+ srcconnA = get_connection(test, &ownsrcA);
+ srcserverA = get_server(srcconnA, &test->srcA, &idA);
+ srcconnB = get_connection(test, &ownsrcB);
+ srcserverB = get_server(srcconnB, &test->srcB, &idB);
+
+ /* remove ,guid=foo part */
+ srcaddr = g_strsplit(g_test_dbus_get_bus_address(srcbus), ",", 2);
+ src_qemu_args =
+ g_strdup_printf("-object dbus-vmstate,id=dv,addr=%s", srcaddr[0]);
+
+ dstbus = g_test_dbus_new(G_TEST_DBUS_NONE);
+ g_test_dbus_up(dstbus);
+ dstconnA = get_connection(test, &owndstA);
+ dstserverA = get_server(dstconnA, &test->dstA, &idA);
+ if (!test->without_dst_b) {
+ dstconnB = get_connection(test, &owndstB);
+ dstserverB = get_server(dstconnB, &test->dstB, &idB);
+ }
+
+ dstaddr = g_strsplit(g_test_dbus_get_bus_address(dstbus), ",", 2);
+ dst_qemu_args =
+ g_strdup_printf("-object dbus-vmstate,id=dv,addr=%s -incoming %s",
+ dstaddr[0], uri);
+
+ src_qemu = qtest_init(src_qemu_args);
+ dst_qemu = qtest_init(dst_qemu_args);
+ set_id_list(test, src_qemu);
+ set_id_list(test, dst_qemu);
+
+ migrate(src_qemu, uri);
+ test->src_qemu = src_qemu;
+ g_timeout_add_seconds(1, wait_for_migration_complete, test);
+
+ g_main_loop_run(test->loop);
+ g_main_loop_unref(test->loop);
+
+ if (test->migrate_fail) {
+ qtest_expect_exit_status(dst_qemu, 1);
+ }
+ qtest_quit(dst_qemu);
+ qtest_quit(src_qemu);
+ g_bus_unown_name(ownsrcA);
+ g_bus_unown_name(ownsrcB);
+ g_bus_unown_name(owndstA);
+ if (!test->without_dst_b) {
+ g_bus_unown_name(owndstB);
+ }
+}
+
+static void
+check_not_migrated(TestServer *s, TestServer *d)
+{
+ assert(!s->save_called);
+ assert(!s->load_called);
+ assert(!d->save_called);
+ assert(!d->load_called);
+}
+
+static void
+check_migrated(TestServer *s, TestServer *d)
+{
+ assert(s->save_called);
+ assert(!s->load_called);
+ assert(!d->save_called);
+ assert(d->load_called);
+}
+
+static void
+test_dbus_vmstate_without_list(void)
+{
+ Test test = { 0, };
+
+ test_dbus_vmstate(&test);
+
+ check_migrated(&test.srcA, &test.dstA);
+ check_migrated(&test.srcB, &test.dstB);
+}
+
+static void
+test_dbus_vmstate_with_list(void)
+{
+ Test test = { .id_list = "idA,idB" };
+
+ test_dbus_vmstate(&test);
+
+ check_migrated(&test.srcA, &test.dstA);
+ check_migrated(&test.srcB, &test.dstB);
+}
+
+static void
+test_dbus_vmstate_only_a(void)
+{
+ Test test = { .id_list = "idA" };
+
+ test_dbus_vmstate(&test);
+
+ check_migrated(&test.srcA, &test.dstA);
+ check_not_migrated(&test.srcB, &test.dstB);
+}
+
+static void
+test_dbus_vmstate_missing_src(void)
+{
+ Test test = { .id_list = "idA,idC", .migrate_fail = true };
+
+ test_dbus_vmstate(&test);
+ check_not_migrated(&test.srcA, &test.dstA);
+ check_not_migrated(&test.srcB, &test.dstB);
+}
+
+static void
+test_dbus_vmstate_missing_dst(void)
+{
+ Test test = { .id_list = "idA,idB",
+ .without_dst_b = true,
+ .migrate_fail = true };
+
+ test_dbus_vmstate(&test);
+
+ assert(test.srcA.save_called);
+ assert(test.srcB.save_called);
+ assert(!test.dstB.save_called);
+}
+
+int
+main(int argc, char **argv)
+{
+ GError *err = NULL;
+ g_autofree char *dbus_daemon = NULL;
+ int ret;
+
+ dbus_daemon = g_build_filename(G_STRINGIFY(SRCDIR),
+ "tests",
+ "dbus-vmstate-daemon.sh",
+ NULL);
+ g_setenv("G_TEST_DBUS_DAEMON", dbus_daemon, true);
+
+ g_test_init(&argc, &argv, NULL);
+
+ workdir = g_dir_make_tmp("dbus-vmstate-test-XXXXXX", &err);
+ if (!workdir) {
+ g_error("Unable to create temporary dir: %s\n", err->message);
+ exit(1);
+ }
+
+ qtest_add_func("/dbus-vmstate/without-list",
+ test_dbus_vmstate_without_list);
+ qtest_add_func("/dbus-vmstate/with-list",
+ test_dbus_vmstate_with_list);
+ qtest_add_func("/dbus-vmstate/only-a",
+ test_dbus_vmstate_only_a);
+ qtest_add_func("/dbus-vmstate/missing-src",
+ test_dbus_vmstate_missing_src);
+ qtest_add_func("/dbus-vmstate/missing-dst",
+ test_dbus_vmstate_missing_dst);
+
+ ret = g_test_run();
+
+ rmdir(workdir);
+ g_free(workdir);
+
+ return ret;
+}
diff --git a/tests/dbus-vmstate1.xml b/tests/dbus-vmstate1.xml
new file mode 100644
index 0000000000..cc8563be4c
--- /dev/null
+++ b/tests/dbus-vmstate1.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
+ <interface name="org.qemu.VMState1">
+ <property name="Id" type="s" access="read"/>
+ <method name="Load">
+ <arg type="ay" name="data" direction="in"/>
+ </method>
+ <method name="Save">
+ <arg type="ay" name="data" direction="out"/>
+ </method>
+ </interface>
+</node>
--
2.23.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object Marc-André Lureau
@ 2019-09-12 14:29 ` Eric Blake
2019-09-16 10:43 ` Dr. David Alan Gilbert
2019-09-17 13:21 ` Daniel P. Berrangé
2 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2019-09-12 14:29 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel
Cc: mprivozn, pbonzini, berrange, dgilbert, quintela
[-- Attachment #1.1: Type: text/plain, Size: 1595 bytes --]
On 9/12/19 7:25 AM, Marc-André Lureau wrote:
> When instanciated, this object will connect to the given D-Bus
instantiated
> bus. During migration, it will take the data from org.qemu.VMState1
> instances.
>
> See documentation for further details.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> +++ b/docs/interop/dbus-vmstate.rst
> @@ -0,0 +1,68 @@
> +=============
> +D-Bus VMState
> +=============
> +
> +Introduction
> +============
> +
> +The QEMU dbus-vmstate object aim is to migrate helpers data running on
> +a QEMU D-Bus bus. (refer to the :doc:`dbus` document for
> +recommendation on D-Bus usage)
object's aim is to migrate helpers' data
> +
> +Upon migration, QEMU will go through the queue of
> +``org.qemu.VMState1`` D-Bus name owners and query their ``Id``. It
> +must be unique among the helpers.
> +
> +It will then save arbitrary data of each Id to be transferred in the
> +migration stream and restored/loaded at the corresponding destination
> +helper.
> +
> +The data amount to be transferred is limited to 1Mb. The state must be
> +saved quickly (a few seconds maximum). (D-Bus imposes a time limit on
> +reply anyway, and migration would fail if data isn't given quickly
> +enough)
s/enough)/enough.)/
> +
> +dbus-vmstate object can be configured with the expected list of
> +helpers by setting its ``id-list`` property, with a coma-separated
comma
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object Marc-André Lureau
2019-09-12 14:29 ` Eric Blake
@ 2019-09-16 10:43 ` Dr. David Alan Gilbert
2019-09-17 13:21 ` Daniel P. Berrangé
2 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-16 10:43 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, berrange, qemu-devel, quintela
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> When instanciated, this object will connect to the given D-Bus
> bus. During migration, it will take the data from org.qemu.VMState1
> instances.
>
> See documentation for further details.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
I could do with this being in smaller chunks; it's a bit of a big
uncommented patch.
Dave
> ---
> MAINTAINERS | 6 +
> backends/Makefile.objs | 4 +
> backends/dbus-vmstate.c | 530 ++++++++++++++++++++++++++++++++++
> configure | 7 +
> docs/interop/dbus-vmstate.rst | 68 +++++
> docs/interop/index.rst | 1 +
> tests/Makefile.include | 19 +-
> tests/dbus-vmstate-daemon.sh | 95 ++++++
> tests/dbus-vmstate-test.c | 386 +++++++++++++++++++++++++
> tests/dbus-vmstate1.xml | 12 +
> 10 files changed, 1127 insertions(+), 1 deletion(-)
> create mode 100644 backends/dbus-vmstate.c
> create mode 100644 docs/interop/dbus-vmstate.rst
> create mode 100755 tests/dbus-vmstate-daemon.sh
> create mode 100644 tests/dbus-vmstate-test.c
> create mode 100644 tests/dbus-vmstate1.xml
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50eaf005f4..f641870c40 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2153,6 +2153,12 @@ F: tests/migration-test.c
> F: docs/devel/migration.rst
> F: qapi/migration.json
>
> +DBus VMState
> +M: Marc-André Lureau <marcandre.lureau@redhat.com>
> +S: Maintained
> +F: backends/dbus-vmstate.c
> +F: tests/dbus-vmstate*
> +
> Seccomp
> M: Eduardo Otubo <otubo@redhat.com>
> S: Supported
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index f0691116e8..28a847cd57 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -17,3 +17,7 @@ endif
> common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_VIRTIO)) += vhost-user.o
>
> common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
> +
> +common-obj-$(CONFIG_GIO) += dbus-vmstate.o
> +dbus-vmstate.o-cflags = $(GIO_CFLAGS)
> +dbus-vmstate.o-libs = $(GIO_LIBS)
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> new file mode 100644
> index 0000000000..559f5430c5
> --- /dev/null
> +++ b/backends/dbus-vmstate.c
> @@ -0,0 +1,530 @@
> +/*
> + * QEMU dbus-vmstate
> + *
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + * Marc-André Lureau <marcandre.lureau@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qom/object_interfaces.h"
> +#include "qapi/qmp/qerror.h"
> +#include "migration/vmstate.h"
> +#include <gio/gio.h>
> +
> +typedef struct DBusVMState DBusVMState;
> +typedef struct DBusVMStateClass DBusVMStateClass;
> +
> +#define TYPE_DBUS_VMSTATE "dbus-vmstate"
> +#define DBUS_VMSTATE(obj) \
> + OBJECT_CHECK(DBusVMState, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(DBusVMStateClass, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(DBusVMStateClass, (klass), TYPE_DBUS_VMSTATE)
> +
> +struct DBusVMStateClass {
> + ObjectClass parent_class;
> +};
> +
> +struct DBusVMState {
> + Object parent;
> +
> + GDBusConnection *bus;
> + char *dbus_addr;
> + char *id_list;
> +
> + uint32_t data_size;
> + uint8_t *data;
> +};
> +
> +static const GDBusPropertyInfo vmstate_property_info[] = {
> + { -1, (char *) "Id", (char *) "s",
> + G_DBUS_PROPERTY_INFO_FLAGS_READABLE, NULL },
> +};
> +
> +static const GDBusPropertyInfo * const vmstate_property_info_pointers[] = {
> + &vmstate_property_info[0],
> + NULL
> +};
> +
> +static const GDBusInterfaceInfo vmstate1_interface_info = {
> + -1,
> + (char *) "org.qemu.VMState1",
> + (GDBusMethodInfo **) NULL,
> + (GDBusSignalInfo **) NULL,
> + (GDBusPropertyInfo **) &vmstate_property_info_pointers,
> + NULL,
> +};
> +
> +#define DBUS_VMSTATE_SIZE_LIMIT (1 * MiB)
> +
> +static char **
> +dbus_get_vmstate1_names(DBusVMState *self, GError **err)
> +{
> + g_autoptr(GDBusProxy) proxy = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) child = NULL;
> +
> + proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE, NULL,
> + "org.freedesktop.DBus",
> + "/org/freedesktop/DBus",
> + "org.freedesktop.DBus",
> + NULL, err);
> + if (!proxy) {
> + return NULL;
> + }
> +
> + result = g_dbus_proxy_call_sync(proxy, "ListQueuedOwners",
> + g_variant_new("(s)", "org.qemu.VMState1"),
> + G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, err);
> + if (!result) {
> + return NULL;
> + }
> +
> + child = g_variant_get_child_value(result, 0);
> + return g_variant_dup_strv(child, NULL);
> +}
> +
> +static GHashTable *
> +get_id_list_set(DBusVMState *self)
> +{
> + g_auto(GStrv) ids = NULL;
> + g_autoptr(GHashTable) set = NULL;
> + int i;
> +
> + if (!self->id_list) {
> + return NULL;
> + }
> +
> + ids = g_strsplit(self->id_list, ",", -1);
> + set = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
> + for (i = 0; ids[i]; i++) {
> + g_hash_table_add(set, ids[i]);
> + ids[i] = NULL;
> + }
> +
> + return g_steal_pointer(&set);
> +}
> +
> +static GHashTable *
> +dbus_get_proxies(DBusVMState *self, GError **err)
> +{
> + g_autoptr(GError) local_err = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + g_autoptr(GHashTable) ids = NULL;
> + g_auto(GStrv) names = NULL;
> + size_t i;
> +
> + ids = get_id_list_set(self);
> + proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
> + g_free, g_object_unref);
> +
> + names = dbus_get_vmstate1_names(self, &local_err);
> + if (!names) {
> + if (g_error_matches(local_err,
> + G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER)) {
> + return proxies;
> + }
> + g_propagate_error(err, local_err);
> + return NULL;
> + }
> +
> + for (i = 0; names[i]; i++) {
> + g_autoptr(GDBusProxy) proxy = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autofree char *id = NULL;
> + size_t size;
> +
> + proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE,
> + (GDBusInterfaceInfo *) &vmstate1_interface_info,
> + names[i],
> + "/org/qemu/VMState1",
> + "org.qemu.VMState1",
> + NULL, err);
> + if (!proxy) {
> + return NULL;
> + }
> +
> + result = g_dbus_proxy_get_cached_property(proxy, "Id");
> + if (!result) {
> + g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "VMState Id property is missing.");
> + return NULL;
> + }
> +
> + id = g_variant_dup_string(result, &size);
> + if (ids && !g_hash_table_remove(ids, id)) {
> + g_clear_pointer(&id, g_free);
> + g_clear_object(&proxy);
> + continue;
> + }
> + if (size == 0 || size >= 256) {
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "VMState Id '%s' is invalid.", id);
> + return NULL;
> + }
> +
> + if (!g_hash_table_insert(proxies, id, proxy)) {
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "Duplicated VMState Id '%s'", id);
> + return NULL;
> + }
> + id = NULL;
> + proxy = NULL;
> +
> + g_clear_pointer(&result, g_variant_unref);
> + }
> +
> + if (ids) {
> + g_autofree char **left = NULL;
> +
> + left = (char **)g_hash_table_get_keys_as_array(ids, NULL);
> + if (*left) {
> + g_autofree char *leftids = g_strjoinv(",", left);
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "Required VMState Id are missing: %s", leftids);
> + return NULL;
> + }
> + }
> +
> + return g_steal_pointer(&proxies);
> +}
> +
> +static int
> +dbus_load_state_proxy(GDBusProxy *proxy, const uint8_t *data, size_t size)
> +{
> + g_autoptr(GError) err = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) value = NULL;
> +
> + value = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
> + data, size, sizeof(char));
> + result = g_dbus_proxy_call_sync(proxy, "Load",
> + g_variant_new("(@ay)",
> + g_steal_pointer(&value)),
> + G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, &err);
> + if (!result) {
> + error_report("Failed to Load: %s", err->message);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int dbus_vmstate_post_load(void *opaque, int version_id)
> +{
> + DBusVMState *self = DBUS_VMSTATE(opaque);
> + g_autoptr(GInputStream) m = NULL;
> + g_autoptr(GDataInputStream) s = NULL;
> + g_autoptr(GError) err = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + uint32_t nelem;
> +
> + proxies = dbus_get_proxies(self, &err);
> + if (!proxies) {
> + error_report("Failed to get proxies: %s", err->message);
> + return -1;
> + }
> +
> + m = g_memory_input_stream_new_from_data(self->data, self->data_size, NULL);
> + s = g_data_input_stream_new(m);
> + g_data_input_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
> +
> + nelem = g_data_input_stream_read_uint32(s, NULL, &err);
> + if (err) {
> + goto error;
> + }
> +
> + while (nelem > 0) {
> + GDBusProxy *proxy = NULL;
> + uint32_t len;
> + gsize bytes_read, avail;
> + char id[256];
> +
> + len = g_data_input_stream_read_uint32(s, NULL, &err);
> + if (err) {
> + goto error;
> + }
> + if (len >= 256) {
> + error_report("Invalid DBus vmstate proxy name %u", len);
> + return -1;
> + }
> + if (!g_input_stream_read_all(G_INPUT_STREAM(s), id, len,
> + &bytes_read, NULL, &err)) {
> + goto error;
> + }
> + g_return_val_if_fail(bytes_read == len, -1);
> + id[len] = 0;
> +
> + proxy = g_hash_table_lookup(proxies, id);
> + if (!proxy) {
> + error_report("Failed to find proxy Id '%s'", id);
> + return -1;
> + }
> +
> + len = g_data_input_stream_read_uint32(s, NULL, &err);
> + avail = g_buffered_input_stream_get_available(
> + G_BUFFERED_INPUT_STREAM(s));
> +
> + if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
> + error_report("Invalid vmstate size: %u", len);
> + return -1;
> + }
> +
> + if (dbus_load_state_proxy(proxy,
> + g_buffered_input_stream_peek_buffer(G_BUFFERED_INPUT_STREAM(s),
> + NULL),
> + len) < 0) {
> + error_report("Failed to restore Id '%s'", id);
> + return -1;
> + }
> +
> + if (!g_seekable_seek(G_SEEKABLE(s), len, G_SEEK_CUR, NULL, &err)) {
> + goto error;
> + }
> +
> + nelem -= 1;
> + }
> +
> + return 0;
> +
> +error:
> + error_report("Failed to read from stream: %s", err->message);
> + return -1;
> +}
> +
> +static void
> +dbus_save_state_proxy(gpointer key,
> + gpointer value,
> + gpointer user_data)
> +{
> + GDataOutputStream *s = user_data;
> + const char *id = key;
> + GDBusProxy *proxy = value;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) child = NULL;
> + g_autoptr(GError) err = NULL;
> + const uint8_t *data;
> + gsize size;
> +
> + result = g_dbus_proxy_call_sync(proxy, "Save",
> + NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, &err);
> + if (!result) {
> + error_report("Failed to Save: %s", err->message);
> + return;
> + }
> +
> + child = g_variant_get_child_value(result, 0);
> + data = g_variant_get_fixed_array(child, &size, sizeof(char));
> + if (!data) {
> + error_report("Failed to Save: not a byte array");
> + return;
> + }
> + if (size > DBUS_VMSTATE_SIZE_LIMIT) {
> + error_report("Too large vmstate data to save: %lu", size);
> + return;
> + }
> +
> + if (!g_data_output_stream_put_uint32(s, strlen(id), NULL, &err) ||
> + !g_data_output_stream_put_string(s, id, NULL, &err) ||
> + !g_data_output_stream_put_uint32(s, size, NULL, &err) ||
> + !g_output_stream_write_all(G_OUTPUT_STREAM(s),
> + data, size, NULL, NULL, &err)) {
> + error_report("Failed to write to stream: %s", err->message);
> + }
> +}
> +
> +static int dbus_vmstate_pre_save(void *opaque)
> +{
> + DBusVMState *self = DBUS_VMSTATE(opaque);
> + g_autoptr(GOutputStream) m = NULL;
> + g_autoptr(GDataOutputStream) s = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + g_autoptr(GError) err = NULL;
> +
> + proxies = dbus_get_proxies(self, &err);
> + if (!proxies) {
> + error_report("Failed to get proxies: %s", err->message);
> + return -1;
> + }
> +
> + m = g_memory_output_stream_new_resizable();
> + s = g_data_output_stream_new(m);
> + g_data_output_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
> +
> + if (!g_data_output_stream_put_uint32(s, g_hash_table_size(proxies),
> + NULL, &err)) {
> + error_report("Failed to write to stream: %s", err->message);
> + return -1;
> + }
> +
> + g_hash_table_foreach(proxies, dbus_save_state_proxy, s);
> +
> + if (g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m))
> + > UINT32_MAX) {
> + error_report("DBus vmstate buffer is too large");
> + return -1;
> + }
> +
> + if (!g_output_stream_close(G_OUTPUT_STREAM(m), NULL, &err)) {
> + error_report("Failed to close stream: %s", err->message);
> + return -1;
> + }
> +
> + g_free(self->data);
> + self->data_size =
> + g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m));
> + self->data =
> + g_memory_output_stream_steal_data(G_MEMORY_OUTPUT_STREAM(m));
> +
> + return 0;
> +}
> +
> +static const VMStateDescription dbus_vmstate = {
> + .name = TYPE_DBUS_VMSTATE,
> + .version_id = 0,
> + .pre_save = dbus_vmstate_pre_save,
> + .post_load = dbus_vmstate_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(data_size, DBusVMState),
> + VMSTATE_VBUFFER_ALLOC_UINT32(data, DBusVMState, 0, 0, data_size),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void
> +dbus_vmstate_complete(UserCreatable *uc, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(uc);
> + GError *err = NULL;
> + GDBusConnection *bus;
> +
> + if (!object_resolve_path_type("", TYPE_DBUS_VMSTATE, NULL)) {
> + error_setg(errp, "There is already an instance of %s",
> + TYPE_DBUS_VMSTATE);
> + return;
> + }
> +
> + if (!self->dbus_addr) {
> + error_setg(errp, QERR_MISSING_PARAMETER, "addr");
> + return;
> + }
> +
> + bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
> + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
> + G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
> + NULL, NULL, &err);
> + if (err) {
> + error_setg(errp, "failed to connect to DBus: '%s'", err->message);
> + g_clear_error(&err);
> + }
> +
> + self->bus = bus;
> +
> + if (vmstate_register(VMSTATE_IF(self), -1, &dbus_vmstate, self) < 0) {
> + error_setg(errp, "Failed to register vmstate");
> + }
> +}
> +
> +static void
> +dbus_vmstate_finalize(Object *o)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + vmstate_unregister(VMSTATE_IF(self), &dbus_vmstate, self);
> +
> + g_clear_object(&self->bus);
> + g_free(self->dbus_addr);
> + g_free(self->id_list);
> + g_free(self->data);
> +}
> +
> +static char *
> +get_dbus_addr(Object *o, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + return g_strdup(self->dbus_addr);
> +}
> +
> +static void
> +set_dbus_addr(Object *o, const char *str, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + g_free(self->dbus_addr);
> + self->dbus_addr = g_strdup(str);
> +}
> +
> +static char *
> +get_id_list(Object *o, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + return g_strdup(self->id_list);
> +}
> +
> +static void
> +set_id_list(Object *o, const char *str, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + g_free(self->id_list);
> + self->id_list = g_strdup(str);
> +}
> +
> +static char *
> +dbus_vmstate_get_id(VMStateIf *vmif)
> +{
> + return g_strdup(TYPE_DBUS_VMSTATE);
> +}
> +
> +static void
> +dbus_vmstate_class_init(ObjectClass *oc, void *data)
> +{
> + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> + VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);
> +
> + ucc->complete = dbus_vmstate_complete;
> + vc->get_id = dbus_vmstate_get_id;
> +
> + object_class_property_add_str(oc, "addr",
> + get_dbus_addr, set_dbus_addr,
> + &error_abort);
> + object_class_property_add_str(oc, "id-list",
> + get_id_list, set_id_list,
> + &error_abort);
> +}
> +
> +static const TypeInfo dbus_vmstate_info = {
> + .name = TYPE_DBUS_VMSTATE,
> + .parent = TYPE_OBJECT,
> + .instance_size = sizeof(DBusVMState),
> + .instance_finalize = dbus_vmstate_finalize,
> + .class_size = sizeof(DBusVMStateClass),
> + .class_init = dbus_vmstate_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_USER_CREATABLE },
> + { TYPE_VMSTATE_IF },
> + { }
> + }
> +};
> +
> +static void
> +register_types(void)
> +{
> + type_register_static(&dbus_vmstate_info);
> +}
> +
> +type_init(register_types);
> diff --git a/configure b/configure
> index 95134c0180..0a6eb2ebcd 100755
> --- a/configure
> +++ b/configure
> @@ -3665,10 +3665,16 @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then
> gio=yes
> gio_cflags=$($pkg_config --cflags gio-2.0)
> gio_libs=$($pkg_config --libs gio-2.0)
> + gdbus_codegen=$($pkg_config --variable=gdbus_codegen gio-2.0)
> else
> gio=no
> fi
>
> +if $pkg_config --atleast-version=$glib_req_ver gio-unix-2.0; then
> + gio_cflags="$gio_cflags $($pkg_config --cflags gio-unix-2.0)"
> + gio_libs="$gio_libs $($pkg_config --libs gio-unix-2.0)"
> +fi
> +
> # Sanity check that the current size_t matches the
> # size that glib thinks it should be. This catches
> # problems on multi-arch where people try to build
> @@ -6831,6 +6837,7 @@ if test "$gio" = "yes" ; then
> echo "CONFIG_GIO=y" >> $config_host_mak
> echo "GIO_CFLAGS=$gio_cflags" >> $config_host_mak
> echo "GIO_LIBS=$gio_libs" >> $config_host_mak
> + echo "GDBUS_CODEGEN=$gdbus_codegen" >> $config_host_mak
> fi
> echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
> if test "$gnutls" = "yes" ; then
> diff --git a/docs/interop/dbus-vmstate.rst b/docs/interop/dbus-vmstate.rst
> new file mode 100644
> index 0000000000..78070be1bd
> --- /dev/null
> +++ b/docs/interop/dbus-vmstate.rst
> @@ -0,0 +1,68 @@
> +=============
> +D-Bus VMState
> +=============
> +
> +Introduction
> +============
> +
> +The QEMU dbus-vmstate object aim is to migrate helpers data running on
> +a QEMU D-Bus bus. (refer to the :doc:`dbus` document for
> +recommendation on D-Bus usage)
> +
> +Upon migration, QEMU will go through the queue of
> +``org.qemu.VMState1`` D-Bus name owners and query their ``Id``. It
> +must be unique among the helpers.
> +
> +It will then save arbitrary data of each Id to be transferred in the
> +migration stream and restored/loaded at the corresponding destination
> +helper.
> +
> +The data amount to be transferred is limited to 1Mb. The state must be
> +saved quickly (a few seconds maximum). (D-Bus imposes a time limit on
> +reply anyway, and migration would fail if data isn't given quickly
> +enough)
> +
> +dbus-vmstate object can be configured with the expected list of
> +helpers by setting its ``id-list`` property, with a coma-separated
> +``Id`` list.
> +
> +Interface
> +=========
> +
> +On object path ``/org/qemu/VMState1``, the following
> +``org.qemu.VMState1`` interface should be implemented:
> +
> +.. code:: xml
> +
> + <interface name="org.qemu.VMState1">
> + <property name="Id" type="s" access="read"/>
> + <method name="Load">
> + <arg type="ay" name="data" direction="in"/>
> + </method>
> + <method name="Save">
> + <arg type="ay" name="data" direction="out"/>
> + </method>
> + </interface>
> +
> +"Id" property
> +-------------
> +
> +A string that identifies the helper uniquely.
> +
> +Load(in u8[] bytes) method
> +--------------------------
> +
> +The method called on destination with the state to restore.
> +
> +The helper may be initially started in a waiting state (with
> +an --incoming argument for example), and it may resume on success.
> +
> +An error may be returned to the caller.
> +
> +Save(out u8[] bytes) method
> +---------------------------
> +
> +The method called on the source to get the current state to be
> +migrated. The helper should continue to run normally.
> +
> +An error may be returned to the caller.
> diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> index fa4478ce2e..75832f44ac 100644
> --- a/docs/interop/index.rst
> +++ b/docs/interop/index.rst
> @@ -14,6 +14,7 @@ Contents:
>
> bitmaps
> dbus
> + dbus-vmstate
> live-block-operations
> pr-helper
> vhost-user
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index d4502a30eb..eb40e8a5e7 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -158,7 +158,9 @@ check-qtest-pci-$(CONFIG_RTL8139_PCI) += tests/rtl8139-test$(EXESUF)
> check-qtest-pci-$(CONFIG_VGA) += tests/display-vga-test$(EXESUF)
> check-qtest-pci-$(CONFIG_HDA) += tests/intel-hda-test$(EXESUF)
> check-qtest-pci-$(CONFIG_IVSHMEM_DEVICE) += tests/ivshmem-test$(EXESUF)
> -
> +ifneq ($(GDBUS_CODEGEN),)
> +check-qtest-pci-$(CONFIG_GIO) += tests/dbus-vmstate-test$(EXESUF)
> +endif
> check-qtest-i386-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF)
> check-qtest-i386-y += tests/fdc-test$(EXESUF)
> check-qtest-i386-y += tests/ide-test$(EXESUF)
> @@ -620,6 +622,19 @@ tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.jso
> @mv tests/qapi-schema/doc-good-qapi-doc.texi $@
> @rm -f tests/qapi-schema/doc-good-qapi-*.[ch] tests/qapi-schema/doc-good-qmp-*.[ch]
>
> +tests/dbus-vmstate1.h tests/dbus-vmstate1.c: tests/dbus-vmstate1-gen-timestamp ;
> +tests/dbus-vmstate1-gen-timestamp: $(SRC_PATH)/tests/dbus-vmstate1.xml
> + $(call quiet-command,$(GDBUS_CODEGEN) $< \
> + --interface-prefix org.qemu --generate-c-code tests/dbus-vmstate1, \
> + "GEN","$(@:%-timestamp=%)")
> + @>$@
> +
> +tests/dbus-vmstate-test.o-cflags := -DSRCDIR="$(SRC_PATH)"
> +tests/dbus-vmstate1.o-cflags := $(GIO_CFLAGS)
> +tests/dbus-vmstate1.o-libs := $(GIO_LIBS)
> +
> +tests/dbus-vmstate-test.o: tests/dbus-vmstate1.h
> +
> tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y)
> tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y)
> tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/test-qapi-events.o
> @@ -822,6 +837,7 @@ tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
> tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
> tests/test-x86-cpuid-compat$(EXESUF): tests/test-x86-cpuid-compat.o $(qtest-obj-y)
> tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
> +tests/dbus-vmstate-test$(EXESUF): tests/dbus-vmstate-test.o tests/dbus-vmstate1.o $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
> tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a
> tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
> tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
> @@ -1176,6 +1192,7 @@ check-clean:
> rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
> rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
> rm -f tests/test-qapi-gen-timestamp
> + rm -f tests/dbus-vmstate1-gen-timestamp
> rm -rf $(TESTS_VENV_DIR) $(TESTS_RESULTS_DIR)
>
> clean: check-clean
> diff --git a/tests/dbus-vmstate-daemon.sh b/tests/dbus-vmstate-daemon.sh
> new file mode 100755
> index 0000000000..c4394ac918
> --- /dev/null
> +++ b/tests/dbus-vmstate-daemon.sh
> @@ -0,0 +1,95 @@
> +#!/bin/sh
> +
> +# dbus-daemon wrapper script for dbus-vmstate testing
> +#
> +# This script allows to tweak the dbus-daemon policy during the test
> +# to test different configurations.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, see <http://www.gnu.org/licenses/>.
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +
> +write_config()
> +{
> + CONF="$1"
> + cat > "$CONF" <<EOF
> +<busconfig>
> + <type>session</type>
> + <listen>unix:tmpdir=/tmp</listen>
> +
> + <policy context="default">
> + <!-- Holes must be punched in service configuration files for
> + name ownership and sending method calls -->
> + <deny own="*"/>
> + <deny send_type="method_call"/>
> +
> + <!-- Signals and reply messages (method returns, errors) are allowed
> + by default -->
> + <allow send_type="signal"/>
> + <allow send_requested_reply="true" send_type="method_return"/>
> + <allow send_requested_reply="true" send_type="error"/>
> +
> + <!-- All messages may be received by default -->
> + <allow receive_type="method_call"/>
> + <allow receive_type="method_return"/>
> + <allow receive_type="error"/>
> + <allow receive_type="signal"/>
> +
> + <!-- Allow anyone to talk to the message bus -->
> + <allow send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.DBus" />
> + <allow send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.DBus.Introspectable"/>
> + <allow send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.DBus.Properties"/>
> + <!-- But disallow some specific bus services -->
> + <deny send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.DBus"
> + send_member="UpdateActivationEnvironment"/>
> + <deny send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.DBus.Debug.Stats"/>
> + <deny send_destination="org.freedesktop.DBus"
> + send_interface="org.freedesktop.systemd1.Activator"/>
> +
> + <allow own="org.qemu.VMState1"/>
> + <allow send_destination="org.qemu.VMState1"/>
> + <allow receive_sender="org.qemu.VMState1"/>
> +
> + </policy>
> +
> + <include if_selinux_enabled="yes"
> + selinux_root_relative="yes">contexts/dbus_contexts</include>
> +
> +</busconfig>
> +EOF
> +}
> +
> +ARGS=
> +for arg in "$@"
> +do
> + case $arg in
> + --config-file=*)
> + CONF="${arg#*=}"
> + write_config "$CONF"
> + ARGS="$ARGS $1"
> + shift
> + ;;
> + *)
> + ARGS="$ARGS $1"
> + shift
> + ;;
> + esac
> +done
> +
> +exec dbus-daemon $ARGS
> diff --git a/tests/dbus-vmstate-test.c b/tests/dbus-vmstate-test.c
> new file mode 100644
> index 0000000000..bcf2372e15
> --- /dev/null
> +++ b/tests/dbus-vmstate-test.c
> @@ -0,0 +1,386 @@
> +#include "qemu/osdep.h"
> +#include <glib/gstdio.h>
> +#include <gio/gio.h>
> +#include "libqtest.h"
> +#include "qemu-common.h"
> +#include "dbus-vmstate1.h"
> +
> +static char *workdir;
> +
> +typedef struct TestServerId {
> + const char *name;
> + const char *data;
> + size_t size;
> +} TestServerId;
> +
> +static const TestServerId idA = {
> + "idA", "I'am\0idA!", sizeof("I'am\0idA!")
> +};
> +
> +static const TestServerId idB = {
> + "idB", "I'am\0idB!", sizeof("I'am\0idB!")
> +};
> +
> +typedef struct TestServer {
> + const TestServerId *id;
> + bool save_called;
> + bool load_called;
> +} TestServer;
> +
> +typedef struct Test {
> + const char *id_list;
> + bool migrate_fail;
> + bool without_dst_b;
> + TestServer srcA;
> + TestServer dstA;
> + TestServer srcB;
> + TestServer dstB;
> + GMainLoop *loop;
> + QTestState *src_qemu;
> +} Test;
> +
> +static gboolean
> +vmstate_load(VMState1 *object, GDBusMethodInvocation *invocation,
> + const gchar *arg_data, gpointer user_data)
> +{
> + TestServer *h = user_data;
> + g_autoptr(GVariant) var = NULL;
> + GVariant *args;
> + const uint8_t *data;
> + size_t size;
> +
> + args = g_dbus_method_invocation_get_parameters(invocation);
> + var = g_variant_get_child_value(args, 0);
> + data = g_variant_get_fixed_array(var, &size, sizeof(char));
> + g_assert_cmpuint(size, ==, h->id->size);
> + g_assert(!memcmp(data, h->id->data, h->id->size));
> + h->load_called = true;
> +
> + g_dbus_method_invocation_return_value(invocation, g_variant_new("()"));
> + return TRUE;
> +}
> +
> +static gboolean
> +vmstate_save(VMState1 *object, GDBusMethodInvocation *invocation,
> + gpointer user_data)
> +{
> + TestServer *h = user_data;
> + GVariant *var;
> +
> + var = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
> + h->id->data, h->id->size, sizeof(char));
> + g_dbus_method_invocation_return_value(invocation,
> + g_variant_new("(@ay)", var));
> + h->save_called = true;
> +
> + return TRUE;
> +}
> +
> +static gboolean
> +wait_for_migration_complete(gpointer user_data)
> +{
> + Test *test = user_data;
> + QDict *rsp_return;
> + bool stop = false;
> + const char *status;
> +
> + qtest_qmp_send(test->src_qemu, "{ 'execute': 'query-migrate' }");
> + rsp_return = qtest_qmp_receive_success(test->src_qemu, NULL, NULL);
> + status = qdict_get_str(rsp_return, "status");
> + if (g_str_equal(status, "completed") || g_str_equal(status, "failed")) {
> + stop = true;
> + g_assert_cmpstr(status, ==,
> + test->migrate_fail ? "failed" : "completed");
> + }
> + qobject_unref(rsp_return);
> +
> + if (stop) {
> + g_main_loop_quit(test->loop);
> + }
> + return stop ? G_SOURCE_REMOVE : G_SOURCE_CONTINUE;
> +}
> +
> +static void migrate(QTestState *who, const char *uri)
> +{
> + QDict *args, *rsp;
> +
> + args = qdict_new();
> + qdict_put_str(args, "uri", uri);
> +
> + rsp = qtest_qmp(who, "{ 'execute': 'migrate', 'arguments': %p }", args);
> +
> + g_assert(qdict_haskey(rsp, "return"));
> + qobject_unref(rsp);
> +}
> +
> +typedef struct WaitNamed {
> + GMainLoop *loop;
> + bool named;
> +} WaitNamed;
> +
> +static void
> +named_cb(GDBusConnection *connection,
> + const gchar *name,
> + gpointer user_data)
> +{
> + WaitNamed *t = user_data;
> +
> + t->named = true;
> + g_main_loop_quit(t->loop);
> +}
> +
> +static GDBusConnection *
> +get_connection(Test *test, guint *ownid)
> +{
> + g_autofree gchar *addr = NULL;
> + WaitNamed *wait;
> + GError *err = NULL;
> + GDBusConnection *c;
> +
> + wait = g_new0(WaitNamed, 1);
> + wait->loop = test->loop;
> + addr = g_dbus_address_get_for_bus_sync(G_BUS_TYPE_SESSION, NULL, &err);
> + g_assert_no_error(err);
> +
> + c = g_dbus_connection_new_for_address_sync(
> + addr,
> + G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION |
> + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT,
> + NULL, NULL, &err);
> + g_assert_no_error(err);
> + *ownid = g_bus_own_name_on_connection(c, "org.qemu.VMState1",
> + G_BUS_NAME_OWNER_FLAGS_NONE,
> + named_cb, named_cb, wait, g_free);
> + if (!wait->named) {
> + g_main_loop_run(wait->loop);
> + }
> +
> + return c;
> +}
> +
> +static GDBusObjectManagerServer *
> +get_server(GDBusConnection *conn, TestServer *s, const TestServerId *id)
> +{
> + g_autoptr(GDBusObjectSkeleton) sk = NULL;
> + g_autoptr(VMState1Skeleton) v = NULL;
> + GDBusObjectManagerServer *os;
> +
> + s->id = id;
> + os = g_dbus_object_manager_server_new("/org/qemu");
> + sk = g_dbus_object_skeleton_new("/org/qemu/VMState1");
> +
> + v = VMSTATE1_SKELETON(vmstate1_skeleton_new());
> + g_object_set(v, "id", id->name, NULL);
> +
> + g_signal_connect(v, "handle-load", G_CALLBACK(vmstate_load), s);
> + g_signal_connect(v, "handle-save", G_CALLBACK(vmstate_save), s);
> +
> + g_dbus_object_skeleton_add_interface(sk, G_DBUS_INTERFACE_SKELETON(v));
> + g_dbus_object_manager_server_export(os, sk);
> + g_dbus_object_manager_server_set_connection(os, conn);
> +
> + return os;
> +}
> +
> +static void
> +set_id_list(Test *test, QTestState *s)
> +{
> + if (!test->id_list) {
> + return;
> + }
> +
> + g_assert(!qmp_rsp_is_err(qtest_qmp(s,
> + "{ 'execute': 'qom-set', 'arguments': "
> + "{ 'path': '/objects/dv', 'property': 'id-list', 'value': %s } }",
> + test->id_list)));
> +}
> +static void
> +test_dbus_vmstate(Test *test)
> +{
> + g_autofree char *src_qemu_args = NULL;
> + g_autofree char *dst_qemu_args = NULL;
> + g_autoptr(GTestDBus) srcbus = NULL;
> + g_autoptr(GTestDBus) dstbus = NULL;
> + g_autoptr(GDBusConnection) srcconnA = NULL;
> + g_autoptr(GDBusConnection) srcconnB = NULL;
> + g_autoptr(GDBusConnection) dstconnA = NULL;
> + g_autoptr(GDBusConnection) dstconnB = NULL;
> + g_autoptr(GDBusObjectManagerServer) srcserverA = NULL;
> + g_autoptr(GDBusObjectManagerServer) srcserverB = NULL;
> + g_autoptr(GDBusObjectManagerServer) dstserverA = NULL;
> + g_autoptr(GDBusObjectManagerServer) dstserverB = NULL;
> + g_auto(GStrv) srcaddr = NULL;
> + g_auto(GStrv) dstaddr = NULL;
> + g_autofree char *uri = NULL;
> + QTestState *src_qemu = NULL, *dst_qemu = NULL;
> + guint ownsrcA, ownsrcB, owndstA, owndstB;
> +
> + uri = g_strdup_printf("unix:%s/migsocket", workdir);
> +
> + test->loop = g_main_loop_new(NULL, TRUE);
> +
> + srcbus = g_test_dbus_new(G_TEST_DBUS_NONE);
> + g_test_dbus_up(srcbus);
> + srcconnA = get_connection(test, &ownsrcA);
> + srcserverA = get_server(srcconnA, &test->srcA, &idA);
> + srcconnB = get_connection(test, &ownsrcB);
> + srcserverB = get_server(srcconnB, &test->srcB, &idB);
> +
> + /* remove ,guid=foo part */
> + srcaddr = g_strsplit(g_test_dbus_get_bus_address(srcbus), ",", 2);
> + src_qemu_args =
> + g_strdup_printf("-object dbus-vmstate,id=dv,addr=%s", srcaddr[0]);
> +
> + dstbus = g_test_dbus_new(G_TEST_DBUS_NONE);
> + g_test_dbus_up(dstbus);
> + dstconnA = get_connection(test, &owndstA);
> + dstserverA = get_server(dstconnA, &test->dstA, &idA);
> + if (!test->without_dst_b) {
> + dstconnB = get_connection(test, &owndstB);
> + dstserverB = get_server(dstconnB, &test->dstB, &idB);
> + }
> +
> + dstaddr = g_strsplit(g_test_dbus_get_bus_address(dstbus), ",", 2);
> + dst_qemu_args =
> + g_strdup_printf("-object dbus-vmstate,id=dv,addr=%s -incoming %s",
> + dstaddr[0], uri);
> +
> + src_qemu = qtest_init(src_qemu_args);
> + dst_qemu = qtest_init(dst_qemu_args);
> + set_id_list(test, src_qemu);
> + set_id_list(test, dst_qemu);
> +
> + migrate(src_qemu, uri);
> + test->src_qemu = src_qemu;
> + g_timeout_add_seconds(1, wait_for_migration_complete, test);
> +
> + g_main_loop_run(test->loop);
> + g_main_loop_unref(test->loop);
> +
> + if (test->migrate_fail) {
> + qtest_expect_exit_status(dst_qemu, 1);
> + }
> + qtest_quit(dst_qemu);
> + qtest_quit(src_qemu);
> + g_bus_unown_name(ownsrcA);
> + g_bus_unown_name(ownsrcB);
> + g_bus_unown_name(owndstA);
> + if (!test->without_dst_b) {
> + g_bus_unown_name(owndstB);
> + }
> +}
> +
> +static void
> +check_not_migrated(TestServer *s, TestServer *d)
> +{
> + assert(!s->save_called);
> + assert(!s->load_called);
> + assert(!d->save_called);
> + assert(!d->load_called);
> +}
> +
> +static void
> +check_migrated(TestServer *s, TestServer *d)
> +{
> + assert(s->save_called);
> + assert(!s->load_called);
> + assert(!d->save_called);
> + assert(d->load_called);
> +}
> +
> +static void
> +test_dbus_vmstate_without_list(void)
> +{
> + Test test = { 0, };
> +
> + test_dbus_vmstate(&test);
> +
> + check_migrated(&test.srcA, &test.dstA);
> + check_migrated(&test.srcB, &test.dstB);
> +}
> +
> +static void
> +test_dbus_vmstate_with_list(void)
> +{
> + Test test = { .id_list = "idA,idB" };
> +
> + test_dbus_vmstate(&test);
> +
> + check_migrated(&test.srcA, &test.dstA);
> + check_migrated(&test.srcB, &test.dstB);
> +}
> +
> +static void
> +test_dbus_vmstate_only_a(void)
> +{
> + Test test = { .id_list = "idA" };
> +
> + test_dbus_vmstate(&test);
> +
> + check_migrated(&test.srcA, &test.dstA);
> + check_not_migrated(&test.srcB, &test.dstB);
> +}
> +
> +static void
> +test_dbus_vmstate_missing_src(void)
> +{
> + Test test = { .id_list = "idA,idC", .migrate_fail = true };
> +
> + test_dbus_vmstate(&test);
> + check_not_migrated(&test.srcA, &test.dstA);
> + check_not_migrated(&test.srcB, &test.dstB);
> +}
> +
> +static void
> +test_dbus_vmstate_missing_dst(void)
> +{
> + Test test = { .id_list = "idA,idB",
> + .without_dst_b = true,
> + .migrate_fail = true };
> +
> + test_dbus_vmstate(&test);
> +
> + assert(test.srcA.save_called);
> + assert(test.srcB.save_called);
> + assert(!test.dstB.save_called);
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> + GError *err = NULL;
> + g_autofree char *dbus_daemon = NULL;
> + int ret;
> +
> + dbus_daemon = g_build_filename(G_STRINGIFY(SRCDIR),
> + "tests",
> + "dbus-vmstate-daemon.sh",
> + NULL);
> + g_setenv("G_TEST_DBUS_DAEMON", dbus_daemon, true);
> +
> + g_test_init(&argc, &argv, NULL);
> +
> + workdir = g_dir_make_tmp("dbus-vmstate-test-XXXXXX", &err);
> + if (!workdir) {
> + g_error("Unable to create temporary dir: %s\n", err->message);
> + exit(1);
> + }
> +
> + qtest_add_func("/dbus-vmstate/without-list",
> + test_dbus_vmstate_without_list);
> + qtest_add_func("/dbus-vmstate/with-list",
> + test_dbus_vmstate_with_list);
> + qtest_add_func("/dbus-vmstate/only-a",
> + test_dbus_vmstate_only_a);
> + qtest_add_func("/dbus-vmstate/missing-src",
> + test_dbus_vmstate_missing_src);
> + qtest_add_func("/dbus-vmstate/missing-dst",
> + test_dbus_vmstate_missing_dst);
> +
> + ret = g_test_run();
> +
> + rmdir(workdir);
> + g_free(workdir);
> +
> + return ret;
> +}
> diff --git a/tests/dbus-vmstate1.xml b/tests/dbus-vmstate1.xml
> new file mode 100644
> index 0000000000..cc8563be4c
> --- /dev/null
> +++ b/tests/dbus-vmstate1.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
> + <interface name="org.qemu.VMState1">
> + <property name="Id" type="s" access="read"/>
> + <method name="Load">
> + <arg type="ay" name="data" direction="in"/>
> + </method>
> + <method name="Save">
> + <arg type="ay" name="data" direction="out"/>
> + </method>
> + </interface>
> +</node>
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object Marc-André Lureau
2019-09-12 14:29 ` Eric Blake
2019-09-16 10:43 ` Dr. David Alan Gilbert
@ 2019-09-17 13:21 ` Daniel P. Berrangé
2 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2019-09-17 13:21 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: mprivozn, pbonzini, qemu-devel, dgilbert, quintela
On Thu, Sep 12, 2019 at 04:25:14PM +0400, Marc-André Lureau wrote:
> When instanciated, this object will connect to the given D-Bus
> bus. During migration, it will take the data from org.qemu.VMState1
> instances.
>
> See documentation for further details.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> MAINTAINERS | 6 +
> backends/Makefile.objs | 4 +
> backends/dbus-vmstate.c | 530 ++++++++++++++++++++++++++++++++++
> configure | 7 +
> docs/interop/dbus-vmstate.rst | 68 +++++
> docs/interop/index.rst | 1 +
> tests/Makefile.include | 19 +-
> tests/dbus-vmstate-daemon.sh | 95 ++++++
> tests/dbus-vmstate-test.c | 386 +++++++++++++++++++++++++
> tests/dbus-vmstate1.xml | 12 +
> 10 files changed, 1127 insertions(+), 1 deletion(-)
> create mode 100644 backends/dbus-vmstate.c
> create mode 100644 docs/interop/dbus-vmstate.rst
> create mode 100755 tests/dbus-vmstate-daemon.sh
> create mode 100644 tests/dbus-vmstate-test.c
> create mode 100644 tests/dbus-vmstate1.xml
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50eaf005f4..f641870c40 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2153,6 +2153,12 @@ F: tests/migration-test.c
> F: docs/devel/migration.rst
> F: qapi/migration.json
>
> +DBus VMState
> +M: Marc-André Lureau <marcandre.lureau@redhat.com>
> +S: Maintained
> +F: backends/dbus-vmstate.c
> +F: tests/dbus-vmstate*
> +
> Seccomp
> M: Eduardo Otubo <otubo@redhat.com>
> S: Supported
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index f0691116e8..28a847cd57 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -17,3 +17,7 @@ endif
> common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_VIRTIO)) += vhost-user.o
>
> common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
> +
> +common-obj-$(CONFIG_GIO) += dbus-vmstate.o
> +dbus-vmstate.o-cflags = $(GIO_CFLAGS)
> +dbus-vmstate.o-libs = $(GIO_LIBS)
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> new file mode 100644
> index 0000000000..559f5430c5
> --- /dev/null
> +++ b/backends/dbus-vmstate.c
> @@ -0,0 +1,530 @@
> +/*
> + * QEMU dbus-vmstate
> + *
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + * Marc-André Lureau <marcandre.lureau@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qom/object_interfaces.h"
> +#include "qapi/qmp/qerror.h"
> +#include "migration/vmstate.h"
> +#include <gio/gio.h>
> +
> +typedef struct DBusVMState DBusVMState;
> +typedef struct DBusVMStateClass DBusVMStateClass;
> +
> +#define TYPE_DBUS_VMSTATE "dbus-vmstate"
> +#define DBUS_VMSTATE(obj) \
> + OBJECT_CHECK(DBusVMState, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(DBusVMStateClass, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(DBusVMStateClass, (klass), TYPE_DBUS_VMSTATE)
> +
> +struct DBusVMStateClass {
> + ObjectClass parent_class;
> +};
> +
> +struct DBusVMState {
> + Object parent;
> +
> + GDBusConnection *bus;
> + char *dbus_addr;
> + char *id_list;
> +
> + uint32_t data_size;
> + uint8_t *data;
> +};
> +
> +static const GDBusPropertyInfo vmstate_property_info[] = {
> + { -1, (char *) "Id", (char *) "s",
> + G_DBUS_PROPERTY_INFO_FLAGS_READABLE, NULL },
> +};
> +
> +static const GDBusPropertyInfo * const vmstate_property_info_pointers[] = {
> + &vmstate_property_info[0],
> + NULL
> +};
> +
> +static const GDBusInterfaceInfo vmstate1_interface_info = {
> + -1,
> + (char *) "org.qemu.VMState1",
> + (GDBusMethodInfo **) NULL,
> + (GDBusSignalInfo **) NULL,
> + (GDBusPropertyInfo **) &vmstate_property_info_pointers,
> + NULL,
> +};
> +
> +#define DBUS_VMSTATE_SIZE_LIMIT (1 * MiB)
> +
> +static char **
> +dbus_get_vmstate1_names(DBusVMState *self, GError **err)
> +{
> + g_autoptr(GDBusProxy) proxy = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) child = NULL;
> +
> + proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE, NULL,
> + "org.freedesktop.DBus",
> + "/org/freedesktop/DBus",
> + "org.freedesktop.DBus",
> + NULL, err);
> + if (!proxy) {
> + return NULL;
> + }
> +
> + result = g_dbus_proxy_call_sync(proxy, "ListQueuedOwners",
> + g_variant_new("(s)", "org.qemu.VMState1"),
> + G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, err);
> + if (!result) {
> + return NULL;
> + }
> +
> + child = g_variant_get_child_value(result, 0);
> + return g_variant_dup_strv(child, NULL);
> +}
I'd probably move this into a util/dbus.c file, as it is the
kind of generic code that is likely to be useful in other
parts of QEMU. Just have "org.qem.VMState1" passed in as a
param.
> +
> +static GHashTable *
> +get_id_list_set(DBusVMState *self)
> +{
> + g_auto(GStrv) ids = NULL;
> + g_autoptr(GHashTable) set = NULL;
> + int i;
> +
> + if (!self->id_list) {
> + return NULL;
> + }
> +
> + ids = g_strsplit(self->id_list, ",", -1);
> + set = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
> + for (i = 0; ids[i]; i++) {
> + g_hash_table_add(set, ids[i]);
> + ids[i] = NULL;
> + }
> +
> + return g_steal_pointer(&set);
> +}
> +
> +static GHashTable *
> +dbus_get_proxies(DBusVMState *self, GError **err)
> +{
> + g_autoptr(GError) local_err = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + g_autoptr(GHashTable) ids = NULL;
> + g_auto(GStrv) names = NULL;
> + size_t i;
> +
> + ids = get_id_list_set(self);
> + proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
> + g_free, g_object_unref);
> +
> + names = dbus_get_vmstate1_names(self, &local_err);
> + if (!names) {
> + if (g_error_matches(local_err,
> + G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER)) {
> + return proxies;
> + }
> + g_propagate_error(err, local_err);
> + return NULL;
> + }
> +
> + for (i = 0; names[i]; i++) {
> + g_autoptr(GDBusProxy) proxy = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autofree char *id = NULL;
> + size_t size;
> +
> + proxy = g_dbus_proxy_new_sync(self->bus, G_DBUS_PROXY_FLAGS_NONE,
> + (GDBusInterfaceInfo *) &vmstate1_interface_info,
> + names[i],
> + "/org/qemu/VMState1",
> + "org.qemu.VMState1",
> + NULL, err);
> + if (!proxy) {
> + return NULL;
> + }
> +
> + result = g_dbus_proxy_get_cached_property(proxy, "Id");
> + if (!result) {
> + g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "VMState Id property is missing.");
> + return NULL;
> + }
> +
> + id = g_variant_dup_string(result, &size);
> + if (ids && !g_hash_table_remove(ids, id)) {
> + g_clear_pointer(&id, g_free);
> + g_clear_object(&proxy);
> + continue;
> + }
> + if (size == 0 || size >= 256) {
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "VMState Id '%s' is invalid.", id);
> + return NULL;
> + }
> +
> + if (!g_hash_table_insert(proxies, id, proxy)) {
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "Duplicated VMState Id '%s'", id);
> + return NULL;
> + }
> + id = NULL;
> + proxy = NULL;
> +
> + g_clear_pointer(&result, g_variant_unref);
> + }
> +
> + if (ids) {
> + g_autofree char **left = NULL;
> +
> + left = (char **)g_hash_table_get_keys_as_array(ids, NULL);
> + if (*left) {
> + g_autofree char *leftids = g_strjoinv(",", left);
> + g_set_error(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> + "Required VMState Id are missing: %s", leftids);
> + return NULL;
> + }
> + }
> +
> + return g_steal_pointer(&proxies);
> +}
This method could probably go to a util/dbus.c file too.
> +
> +static int
> +dbus_load_state_proxy(GDBusProxy *proxy, const uint8_t *data, size_t size)
> +{
> + g_autoptr(GError) err = NULL;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) value = NULL;
> +
> + value = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
> + data, size, sizeof(char));
> + result = g_dbus_proxy_call_sync(proxy, "Load",
> + g_variant_new("(@ay)",
> + g_steal_pointer(&value)),
> + G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, &err);
> + if (!result) {
> + error_report("Failed to Load: %s", err->message);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int dbus_vmstate_post_load(void *opaque, int version_id)
> +{
> + DBusVMState *self = DBUS_VMSTATE(opaque);
> + g_autoptr(GInputStream) m = NULL;
> + g_autoptr(GDataInputStream) s = NULL;
> + g_autoptr(GError) err = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + uint32_t nelem;
> +
> + proxies = dbus_get_proxies(self, &err);
> + if (!proxies) {
> + error_report("Failed to get proxies: %s", err->message);
> + return -1;
> + }
> +
> + m = g_memory_input_stream_new_from_data(self->data, self->data_size, NULL);
> + s = g_data_input_stream_new(m);
> + g_data_input_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
> +
> + nelem = g_data_input_stream_read_uint32(s, NULL, &err);
> + if (err) {
> + goto error;
> + }
> +
> + while (nelem > 0) {
> + GDBusProxy *proxy = NULL;
> + uint32_t len;
> + gsize bytes_read, avail;
> + char id[256];
> +
> + len = g_data_input_stream_read_uint32(s, NULL, &err);
> + if (err) {
> + goto error;
> + }
> + if (len >= 256) {
> + error_report("Invalid DBus vmstate proxy name %u", len);
> + return -1;
> + }
> + if (!g_input_stream_read_all(G_INPUT_STREAM(s), id, len,
> + &bytes_read, NULL, &err)) {
> + goto error;
> + }
> + g_return_val_if_fail(bytes_read == len, -1);
> + id[len] = 0;
> +
> + proxy = g_hash_table_lookup(proxies, id);
> + if (!proxy) {
> + error_report("Failed to find proxy Id '%s'", id);
> + return -1;
> + }
> +
> + len = g_data_input_stream_read_uint32(s, NULL, &err);
> + avail = g_buffered_input_stream_get_available(
> + G_BUFFERED_INPUT_STREAM(s));
> +
> + if (len > DBUS_VMSTATE_SIZE_LIMIT || len > avail) {
> + error_report("Invalid vmstate size: %u", len);
> + return -1;
> + }
> +
> + if (dbus_load_state_proxy(proxy,
> + g_buffered_input_stream_peek_buffer(G_BUFFERED_INPUT_STREAM(s),
> + NULL),
> + len) < 0) {
> + error_report("Failed to restore Id '%s'", id);
> + return -1;
> + }
> +
> + if (!g_seekable_seek(G_SEEKABLE(s), len, G_SEEK_CUR, NULL, &err)) {
> + goto error;
> + }
> +
> + nelem -= 1;
> + }
> +
> + return 0;
> +
> +error:
> + error_report("Failed to read from stream: %s", err->message);
> + return -1;
> +}
> +
> +static void
> +dbus_save_state_proxy(gpointer key,
> + gpointer value,
> + gpointer user_data)
> +{
> + GDataOutputStream *s = user_data;
> + const char *id = key;
> + GDBusProxy *proxy = value;
> + g_autoptr(GVariant) result = NULL;
> + g_autoptr(GVariant) child = NULL;
> + g_autoptr(GError) err = NULL;
> + const uint8_t *data;
> + gsize size;
> +
> + result = g_dbus_proxy_call_sync(proxy, "Save",
> + NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START,
> + -1, NULL, &err);
> + if (!result) {
> + error_report("Failed to Save: %s", err->message);
> + return;
> + }
> +
> + child = g_variant_get_child_value(result, 0);
> + data = g_variant_get_fixed_array(child, &size, sizeof(char));
> + if (!data) {
> + error_report("Failed to Save: not a byte array");
> + return;
> + }
> + if (size > DBUS_VMSTATE_SIZE_LIMIT) {
> + error_report("Too large vmstate data to save: %lu", size);
> + return;
> + }
> +
> + if (!g_data_output_stream_put_uint32(s, strlen(id), NULL, &err) ||
> + !g_data_output_stream_put_string(s, id, NULL, &err) ||
> + !g_data_output_stream_put_uint32(s, size, NULL, &err) ||
> + !g_output_stream_write_all(G_OUTPUT_STREAM(s),
> + data, size, NULL, NULL, &err)) {
> + error_report("Failed to write to stream: %s", err->message);
> + }
> +}
> +
> +static int dbus_vmstate_pre_save(void *opaque)
> +{
> + DBusVMState *self = DBUS_VMSTATE(opaque);
> + g_autoptr(GOutputStream) m = NULL;
> + g_autoptr(GDataOutputStream) s = NULL;
> + g_autoptr(GHashTable) proxies = NULL;
> + g_autoptr(GError) err = NULL;
> +
> + proxies = dbus_get_proxies(self, &err);
> + if (!proxies) {
> + error_report("Failed to get proxies: %s", err->message);
> + return -1;
> + }
> +
> + m = g_memory_output_stream_new_resizable();
> + s = g_data_output_stream_new(m);
> + g_data_output_stream_set_byte_order(s, G_DATA_STREAM_BYTE_ORDER_BIG_ENDIAN);
> +
> + if (!g_data_output_stream_put_uint32(s, g_hash_table_size(proxies),
> + NULL, &err)) {
> + error_report("Failed to write to stream: %s", err->message);
> + return -1;
> + }
> +
> + g_hash_table_foreach(proxies, dbus_save_state_proxy, s);
> +
> + if (g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m))
> + > UINT32_MAX) {
> + error_report("DBus vmstate buffer is too large");
> + return -1;
> + }
> +
> + if (!g_output_stream_close(G_OUTPUT_STREAM(m), NULL, &err)) {
> + error_report("Failed to close stream: %s", err->message);
> + return -1;
> + }
> +
> + g_free(self->data);
> + self->data_size =
> + g_memory_output_stream_get_size(G_MEMORY_OUTPUT_STREAM(m));
> + self->data =
> + g_memory_output_stream_steal_data(G_MEMORY_OUTPUT_STREAM(m));
> +
> + return 0;
> +}
> +
> +static const VMStateDescription dbus_vmstate = {
> + .name = TYPE_DBUS_VMSTATE,
> + .version_id = 0,
> + .pre_save = dbus_vmstate_pre_save,
> + .post_load = dbus_vmstate_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(data_size, DBusVMState),
> + VMSTATE_VBUFFER_ALLOC_UINT32(data, DBusVMState, 0, 0, data_size),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void
> +dbus_vmstate_complete(UserCreatable *uc, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(uc);
> + GError *err = NULL;
> + GDBusConnection *bus;
> +
> + if (!object_resolve_path_type("", TYPE_DBUS_VMSTATE, NULL)) {
> + error_setg(errp, "There is already an instance of %s",
> + TYPE_DBUS_VMSTATE);
> + return;
> + }
> +
> + if (!self->dbus_addr) {
> + error_setg(errp, QERR_MISSING_PARAMETER, "addr");
> + return;
> + }
> +
> + bus = g_dbus_connection_new_for_address_sync(self->dbus_addr,
> + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT |
> + G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
> + NULL, NULL, &err);
> + if (err) {
> + error_setg(errp, "failed to connect to DBus: '%s'", err->message);
> + g_clear_error(&err);
> + }
> +
> + self->bus = bus;
> +
> + if (vmstate_register(VMSTATE_IF(self), -1, &dbus_vmstate, self) < 0) {
> + error_setg(errp, "Failed to register vmstate");
> + }
> +}
> +
> +static void
> +dbus_vmstate_finalize(Object *o)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + vmstate_unregister(VMSTATE_IF(self), &dbus_vmstate, self);
> +
> + g_clear_object(&self->bus);
> + g_free(self->dbus_addr);
> + g_free(self->id_list);
> + g_free(self->data);
> +}
> +
> +static char *
> +get_dbus_addr(Object *o, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + return g_strdup(self->dbus_addr);
> +}
> +
> +static void
> +set_dbus_addr(Object *o, const char *str, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + g_free(self->dbus_addr);
> + self->dbus_addr = g_strdup(str);
> +}
> +
> +static char *
> +get_id_list(Object *o, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + return g_strdup(self->id_list);
> +}
> +
> +static void
> +set_id_list(Object *o, const char *str, Error **errp)
> +{
> + DBusVMState *self = DBUS_VMSTATE(o);
> +
> + g_free(self->id_list);
> + self->id_list = g_strdup(str);
> +}
> +
> +static char *
> +dbus_vmstate_get_id(VMStateIf *vmif)
> +{
> + return g_strdup(TYPE_DBUS_VMSTATE);
> +}
> +
> +static void
> +dbus_vmstate_class_init(ObjectClass *oc, void *data)
> +{
> + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> + VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);
> +
> + ucc->complete = dbus_vmstate_complete;
> + vc->get_id = dbus_vmstate_get_id;
> +
> + object_class_property_add_str(oc, "addr",
> + get_dbus_addr, set_dbus_addr,
> + &error_abort);
> + object_class_property_add_str(oc, "id-list",
> + get_id_list, set_id_list,
> + &error_abort);
> +}
> +
> +static const TypeInfo dbus_vmstate_info = {
> + .name = TYPE_DBUS_VMSTATE,
> + .parent = TYPE_OBJECT,
> + .instance_size = sizeof(DBusVMState),
> + .instance_finalize = dbus_vmstate_finalize,
> + .class_size = sizeof(DBusVMStateClass),
> + .class_init = dbus_vmstate_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_USER_CREATABLE },
> + { TYPE_VMSTATE_IF },
> + { }
> + }
> +};
> +
> +static void
> +register_types(void)
> +{
> + type_register_static(&dbus_vmstate_info);
> +}
> +
> +type_init(register_types);
> diff --git a/configure b/configure
> index 95134c0180..0a6eb2ebcd 100755
> --- a/configure
> +++ b/configure
> @@ -3665,10 +3665,16 @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then
> gio=yes
> gio_cflags=$($pkg_config --cflags gio-2.0)
> gio_libs=$($pkg_config --libs gio-2.0)
> + gdbus_codegen=$($pkg_config --variable=gdbus_codegen gio-2.0)
> else
> gio=no
> fi
>
> +if $pkg_config --atleast-version=$glib_req_ver gio-unix-2.0; then
> + gio_cflags="$gio_cflags $($pkg_config --cflags gio-unix-2.0)"
> + gio_libs="$gio_libs $($pkg_config --libs gio-unix-2.0)"
> +fi
Unless I'm missing it, nothing in this patch uses gio-unix APIs
> diff --git a/docs/interop/dbus-vmstate.rst b/docs/interop/dbus-vmstate.rst
> new file mode 100644
> index 0000000000..78070be1bd
> --- /dev/null
> +++ b/docs/interop/dbus-vmstate.rst
> @@ -0,0 +1,68 @@
> +=============
> +D-Bus VMState
> +=============
> +
> +Introduction
> +============
> +
> +The QEMU dbus-vmstate object aim is to migrate helpers data running on
> +a QEMU D-Bus bus. (refer to the :doc:`dbus` document for
> +recommendation on D-Bus usage)
> +
> +Upon migration, QEMU will go through the queue of
> +``org.qemu.VMState1`` D-Bus name owners and query their ``Id``. It
> +must be unique among the helpers.
> +
> +It will then save arbitrary data of each Id to be transferred in the
> +migration stream and restored/loaded at the corresponding destination
> +helper.
> +
> +The data amount to be transferred is limited to 1Mb. The state must be
> +saved quickly (a few seconds maximum). (D-Bus imposes a time limit on
> +reply anyway, and migration would fail if data isn't given quickly
> +enough)
> +
> +dbus-vmstate object can be configured with the expected list of
> +helpers by setting its ``id-list`` property, with a coma-separated
> +``Id`` list.
> +
> +Interface
> +=========
> +
> +On object path ``/org/qemu/VMState1``, the following
> +``org.qemu.VMState1`` interface should be implemented:
> +
> +.. code:: xml
> +
> + <interface name="org.qemu.VMState1">
> + <property name="Id" type="s" access="read"/>
> + <method name="Load">
> + <arg type="ay" name="data" direction="in"/>
> + </method>
> + <method name="Save">
> + <arg type="ay" name="data" direction="out"/>
> + </method>
> + </interface>
> +
> +"Id" property
> +-------------
> +
> +A string that identifies the helper uniquely.
Is there any association to the "id" values used in QEMU
-object/-device objects, or is this a separate ID namespace
entirely ?
> +Load(in u8[] bytes) method
> +--------------------------
> +
> +The method called on destination with the state to restore.
> +
> +The helper may be initially started in a waiting state (with
> +an --incoming argument for example), and it may resume on success.
> +
> +An error may be returned to the caller.
> +
> +Save(out u8[] bytes) method
> +---------------------------
> +
> +The method called on the source to get the current state to be
> +migrated. The helper should continue to run normally.
> +
> +An error may be returned to the caller.
> diff --git a/docs/interop/index.rst b/docs/interop/index.rst
> index fa4478ce2e..75832f44ac 100644
> --- a/docs/interop/index.rst
> +++ b/docs/interop/index.rst
> @@ -14,6 +14,7 @@ Contents:
>
> bitmaps
> dbus
> + dbus-vmstate
> live-block-operations
> pr-helper
> vhost-user
Perhaps the 'dbus.rst' doc should have a link to the
'dbus-vmstate' block under a "QEMU services/interfaces" heading
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate
2019-09-12 12:25 [Qemu-devel] [PATCH v3 0/6] Add dbus-vmstate Marc-André Lureau
` (5 preceding siblings ...)
2019-09-12 12:25 ` [Qemu-devel] [PATCH v3 6/6] Add dbus-vmstate object Marc-André Lureau
@ 2019-09-12 13:50 ` no-reply
6 siblings, 0 replies; 32+ messages in thread
From: no-reply @ 2019-09-12 13:50 UTC (permalink / raw)
To: marcandre.lureau
Cc: berrange, quintela, mprivozn, qemu-devel, dgilbert, pbonzini,
marcandre.lureau
Patchew URL: https://patchew.org/QEMU/20190912122514.22504-1-marcandre.lureau@redhat.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
libudev no
default devices yes
warning: Python 2 support is deprecated
warning: Python 3 will be required for building future versions of QEMU
NOTE: cross-compilers enabled: 'cc'
GEN x86_64-softmmu/config-devices.mak.tmp
---
Could not get password database information for UID of current process: User "???" unknown or no memory to allocate password entry
**
ERROR:/tmp/qemu-test/src/tests/dbus-vmstate-test.c:150:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
cleaning up pid 28575
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/dbus-vmstate-test.c:150:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
TEST iotest-qcow2: 080
The full log is available at
http://patchew.org/logs/20190912122514.22504-1-marcandre.lureau@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 32+ messages in thread