[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: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t
Date: Tue, 20 Dec 2011 14:56:41 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15

On 12/20/2011 08:30 AM, Paolo Bonzini wrote:
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

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).

You are tying Visitors too closely to JSON. We should be able to write a Visitor that can output a serialization format that has more interesting integer types and maintains better fidelity with standard C types.

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

Yes...  But I don't see the relevance here.

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

This is mixing up too many concepts.

A visit function -> knows only how to walk a C data structure. It's just saying, I have an int, it's name is X, i have a double, it's name is Y.

The Visitor is the thing that plugs into the visit function and decides what to do with this information.

Having a "QEMUFile" decorator just doesn't fit the model. I'm not even sure what it means.

As always, you can implement that in many ways. However, I think the point of
using Visitors is not to remove QEMUFile.

Yes, it is.

It is to provide a backend-independent
representation that backends can transform and that secondarily can be exposed
by QOM.

The point of Visitors is to make up for the fact that C lacks introspection. It's meant to be a standard way to introspect a C data structure (or type).

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/...

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.

Yes, but there's no way to do this today without breaking the format. There's just too much magic in VMState right now. We need something like a migration filter capability where we can encapsulate this kind of logic such that we can ween VMState away from these things (and ultimately switch to an IDL compiler).

We can't do a migration filter until we have something like Michael's series.

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

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.

And Mike's series does this, no?

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

Mike's series also does this, no?

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 don't see how.


Anthony Liguori


reply via email to

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