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

[Paolo Bonzini]

On 12/20/2011 02:50 PM, Anthony Liguori wrote:
> > 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).
>
> I don't understand this. The visitor interface should expose the C
> level primitives so that we can maintain fidelity when visiting
> something. The fact that it only knows about "ints" today is a short
> cut.

Why does this need to be in Visitor?  You can always embed C knowledge 
in an adaptor or decorator.  Visitors only need to know about names and 
JSON types (well, they also distinguish int from double).

We already have such an adaptor:  QOM static properties know about 
names, JSON types, C type and struct offset.

VMState fields know about all that plus QEMUFile encoding.  QEMUFile 
encoding can be hidden in the decorator, it does not need to become 
visible to the concrete visitors.

As always, you can implement that in many ways.  However, I think the 
point of using Visitors is not to remove QEMUFile.  It is to provide a 
backend-independent representation that backends can transform and that 
secondarily can be exposed by QOM.

This is only half-true in Michael's code, because he relies on 
primitives that QMPInputVisitor and QMPOutputVisitor do not implement. 
Fixing this is quite easy, you only need to add a base-class 
implementation of the int8/int16/... primitives.

On top of this the representation he passes to visitors is somewhat 
redundant.  For example, VMState has "equal" fields; they are fields 
that are serialized but are really fixed at compile- or realize-time. 
Such fields should not be part of the backend-independent 
representation.  With Michael's approach they are, and that's quite deep 
in the implementation.

> > 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.
>
> That doesn't make effective use of visitors. Visitors should preserve
> as much type information as possible. I'm not really sure I
> understand the whole QEMUFile tie in either. This series:
>
> 1) Makes a fully compatible QEMUFile input and output Visitor
>
> 2) Makes VMState no longer know about QEMUFile by using (1)
>
> (2) is really the end goal. If we have an interface that still uses
> QEMUFile, we're doing something wrong IMHO.

Yes, this is accurate, but I see the goals differently.  We should:

(1) First and foremost, provide a backend-independent representation of 
device state so that we can add other backends later.

(2) Serialize this with QEMUFile, both for backwards-compatibility and 
to ensure that the whole thing works.

Whether you do (2) directly with QEMUFile or, like Michael does, with 
QEMUFile*Visitors is secondary.  I don't have big objections to either 
approach.  However, the series is missing (1).

Paolo