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);
+    }
+
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
 
-- 
2.35.3

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
> 

-- 
Peter Xu

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.

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.

-- 
Peter Xu

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.

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>

-- 
Peter Xu