Re: [PATCH v2 11/11] tests/migration-test: Add a test for postcopy hangs during RECOVER

[Peter Xu]

On Wed, Sep 13, 2023 at 11:27:13AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> >  
> > +static void wait_for_postcopy_status(QTestStatus *one, const char *status)
> > +{
> 
> QTestState *who
> 
> > +    wait_for_migration_status(from, status,
> 
> s/from/who
> 
> > +                              (const char * []) { "failed", "active",
> > +                                                  "completed", NULL });
> > +}
> > +
> > +static void postcopy_recover_fail(QTestState *from, QTestState *to)
> > +{
> > +    int ret, pair1[2], pair2[2];
> > +    char c;
> > +
> > +    /* Create two unrelated socketpairs */
> > +    ret = qemu_socketpair(PF_LOCAL, SOCK_STREAM, 0, pair1);
> > +    g_assert_cmpint(ret, ==, 0);
> > +
> > +    ret = qemu_socketpair(PF_LOCAL, SOCK_STREAM, 0, pair2);
> > +    g_assert_cmpint(ret, ==, 0);
> > +
> > +    /*
> > +     * Give the guests unpaired ends of the sockets, so they'll all blocked
> > +     * at reading.  This mimics a wrong channel established.
> > +     */
> > +    qtest_qmp_fds_assert_success(from, &pair1[0], 1,
> > +                                 "{ 'execute': 'getfd',"
> > +                                 "  'arguments': { 'fdname': 'fd-mig' }}");
> > +    qtest_qmp_fds_assert_success(to, &pair2[0], 1,
> > +                                 "{ 'execute': 'getfd',"
> > +                                 "  'arguments': { 'fdname': 'fd-mig' }}");
> > +
> > +    /*
> > +     * Write the 1st byte as QEMU_VM_COMMAND (0x8) for the dest socket, to
> > +     * emulate the 1st byte of a real recovery, but stops from there to
> > +     * keep dest QEMU in RECOVER.  This is needed so that we can kick off
> > +     * the recover process on dest QEMU (by triggering the G_IO_IN event).
> > +     *
> > +     * NOTE: this trick is not needed on src QEMUs, because src doesn't
> > +     * rely on an pre-existing G_IO_IN event, so it will always trigger the
> > +     * upcoming recovery anyway even if it can read nothing.
> > +     */
> > +#define QEMU_VM_COMMAND              0x08
> > +    c = QEMU_VM_COMMAND;
> > +    ret = send(pair2[1], &c, 1, 0);
> > +    g_assert_cmpint(ret, ==, 1);
> > +
> > +    migrate_recover(to, "fd:fd-mig");
> > +    migrate_qmp(from, "fd:fd-mig", "{'resume': true}");
> > +
> > +    /*
> > +     * Make sure both QEMU instances will go into RECOVER stage, then test
> > +     * kicking them out using migrate-pause.
> > +     */
> > +    wait_for_postcopy_status(from, "postcopy-recover")
> 
> semicolon

Sorry, I forgot to amend into this patch when I last touched the
bits.. here's the diff I'll amend into it:

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6105c2da65..de5aa90483 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1358,9 +1358,9 @@ static void test_postcopy_preempt_tls_psk(void)
 }
 #endif
 
-static void wait_for_postcopy_status(QTestStatus *one, const char *status)
+static void wait_for_postcopy_status(QTestState *one, const char *status)
 {
-    wait_for_migration_status(from, status,
+    wait_for_migration_status(one, status,
                               (const char * []) { "failed", "active",
                                                   "completed", NULL });
 }
@@ -1410,7 +1410,7 @@ static void postcopy_recover_fail(QTestState *from, 
QTestState *to)
      * Make sure both QEMU instances will go into RECOVER stage, then test
      * kicking them out using migrate-pause.
      */
-    wait_for_postcopy_status(from, "postcopy-recover")
+    wait_for_postcopy_status(from, "postcopy-recover");
     wait_for_postcopy_status(to, "postcopy-recover");
 
     /*

-- 
Peter Xu