Re: [Qemu-devel] [qemu RFC v3 2/3] qapi: change the type of TargetInfo.arch from string to enum SysEmuTarget

[Markus Armbruster <armbru@redhat.com>]

Laszlo Ersek <lersek@redhat.com> writes:

> Now that we have @SysEmuTarget, it makes sense to restict
> @TargetInfo.@arch to valid sysemu targets at the schema level.

We could mention that supported targets become visible in QMP
introspection.  But I can't see what QMP clients could do with this
information, so let's not bother.

>
> Cc: "Daniel P. Berrange" <berrange@redhat.com>
> Cc: David Gibson <dgibson@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Kashyap Chamarthy <kchamart@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
>     RFCv3:
>     
>     - The patch is new in this version. [Markus]
>
>  qapi/misc.json |  6 ++++--
>  arch_init.c    | 18 +++++++++++++++++-
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 5636f4a14997..3b4fbc534089 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -5,6 +5,8 @@
>  # = Miscellanea
>  ##
>  
> +{ 'include': 'common.json' }
> +
>  ##
>  # @qmp_capabilities:
>  #
> @@ -2449,12 +2451,12 @@
>  #
>  # Information describing the QEMU target.
>  #
> -# @arch: the target architecture (eg "x86_64", "i386", etc)
> +# @arch: the target architecture
>  #
>  # Since: 1.2.0
>  ##
>  { 'struct': 'TargetInfo',
> -  'data': { 'arch': 'str' } }
> +  'data': { 'arch': 'SysEmuTarget' } }
>  
>  ##
>  # @query-target:
> diff --git a/arch_init.c b/arch_init.c
> index 6ee07478bd11..4367f30291f8 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -29,6 +29,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/audio/soundhw.h"
>  #include "qapi/qapi-commands-misc.h"
> +#include "qapi/error.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "hw/acpi/acpi.h"
> @@ -111,8 +112,23 @@ int xen_available(void)
>  TargetInfo *qmp_query_target(Error **errp)
>  {
>      TargetInfo *info = g_malloc0(sizeof(*info));
> +    Error *local_err = NULL;
>  
> -    info->arch = g_strdup(TARGET_NAME);
> +    /*
> +     * The fallback enum value is irrelevant here (TARGET_NAME is a
> +     * macro and can never be NULL), so simply pass zero. Also, the

We pass -1 in similar cases elsewhere.

> +     * lookup should never fail -- if it fails, then @SysEmuTarget needs
> +     * extending. Catch that with an assertion, but also handle it
> +     * gracefully, in case someone adventurous disables assertions.
> +     */
> +    info->arch = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, 0,
> +                                 &local_err);
> +    g_assert(!local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        qapi_free_TargetInfo(info);
> +        return NULL;
> +    }

Simpler:

       info->arch = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, 0,
                                    &error_abort);

>  
>      return info;
>  }