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
