Re: [Qemu-devel] [PATCH v2 5/6] e1000: Choose which set of props to migrate

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Jason Wang (jasowang@redhat.com) wrote:
> 
> 
> On 2018年03月29日 16:08, Dr. David Alan Gilbert wrote:
> > * Jason Wang (jasowang@redhat.com) wrote:
> > > 
> > > On 2018年03月29日 00:36, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > When we're using the subsection we migrate both
> > > > the 'props' and 'tso_props' data; when we're not using
> > > > the subsection (to migrate to 2.11 or old machine types) we've
> > > > got to choose what to migrate in the main structure.
> > > > 
> > > > If we're using the subsection migrate 'props' in the main structure.
> > > > If we're not using the subsection then migrate the last one
> > > > that changed, which gives behaviour similar to the old behaviour.
> > > > 
> > > > 
> > > But only after migration. Why not simply switch back to the old behavior if
> > > migrate_tso_props if false?
> > Because:
> >    1) We know it's a broken behaviour so it's better not to unfix it
> >    2) The fix doesn't change guest visible behaviour other than actually
> >       sending the right packets; so there's no reason to make the fix
> >       itself dependent on the machine type.
> >    3) Gating the fix itself on the flag is actually more complex and
> >       would need checking the flag in lots of places that are already
> >       pretty complex, rather than what this does which is just check it
> >       in one place at migration.
> 
> It looks to me it was just something like:
> 
>     struct e1000x_txd_props *props = tp->cptse && chkflag(TSO) ?
> &tp->tso_props : &tp->props;

is that the only thing that would change?

> Btw, did this patch work when:
> 
> migrate A to B
> migrate B to C
> 
> But there's no tx during B?

Hmm, good question; I only tried it keeping the stream alive during
migration.
Lets see what happens.

For this code to be used we have to be running with an old machine
type/property.
That means A->B will have either come from 2.11 or a 2.12 with this same
patch.
But given patch 2 that duplicates on loading; that means A->B should
end up with B having the same data in both sets of props, and
thus it doesn't matter which set this patch picks.

Dave

> Thanks
> 
> > 
> > Dave
> > > Thanks
> > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK