[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors |
Date: |
Fri, 15 Apr 2016 17:05:24 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 04/13/2016 06:48 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> Rather than having two separate visitor callbacks with items
>>> already broken out, pass the actual QAPISchemaObjectType object
>>> to the visitor. This lets the visitor access things like
>>> type.is_implicit() without needing another parameter, resolving
>>> a TODO from previous patches.
>>>
>>> For convenience and consistency, the 'name' and 'info' parameters
>>> are still provided, even though they are now redundant with
>>> 'typ.name' and 'typ.info'.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>
>> I've made you push this one back in the queue a couple of times, because
>> there are pros and cons, and the work at hand didn't actually require
>> the patch. At some point we need to decide. Perhaps that point is now.
>>
>> The patch replaces two somewhat unclean "is implicit" tests by clean
>> .is_implicit() calls. Any other use of the change in this series?
>
> I'm not seeing any other direct simplification in this series. As a
> quick test, I just rebased it to the end of this series with no merge
> conflicts, and everything else still compiled and passed without it.
> I'm less sure whether any of my later pending series depend on it.
>
>>
>> Recap of pros and cons:
>>
>> * The existing interface
>>
>> def visit_object_type(self, name, info, base, members, variants):
>> def visit_object_type_flat(self, name, info, members, variants):
>>
>> is explicit and narrow, but when you need more information, you have
>> to add parameters or functions.
>>
>> * The new interface
>>
>> def visit_object_type(self, name, info, typ):
>>
>> avoids that, but now its users can access everything.
>>
>> This patch touches only visiting of objects, because only for objects we
>> have a TODO. Should we change the other visit_ methods as well, for
>> consistency?
>
> I have a pending patch in subset F (last posted at v6) that adds a 'box'
> parameter to visit_event and visit_command:
> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04397.html
> If we change all the other visit_ methods for consistency, then those
> methods would directly access command.box and event.box instead of
> needing to add a separate parameter.
Let's move this patch into subset F (should be the next series, as this
one is E), and figure it out there.
- Re: [Qemu-devel] [PATCH v14 09/19] tests: Add check-qnull, (continued)
[Qemu-devel] [PATCH v14 06/19] qmp-input: Don't consume input when checking has_member, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 11/19] qmp: Support explicit null during visits, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classification, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 12/19] spapr_drc: Expose 'null' in qom-get when there is no fdt, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 15/19] qapi-commands: Wrap argument visit in visit_start_struct, Eric Blake, 2016/04/08