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

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/21/2011 05:24 PM, Anthony Liguori wrote:
> Ok, I get what you're suggesting.  You would like to continue to keep
> VMStateDescription describing both the QEMUFile format and the fields.

I don't "like" to do that, but yes, that's what I'm suggesting. :)

You envision having the front-end (state->introspectable state 
representation) and back-end (state representation->serialization) 
completely decoupled in the future, with migration filters in the middle 
if necessary.

I actually agree this is as the final direction.  For this reason, the 
first step should be to decouple the front-end and backend and actually 
have this introspectable state representation.  Then you can also break 
VMStateDescription apart into a front-end and backend part.

Mike's patches change the way you write to QEMUFile, so the new code 
_does_ go in the right direction.  However, they fail to provide an 
introspectable, backend-independent state representation.  Let's put it 
in a few diagrams:

where we are now:        struct ---> QEMUFile
                                  ^
                                  |
                             VMStateDescription


what Mike's patches do:  struct ---> QEMUFileVisitor ---> QEMUFile
                                  ^
                                  |
                             VMStateDescription


what I proposed:         struct ---> QMPOutputVisitor ---> QObject
                                  ^
                                  |
                             VMStateDescription

                          QObject ---> QEMUFile
                                   ^
                                   |
                            VMStateDescription

where you want to go:    struct ---> QMPOutputVisitor ---> QObject
                                  ^
                                  |
                             DeviceStateDescription

                         QObject ---> QEMUFileOutputVisitor ---> QEMUFile
                                  ^
                                  |
                           VMStateDescription

As Mike's patches stand, I'm worried that they would make further steps 
harder rather than easier.  This is because I'm not convinced that the 
QEMUFileOutputVisitor is actually useful.

However, the new code from Mike's patches is very close to the 
backend-independent representation from the VMStateDescription.  So, 
Mike's patches could be morphed into this pretty easily:

                         struct ---> QMPOutputVisitor ---> QObject
                                 ^
                                 |
                          VMStateDescription

                         struct ---> QEMUFile         \
                                 ^                     \ that's savevm.c,
                                 |                     / unchanged
                            VMStateDescription        /

This is an intermediate state that lets us do the next steps:

- serialize to QEMUFile from a QObject;

- reintroduce Mike's QEMUFileOutputVisitor [I don't really see the 
benefit of this];

- split the DeviceStateDescription and the VMStateDescription;

None of these three steps are a prerequisite for introducing a new 
migration format.

> One of the reasons for this split up is because I would like to generate
> the first table by IDL and make the second table not dependent on
> structure members (so we don't end up with all the hacks we have with
> dummy struct fields).

That would be a few years away.  Let's reason in incremental steps.

Paolo