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
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).
The main goal is to abstract away data serialization schemes
(QObject->JSON, C->QEMUFile, etc). 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.
And beyond QEMUFile, we'd like to eventually move to a serialization
scheme that is self-describing in the types of the fields it stores so
that we can do stricter checking in the deserialization/input visitor
routines, or just plain be able to make sense of the serialized data
without any outside information, since those schemes would eventually be
used directly in implementing a new migration wire protocol and/or
device state introspection.
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.
And these are both requirements to implementing a robust, flexible
serialization/Visitor interface with pluggable back-ends, but if those
interface throw away the type/field names then the only way to get them
back is to deserialize, which isn't useful for introspection, and
volatile for migration (since type errors can be silently missed in a
lot of cases)
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.
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.
Yup, that's the plan. These patches are a bit lazy in that regard. I
agree that if we get into the habit of adding interfaces for a specific
back-end without mapping these to the base-class implementations in
other backends things will get out of hand quickly. Fortunately we
haven't yet hit a situation where one backend ends up adding an
interface that other backends cant' handle in some form.
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.
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).
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)?
Paolo
