From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51227) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQd8p-0005Dz-Sr for qemu-devel@nongnu.org; Fri, 22 Jul 2016 12:17:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQd8m-0006wu-5d for qemu-devel@nongnu.org; Fri, 22 Jul 2016 12:17:34 -0400 Received: from mail-wm0-x22e.google.com ([2a00:1450:400c:c09::22e]:37700) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQd8l-0006wq-Us for qemu-devel@nongnu.org; Fri, 22 Jul 2016 12:17:32 -0400 Received: by mail-wm0-x22e.google.com with SMTP id i5so73262783wmg.0 for ; Fri, 22 Jul 2016 09:17:31 -0700 (PDT) References: <1464986428-6739-1-git-send-email-alex.bennee@linaro.org> <1464986428-6739-12-git-send-email-alex.bennee@linaro.org> <5771957F.4070102@gmail.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <5771957F.4070102@gmail.com> Date: Fri, 22 Jul 2016 17:17:38 +0100 Message-ID: <87lh0tr6jh.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v3 11/19] tcg: add options for enabling MTTCG List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov Cc: mttcg@listserver.greensocs.com, qemu-devel@nongnu.org, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, bobby.prani@gmail.com, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite I've taken all the suggestions apart from the bellow (comment inline): Sergey Fedorov writes: >> --- 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. >> } >> + 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