Re: [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests

[Peter Xu <peterx@redhat.com>]

On Tue, Nov 04, 2025 at 01:35:53PM +0100, Juraj Marcin wrote:
> Hi Peter,
> 
> On 2025-10-31 12:49, Peter Xu wrote:
> > error-desc should present on dest QEMU after migration failed on dest when
> > exit-on-error is set to FALSE.  Check the error message.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  tests/qtest/migration/precopy-tests.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> > index 57ca623de5..5f02e35324 100644
> > --- a/tests/qtest/migration/precopy-tests.c
> > +++ b/tests/qtest/migration/precopy-tests.c
> > @@ -759,6 +759,14 @@ static void test_cancel_src_after_none(QTestState *from, QTestState *to,
> >      wait_for_migration_complete(to);
> >  }
> >  
> > +static void assert_migration_error(QTestState *vm)
> > +{
> > +    QDict *rep = migrate_query(vm);
> > +
> > +    g_assert(qdict_get_str(rep, "error-desc"));
> 
> I think it would be beneficial to also check if there even is
> "error-desc". That way if the "error-desc" is missing, it fails on
> assert with SIGABRT instead of SIGSEGV inside qdict_get_str().

IMHO it doesn't matter on how the test would crash.

> 
> With this change you can add my:
> 
> Reviewed-by: Juraj Marcin <jmarcin@redhat.com>

I would go ahead and merge a test patch if it had both lines, definitely
not a huge deal.

However strictly speaking, qdict_get_str() is actually pretty efficient to
make sure both that exists && is_string when used in testings. Would you
agree?

I definitely still want your R-b one way or another!

> 
> 
> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> index 5f02e35324..87e33b8965 100644
> --- a/tests/qtest/migration/precopy-tests.c
> +++ b/tests/qtest/migration/precopy-tests.c
> @@ -763,6 +763,7 @@ static void assert_migration_error(QTestState *vm)
>  {
>      QDict *rep = migrate_query(vm);
> 
> +    g_assert(qdict_get(rep, "error-desc"));
>      g_assert(qdict_get_str(rep, "error-desc"));
>      qobject_unref(rep);
>  }
> 
> 
> > +    qobject_unref(rep);
> > +}
> > +
> >  static void test_cancel_src_pre_switchover(QTestState *from, QTestState *to,
> >                                             const char *uri, const char *phase)
> >  {
> > @@ -784,6 +792,7 @@ static void test_cancel_src_pre_switchover(QTestState *from, QTestState *to,
> >  
> >      wait_for_migration_status(to, "failed",
> >                                (const char * []) { "completed", NULL });
> > +    assert_migration_error(to);
> >  }
> >  
> >  static void test_cancel_src_after_status(void *opaque)
> > -- 
> > 2.50.1
> > 
> 

-- 
Peter Xu