On 10/03/2011 08:24 AM, Michael S. Tsirkin wrote:
On Mon, Oct 03, 2011 at 07:51:00AM -0500, Anthony Liguori wrote:
Here are some suggestions:
- Let's make the protocol be BER directly.
As a first step, use a single octet string for
the whole of data. Next, start splitting this up.
This can't be done without breaking the old style migration
protocol. I don't have a problem with that but I do have a problem
with repeatedly breaking migration protocol.
As long as this is within a release cycle, is this a real problem?
I think if we try to fit it within a release we'll either end up with a 2 year
long release or a half-broken conversion.
I'd rather buy ourselves time by supporting both formats. That way we can
remove the old format when we're satisfied with the ASN.1 encoding.
The Visitor interface is very little abstraction over a native BER
interface. It lets us introduce BER incrementally while still
supporting the old protocol.
- Don't try to use arrays at all. They don't make sense
in either jason or asn. Instead, use sets for unordered
data and sequences for ordered data.
sequences map to visit_start_array() pretty well. You would do something like:
visit_start_array(v, "entries", errp);
for (int i = 0; i< s->size; i++) {
visit_type_int(v, NULL,&s->entry[i], errp);
}
visit_end_array(v, errp);
Sequences can encode structures not just arrays.
How would you encode this for example:
SEQUENCE OF { VQN: INTEGER, SEQUENCE { OPTIONAL VECTOR: INTEGER} }
visit_start_array(v, "vqs", errp);
for (i = 0; i < s->n_vqs; i++) {
// Array elements never have a name, hence NULL name
visit_start_struct(v, "VirtQueue", NULL, errp);
visit_type_int(v, &s->vq[i].num, "vqn", errp);
// Given this sub-struct an arbitrary name. It could also be anonymous.
visit_start_struct(v, "MsixInfo", "msix_info", errp);
if (s->vq[i].msix_enabled) {
visit_type_int(v, &s->vq[i].vector, "vector", errp);
}
visit_end_struct(v, errp);
visit_end_struct(v, errp);
}
visit_end_array(v, errp);
This would also generate JSON of:
'vqs': [ { 'vqn': 2, 'msix_info': { 'vector': 3 } } ]
This is a natural description of a virtio device.
Yes we can unwrap this to a structure of arrays but
this is tweaking protocol to match implementation.
There's no need to from a Visitor perspective. It doesn't make it any easier.
- Most data should be unordered: we don't normally care
in which order we get device attributes.
So we would have set of sequences, and
each sequence is attribute name (ascii string) + attribute data
I'd actually be inclined to make everything ordered by default and
not encode explicit field names. I'm afraid that encoding field
names on the wire is going to easily increase the amount of device
state by an order of magnitude.
Assuming we worry about space used by name tags, let's use
content-specific tags.
- This does not discuss how we will handle cross version
migration in a lot of detail. Presumably you expect capability magic to
work for this. However this does not work for migration to file.
A self describing format makes a much simpler solution possible:
separating attributes which destination must parse from those
it can skip safely. If destination sees an attribute
that was marked as must parse and does not recognize it,
migration fails. If not, just skip the attribute.
Compatibility and the wire format are more or less orthogonal. A
self describing format helps eliminate a class of bugs but getting
compatibility right requires quite a bit more.
Regards,
Anthony Liguori
So I think we do care about compatibility.
I think this is by far the most interesting part of the problem.
And making that robust IMO pretty much requires not relying
on field order whenever we are not absolutely sure
no other order makes sense.
There are multiple things to consider with compatibility:
1) Creating compatible device models. This is a qdev problem and can't be
solved in the protocol.
2) Ensuring we are sending all the data we need to. I think we solve this
problem by autogenerating Visitors from the C definitions of the device structures.
3) Having the flexibility to morph what's sent over the wire into something that
we matches our internal state. I don't think the wire format solves this. I
think what solves this the ability to read the protocol into a data structure
such that we can manipulate the data structure in memory.
Having the ability to ignore some fields is not enough. We need to also be able
to split a single field into multiple fields, and event split a single device
into multiple devices. If we're dealing with a tree data structure in memory,
we have limitless ability to fudge things.
Visitors buy us this. Right now, we're talking about:
void virtio_marshal(QEMUFile *f, VirtioDevice *vdev);
If you just do BER encoding, that doesn't help us. We still can only read from
the wire to a VirtioDevice. If we do:
void virtio_marshal(Visitor *v, VirtioDevice *vdev);
We can still do BER encoding. We can also do QEMUFile encoding in the current
format.
But we also can Visit from a QObject. That means we can write something that
reads in BER off the wire and generates QObjects corresponding to whatever is
sent. We can then manipulate those QObjects in memory as much as we'd like
before handing a QObject Visitor to virtio_marshal.
Separating parsing off the wire from handing something to the device models and
allowing transformation in between is the key to addressing compatibility in a
robust fashion.
Regards,
Anthony Liguori
Content-specific tags as suggested above let us do unordered.
We lose the ability to easily examine
input by external tools but without field names
they won't be able to parse it so it's not all that valuable anyway.