Re: [Qemu-devel] [PATCH v9] vhost: used_memslots refactoring

[Michael S. Tsirkin]

On Tue, Mar 20, 2018 at 02:09:34AM +0000, Zhoujian (jay) wrote:
> Hi Michael,
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:address@hidden
> > Sent: Tuesday, March 20, 2018 9:34 AM
> > To: Zhoujian (jay) <address@hidden>
> > Cc: address@hidden; address@hidden; Huangweidong (C)
> > <address@hidden>; wangxin (U) <address@hidden>; Gonglei
> > (Arei) <address@hidden>; Liuzhe (Ahriy, Euler) <address@hidden>
> > Subject: Re: [PATCH v9] vhost: used_memslots refactoring
> > 
> > On Mon, Mar 05, 2018 at 05:12:49PM +0800, Jay Zhou wrote:
> > > Used_memslots is shared by vhost kernel and user, it is equal to
> > > dev->mem->nregions, which is correct for vhost kernel, but not for
> > > vhost user, the latter one uses memory regions that have file
> > > descriptor. E.g. a VM has a vhost-user NIC and 8(vhost user memslot
> > > upper limit) memory slots, it will be failed to hotplug a new DIMM
> > > device since vhost_has_free_slot() finds no free slot left. It should
> > > be successful if only part of memory slots have file descriptor, so
> > > setting used memslots for vhost-user and vhost-kernel respectively.
> > 
> > 
> > Below should go after ---
> 
> Thanks for reminding.
> 
> > 
> > > v7 ... v9:
> > >  - rebased on the master
> > > v2 ... v6:
> > >  - delete the "used_memslots" global variable, and add it
> > >    for vhost-user and vhost-kernel separately
> > >  - refine the function, commit log
> > >  - used_memslots refactoring
> > >
> > > Signed-off-by: Igor Mammedov <address@hidden>
> > > Signed-off-by: Jay Zhou <address@hidden>
> > > Signed-off-by: Liuzhe <address@hidden>
> > 
> > When built with clang this causes runtime warnings (during make check) about
> > misaligned access to structures.
> > 
> > The issue is that vhost_user_prepare_msg requests VhostUserMemory which
> > compiler assumes but is then used with a pointer into a packed structure -
> > where fields are not aligned.
> 
> Sorry I missed the patch you have sent to fix the alignment, I have replied
> to that thread.

I've dropped this from the pull for now.  Sorry about that.  Once next
pull is merged, pls rebase and post a version fixing up the alignment
issues.

Thanks for your effort!

-- 
MST