Re: [PATCH v2 00/10] migration: New postcopy state, and some cleanups

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Hello,
>
> On Mon, Jun 17, 2024 at 02:15:24PM -0400, Peter Xu wrote:
>> v2:
>> - Collect tags
>> - Patch 3
>>   - cover all states in migration_postcopy_is_alive()
>> - Patch 4 (old)
>>   - English changes [Fabiano]
>>   - Split the migration_incoming_state_setup() cleanup into a new patch
>>     [Fabiano]
>>   - Drop RECOVER_SETUP in fill_destination_migration_info() [Fabiano]
>>   - Keep using explicit state check in migrate_fd_connect() for resume
>>     [Fabiano]
>> - New patches
>>   - New doc update: "migration/docs: Update postcopy recover session for
>>     SETUP phase"
>>   - New test case: last four patches
>
> I just found that this won't apply on top of latest master, and also has a
> trivial conflict against the direct-io stuffs.  Fabiano, I'll wait for a
> few days on comments, and resend v3 on top of your direct-io stuff.
>
> Meanwhile I also plan to squash below fixup to the last test patch, just to
> fix up a spelling error I just found, and also renamed the test cases (as
> the new test is actually also a "double failure" test, just at different
> phase).  Comments welcomed for that fixup even before a repost.
>
> ===8<===
> From 5b8fbc3a9d9e87ebfef1a3e5592fd196eecd5923 Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Mon, 17 Jun 2024 14:40:15 -0400
> Subject: [PATCH] fixup! tests/migration-tests: Cover postcopy failure on
>  reconnect
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-test.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index a4fed4cc6b..fe33b86783 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1474,7 +1474,7 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to,
>  
>      /*
>       * Kick dest QEMU out too. This is normally not needed in reality
> -     * because when the channel is shutdown it should also happens on src.
> +     * because when the channel is shutdown it should also happen on src.
>       * However here we used separate socket pairs so we need to do that
>       * explicitly.
>       */
> @@ -1565,7 +1565,7 @@ static void test_postcopy_recovery(void)
>      test_postcopy_recovery_common(&args);
>  }
>  
> -static void test_postcopy_recovery_double_fail(void)
> +static void test_postcopy_recovery_fail_handshake(void)
>  {
>      MigrateCommon args = {
>          .postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY,
> @@ -1574,7 +1574,7 @@ static void test_postcopy_recovery_double_fail(void)
>      test_postcopy_recovery_common(&args);
>  }
>  
> -static void test_postcopy_recovery_channel_reconnect(void)
> +static void test_postcopy_recovery_fail_reconnect(void)
>  {
>      MigrateCommon args = {
>          .postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH,
> @@ -3759,10 +3759,10 @@ int main(int argc, char **argv)
>                             test_postcopy_preempt);
>          migration_test_add("/migration/postcopy/preempt/recovery/plain",
>                             test_postcopy_preempt_recovery);
> -        migration_test_add("/migration/postcopy/recovery/double-failures",
> -                           test_postcopy_recovery_double_fail);
> -        migration_test_add("/migration/postcopy/recovery/channel-reconnect",
> -                           test_postcopy_recovery_channel_reconnect);
> +        migration_test_add("/migration/postcopy/recovery/double-failures/handshake",
> +                           test_postcopy_recovery_fail_handshake);
> +        migration_test_add("/migration/postcopy/recovery/double-failures/reconnect",
> +                           test_postcopy_recovery_fail_reconnect);
>          if (is_x86) {
>              migration_test_add("/migration/postcopy/suspend",
>                                 test_postcopy_suspend);
> -- 
> 2.45.0

LGTM