Re: [Qemu-devel] [RFC v3 11/19] tcg: add options for enabling MTTCG

[Alex Bennée]

I've taken all the suggestions apart from the bellow (comment inline):

Sergey Fedorov <address@hidden> writes:

<snip>
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -99,6 +99,26 @@ STEXI
>>  Select CPU model (@code{-cpu help} for list and additional feature 
>> selection)
>>  ETEXI
>>
>> +DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>> +    "-accel [accel=]accelerator[,thread=single|multi]\n"
>> +    "               select accelerator ('-accel help for list')\n"
>> +    "               thread=single|multi (enable multi-threaded TCG)", 
>> QEMU_ARCH_ALL)
>
> "mttcg=on|off" or "multithread=on|off", instead of
> "thread=single|multi", are another possible variants. The former has
> less letters :)

I went with the previous suggestion during the last round. mttcg could
be a little cryptic, multithread=on|off is much of a muchness.

<snip>
>>              }
>> +            case QEMU_OPTION_accel:
>> +                opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
>> +                                               optarg, true);
>> +                optarg = qemu_opt_get(opts, "accel");
>> +
>> +                olist = qemu_find_opts("machine");
>> +                if (strcmp("kvm", optarg) == 0) {
>> +                    qemu_opts_parse_noisily(olist, "accel=kvm", false);
>> +                } else if (strcmp("xen", optarg) == 0) {
>> +                    qemu_opts_parse_noisily(olist, "accel=xen", false);
>> +                } else if (strcmp("tcg", optarg) == 0) {
>> +                    qemu_opts_parse_noisily(olist, "accel=tcg", false);
>> +                    qemu_tcg_configure(opts);
>> +                } else {
>> +                    if (!is_help_option(optarg)) {
>> +                        error_printf("Unknown accelerator: %s", optarg);
>> +                    }
>> +                    error_printf("Supported accelerators: kvm, xen, tcg\n");
>> +                    exit(1);
>> +                }
>
> I am wondering if we should use accel_find() here like in
> configure_accelerator() and probably also make checks with
> AccelClass::available().

I'm not sure exactly what magic is going on here in the pseudo-class
stuff.

>
> Please consider wrapping this code in a separate function.

It seems broadly in line with other snippets in vl.c so I've left it for now.

>
>
>> +                break;
>>              case QEMU_OPTION_usb:
>>                  olist = qemu_find_opts("machine");
>>                  qemu_opts_parse_noisily(olist, "usb=on", false);
>
> Kind regards,
> Sergey


--
Alex Bennée