diff mbox series

[v1,5/7] tests/qtest/migration: Print migration incoming errors

Message ID 20231124161432.3515-6-farosas@suse.de
State New
Headers show
Series migration cleanups and testing improvements | expand

Commit Message

Fabiano Rosas Nov. 24, 2023, 4:14 p.m. UTC
We're currently just asserting when incoming migration fails. Let's
print the error message from QMP as well.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Peter Xu Nov. 27, 2023, 2:50 p.m. UTC | #1
On Fri, Nov 24, 2023 at 01:14:30PM -0300, Fabiano Rosas wrote:
> We're currently just asserting when incoming migration fails. Let's
> print the error message from QMP as well.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/migration-helpers.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 24fb7b3525..f1106128a9 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -118,6 +118,12 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
>  
>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
>                      args);
> +
> +    if (!qdict_haskey(rsp, "return")) {
> +        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
> +        g_test_message("%s", s->str);
> +    }

This traps the "migrate-incoming" command only (which, afaiu, only setup
the listening), would this capture the incoming error?

> +
>      g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
>  
> -- 
> 2.35.3
>
Fabiano Rosas Nov. 27, 2023, 3:52 p.m. UTC | #2
Peter Xu <peterx@redhat.com> writes:

> On Fri, Nov 24, 2023 at 01:14:30PM -0300, Fabiano Rosas wrote:
>> We're currently just asserting when incoming migration fails. Let's
>> print the error message from QMP as well.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/migration-helpers.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index 24fb7b3525..f1106128a9 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -118,6 +118,12 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
>>  
>>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
>>                      args);
>> +
>> +    if (!qdict_haskey(rsp, "return")) {
>> +        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
>> +        g_test_message("%s", s->str);
>> +    }
>
> This traps the "migrate-incoming" command only (which, afaiu, only setup
> the listening), would this capture the incoming error?

This is about the migrate-incoming only. We could replace "incoming
migration" with "qmp_migrate_incoming" in the commit message to clarify.
Peter Xu Nov. 27, 2023, 8:23 p.m. UTC | #3
On Mon, Nov 27, 2023 at 12:52:38PM -0300, Fabiano Rosas wrote:
> >> @@ -118,6 +118,12 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
> >>  
> >>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
> >>                      args);
> >> +
> >> +    if (!qdict_haskey(rsp, "return")) {
> >> +        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
> >> +        g_test_message("%s", s->str);
> >> +    }
> >
> > This traps the "migrate-incoming" command only (which, afaiu, only setup
> > the listening), would this capture the incoming error?
> 
> This is about the migrate-incoming only. We could replace "incoming
> migration" with "qmp_migrate_incoming" in the commit message to clarify.

Ah.. Did you ever see this failure in any of your runs in these tests?  I
think it means you hit the assertion right below this part, but I'm just
curious how, as the URIs in the test cases are pretty constant.
Fabiano Rosas Nov. 27, 2023, 8:32 p.m. UTC | #4
Peter Xu <peterx@redhat.com> writes:

> On Mon, Nov 27, 2023 at 12:52:38PM -0300, Fabiano Rosas wrote:
>> >> @@ -118,6 +118,12 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
>> >>  
>> >>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
>> >>                      args);
>> >> +
>> >> +    if (!qdict_haskey(rsp, "return")) {
>> >> +        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
>> >> +        g_test_message("%s", s->str);
>> >> +    }
>> >
>> > This traps the "migrate-incoming" command only (which, afaiu, only setup
>> > the listening), would this capture the incoming error?
>> 
>> This is about the migrate-incoming only. We could replace "incoming
>> migration" with "qmp_migrate_incoming" in the commit message to clarify.
>
> Ah.. Did you ever see this failure in any of your runs in these tests?  I
> think it means you hit the assertion right below this part, but I'm just
> curious how, as the URIs in the test cases are pretty constant.

Yes, I don't remember what exactly, but we changed the code that parses
the URIs in this release and I'm also working on
file_start_incoming_migration.
Peter Xu Nov. 27, 2023, 8:52 p.m. UTC | #5
On Mon, Nov 27, 2023 at 05:32:45PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Mon, Nov 27, 2023 at 12:52:38PM -0300, Fabiano Rosas wrote:
> >> >> @@ -118,6 +118,12 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
> >> >>  
> >> >>      rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
> >> >>                      args);
> >> >> +
> >> >> +    if (!qdict_haskey(rsp, "return")) {
> >> >> +        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
> >> >> +        g_test_message("%s", s->str);
> >> >> +    }
> >> >
> >> > This traps the "migrate-incoming" command only (which, afaiu, only setup
> >> > the listening), would this capture the incoming error?
> >> 
> >> This is about the migrate-incoming only. We could replace "incoming
> >> migration" with "qmp_migrate_incoming" in the commit message to clarify.
> >
> > Ah.. Did you ever see this failure in any of your runs in these tests?  I
> > think it means you hit the assertion right below this part, but I'm just
> > curious how, as the URIs in the test cases are pretty constant.
> 
> Yes, I don't remember what exactly, but we changed the code that parses
> the URIs in this release and I'm also working on
> file_start_incoming_migration.

OK then.

Reviewed-by: Peter Xu <peterx@redhat.com>
diff mbox series

Patch

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 24fb7b3525..f1106128a9 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -118,6 +118,12 @@  void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...)
 
     rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
                     args);
+
+    if (!qdict_haskey(rsp, "return")) {
+        g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
+        g_test_message("%s", s->str);
+    }
+
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);