[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/7] nbd: Update qapi to support exporting multiple bitmap
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v4 4/7] nbd: Update qapi to support exporting multiple bitmaps |
Date: |
Wed, 21 Oct 2020 06:19:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> On 10/20/20 3:51 AM, Markus Armbruster wrote:
>
>>> #define QAPI_LIST_ADD(list, element) do { \
>>> typeof(list) _tmp = g_new(typeof(*(list)), 1); \
>>> _tmp->value = (element); \
>>> _tmp->next = (list); \
>>> (list) = _tmp; \
>>> } while (0)
>>>
>>>
>>> Markus, thoughts on if we should publish this macro,
>>
>> If it's widely useful.
>>
>> "git-grep -- '->value ='" matches ~200 times. A patch converting these
>> to the macro where possible would make a strong case for having the
>> macro.
>>
>>> and if so, which
>>> header would be best?
>>
>> The macro is generic: @list's type may be any of the struct TYPEList we
>> generate for the QAPI type ['TYPE'].
>>
>> We don't want to generate this macro next to each of these struct
>> definitions. A non-generic macro would go there, but let's not generate
>> almost a hundred non-generic macros where a single generic one can do
>> the job.
>
> Agreed.
>
>>
>> The closest we have to a common base is GenericList. It's in in
>> visitor.h because it's only used by visitors so far. Adding the macro
>> next it is not so smart, because we don't want non-visitor code to
>> include visitor.h just for this macro.
>
> Also agreed.
>
>>
>> Perhaps the macro should go into qapi/util.h, and perhaps GenericList
>> and GenericAlternate should move there, too.
>
> GenericList is easy, but GenericAlternate is harder: it would introduce
> a cyclic declaration dependency (generated qapi-builtin-types.h includes
> qapi/util.h for the definition of QEnumLookup, but qapi/util.h declaring
> GenericAlternate would depend on including qapi-builtin-types.h for the
> definition of QType).
You're right.
QType is a built-in QAPI type. The typedef is generated into
qapi-builtin-types.h.
It needs to be a QAPI type because it's the type of alternates'
(implicit) member @type.
I figure the easiest way to move GenericAlternate (if we want to), is
creating a new header, or rather splitting qapi/util.h into the part
needed by qapi-builtin-types.h and the part that needs
qapi-builtin-types.h.
Doesn't seem to be worth our while now. We can simply put the macro
into qapi/util.h and call it a day.
- Re: [PATCH v4 2/7] nbd: Add new qemu:allocation-depth metadata context, (continued)
[PATCH v4 5/7] nbd: Simplify qemu bitmap context name, Eric Blake, 2020/10/09
[PATCH v4 7/7] nbd: Allow export of multiple bitmaps for one device, Eric Blake, 2020/10/09
Message not available