[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v3 11/19] tcg: add options for enabling MTTCG
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [RFC v3 11/19] tcg: add options for enabling MTTCG |
Date: |
Fri, 22 Jul 2016 17:17:38 +0100 |
User-agent: |
mu4e 0.9.17; emacs 25.0.95.10 |
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