From: Juan Quintela <quintela@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>,
Paul Brook <paul@codesourcery.com>,
qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
Date: Tue, 23 Mar 2010 13:32:11 +0100 [thread overview]
Message-ID: <m3hbo71ano.fsf@trasno.mitica> (raw)
In-Reply-To: <20100323115836.GA24552@redhat.com> (Michael S. Tsirkin's message of "Tue, 23 Mar 2010 13:58:36 +0200")
"Michael S. Tsirkin" <mst@redhat.com> 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.
next prev parent reply other threads:[~2010-03-23 12:32 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-16 18:51 [Qemu-devel] [PATCH 0/9] Virtio cleanups Juan Quintela
2010-03-16 18:51 ` [Qemu-devel] [PATCH 1/9] qemu/pci: document msix_entries_nr field Juan Quintela
2010-03-16 18:51 ` [Qemu-devel] [PATCH 2/9] virtio: Teach virtio-balloon about DO_UPCAST Juan Quintela
2010-03-18 7:29 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-16 18:51 ` [Qemu-devel] [PATCH 3/9] virtio: Teach virtio-blk " Juan Quintela
2010-03-18 7:29 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-16 18:51 ` [Qemu-devel] [PATCH 4/9] virtio: Teach virtio-net " Juan Quintela
2010-03-18 7:29 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-16 18:51 ` [Qemu-devel] [PATCH 5/9] virtio: Use DO_UPCAST instead of a cast Juan Quintela
2010-03-18 7:30 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-16 18:51 ` [Qemu-devel] [PATCH 6/9] virtio-pci: Remove duplicate test Juan Quintela
2010-03-18 7:25 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-18 8:26 ` Juan Quintela
2010-03-18 8:47 ` Michael S. Tsirkin
2010-03-18 8:59 ` Juan Quintela
2010-03-18 9:11 ` Michael S. Tsirkin
2010-03-18 11:40 ` Juan Quintela
2010-03-18 13:24 ` Michael S. Tsirkin
2010-03-18 13:47 ` Juan Quintela
2010-03-16 18:51 ` [Qemu-devel] [PATCH 7/9] QLIST: Introduce QLIST_COPY_HEAD Juan Quintela
2010-03-16 18:51 ` [Qemu-devel] [PATCH 8/9] virtio-blk: change rq type to VirtIOBlockReq Juan Quintela
2010-03-18 7:27 ` [Qemu-devel] " Michael S. Tsirkin
2010-03-16 18:51 ` [Qemu-devel] [PATCH 9/9] virtio-blk: use QLIST for the list of requests Juan Quintela
2010-03-18 6:40 ` [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups Michael S. Tsirkin
2010-03-18 7:36 ` Juan Quintela
2010-03-18 7:42 ` Michael S. Tsirkin
2010-03-18 8:36 ` Juan Quintela
2010-03-18 9:07 ` Michael S. Tsirkin
2010-03-18 11:53 ` Juan Quintela
2010-03-18 12:33 ` Michael S. Tsirkin
2010-03-18 13:43 ` Juan Quintela
2010-03-18 13:47 ` Michael S. Tsirkin
2010-03-18 14:21 ` Juan Quintela
2010-03-18 17:13 ` Michael S. Tsirkin
2010-03-19 1:41 ` Jamie Lokier
2010-03-21 14:31 ` Michael S. Tsirkin
2010-03-21 18:11 ` Jamie Lokier
2010-03-21 19:16 ` Michael S. Tsirkin
2010-03-22 1:06 ` Juan Quintela
2010-03-22 2:51 ` Anthony Liguori
2010-03-22 13:30 ` Paul Brook
2010-03-22 14:49 ` Anthony Liguori
2010-03-22 14:50 ` Michael S. Tsirkin
2010-03-22 15:03 ` Anthony Liguori
2010-03-22 15:17 ` Michael S. Tsirkin
2010-03-22 15:50 ` Anthony Liguori
2010-03-22 16:16 ` Paul Brook
2010-03-22 18:48 ` Anthony Liguori
2010-03-22 21:00 ` Paul Brook
2010-03-23 1:13 ` Anthony Liguori
2010-03-22 15:51 ` Paul Brook
2010-03-22 17:19 ` Michael S. Tsirkin
2010-03-22 22:16 ` Juan Quintela
2010-03-23 0:49 ` Paul Brook
2010-03-23 1:16 ` Anthony Liguori
2010-03-23 10:47 ` Michael S. Tsirkin
2010-03-23 11:11 ` Gerd Hoffmann
2010-03-23 11:40 ` Paul Brook
2010-03-23 11:58 ` Michael S. Tsirkin
2010-03-23 12:32 ` Juan Quintela [this message]
2010-03-21 19:57 ` Michael S. Tsirkin
2010-03-22 1:13 ` Juan Quintela
2010-03-22 8:37 ` Michael S. Tsirkin
2010-03-18 10:05 ` Michael S. Tsirkin
2010-03-18 15:43 ` Gerd Hoffmann
2010-03-18 16:20 ` Juan Quintela
2010-03-18 16:36 ` Gerd Hoffmann
2010-03-18 17:08 ` Juan Quintela
2010-03-18 17:21 ` Michael S. Tsirkin
2010-03-18 19:37 ` Juan Quintela
2010-03-18 20:07 ` Anthony Liguori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3hbo71ano.fsf@trasno.mitica \
--to=quintela@redhat.com \
--cc=kraxel@redhat.com \
--cc=mst@redhat.com \
--cc=paul@codesourcery.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.