Re: [Qemu-devel] [PATCH v2 5/6] contrib: add libvhost-user

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 18, 2016 at 04:21:57PM +0000, Felipe Franciosi wrote:
> 
> > On 18 Oct 2016, at 16:25, Marc-André Lureau <mlureau@redhat.com> wrote:
> > 
> > Hi Felipe
> > 
> > ----- Original Message -----
> >> Hello,
> >> 
> >>> On 18 Oct 2016, at 10:24, Marc-André Lureau <marcandre.lureau@redhat.com>
> >>> wrote:
> >>> 
> >>> <...>
> >>> 
> >>> diff --git a/contrib/libvhost-user/libvhost-user.h
> >>> b/contrib/libvhost-user/libvhost-user.h
> >>> 
> >>> <...>
> >>> 
> >>> +#define VHOST_MAX_NR_VIRTQUEUE 8
> >>> +#define VIRTQUEUE_MAX_SIZE 1024
> >> 
> >> I think that the maximum number of VQs should be 1024 to match Qemu's.
> >> 
> >> http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio.h;h=b913aac45589449bcc5d8161651332f4b0d69c7f;hb=HEAD#l55
> > 
> > That would make the VuDev structure quite big. We may want to set the nr of max queues in vu_init() instead, and allocate it there. I think this is rather a current limitation, but does not prevent iterating from there.
> 
> Well, it depends what we call "quite big". The only thing that depends on that is:
> 
> > struct VuDev {
> > <...>
> >     VuVirtq vq[VHOST_MAX_NR_VIRTQUEUE];
> > <...>
> 
> And VuVirtq is 96 bytes in size (x86_64). So we're really talking about 768 bytes vs 96 KiB.
> 
> Actually, you can bring it down to 88 bytes by...
> 
> > typedef struct VuRing {
> >     unsigned int num;
> >     struct vring_desc *desc;
> >     struct vring_avail *avail;
> >     struct vring_used *used;
> >     uint64_t log_guest_addr;
> >     uint32_t flags;
> 
> ... moving "flags" just below "num". Allows gcc to compact the struct better.
> 
> > } VuRing;
> 
> 
> I'd rather fix this kind of thing sooner than later, but it's ultimately up to you.
> 
> Felipe

I suspect the right thing to do is allocating it dynamically,
I'm fine merging as is for now though.

-- 
MST