Re: [PATCH v4 4/6] migration-test: Make sure that multifd and cancel works

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Wed, 22 Jan 2020 at 11:20, Juan Quintela <quintela@redhat.com> wrote:
> >
> > Test that this sequerce works:
> >
> > - launch source
> > - launch target
> > - start migration
> > - cancel migration
> > - relaunch target
> > - do migration again
> >
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> A year-old commit, but we've just got around to running Coverity
> on the tests/ directory, and it spotted this one:
> 
> >  static void migrate_set_capability(QTestState *who, const char *capability,
> >                                     bool value)
> 
> The 3rd argument to migrate_set_capability() is a bool...

oops

> 
> > +static void test_multifd_tcp_cancel(void)
> > +{
> 
> > +    migrate_set_parameter_int(from, "downtime-limit", 1);
> > +    /* 300MB/s */
> > +    migrate_set_parameter_int(from, "max-bandwidth", 30000000);
> > +
> > +    migrate_set_parameter_int(from, "multifd-channels", 16);
> > +    migrate_set_parameter_int(to, "multifd-channels", 16);
> > +
> > +    migrate_set_capability(from, "multifd", "true");
> > +    migrate_set_capability(to, "multifd", "true");
> 
> ...but here you pass it the character string '"true"' rather than
> the boolean value 'true'.
> 
> This works by fluke since the implicit comparison of the literal string
> against NULL will evaluate to true, but it isn't really right :-)
> 
> CID 1432373, 1432292, 1432288.
> 
> There seem to be 7 uses of the string "true" when the boolean
> was intended; I don't know why Coverity only found 3 issues.

I'll send a patch.

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK