On Wed, Nov 13, 2013 at 03:26:36PM -0500, Vlad Yasevich wrote: > On 11/13/2013 03:00 PM, Michael S. Tsirkin wrote: > >On Wed, Nov 13, 2013 at 11:21:18AM -0500, Vlad Yasevich wrote: > >>On 11/10/2013 07:11 AM, Michael S. Tsirkin wrote: > >>>On Fri, Nov 08, 2013 at 02:42:27PM -0500, Vlad Yasevich wrote: > >>>>What about this approach? This only updates the monitory when all the > >>>>bits have been written to. > >>>> > >>>>-vlad > >>> > >>> > >>>Thanks! > >>>Some comments below. > >>> > >>>>-- >8 -- > >>>>Subject: [PATCH] e1000/rtl8139: update HMP NIC when every bit is written > >>>> > >>>>We currently just update the HMP NIC info when the last bit of macaddr > >>>>is written. This assumes that guest driver will write all the macaddr > >>>>from bit 0 to bit 5 when it changes the macaddr, this is the current > >>>>behavior of linux driver (e1000/rtl8139cp), but we can't do this > >>>>assumption. > >>> > >>>I would rather say "it seems better not to make this assumption". > >>>This does look somewhat safer than what Amos proposed. > >>> > >>>> > >>>>The macaddr that is used for rx-filter will be updated when every bit > >>>>is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC > >>>>info when every bit has been changed. It will be same as virtio-net. > >>>> > >>>>Signed-off-by: Vlad Yasevich <vyase...@redhat.com> > >>>>--- > >>>> hw/net/e1000.c | 17 ++++++++++++++++- > >>>> hw/net/rtl8139.c | 14 +++++++++----- > >>>> 2 files changed, 25 insertions(+), 6 deletions(-) > >>>> > >>>>diff --git a/hw/net/e1000.c b/hw/net/e1000.c > >>>>index 8387443..a5967ed 100644 > >>>>--- a/hw/net/e1000.c > >>>>+++ b/hw/net/e1000.c > >>>>@@ -149,6 +149,10 @@ typedef struct E1000State_st { > >>>> #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT) > >>>> #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT) > >>>> uint32_t compat_flags; > >>>>+ uint32_t mac_changed; > >>> > >>>Hmm why uint32_t? uint8_t is enough here isn't? > >>> > >>>This new state has to be migrated then, and > >>>we need to fallback to old behaviour if migrating to/from > >>>an old version (see compat_flags for one way to > >>>detect this compatibility mode). > >>> > >> > >>Hi Michael > >> > >>I started looking at migrating this thing and I now starting to question > >>the whole approach. > >> > >>The only reason to migrate this is if we can migrate between writes to > >>the mac address registers. > > > >Absolutely. For some reason below you only discuss cross version > >migration but it needs to be migrated for same version migration too. > > > >> We can fairly easily solve the issue of > >>migrating from net to old versions. > > > >I'm not sure it's easier, but it needs to happen anymore. > > > >> The more interesting question > >>is migrating from old to new versions. > >> > >>If someone is migrating from an older version (without this feature) > >>to a newer version and doing so between writes, the bitmap state will > >>have no idea that a partial write has already happened. The completing > >>write will just set one of the bits and notifications that we are > >>looking for do not happen. > >> > >>-vlad > > > >Yep, that's a problem too. > > > > > >For 1.8 just send the bitmap across. > > Right. That's simple enough. > > > > >For cross version migration I would say we should just detect -M 1.7 > >and older and just emulate old behaviour, disregard the bitmap > >completely. > >Don't do special tricks just for migration. > > > > The compat flags allow for simple migration to 1.7. However, it's the > migration from 1.7 to 1.8 that's the issue.
It's an issue because you are trying to migrate to a machine that behaves differently from 1.7. If we update mac on last byte write there would not be an issue. > There is a version number on the E1000 state and I am thinking of maybe > bumping that so that we can catch older state that doesn't support mac > sate change. Thoughts? > > -vlad No, we need migration to work cross version if matching -M flag is used on both sides. Stop thinking about migration specifically. think about emulation with -M 1.7 generally. What it should do is behave same way as 1.7 behaves. So just implement that: with -M 1.7 only update on last byte write. And then migration becomes simple, with no need for version bumps. -- MST