Peter Xu <pet...@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;
>> >> + }
