Am 05.12.2012 17:25, schrieb Peter Maydell: > On 4 December 2012 14:35, <fred.kon...@greensocs.com> wrote: >> From: KONRAD Frederic <fred.kon...@greensocs.com> >> >> Create virtio-blk which extends virtio-device, so it can be connected on >> virtio-bus. >> >> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com> >> --- >> 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 <scsi/sg.h> >> #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 functionality. >> + */ >> VirtIODevice vdev; > > 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). > >> 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; >> + * } >> + */ > > 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 users. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg