Re: [PATCH v2 0/3] Allow to enable multifd and postcopy migration together

[Prasad Pandit]

Hello Peter,

On Fri, 29 Nov 2024 at 22:16, Peter Xu <peterx@redhat.com> wrote:
> I saw that there's still discussion in the previous version, while this
> cover letter doesn't mention why it was ignored.  Especially, at least to
> me, what Fabiano commented on simplifying the flush condition check makes
> senes to me to be addressed.

* It is not ignored. Simplifying flush conditionals makes sense to me
too, that is why in the 'v0' version of this series I had added the
!migration_in_postcopy() check to the migrate_multifd() function,
right? I tried to discuss in the 'v1' thread if there's another way to
simplify conditionals. Not sure if you've followed all comments in the
thread.

* Secondly, as you mention above, I also thought Fabiano is pointing
at the complexity of the 'if' conditionals and thus I replied that his
proposed patch does not seem to solve for that complexity. But in his
subsequent reply Fabiano mentions that it is not just about
conditionals, but larger complexity of how and when multifd threads do
flush and sync amongst them.

* IMHO, simplifying that larger complexity of how multifd threads do
flush and sync can be done independently, outside the scope of this
patch series, which is about enabling multifd and postcopy together.

> Meanwhile, before I read into any details, I found that all the tests I
> requested are still missing.  Would you please consider adding them?
>
> My previous comment regarding to test is here:
> https://lore.kernel.org/qemu-devel/ZykJBq7ME5jgSzCA@x1n/
>
> I listed exactly the minimum set of tests that I think we should have.
...
> In general, any migration new feature must have both doc updates and test
> coverage as long as applicable.
>
> Multifd still has its doc missing, which is unfortunate.  We could have one
> doc prior to this work, but it can also be done later.
>
> OTOH on testing: this is not a new feature alone, but it's more dangerous
> than a new feature due to what I mentioned before, that postcopy needs
> atomicity on page movements, and multifd is completely against that from
> design POV due to NIC drivers being able to modify guest pages directly.
>
> It means multifd+postcopy bugs will be extremely hard to debug if we don't
> put it right first.  So please be serious on the test coverage on this
> work.

* I'm yet to get to the test cases. The revised series(v1 and v2) are
posted to share patch updates which were suggested in the previous
reviews. Test cases are a separate/different effort from source
patches. If we want to hold on this patch series till we get the test
cases and documentation in place, that is okay, I'll work on that
next.

Thank you.
---
  - Prasad