Re: [Qemu-devel] [PULL 14/15] qapi: Don't box branches of flat unions

From: Eric Blake <eblake@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PULL 14/15] qapi: Don't box branches of flat unions
Date: Tue, 23 Feb 2016 08:18:55 -0700	[thread overview]
Message-ID: <56CC785F.9020708@redhat.com> (raw)
In-Reply-To: <20160223145039.GH22780@redhat.com>

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

On 02/23/2016 07:50 AM, Daniel P. Berrange wrote:
> On Fri, Feb 19, 2016 at 01:18:05PM +0100, Markus Armbruster wrote:
>> From: Eric Blake <eblake@redhat.com>
>>
>> There's no reason to do two malloc's for a flat union; let's just
>> inline the branch struct directly into the C union branch of the
>> flat union.
>>

> 
> My code needs todo something a little different though - it needs
> to directly visit one of the union branches. Previously, I could
> allocate a QCryptoBlockOptions and then visit a specific union
> branch like this:
> 
>    QCryptoBlockOptions *opts = NULL;
> 
>    opts = g_new0(QCryptoBlockOptions, 1);
>    opts.format = Q_CRYPTO_BLOCK_FORMAT_LUKS
> 
>    visit_type_QCryptoBlockOptionsLUKS(v, "luks", &opts.u.luks, errp)

which allocated a pointer and visited the fields.  We no longer need the
pointer, but still need the fields visited.

> 
> 
> I need todo this, because my visitor instance does not have any
> 'format' field to visit - we know the format upfront from the
> block layer driver choice. So we need to directly visit the
> appropriate inline union branch. This no longer works, because
> the opts.u.luks field is now inlined instead of being boxed :-(

You're using OptsVisitor, right? Is it possible to hack the QemuOpts
that you feed to opts_visitor_new() to include a 'format' field?

> 
> I could visit_type_QCryptoBlockOptionsLUKS_fields(v, opts.u.luks, errp)
> but the '_fields' methods are all declared static in qapi-visit.c
> preventing their use.
> 
> IMHO, now that QAPI inlines the flat unions, we should be making
> the _fields() methods public.

But my suggestion would just be a hack. Yours makes more sense in the
long run, so I'll go ahead and propose that patch.

-- 
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 --]