* [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests @ 2025-10-31 16:49 Peter Xu 2025-11-04 12:35 ` Juraj Marcin 0 siblings, 1 reply; 5+ messages in thread From: Peter Xu @ 2025-10-31 16:49 UTC (permalink / raw) To: qemu-devel; +Cc: Fabiano Rosas, Juraj Marcin, peterx 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")); + 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests 2025-10-31 16:49 [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests Peter Xu @ 2025-11-04 12:35 ` Juraj Marcin 2025-11-05 19:52 ` Peter Xu 0 siblings, 1 reply; 5+ messages in thread From: Juraj Marcin @ 2025-11-04 12:35 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas 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(). With this change you can add my: Reviewed-by: Juraj Marcin <jmarcin@redhat.com> 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 > ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests 2025-11-04 12:35 ` Juraj Marcin @ 2025-11-05 19:52 ` Peter Xu 2025-11-06 11:27 ` Juraj Marcin 0 siblings, 1 reply; 5+ messages in thread From: Peter Xu @ 2025-11-05 19:52 UTC (permalink / raw) To: Juraj Marcin; +Cc: qemu-devel, Fabiano Rosas 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests 2025-11-05 19:52 ` Peter Xu @ 2025-11-06 11:27 ` Juraj Marcin 2025-11-06 16:15 ` Peter Xu 0 siblings, 1 reply; 5+ messages in thread From: Juraj Marcin @ 2025-11-06 11:27 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas 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 <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? 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests 2025-11-06 11:27 ` Juraj Marcin @ 2025-11-06 16:15 ` Peter Xu 0 siblings, 0 replies; 5+ messages in thread From: Peter Xu @ 2025-11-06 16:15 UTC (permalink / raw) To: Juraj Marcin; +Cc: qemu-devel, Fabiano Rosas On Thu, Nov 06, 2025 at 12:27:49PM +0100, Juraj Marcin wrote: > 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 <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? > > 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. Thanks. I wanted to have this coverage asap, so I collected it for -rc. -- Peter Xu ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-06 16:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-31 16:49 [PATCH] tests/migration-test: Check error-desc after pre-switch cancel tests Peter Xu 2025-11-04 12:35 ` Juraj Marcin 2025-11-05 19:52 ` Peter Xu 2025-11-06 11:27 ` Juraj Marcin 2025-11-06 16:15 ` Peter Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).