[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
From: |
Eric Blake |
Subject: |
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command |
Date: |
Mon, 15 Mar 2021 12:53:01 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
> Introduce the 'query-accels' QMP command which returns a list
> of built-in accelerators names.
accelerator names
>
> - Accelerator is an QAPI enum of all existing accelerators,
is a QAPI enum
>
> - AcceleratorInfo is a QAPI structure providing accelerator
> specific information. Currently the common structure base
> provides the name of the accelerator, while the specific
> part is empty, but each accelerator can expand it.
Do we expand it later in this series? If not,...
>
> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>
> For example on a KVM-only build we get:
>
> { "execute": "query-accels" }
> {
> "return": [
> {
> "type": "qtest"
> },
> {
> "type": "kvm"
> }
Inconsistent with the code, already pointed out in other reviews.
> ]
> }
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> +##
> +# @AcceleratorInfo:
> +#
> +# Accelerator information.
> +#
> +# @name: The accelerator name.
> +#
> +# Since: 6.0
> +##
> +{ 'union': 'AcceleratorInfo',
> + 'base': {'name': 'Accelerator'},
> + 'discriminator': 'name',
> + 'data': { } }
...it feels a bit over-engineered (we can turn it into a union later
while still preserving back-compat). On the other hand, since you are
using conditional compilation...
> +
> +##
> +# @query-accels:
> +#
> +# Get a list of AcceleratorInfo for all built-in accelerators.
> +#
> +# Returns: a list of @AcceleratorInfo describing each accelerator.
> +#
> +# Since: 6.0
> +#
> +# Example:
> +#
> +# -> { "execute": "query-accels" }
> +# <- { "return": [
> +# {
> +# "type": "qtest"
> +# },
> +# {
> +# "type": "kvm"
Another inconsistent example.
> +# }
> +# ] }
> +#
> +##
> +{ 'command': 'query-accels',
> + 'returns': ['AcceleratorInfo'] }
> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
> new file mode 100644
> index 00000000000..f16e49b8956
> --- /dev/null
> +++ b/accel/accel-qmp.c
> @@ -0,0 +1,47 @@
> +/*
> + * QEMU accelerators, QMP commands
> + *
> + * Copyright (c) 2021 Red Hat Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/qapi-commands-machine.h"
> +
> +static const Accelerator accel_list[] = {
> + ACCELERATOR_QTEST,
> +#ifdef CONFIG_TCG
> + ACCELERATOR_TCG,
> +#endif
> +#ifdef CONFIG_KVM
> + ACCELERATOR_KVM,
> +#endif
...would it be worth compiling the enum to only list enum values that
were actually compiled in? That would change it to:
{ 'enum': 'Accelerator',
'data': [ 'qtest',
{ 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
...
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command,
Eric Blake <=
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Markus Armbruster, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Paolo Bonzini, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Philippe Mathieu-Daudé, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Philippe Mathieu-Daudé, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Markus Armbruster, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Philippe Mathieu-Daudé, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Markus Armbruster, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Thomas Huth, 2021/03/16
- Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command, Philippe Mathieu-Daudé, 2021/03/16
[PATCH 2/6] tests/qtest: Add qtest_probe_accel() method, Philippe Mathieu-Daudé, 2021/03/11