Re: [PATCH 1/2] tests/qtest/migration: Ignore if socket-address is missing to avoid crash below

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> On 20/03/24 3:27 am, Peter Xu wrote:
>> On Tue, Mar 19, 2024 at 08:48:39PM +0000, Het Gala wrote:
>>> 'object' can return NULL if there is no socket-address, such as with a
>>> file migration. Then the visitor code below fails and the test crashes.
>>>
>>> Ignore and return NULL when socket-address is missing in the reply so
>>> we don't break future tests that use a non-socket type.
>> Hmm, this patch isn't as clear to me.  Even if this can return NULL now,
>> it'll soon crash at some later point, no?
> It won't crash IMO, the next function SocketAddress_to_str for a non-socket
> type would return an proper error ?
>> IMHO such patch is more suitable to be included in the same patch where
>> such new tests will be introduced, then we're addressing some real test
>> code changes that will work, rather than worrying on what will happen in
>> the future (and as I mentioned, i don't think it fully resolved that 
>> either..)
>>
>> Thanks,
> Maybe, Fabiano can pick this patch, and add along file migration qtests 
> patch ?

Yep.

>>> Suggested-by: Fabiano Rosas<farosas@suse.de>
>>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>>> ---
>>>   tests/qtest/migration-helpers.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tests/qtest/migration-helpers.c 
>>> b/tests/qtest/migration-helpers.c
>>> index b2a90469fb..fb7156f09a 100644
>>> --- a/tests/qtest/migration-helpers.c
>>> +++ b/tests/qtest/migration-helpers.c
>>> @@ -90,6 +90,10 @@ static SocketAddress 
>>> *migrate_get_socket_address(QTestState *who)
>>>       QObject *object;
>>>   
>>>       rsp = migrate_query(who);
>>> +
>>> +    if (!qdict_haskey(rsp, "socket-address")) {
>>> +        return NULL;
>>> +    }
>>>       object = qdict_get(rsp, "socket-address");
>>>   
>>>       iv = qobject_input_visitor_new(object);
>>> -- 
>>> 2.22.3
>>>
> Regards,
> Het Gala