Thomas Huth <th...@redhat.com> wrote:
> On 30/01/2023 05.44, Juan Quintela wrote:
>> Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>> Reviewed-by: Juan Quintela <quint...@redhat.com>
>> I am assuming that you will pull this patches through tests tree,
>> not
>> migration tree.
>> Otherwise, let me know.
>
> I had some remarks (in v2 of the series), so I'm not going to pick
> this up (yet). If you want to take the migration part, feel free to do
> so.
>
> I still think it's quite a lot of code churn for just supporting
> multiple "-accel" statements here.
>
> What about introducing a new lib functions like this:
>
> char *qtest_get_accel_params(bool use_tcg_first)
> {
> bool tcg = qtest_has_accel("tcg");
>
> return g_strdup_printf("%s %s %s %s",
> use_tcg_first && tcg ? "-accel tcg" : "",
> qtest_has_accel("kvm") ? "-accel kvm" : "",
> qtest_has_accel("hvf") ? "-accel hvf" : "",
> !use_tcg_first && tcg ? "-accel tcg" : "");
I know that it is me, but I don't find the ? operator especially
readable.
What about:
GString *s = g_string_new("");
bool tcg = qtest_has_accel("tcg");
if (use_tcg_first && tcg)
g_string_append(s, "-accel tcg ");
if (qtest_has_accel("kvm"))
g_string_append(s, "-accel kvm ");
if (qtest_has_accel("hvf"))
g_string_append(s, "-accel hvf ");
if (!use_tcg_first && tcg)
g_string_append(s, "-accel tcg");
return s;
Yes, in the function that Phillipe is changing now, each time that I
have to change it, I have to count to see what and where I need to put
the format/change/whatever.
> }
>
> ... then all tests that want to run real code could simply call this
> function instead of having to deal with the list of supported
> accelerators again and again?
It is ok with me.
Later, Juan.
