Re: [Qemu-devel] [PATCH 0/2] vhost: two fixes

[Zhoujian (jay)]

Hi Michael,

> -----Original Message-----
> From: Zhoujian (jay)
> Sent: Friday, December 15, 2017 12:52 PM
> To: 'Michael S. Tsirkin' <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>; address@hidden;
> Huangweidong (C) <address@hidden>; Gonglei (Arei)
> <address@hidden>; wangxin (U) <address@hidden>;
> Liuzhe (Cloud Open Labs, NFV) <address@hidden>; Igor Mammedov
> <address@hidden>
> Subject: RE: [PATCH 0/2] vhost: two fixes
> 
> Hi Michael,
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:address@hidden
> > Sent: Friday, December 15, 2017 12:36 PM
> > To: Zhoujian (jay) <address@hidden>
> > Cc: Dr. David Alan Gilbert <address@hidden>;
> > address@hidden; Huangweidong (C) <address@hidden>;
> > Gonglei (Arei) <address@hidden>; wangxin (U)
> > <address@hidden>; Liuzhe (Cloud Open Labs, NFV)
> > <address@hidden>; Igor Mammedov <address@hidden>
> > Subject: Re: [PATCH 0/2] vhost: two fixes
> >
> > On Fri, Dec 15, 2017 at 02:38:35AM +0000, Zhoujian (jay) wrote:
> > > Hi Dave,
> > >
> > > > -----Original Message-----
> > > > From: Dr. David Alan Gilbert [mailto:address@hidden
> > > > Sent: Friday, December 15, 2017 3:49 AM
> > > > To: Michael S. Tsirkin <address@hidden>
> > > > Cc: Zhoujian (jay) <address@hidden>;
> > > > address@hidden; Huangweidong (C)
> > > > <address@hidden>; Gonglei (Arei)
> > > > <address@hidden>; wangxin (U)
> > > > <address@hidden>; Liuzhe (Cloud Open Labs, NFV)
> > > > <address@hidden>; Igor Mammedov <address@hidden>
> > > > Subject: Re: [PATCH 0/2] vhost: two fixes
> > > >
> > > > * Michael S. Tsirkin (address@hidden) wrote:
> > > > > On Fri, Dec 15, 2017 at 12:36:30AM +0800, Jay Zhou wrote:
> > > > > > Jay Zhou (2):
> > > > > >   vhost: add used memslot number for vhost-user
> > > > > >   vhost: double check memslot number
> > > > > >
> > > > > >  hw/virtio/vhost-user.c            | 31
> +++++++++++++++++++++++++
> > > > > >  hw/virtio/vhost.c                 | 49
> > > > ++++++++++++++++++++++++++++++++++-----
> > > > > >  include/hw/virtio/vhost-backend.h |  4 ++++
> > > > > >  3 files changed, 78 insertions(+), 6 deletions(-)
> > > > >
> > > > > Cc two developers working on these files right now.
> > > >
> > > > I have to admit to not understanding the 'used_memslots' variable.
> > > >
> > > > * It's a global in vhost.c
> > > > * but set by vhost_set_memory that's called from the listener
> > associated
> > > >   with each individual vhost
> > > > * While they're probably always the same, the merging code calls
> > > >   the vhost_backend_can_merge method for each device, so the number
> > > >   of regions can be different.
> > > >
> > >
> > > Your mean for some devices the new added MemoryRegionSection can be
> > > merged, but for others it can not be merged? IIUC the vhost_mem for
> > > each vhost_dev is the same.
> > >
> > > Meanwhile, I think it is more reasonable to add globals in
> > > vhost-backend.c and vhost-user.c respectively instead of
> > > 'used_memslots'. The reason is explained in patch 1. What do you think?
> > >
> > > Regards,
> > > Jay
> >
> > I'd rather avoid globals completely if possible.
> >
> 
> It is possible, we could add a 'used_memslots' variable in struct
> vhost_dev for per device. I will try to do it in v2.
> 

If the globals don't exist, the disadvantage I found is that the check
"if memslots number exceeds" will be moved from the beginning to the end
in vhost_dev_init, does it acceptable? Or are there other ideas to avoid
globals?

To be honest, I prefer to add globals in vhost-backend.c and vhost-user.c
respectively, the value of used_memslots for the same type of backend is the
same.

Regards,
Jay