[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
Date: Tue, 20 Dec 2011 12:43:31 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1

On 12/20/2011 12:12 PM, Paolo Bonzini wrote:

I think this approach is wrong.  We're mashing the design of vmstate
with that of visitors and getting something that is not a visitor and
not vmstate.

Instead, I think you should have something like this:

     struct VMStateInfo {
         const char *name;
         // takes a QMPOutputVisitor and a QEMUFile open for reading
         int (*load)(QEMUFile *f, const char *name, Visitor *v,
                     size_t size, Error **err);

         // takes a QMPInputVisitor and a QEMUFile open for writing
         void (*save)(QEMUFile *f, const char *name, Visitor *v,
                     size_t size, Error **err);

         // takes a QMPOutputVisitor and reads from *pv
         int (*get)(Visitor *v, const char *name, void *pv,
                    size_t size, Error **err);

         // takes a QMPInputVisitor and writes into *pv
         void (*set)(Visitor *v, const char *name, void *pv,
                    size_t size, Error **err);

that splits the existing callbacks in two steps.

For saving, you would adapt your visitor-based vmstate "put" routines so
that they put things in a dictionary with no regard for integer types (a
bit ugly for uint64, but perfectly fine for everything else).  You take
the dictionary from the output visitor and (with an input visitor) you
feed it back to the "save" routines, which convert the dictionary to a
QEMUFile.  Both steps keep the types internal to vmstate.

For loading, it's the other way round: you interpret the vmstate with
the QEMUFile reading routines, and create a dictionary.  Then make an
input visitor and use the vmstate "set" interpreter to fill in the
struct fields.

I'm sorry for noticing this just now, I was waiting for Anthony's QOM
plans to be committed so that I could understand better how vmstate and
QOM properties would interact.  In fact, it would be great and not hard
if the struct<->visitor step (get/set) was also exposed as a QOM property.

Note that this doesn't prevent sharing the code for loading and saving.

1) You can still add a vtable to QEMUFile for "visit_type_int*" and "visit_type_uint*". But this vtable doesn't need start/end callbacks.

2) Similarly, I don't object to adding visit_type_int* and visit_type_uint* to Visitor. However, these should be exclusively a convenience for the callers, so that they do not have to convert between int64 and other types. There should be exactly two implementations of these callbacks, one for input visitors (including e.g. the dealloc visitor) and one for output visitors. I attach a sample patch that does this for int16 only.


Attachment: qapi-visitor-types.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]