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

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

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

Ah ok, code below is wrong.

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

Ok, I'll change it to something else.

>
> [...]
>
>> >> @@ -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;
>> >> +        }