Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

On 03/08/2016 11:09 AM, Markus Armbruster wrote:

> This patch actually fixes a similar issue in the qmp_marshal_FOO()
> functions.

Indeed, and I didn't even realize it. I'll add that to the commit message :)

> 
> To keep ignoring it in the qapi_event_send_BAR() functions is okay.
> It's fairly easy to fix now, though: split them into two, so that the
> outer half does nothing but parameter wrapping.  For instance,
> 
>     void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
>     {
>         QDict *qmp;
>         Error *err = NULL;
>         QMPEventFuncEmit emit;
>         QmpOutputVisitor *qov;
>         Visitor *v;
>         QObject *obj;
>         _obj_BLOCK_IO_ERROR_arg param = {
>             (char *)device, operation, action, has_nospace, nospace, (char *)reason
>         };
> 
>         [do stuff...]
>     }
> 
> becomes
> 
>     static inline void do_event_send_block_io_error(_obj_BLOCK_IO_ERROR_arg param, Error **errp)
>     {
>         QDict *qmp;
>         Error *err = NULL;
>         QMPEventFuncEmit emit;
>         QmpOutputVisitor *qov;
>         Visitor *v;
>         QObject *obj;
> 
>         [do stuff...]
>     }
> 
>     void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)

Still means we can't have 'errp' as a QMP member of the error, without
some sort of renaming.  Again, not worth worrying about until we
actually want to avoid the collision.

>     {
>         do_event_send_block_io_error((_obj_BLOCK_IO_ERROR_arg){
>                 (char *)device, operation, action, has_nospace, nospace, (char *)reason
>             }, errp);
>         };
>     }
> 
> Feel free not to do that now, but mark the spot with a comment then.
> Since it's technically wrong, we could even mark it FIXME.

In fact, I have a patch in a later series [1] that WANTS to let the user
supply a boxed parameter - at which point, the difference between two
vs. one function would be whether the user requested boxing.  Sounds
like I add the FIXME here, and then that series can take care of the
possible split.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]