Laszlo Ersek <ler...@redhat.com> writes:
> On 04/23/18 11:50, Markus Armbruster wrote:
>> Laszlo Ersek <ler...@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" <berra...@redhat.com>
>>> Cc: David Gibson <dgib...@redhat.com>
>>> Cc: Eric Blake <ebl...@redhat.com>
>>> Cc: Gerd Hoffmann <kra...@redhat.com>
>>> Cc: Kashyap Chamarthy <kcham...@redhat.com>
>>> Cc: Markus Armbruster <arm...@redhat.com>
>>> Cc: Paolo Bonzini <pbonz...@redhat.com>
>>> Cc: Thomas Huth <th...@redhat.com>
>>> Signed-off-by: Laszlo Ersek <ler...@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);
>
> OK, I'll use this form -- but should I use -1 or 0, after all, for
> default value?
I count more than a dozen instances of -1, and none of zero. Please use
-1.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
