All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
Date: Thu, 18 Mar 2010 08:36:26 +0100	[thread overview]
Message-ID: <m3iq8unksl.fsf@trasno.mitica> (raw)
In-Reply-To: <20100318064015.GA16973@redhat.com> (Michael S. Tsirkin's message of "Thu, 18 Mar 2010 08:40:15 +0200")

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Mar 16, 2010 at 07:51:16PM +0100, Juan Quintela wrote:
>> Hi
>> 
>> This series introduces several virtio cleanups:
>> - add comment to pci (mst)
>> - tell virtio about DO_UPCAST
>
> I think we should move away from struct layout assumptions that
> DO_UPCAST enforces, and to use container_of where possible.
> I'll post a series shortly that do this for virtio.

Not in this case.  qdev needs it to be in that order, and that will not
change without changing everything again.   Look at the rest of hw/*.

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".

The improtant bit is the patch is:

+    VirtIODevice *vdev = virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
+                                            sizeof(struct virtio_blk_config),
+                                            sizeof(VirtIOBlock));
+    s = DO_UPCAST(VirtIOBlock, vdev, vdev);

You can't have a virtio_common_init() that initializes the shared bits
ad init them in the middle of nowhere.  It _needs_ to be at the
beginning of the shared struct.  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.

>> - use QLIST instead of one open list
>> - virtio-pci/msix: remove duplicated test
>> 
>> Please review and apply.
>> 
>> This is split for a series previously sent.  Will send the vmstate
>> conversions as a different series on top of this one.
>> 
>> Later, Juan.
>> 
>> Juan Quintela (8):
>>   virtio: Teach virtio-balloon about DO_UPCAST
>>   virtio: Teach virtio-blk about DO_UPCAST
>>   virtio: Teach virtio-net about DO_UPCAST
>>   virtio: Use DO_UPCAST instead of a cast
>>   virtio-pci: Remove duplicate test
>>   QLIST: Introduce QLIST_COPY_HEAD
>>   virtio-blk: change rq type to VirtIOBlockReq
>>   virtio-blk: use QLIST for the list of requests
>> 
>> Michael S. Tsirkin (1):
>>   qemu/pci: document msix_entries_nr field
>> 
>>  hw/msix.c           |    8 -------
>>  hw/pci.h            |    4 ++-
>>  hw/virtio-balloon.c |   15 ++++---------
>>  hw/virtio-blk.c     |   54 ++++++++++++++++++++++++--------------------------
>>  hw/virtio-net.c     |   29 +++++++++++----------------
>>  hw/virtio-pci.c     |    7 +++--
>>  qemu-queue.h        |    4 +++
>>  7 files changed, 54 insertions(+), 67 deletions(-)
>> 
>> 

  reply	other threads:[~2010-03-18  7:36 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 [this message]
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
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=m3iq8unksl.fsf@trasno.mitica \
    --to=quintela@redhat.com \
    --cc=mst@redhat.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.