Re: [Qemu-devel] [PATCH] vhost: fix a migration failed because of vhost region merge

[Igor Mammedov]

On Wed, 19 Jul 2017 12:46:13 +0100
"Dr. David Alan Gilbert" <address@hidden> wrote:

> * Igor Mammedov (address@hidden) wrote:
> > On Wed, 19 Jul 2017 23:17:32 +0800
> > Peng Hao <address@hidden> wrote:
> >   
> > > When a guest that has several hotplugged dimms is migrated, in
> > > destination host it will fail to resume. Because vhost regions of
> > > several dimms in source host are merged and in the restore stage
> > > in destination host it computes whether more than vhost slot limit
> > > before merging vhost regions of several dimms.  
> > could you provide a bit more detailed description of the problem
> > including command line+used device_add commands on source and
> > command line on destination?  
> 
> (ccing in Marc Andre and Maxime)
> 
> Hmm, I'd like to understade the situation where you get merging between
> RAMBlocks; that complicates some stuff for postcopy.
and probably inconsistent merging breaks vhost as well

merging might happen if regions are adjacent or overlap
but for that to happen merged regions must have equal
distance between their GPA:HVA pairs, so that following
translation would work:

if gva in regionX[gva_start, len, hva_start]
   hva = hva_start + gva - gva_start

while GVA of regions is under QEMU control and deterministic
HVA is not, so in migration case merging might happen on source
side but not on destination, resulting in different memory maps.

Maybe Michael might know details why migration works in vhost usecase,
but I don't see vhost sending any vmstate data.


> 
> > > 
> > > Signed-off-by: Peng Hao <address@hidden>
> > > Signed-off-by: Wang Yechao <address@hidden>
> > > ---
> > >  hw/mem/pc-dimm.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > > index ea67b46..bb0fa08 100644
> > > --- a/hw/mem/pc-dimm.c
> > > +++ b/hw/mem/pc-dimm.c
> > > @@ -101,7 +101,7 @@ void pc_dimm_memory_plug(DeviceState *dev, 
> > > MemoryHotplugState *hpms,
> > >          goto out;
> > >      }
> > >  
> > > -    if (!vhost_has_free_slot()) {
> > > +    if (!vhost_has_free_slot() && runstate_is_running()) {
> > >          error_setg(&local_err, "a used vhost backend has no free"
> > >                                 " memory slots left");
> > >          goto out;  
> 
> Even this produces the wrong error message in this case,
> it also makes me think if the existing code should undo a lot of
> the object_property_set's that happen.
> 
> Dave
> > 
> >   
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK