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

[Het Gala]

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 ?

> > 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