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

[Paolo Bonzini]

On 12/20/2011 09:22 PM, Michael Roth wrote:
> The main goal is to abstract away data serialization schemes
> (QObject->JSON, C->QEMUFile, etc).

Right.  And the simplest way to abstract the scheme is to provide a 
backend-independent representation of device state.

As a convention, I'll call:

- "device state" the fields in the struct

- "QEMUFile" the serialized output that VMState save produces

- "device state representation" a representation of device state that is 
independent of the actual serialization backend.

- "device configuration" is the parameters that are used to create the 
device (coming from compile-time constants or the command-line)

> In the case of a JSON-based serialization, the visitor interface for
> fixed-with types would end up serializing everything as
> int64_t/double, but QEMUFile requires byte-length affinity to remain
> backward-compatible, so that information must be passed on to the
> Visitor interface when we call it.

When creating the device state representation from either device state 
or QEMUFile, you need the byte length to fetch fields correctly.

When recreating device state from its representation, or saving to 
QEMUFile state, you need the byte length to store fields correctly.

However, you do not need the byte length in the device state 
representation.  In all four cases (from/to device state, from/to 
QEMUFile) the byte length can be fetched from the VMState.

> > 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.
>
> Agreed, it's just a matter of wanting to maintain that information from
> start to finish.

I don't see the point of maintaining the type hints from start to 
finish, as long as you can reconstruct it any time you want it.

> > 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 mean, for instance, put_int32()/get_int32_equal()? If so, I'm not
> sure I follow. In that case we use a Visitor purely to
> serialize/deserialize an int32, vmstate adds the *_equal() interface as
> helper function on top of that, but it's not part of the Visitor
> interfaces.

It should be only in QEMUFile, because it's one of its quirks.  It is a 
separate property that is fixed at compile-time or realize-time, so it's 
part of device configuration; it's not part of device state representation.

> > 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).
>
> I'll fix up the existing non-QEMUFile Visitor backends with base-class
> implementations for all the new interfaces. Beyond that, is there
> anything else missing to achieve 1)?

I think almost nothing (you need to integrate into the QOM properties, 
and to handle a few special cases such as VMSTATE_EQUAL and 
VMSTATE_UNUSED).  That's quite good.

However, once you do this, you will still be serializing QEMUFile 
directly from device state rather than from device state representation. 
 This is fine, but not what we should do when working on BER 
serialization or similar.

Paolo