Re: [Qemu-devel] [PATCH v3 36/50] qapi: add conditions to VNC type/commands/events on the schema

[Daniel P. Berrange]

On Wed, Dec 13, 2017 at 03:12:26PM +0100, Markus Armbruster wrote:
> Cc: Daniel for his opinion on QCryptoCipherAlgorithm member des-rfb.

> > Enum made conditional:
> 
> The enum isn't made conditional, only one of its values is.  Suggest "
> Enumeration values made conditional:".
> 
> > * QCryptoCipherAlgorithm
> >
> >     # @des-rfb: RFB specific variant of single DES. Do not use except in 
> > VNC.
> 
> Daniel, is this okay?

I don't think we should touch the crypto/ code at all here.

Although the VNC server is the only intended user of the des-rfb choice,
I don't really think we should make this conditional on CONFIG_VNC. It
isn't reducing the amount of code we build in any meaningful way and is
littering the crypto code with '#ifdef CONFIG_VNC' conditionals, which
harms readability and I think it is a code layering violation.

So please drop the following changes:

> >  qapi/crypto.json           |  3 ++-
> >  crypto/cipher-builtin.c    |  9 +++++++++
> >  crypto/cipher-gcrypt.c     | 10 ++++++++--
> >  crypto/cipher-nettle.c     | 14 +++++++++++---
> >  crypto/cipher.c            | 13 +++++++++++--
> >  tests/test-crypto-cipher.c |  2 ++

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|