[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators |
Date: |
Wed, 8 Nov 2017 14:56:19 -0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Nov 08, 2017 at 01:20:49PM -0300, Philippe Mathieu-Daudé wrote:
> On 11/02/2017 09:35 PM, Eduardo Habkost wrote:
> > On Mon, Oct 30, 2017 at 03:14:59PM -0300, Philippe Mathieu-Daudé wrote:
> >> examples configuring with '--enable-kvm --disable-tcg'
> >>
> >> - before
> >>
> >> $ qemu-system-x86_64 -accel help
> >> Possible accelerators: kvm, xen, hax, tcg
> >>
> >> $ qemu-system-x86_64 -accel tcg
> >> qemu-system-x86_64: -machine accel=tcg: No accelerator found
> >>
> >> # qemu-system-x86_64 -accel hax
> >> qemu-system-x86_64: -machine accel=hax: No accelerator found
> >>
> >> - after
> >>
> >> $ qemu-system-x86_64 -accel help
> >> Possible accelerators:
> >> xen
> >> kvm
> >>
> >> Suggested-by: Eduardo Habkost <address@hidden>
> >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> >> ---
> >> since RFC:
> >> - use much cleaner object_class_get_list(TYPE_ACCEL, false)
> >>
> >> vl.c | 39 ++++++++++++++++++++++++++++++++++-----
> >> 1 file changed, 34 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/vl.c b/vl.c
> >> index ec299099ff..0f13641715 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2764,6 +2764,39 @@ static gint machine_class_cmp(gconstpointer a,
> >> gconstpointer b)
> >> exit(!name || !is_help_option(name));
> >> }
> >>
> >> +static void accel_list_entry(gpointer data, gpointer user_data)
> >> +{
> >> + ObjectClass *oc = data;
> >> + const char *typename = object_class_get_name(oc);
> >> + int len;
> >> +
> >> + if (!qtest_driver() && !g_strcmp0(typename,
> >> ACCEL_CLASS_NAME("qtest"))) {
> >> + return; /* used by test cases */
> >> + }
> >
> > I would prefer to not hardcode the accel name here. If you want
> > qtest to have special behavior, you can add a new field to
> > AccelClass to implement that.
> >
> > However, I don't think I agree we should hide qtest from the
> > user: it is accepted on the command-line, so why should we lie?
>
> Ok.
>
> > Why not add a description field and indicate that qtest is useful
> > only for testing?
>
> This is cleaner indeed.
>
> >> +
> >> + len = strlen(typename) - strlen("-" TYPE_ACCEL);
> >
> > You can use AccelClass::name here.
>
> Hmm yes but currently this is not what the user should use, since the
> accel_find() matches the ACCEL_CLASS_NAME:
>
> $ ppc64-softmmu/qemu-system-ppc64 -accel QTest
> qemu-system-ppc64: -machine accel=QTest: No accelerator found
Oops, you're correct.
In this case, I suggest using ACCEL_CLASS_SUFFIX instead of
hardcoding ("-" TYPE_ACCEL).
Maybe a accel_class_get_accel_name() helper similar to
x86_cpu_class_get_model_name() would be even better. It would
require an extra memory allocation, but the code would be
cleaner. We might add a QMP command returning the accel list in
the future, so a common helper would be useful.
>
> I'll respin but the change will be bigger so I is probably too late for
> 2.11.
>
> >
> >> + if (len > 0) {
> >> + error_printf(" %.*s\n", len, typename);
> >> + }
> >> +}
> >> +
> >> +static void accel_parse(const char *name, QemuOpts *accel_opts)
> >> +{
> >> + const char *optarg = qemu_opt_get(accel_opts, "accel");
> >> + GSList *list;
> >> +
> >> + if (!is_help_option(optarg)) {
> >> + return;
> >> + }
> >> +
> >> + list = object_class_get_list(TYPE_ACCEL, false);
> >> + error_printf("Possible accelerators:\n");
> >> + g_slist_foreach(list, accel_list_entry, NULL);
> >> + g_slist_free(list);
> >> +
> >> + exit(0);
> >> +}
> >> +
> >> void qemu_add_exit_notifier(Notifier *notify)
> >> {
> >> notifier_list_add(&exit_notifiers, notify);
> >> @@ -3881,11 +3914,7 @@ int main(int argc, char **argv, char **envp)
> >> case QEMU_OPTION_accel:
> >> accel_opts =
> >> qemu_opts_parse_noisily(qemu_find_opts("accel"),
> >> optarg, true);
> >> - optarg = qemu_opt_get(accel_opts, "accel");
> >> - if (!optarg || is_help_option(optarg)) {
> >> - error_printf("Possible accelerators: kvm, xen, hax,
> >> tcg\n");
> >> - exit(0);
> >> - }
> >> + accel_parse(optarg, accel_opts);
> >> opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
> >> false, &error_abort);
> >> qemu_opt_set(opts, "accel", optarg, &error_abort);
> >> --
> >> 2.15.0.rc2
> >>
> >
--
Eduardo
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Eduardo Habkost, 2017/11/02
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Daniel P. Berrange, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Philippe Mathieu-Daudé, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Eduardo Habkost, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Daniel P. Berrange, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Eduardo Habkost, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Daniel P. Berrange, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Philippe Mathieu-Daudé, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Eduardo Habkost, 2017/11/08
- Re: [Qemu-devel] [RFC PATCH] vl: only display available accelerators, Philippe Mathieu-Daudé, 2017/11/08