[Qemu-devel] Re: [PATCH] rtl8139: IO memory is not part of vmstate

[Juan Quintela]

Alex Williamson <address@hidden> wrote:
> On Sun, 2010-12-12 at 20:07 +0530, Juan Quintela wrote:
>> "Michael S. Tsirkin" <address@hidden> wrote:
>> > On Sun, Dec 12, 2010 at 05:23:39PM +0530, Juan Quintela wrote:
>> >> "Michael S. Tsirkin" <address@hidden> wrote:
>> >> > On Thu, Dec 09, 2010 at 03:14:17PM -0700, Alex Williamson wrote:
>> 
>> >> > How about we keep migrating the index for the benefit of
>> >> > old versions, but ignore the value on load?
>> >> > Something like the following:
>> >> 
>> >> This was my 1st suggestion to Alex O:-)
>> >
>> > The difference here is that instead of sending garbage to the
>> > old version we send an actual index value.
>> >
>> >> So, I am in.  he think this is bad for upstream,  I don't think so (but
>> >> I understand that it is oppinable).
>> >> 
>> >> Later, Juan.
>> >
>> > I think it makes sense to fix this for the stable branch,
>> > and I think we should try as hard as we can to avoid bumping up the
>> > version number there.
>> >
>> > For master we can bump the version number but it might be easier to
>> > just keep the code the same there.
>> 
>> I think that your solution is better.  For older versions, it works as
>> expected.  For new versions, problem is fixed.  Solution is not the
>> "purest", but you can say the same about uping the version for a state
>> that is exactly the same length & fields O:-)
>
> I disagree, without bumping the version number, we can never guarantee
> the problem is behind us.

we can, if we use the latest version.

> We can always migrate to the bad version,

That is the whole point.  Bumping the version makes this impossible.

> which puts our users at risk.  The responsible behavior is to allow
> forward migrations and prevent migrations to a version with an issue
> known to compromise VM integrity.  Perhaps I feel more strongly about
> this because I actually had to debug this problem.  Obvious in
> retrospect, but a huge pain in the butt to get there.

Obviously, my point of view is different, and is related with
maintaining a stable migration ABI. So, ... I am also "biased".

We have to make a decission (in general, not just this case):
- we are going to never bump the version:
  this gives an stable ABI, but bugs stay with us forever
- we are not ever going to prettend that we care
  this makes changes trivial, as we don't have to maintain
  backward compatiblity.

And that is it.  Basically anything in the middle don't matter.  If I
have a machine definition, with only a single device that has bumped
version, I can't migrate to the backwards one.

This is the reason why I am against the changes like this, if we are
prettending that we are going to maintain the versions stable.

Notice that there are (at least) two ways to look at this specific
problem:
- don't bump the version.
  * new -> new : works
  * old -> new : works
  * new -> old : works (at least as well as old -> old that existed
                        before)
- bump the version
  * new -> new: works
  * old -> new: works
  * new -> old: fails always

Obviously, never of the options is perfect :(

Thanks, Juan.