On Mon, May 16, 2016 at 01:48:01AM +0000, Tan, Jianfeng wrote:
> > On Fri, May 13, 2016 at 09:54:33AM +0800, Tan, Jianfeng wrote:
> > >
> > > So, I'd suggest something like following:
> > >
> > > if (is_vdev(..)) {
> > >
> > >
> > > The blocker issue of your suggestion is that we have no such condition.
> > >
> > > Previously, I use dev_type, but as David's comment said:
> >
> > That's not the only option. There should be others, for example,
> > checking the existence of virtio_user_device. Or even, you could
> > add a new flag inside virtio hw while initiating your vdev.
> >
> > > May I ask how many more such handling are needed, excluding the tx
> > queue
> > > header desc setup? And as stated, in generic, yes, we should try that.
> > >
> > >
> > > Those which need special handling:
> > > (1) vq->vq_ring_mem: it is set but never used, so it's out of question.
> > > (2) vq->virtio_net_hdr_mem and vring_hdr_desc_init
> >
> > vring_hdr_desc_init is common.
> >
> > > (3) vq->offset
> > >
> > > Just (2) and (3) so far. And the question is quite clear: where to put
> > > these
> > > two special handling.
> >
> > Apparently, you can't put it into the queue_setup(). And I still think
> > my proposal works great here.
>
> OK, since it's indeed inappropriate to put these special handlings inside
> queue_setup() from semantic perspective, I'll add them according to if
> hw->vdev_private == NULL in the driver.
I'm thinking maybe we could rename "vdev_private" to "virtio_user_dev"
(or something like that), to make it explicit that it's for virtio user
device. I mean, there should be no other vdevs after all. OTOH, using
"hw->vdev_private != NULL" to say it's a virtio-user device is a bit
weird; it doesn't even make too much sense.
--yliu
