Re: [Qemu-devel] [PATCH 3/3] e1000: Old machine types, turn new subsection off

[Dr. David Alan Gilbert]

* Paolo Bonzini (address@hidden) wrote:
> On 27/03/2018 13:34, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > Turn the newly added subsection off for old machine types
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  include/hw/compat.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index bc9e3a6627..13242b831a 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -14,6 +14,10 @@
> >          .driver   = "vhost-user-blk-pci",\
> >          .property = "vectors",\
> >          .value    = "2",\
> > +    },{\
> > +        .driver   = "e1000",\
> > +        .property = "migrate_tso_props",\
> > +        .value    = "off",\
> >      },
> >  
> >  #define HW_COMPAT_2_10 \
> > 
> 
> Unfortunately it's not that easy, because on old machine types
> tx.tso_props was stored in tx.props.

OK, this pretty much sounds like the answer to the question I asked on
the cover letter.

> So if the subsection is absent you
> have to migrate either tx.tso_props or tx.props, depending on s->tx.cptse.

Do you mean when sending you have to decide which set to send in the
non-subsection data?  And with cptse true that means use tso_props?

> Likewise if you migrate from older versions: if s->tx.props.tse &&
> s->tx.cptse, you have to copy s->tx.props to s->tx.tso_props and clear
> s->tx.props.


I don't see any equivalent code in the existing non-subsection postload to
do this; so I'm guessing there are some cases of 2.11->2.12 that will
break at the moment?

> My understanding is that s->tx.tso_props.tse will be 1 if
> and only if the source sent s->tx.tso_props.

I don't see anything in the current code that migrates tso_props.tse -
where does it come from?

> This seems most easily done with a new field (e.g. vmstate_fixed_props)
> that is written in pre_save and set in post_load.

It might need a VMSTATE_WITH_TMP to be able to do the saving part;
when saving we can't change the current state when migrating
to an old destination in case the migration fails and we just continue.

How would I test this lot?

Dave

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