On 2025-11-05 14:52, Peter Xu wrote:
> 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
>
> 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?
It is an efficient way, I just thought the less efficient might be a
little bit easier to deduce why the test failed. But if nobody else
opposes, you can also keep it as proposed,
>
> I definitely still want your R-b one way or another!
and also add my R-b.
>
> >
> >
> > 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
>
