All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Fabiano Rosas <farosas@suse.de>
Cc: qemu-devel@nongnu.org, Juan Quintela <quintela@redhat.com>,
	Xiaohui Li <xiaohli@redhat.com>
Subject: Re: [PATCH 9/9] migration/postcopy: Allow network to fail even during recovery
Date: Tue, 12 Sep 2023 18:16:45 -0400	[thread overview]
Message-ID: <ZQDjTRtNVbtruUtD@x1n> (raw)
In-Reply-To: <ZQDEh85X2IcC+o8M@x1n>

On Tue, Sep 12, 2023 at 04:05:27PM -0400, Peter Xu wrote:
> Thanks for contributing the test case!
> 
> Do you want me to pick this patch up (with modifications) and repost
> together with this series?  It'll also work if you want to send a separate
> test patch.  Let me know!

It turns out I found more bug when I was reworking that test case based on
yours.  E.g., currently we'll crash dest qemu if we really fail during
recovery, because we miss:

diff --git a/migration/savevm.c b/migration/savevm.c
index bb3e99194c..422406e0ee 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2723,7 +2723,8 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
         qemu_mutex_unlock(&mis->postcopy_prio_thread_mutex);
     }
 
-    migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+    /* Current state can be either ACTIVE or RECOVER */
+    migrate_set_state(&mis->state, mis->state,
                       MIGRATION_STATUS_POSTCOPY_PAUSED);
 
     /* Notify the fault thread for the invalidated file handle */

So in double failure case we'll not set RECOVER to PAUSED, and we'll crash
right afterwards, as we'll skip the semaphore:

    while (mis->state == MIGRATION_STATUS_POSTCOPY_PAUSED) {  <--- not true, continue
        qemu_sem_wait(&mis->postcopy_pause_sem_dst);
    }

Now within the new test case I am 100% sure I can kick both sides into
RECOVER state (one trick still needed along the way; the test patch will
tell soon), then kick them back, then proceed with a successful migration.

Let me just repost everything with the new test case.

Thanks,

-- 
Peter Xu



  reply	other threads:[~2023-09-12 22:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-29 21:42 [PATCH 0/9] migration: Better error handling in rp thread, allow failures in recover Peter Xu
2023-08-29 21:42 ` [PATCH 1/9] migration: Display error in query-migrate irrelevant of status Peter Xu
2023-08-29 21:42 ` [PATCH 2/9] migration: Let migrate_set_error() take ownership Peter Xu
2023-09-12 19:40   ` Fabiano Rosas
2023-09-12 20:14     ` Peter Xu
2023-08-29 21:42 ` [PATCH 3/9] migration: Introduce migrate_has_error() Peter Xu
2023-08-29 21:42 ` [PATCH 4/9] migration: Refactor error handling in source return path Peter Xu
2023-08-29 21:42 ` [PATCH 5/9] migration: Deliver return path file error to migrate state too Peter Xu
2023-08-29 21:42 ` [PATCH 6/9] qemufile: Always return a verbose error Peter Xu
2023-08-29 21:42 ` [PATCH 7/9] migration: Remember num of ramblocks to sync during recovery Peter Xu
2023-09-12  0:33   ` Fabiano Rosas
2023-08-29 21:42 ` [PATCH 8/9] migration: Add migration_rp_wait|kick() Peter Xu
2023-09-12  0:32   ` Fabiano Rosas
2023-08-29 21:42 ` [PATCH 9/9] migration/postcopy: Allow network to fail even during recovery Peter Xu
2023-09-12  0:31   ` Fabiano Rosas
2023-09-12 20:05     ` Peter Xu
2023-09-12 22:16       ` Peter Xu [this message]
2023-09-12 22:49       ` Fabiano Rosas
2023-09-13  0:38         ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZQDjTRtNVbtruUtD@x1n \
    --to=peterx@redhat.com \
    --cc=farosas@suse.de \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=xiaohli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.