Re: [PATCH v3 2/4] tests/qtest/migration: Add infrastructure to skip tests on older QEMUs

[Peter Xu <peterx@redhat.com>]

On Mon, Jan 08, 2024 at 11:49:45AM -0300, Fabiano Rosas wrote:
> >> +
> >> +    if (major > tgt_major) {
> >> +        return -1;
> >
> > This means the QEMU version is newer, the function will return negative.
> > Is this what we want?  It seems it's inverted.
> 
> The return "points" to which once is the more recent:
> 
> QEMU version | since: version
> -1           0         1

Here if returns -1, then below [1] will skip the test?

> 
> > In all cases, document this function with retval would be helpful too.
> >
> 
> Ok.
> 
> >> +    }
> >> +    if (major < tgt_major) {
> >> +        return 1;
> >> +    }
> >
> > Instead of all these, I'm wondering whether we can allow "since" to be an
> > array of integers, like [8, 2, 0].  Would that be much easier?
> 
> I don't see why push the complexity towards the person writing the
> tests. The string is much more natural to specify.

To me QEMU_VER(8,2,0) is as easy to write and read, too.  What Dan proposed
looks also good in the other thread.

I don't really have a strong opinion here especially for the test case. But
imho it'll be still nice to avoid string <-> int if the string is not required.

[...]

> >> @@ -850,6 +856,17 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> >>          qtest_qmp_set_event_callback(*from,
> >>                                       migrate_watch_for_stop,
> >>                                       &got_src_stop);
> >> +
> >> +        if (args->since && migration_vercmp(*from, args->since) < 0) {

[1]

> >> +            g_autofree char *msg = NULL;
> >> +
> >> +            msg = g_strdup_printf("Test requires at least QEMU version %s",
> >> +                                  args->since);
> >> +            g_test_skip(msg);
> >> +            qtest_quit(*from);
> >> +
> >> +            return -1;
> >> +        }

-- 
Peter Xu