From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59252) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgIga-0000rE-Ax for qemu-devel@nongnu.org; Wed, 05 Dec 2012 12:23:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TgIgV-00079E-Ud for qemu-devel@nongnu.org; Wed, 05 Dec 2012 12:23:04 -0500 Received: from cantor2.suse.de ([195.135.220.15]:50503 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgIgV-00079A-Kk for qemu-devel@nongnu.org; Wed, 05 Dec 2012 12:22:59 -0500 Message-ID: <50BF82ED.4070205@suse.de> Date: Wed, 05 Dec 2012 18:22:53 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1354631742-4693-1-git-send-email-fred.konrad@greensocs.com> <1354631742-4693-7-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH v5 6/6] virtio-blk : Refactor virtio-blk. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: fred.konrad@greensocs.com Cc: Peter Maydell , aliguori@us.ibm.com, e.voevodin@samsung.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, stefanha@redhat.com, cornelia.huck@de.ibm.com Am 05.12.2012 17:25, schrieb Peter Maydell: > On 4 December 2012 14:35, wrote: >> From: KONRAD Frederic >> >> Create virtio-blk which extends virtio-device, so it can be connected = on >> virtio-bus. >> >> Signed-off-by: KONRAD Frederic >> --- >> hw/virtio-blk.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++= +-------- >> hw/virtio-blk.h | 4 ++ >> 2 files changed, 150 insertions(+), 24 deletions(-) >> >> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >> index e25cc96..ee1ea8b 100644 >> --- a/hw/virtio-blk.c >> +++ b/hw/virtio-blk.c >> @@ -21,24 +21,42 @@ >> #ifdef __linux__ >> # include >> #endif >> +#include "virtio-bus.h" >> >> +/* Take this structure as our device structure. */ >> typedef struct VirtIOBlock >> { >> + /* >> + * Adding parent_obj breaks to_virtio_blk cast function, >> + * and virtio_blk_init. >> + */ >> + DeviceState parent_obj; >> + /* >> + * We don't need that anymore, as we'll use QOM cast to get the >> + * VirtIODevice. Just temporary keep it, for not breaking functio= nality. >> + */ >> VirtIODevice vdev; >=20 > This doesn't make sense. After your previous patch, VirtIODevice > is-a DeviceState, and VirtIOBlock already is-a VirtIODevice, > so there's nothing to do here. Adding this parent_obj field > here is just breaking things (it would make the VirtIOBlock > into a direct child of DeviceState, which isn't what we want). >=20 >> BlockDriverState *bs; >> VirtQueue *vq; >> void *rq; >> QEMUBH *bh; >> BlockConf *conf; >> - VirtIOBlkConf *blk; >> + /* >> + * We can't use pointer with properties. >> + */ >> + VirtIOBlkConf blk; >> unsigned short sector_mask; >> DeviceState *qdev; >> } VirtIOBlock; >> >> -static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) >> -{ >> - return (VirtIOBlock *)vdev; >> -} >> +/* >> + * Use the QOM cast, so we don't need that anymore. >> + * >> + * static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) >> + * { >> + * return (VirtIOBlock *)vdev; >> + * } >> + */ >=20 > If we don't need it, just delete it. Seconded. You need to introduce a VIRTIO_BLOCK() macro, backed by OBJECT_CHECK(), and replace all callers of to_virtio_blk() with VIRTIO_BLOCK(). Compare my ISA series that I intentionally cc'ed you on. You can then rename vdev to parent_obj as a check that you caught all use= rs. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg