Re: [Qemu-devel] [PATCH v8 14/22] qapi: add an error in case a discriminator is conditional

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Marc-André Lureau <address@hidden> writes:
>
>> Making a discriminator conditional doesn't make much sense.  "qapi:

I guess it could be made to work as long as the discriminator's
condition implies each variant's condition.  Example:

    { 'union': 'BlockdevQcow2Encryption',
      'base': {
          'format': { 'type': 'BlockdevQcow2EncryptionFormat',
                      'if': 'defined(CONFIG_AES) || defined(CONFIG_LUKS)' } },
      'discriminator': 'format',
      'data': {
          'aes': { 'type': 'QCryptoBlockOptionsQCow',
                   'if': 'defined(CONFIG_AES)' },
          'luks': 'type': 'QCryptoBlockOptionsLUKS',
                  'if': 'defined(CONFIG_LUKS)' } }

Now, this example looks like a bad idea.  Whether there's a more
legitimate use remains to be seen.  Until then, outlawing conditional
discriminators is totally fine with me.

>> add a dictionary form for TYPE" allows it, this patch fixes
>> it. Eventually the two could be merged, but for clarity of review it
>> is kept separate.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>
> Reviewed-by: Markus Armbruster <address@hidden>
>
> I may accept the offer to let me squash it into the previous patch when
> I apply.

Squashed patch's commit message:

    qapi: Add 'if' to implicit struct members

    The generated code is for now *unconditional*.  Later patches generate
    the conditionals.

    Note that union discriminators may not have 'if' conditionals.

Avoids passing judgement :)