Re: [PULL 01/30] migration: Fix migration crash when target psize larger than host

[Peter Xu <peterx@redhat.com>]

On Fri, Feb 10, 2023 at 06:28:36PM +0300, Michael Tokarev wrote:
> 10.02.2023 18:01, Peter Xu пишет:
> 
> > Thanks, Juan.
> > 
> > I think Michael was correct that d9e474ea56 is only merged after our most
> > recent release (which is v7.2.0).  So it doesn't need to have stable copied
> > (aka, it doesn't need to be applied to any QEMU's stable tree).
> > 
> > Juan, could you help drop the "cc: stable" line if the pull is going to
> > have a new version?
> 
> It has been applied to master already, - this is where I picked it from.

Ah I didn't notice that.

> 
> > Side note: I think in the case where we have wrongly have the cc:stable it
> > shouldn't hurt either, because when the stable tree tries to pick it up
> > it'll find it doesn't apply at all, then a downstream-only patch will be
> > needed, then we'll also figure all things out, just later.
> 
> There's absolutely nothing wrong with that.  I was just not sure about my
> own sanity here, and decided to ask.  Maybe the problem was deeper and a
> more careful backport is needed.

Thanks for raising this.

The old tree should be good with guest psize > host psize not only because
the assertion was not there before, but also because we used the right
boundary (as hostpage_boundary here):

     size_t pagesize_bits =
         qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
     unsigned long hostpage_boundary =
         QEMU_ALIGN_UP(pss->page + 1, pagesize_bits);

And it also matches with Thomas's bisection result, where the bug report
came from.

TL;DR: I'm pretty sure the old code was good, hence no explicit backport
needed.

> 
> Speaking of -stable, on my view, it is better if "too many" things will be
> tagged for it and filtered out, than some important things will not be
> tagged.

Agreed.

I guess we shouldn't blindly apply cc:stable because it'll add unnecessary
burden to the stable tree maintainers on figuring things out later, IOW it
should be a question being thought thoroughly when the developer was
working on the patch.

Normally it should be the maintainers' role to double check especially when
one patch should apply stable but it got missing (which should happen more
frequently..).

In the ideal world of perfect developers and active tree maintainers, the
cc:stable should be 100% accurate because the judgement should be able to
have been made with existing knowledge, then stable maintainance should be
hopefully even smoother than the reality.

In this case definitely my fault to applied the cc:stable wrongly, luckily
in the healthy way rather than losing one of them when really needed.

> 
> Thank you!

Thanks again to both!

-- 
Peter Xu