Re: [Qemu-devel] [PATCH 0/3] e1000 migration changes for 2.12

[Jason Wang]

On 2018年03月27日 22:26, Dr. David Alan Gilbert wrote:
> * Jason Wang (address@hidden) wrote:
> > On 2018年03月27日 19:34, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <address@hidden>
> > >
> > > Hi Ed, Jason,
> > >     This set of patches change the e1000 migration code to make
> > > it easier to keep with compatibility with older versions in backwards
> > > migration;  but I do need some advice whether I need to do more as well.
> > >
> > > I think the first and second patch are fairly uncontrovercial and I
> > > would like them for 2.12, since it'll make any future changes easier.
> > > The third one changes the default behaviour, so again I'd prefer it but
> > > lets see what you think.
> > The patches looks good to me. So for the changes of default behavior, did
> > you mean we can make the migration to older versions work?
> Right.
>
> > > My question however, without knowing the internals of the e1000, is
> > > whether when ommitting the subsection, should the code in 2.12 be
> > > changing the data it sends back in the main section of data?
> > I'm not sure I get the meaning here. But it looks to me turning it off for
> > old machine types makes sense, otherwise, management need to set it
> > explicitly.
> OK, let me expand the question a bit.
> If I understand correctly the d6244b description, there's two pieces of
> state (TSO and non-TSO) where there used to be only one.
> Now, with the new format we migrate both pieces of state, but lets think
> about what happens if we have to migrate only one piece.
>
> a) 2.11->2.11 migrated the only piece it knew about; so I guess the only
> problem was really the UDP corruption mentioned in the commit.
>
> b) 2.11->2.12 - it receives the wrongly merged piece of state and puts it
> in - well which of the two states does it load it into?  What's the
> effect of that?

So I think what we can do in this case is keep the (buggy) behavior 
here. E.g put the state into both props and tso_props.

> c) 2.12(+my mod)->2.11 ok, so 2.12 will have filled in both sets of state
> internally; but now it's only going to send one of them over to 2.11 -
> which one gets sent to 2.11? Is it the one that 2.11 is expecting?

Then we can keep the behavior of 2.11 when migrate_tso_props (probably 
need a better name) is false we will use props for all contexts.

> d) 2.12(+my mod)->2.12(+my mod) with an old machine type, again we're only
> going to send one set of data (same as c) - but what's 2.12 going to
> make of the one piece of state received?

So if we do like above, guest will see buggy device after migration. 
(But do we really care this case?).

Thanks

> (b) is an existing question.
>
> Dave
>
> > Thanks
> >
> > > Dave
> > >
> > >
> > > Dr. David Alan Gilbert (3):
> > >     e1000: Convert v3 fields to subsection
> > >     e1000: wire new subsection to property
> > >     e1000: Old machine types, turn new subsection off
> > >
> > >    hw/net/e1000.c      | 46 ++++++++++++++++++++++++++++++++++------------
> > >    include/hw/compat.h |  4 ++++
> > >    2 files changed, 38 insertions(+), 12 deletions(-)
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK