All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
@ 2019-05-22 22:22 rkir--- via Qemu-devel
  2019-05-23 10:22 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 6+ messages in thread
From: rkir--- via Qemu-devel @ 2019-05-22 22:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Roman Kiryanov

From: Roman Kiryanov <rkir@google.com>

VMSTATE_OPAQUE allows passing user defined functions to save
and load vmstate for cases when data structures do not fit
into int/struct/array terms.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 include/migration/vmstate.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 9224370ed5..2736daef17 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -737,6 +737,19 @@ extern const VMStateInfo vmstate_info_qtailq;
     .start        = offsetof(_type, _next),                              \
 }
 
+/* Provides a way to save/load complex data structures that do not
+ * fit into int/struct/array terms.
+ * _info: a user defined instance of VMStateInfo to handle saving and loading.
+ */
+#define VMSTATE_OPAQUE(_name, _version, _info) {                      \
+    .name         = _name,                                            \
+    .version_id   = (_version),                                       \
+    .size         = 0,                                                \
+    .info         = &(_info),                                         \
+    .flags        = VMS_SINGLE,                                       \
+    .offset       = 0,                                                \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
-- 
2.21.0.1020.gf2820cf01a-goog



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
  2019-05-22 22:22 [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures rkir--- via Qemu-devel
@ 2019-05-23 10:22 ` Dr. David Alan Gilbert
  2019-05-23 17:00   ` Roman Kiryanov via Qemu-devel
  0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2019-05-23 10:22 UTC (permalink / raw)
  To: rkir, quintela; +Cc: qemu-devel

* rkir--- via Qemu-devel (qemu-devel@nongnu.org) wrote:
> From: Roman Kiryanov <rkir@google.com>
> 
> VMSTATE_OPAQUE allows passing user defined functions to save
> and load vmstate for cases when data structures do not fit
> into int/struct/array terms.
> 
> Signed-off-by: Roman Kiryanov <rkir@google.com>

Hi Roman,
  Thanks for the patch.

Can you give me an example of where you would use it?  I've been
trying to get rid as many of the open-coded cases as possible
and try and make sure vmstate can handle it.

Having said that;  would it be easier to pass the get/put functions
rather than the info ?

Dave

> ---
>  include/migration/vmstate.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 9224370ed5..2736daef17 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -737,6 +737,19 @@ extern const VMStateInfo vmstate_info_qtailq;
>      .start        = offsetof(_type, _next),                              \
>  }
>  
> +/* Provides a way to save/load complex data structures that do not
> + * fit into int/struct/array terms.
> + * _info: a user defined instance of VMStateInfo to handle saving and loading.
> + */
> +#define VMSTATE_OPAQUE(_name, _version, _info) {                      \
> +    .name         = _name,                                            \
> +    .version_id   = (_version),                                       \
> +    .size         = 0,                                                \
> +    .info         = &(_info),                                         \
> +    .flags        = VMS_SINGLE,                                       \
> +    .offset       = 0,                                                \
> +}
> +
>  /* _f : field name
>     _f_n : num of elements field_name
>     _n : num of elements
> -- 
> 2.21.0.1020.gf2820cf01a-goog
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
  2019-05-23 10:22 ` Dr. David Alan Gilbert
@ 2019-05-23 17:00   ` Roman Kiryanov via Qemu-devel
  2019-05-24  9:03     ` Peter Maydell
  0 siblings, 1 reply; 6+ messages in thread
From: Roman Kiryanov via Qemu-devel @ 2019-05-23 17:00 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Lingfeng Yang, qemu-devel, quintela

Hi Dave, thank you for looking.

> Can you give me an example of where you would use it?

We use it in our host memory sharing device. I used the existing
macros for all fields I could, but unfortunately some state does not fit
into them. We use this new macro to save/load memory
allocators (for now we have malloc, but we are working on adding
Vulkan calls). For now the state looks this way:

class Allocator;
unordered_map<int, Allocator *> state;

class MallocAllocator: public Allocator {
    unordered_map<int, vector<char>> state;
};

class VulkanAllocator: public Allocator {
    // TBD
};

>  I've been trying to get rid as many of the open-coded cases as possible

I saw this in the migration document. I used VMSTATE_INT32,
VMSTATE_STRUCT and VMSTATE_STRUCT_VARRAY_ALLOC
where I could.

> Having said that;  would it be easier to pass the get/put functions
> rather than the info ?

Sure, function pointer will be even better, but since VMStateField
takes "const VMStateInfo *", I added this way.

Regards,
Roman.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
  2019-05-23 17:00   ` Roman Kiryanov via Qemu-devel
@ 2019-05-24  9:03     ` Peter Maydell
  2019-05-24 17:00       ` Roman Kiryanov via Qemu-devel
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-05-24  9:03 UTC (permalink / raw)
  To: Roman Kiryanov
  Cc: QEMU Developers, Juan Quintela, Dr. David Alan Gilbert, Lingfeng Yang

On Thu, 23 May 2019 at 18:02, Roman Kiryanov via Qemu-devel
<qemu-devel@nongnu.org> wrote:
>
> Hi Dave, thank you for looking.
>
> > Can you give me an example of where you would use it?
>
> We use it in our host memory sharing device. I used the existing
> macros for all fields I could, but unfortunately some state does not fit
> into them. We use this new macro to save/load memory
> allocators (for now we have malloc, but we are working on adding
> Vulkan calls). For now the state looks this way:
>
> class Allocator;
> unordered_map<int, Allocator *> state;
>
> class MallocAllocator: public Allocator {
>     unordered_map<int, vector<char>> state;
> };
>
> class VulkanAllocator: public Allocator {
>     // TBD
> };

This is all C++, so it's not relevant for upstream QEMU...

In any case, migration state on the wire needs to be
architecture/endianness/implementation-independent,
so you can't just send raw complex data structures -- you
need to marshal these into something else (by having
pre-load/post-save functions in whatever device is using
these to marshal between the complex data structures and a
plain-old-array-of-whatevers, or by having the migration
code specifically support migration of the complex data
structures.)

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
  2019-05-24  9:03     ` Peter Maydell
@ 2019-05-24 17:00       ` Roman Kiryanov via Qemu-devel
  2019-05-24 17:29         ` Peter Maydell
  0 siblings, 1 reply; 6+ messages in thread
From: Roman Kiryanov via Qemu-devel @ 2019-05-24 17:00 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Juan Quintela, Dr. David Alan Gilbert, Lingfeng Yang

Hi Peter,

> In any case, migration state on the wire needs to be
> architecture/endianness/

Could you please point how the proposed change is
architecture/endianness/ dependent?

> implementation-independent,

Could you please elaborate, what "implementation"
you mean here?

> so you can't just send raw complex data structures

Do we need to serialize (in pre_save and then release in
post_save) our state into a buffer and to write it as one
piece using the existing macro? This looks ok, but how
is this different from what we are doing?

Regards,
Roman.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures
  2019-05-24 17:00       ` Roman Kiryanov via Qemu-devel
@ 2019-05-24 17:29         ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2019-05-24 17:29 UTC (permalink / raw)
  To: Roman Kiryanov
  Cc: QEMU Developers, Juan Quintela, Dr. David Alan Gilbert, Lingfeng Yang

On Fri, 24 May 2019 at 18:00, Roman Kiryanov <rkir@google.com> wrote:
>
> Hi Peter,
>
> > In any case, migration state on the wire needs to be
> > architecture/endianness/
>
> Could you please point how the proposed change is
> architecture/endianness/ dependent?

That's really hard to say, because this patch doesn't
come with any example of its use. So I'm basically
guessing that when you say "load/save complex data
structures" and call your macro OPAQUE that you mean
"I am going to just feed the raw in-memory representation
of this data structure into the migration stream in an
opaque way". Perhaps my assumptions here are wrong ?

> > implementation-independent,
>
> Could you please elaborate, what "implementation"
> you mean here?

I had in mind the C++ implementation of unordered_map<>.

> > so you can't just send raw complex data structures
>
> Do we need to serialize (in pre_save and then release in
> post_save) our state into a buffer and to write it as one
> piece using the existing macro? This looks ok, but how
> is this different from what we are doing?

I guess essentially what I'm asking for here is that
this patch comes as part of a series which makes
use of it. It's really hard to evaluate the design
of a utility feature without a concrete example of
how it's being used. That then makes it easier to understand
the abstract feature and also allows us to sometimes
suggest better ways to achieve the underlying aim
(and to avoid making suggestions which don't make sense!)

A corollary of this is that in general we don't like to
take patches upstream that implement facilities that don't
have a use upstream. Is there some existing vmstate
handling in upstream QEMU that we could refactor to
be more cleanly implemented using this?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-05-24 17:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 22:22 [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures rkir--- via Qemu-devel
2019-05-23 10:22 ` Dr. David Alan Gilbert
2019-05-23 17:00   ` Roman Kiryanov via Qemu-devel
2019-05-24  9:03     ` Peter Maydell
2019-05-24 17:00       ` Roman Kiryanov via Qemu-devel
2019-05-24 17:29         ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.