[Qemu-devel] Re: [PATCH 0/9] Virtio cleanups

[Juan Quintela]

"Michael S. Tsirkin" <address@hidden> wrote:

>>   Look at the rest of hw/*.
>
> I think you mean that a similar assumption is made by lots of
> other code. Does not mean it's always a good idea and that
> we need to force this assumption everywhere.

Principle of Least Surprise.  as posted in the other email, removing
that assumption don't bring us anything and just make code more complex
(not a huge ammount, but more complex).

>> The only "sane" way of doing OOP on C is to use the super-class memmbers as 
>> the
>> 1st member of the sub-classes.  That will not change anytime soon.  And
>> trying to "emulate" multiple inheritance in C is completely "insane".
>
> container_of does it and I think it's sane.
> People like comparing it to inheritance but for me it's just
> using a field in a structure. multiple fields in a structure are insane?

It is inheritance.

we assume in virtio_common_init() can be called for all virtio devices.
That means that all virtio devices have "something" in common.
That is the basic concept of super-class and sub-class.  Yes, C sucks
for describing that kind of relationships, but that is a different matter.

>> This "assumption" is used for all the
>> code left and right.  It is an essentioal part of the qdev design, not
>> something that can be changed easily.
>
> I do not think it is essential at all. We just add an offset.
> Yes we make such assumptions a lot but it's a bug, not a feature.

Clearly, we don't agree here.  For me it is a "feature" that all virtio
devices are in the way:

struct virtio_common {
...
}

struct virtio_specific {
       struct virtio_common common;
       <specific fields>
}

And then I can pass virtio_specific pointers to virtio_common routines
and things just work.  Special importance when we have callbacks, and
virtio_specific parts are only used in virtio_specific.c file.

Why do you want to break that assumption that is used (in a good place)
in a lot of qemu code (qdev "requires" it)?  For me is a clear case of
coherence.  Virtio* can live with container_of() instead of DO_UPCAST()
because they are not exposed (directly) through qdev.  Then mark it as
different that everything else, indeed if it don't bring us anything,
just to confuse new users.  This is one of the features that I hate
more,  searching for how to use a qemu api because from only the
prototypes it is not clear, and just pick an example, and that one uses
it in a non-conventional way.

Later, Juan.