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

[Juan Quintela]

"Michael S. Tsirkin" <address@hidden> wrote:
> On Tue, Mar 23, 2010 at 11:40:46AM +0000, Paul Brook wrote:
>> > > Right.  The only real challenge is dealing with legacy save/restore and
>> > > command line syntax.  For save/restore, we can possibly have a dummy
>> > > device that can split the VirtioPCI device state from the virtio device
>> > > states and do the right thing.
>> > >
>> > > I'm not sure what we should do for command line syntax.  We can keep
>> > > -drive working as is.  Do we need to support -device based creation?  I
>> > > don't think we've really considered what to do in a situation like this.
>> > 
>> > If we need to change command line because of an implementation
>> > change, IMO something is wrong with the design.
>> > Users shouldn't care about non-existent virtio bus.
>> 
>> I don't find this argument convincing. If we need to change the
>> internal structure of a machine, then users who manipulate the machine
>> configuration are going to have to compensate for this.
>> This kind of change is pretty much unavoidable when we get the device
>> model wrong.
>
> I am yet to see why the model is wrong. virtio devices
> on pci bus and on s390 bus are different virtual hardware
> devices. There's no emulated bus.

Look the other way around.  You don't want to see :(

> This is not much different from e100 - it can emulate tens
> of device variants - e100 bus?

it is.  very different.  All e100 implementation is in eepro100.c.  And
all hangs from the PCI bus -> where to put PCIDevice (or DeviceState if
your preffer is trivial) -> in PCIDevice.

In this case you want to:

- share virtio bits between virtio devices
- share virtio-pci bits between virtio-pci devices
- implement each virtio-device in a different file

Fix is trivial if you are ok with the e100 example.  Just concatenate
all virtio* files in a single one.  Force all virtio-pci to know about
virtio-sysborg and virtio-s390.  And the rest of things.  And you are
done.

And no, for more that you complain that qemu model is wrong, that it
should allow multiple inheritance, it would not appear from nowhere.
support is not there at this point -> virtio devices use a hack to
simulate it.


> Anyway, people really want to share implementation and
> want to do this by means of a bus, ok, but there is nothing here
> that users care about.

Here we agree.  I haven't think about the vmstate changes because I
don't have still so clear how our system would look.

> And if we let implermentation leak out to command line, we will have
> compat problems down the line.

Problem here was the model=virtio vs model=virtio-{net,blk,...} that
gerd showed.  I don't know if there is a way to hack qdev to allow this
sharing of a single name.

>> The best we can realistically do is avoid making these
>> changes on a stable branch, and arrange for outdated configs to be
>> rejected rather than silently doing the wrong thing.
>> 
>> Paul
>
> Practically speaking, we are bound to change internal representation in
> the future, and breaking scripts, management tools, documentation and
> simple user habits with each such change would be very bad.

Here we are (again), back at square 0.

Current implementation is hackish, every "cleaner" alternative is not
backward compatible.  And to make it backward compatible, we need to add
(at least) as much hackinesh as current implementation.

Later, Juan.