| Message ID | 20190522222224.244714-1-rkir@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vmstate: Add VMSTATE_OPAQUE to save/load complex data structures | expand |
* 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
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.
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
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.
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
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