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

[Jason Wang]

On 2018年03月29日 16:44, Dr. David Alan Gilbert wrote:
> * Jason Wang (address@hidden) wrote:
> > On 2018年03月29日 16:08, Dr. David Alan Gilbert wrote:
> > > * Jason Wang (address@hidden) wrote:
> > > > On 2018年03月29日 00:36, Dr. David Alan Gilbert (git) wrote:
> > > > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > > >
> > > > > 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?

Looks like and we can avoid using tricks like patch 4.

> > 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.

Yes it is. I think I would agree with your idea now consider it was 
slightly better than using the old behavior.

Thanks

> Dave
>
> > Thanks
> >
> > > Dave
> > > > Thanks
> > > --
> > > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK