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

[Markus Armbruster <armbru@redhat.com>]

Cc: Daniel for his opinion on QCryptoCipherAlgorithm member des-rfb.

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Add #if defined(CONFIG_VNC) in generated code, and adjust the
> qmp/hmp code accordingly.
>
> Commands made conditional:
>
> * query-vnc, query-vnc-servers, change-vnc-password
>
>   Before the patch, the commands for !CONFIG_VNC are stubs that fail
>   like this:
>
>     {"error": {"class": "GenericError",
>                "desc": "The feature 'vnc' is not enabled"}}
>
>   Afterwards, they fail like this:
>
>     {"error": {"class": "CommandNotFound",
>                "desc": "The command FOO has not been found"}}
>
>   I call that an improvement, because it lets clients distinguish
>   between command unavailable (class CommandNotFound) and command failed
>   (class GenericError).

Moreover, query-qmp-schema no longer reports the command as available.
That's the point of this series!  Let's spell it out in the commit
message.

Same for events etc.

How this affects HMP is worth describing, too.

> Events made conditional:
>
> * VNC_CONNECTED, VNC_INITIALIZED, VNC_DISCONNECTED
>
> 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?

> Occurrences of VNC (case insensitive) in the schema that aren't
> covered by this change:
>
> * add_client
>
>   Command has other uses, including "socket bases character devices".
>   These are unconditional as far as I can tell.
>
> * set_password, expire_password
>
>   In theory, these commands could be used for managing any service's
>   password.  In practice, they're used for VNC and SPICE services.
>   They're documented for "remote display session" / "remote display
>   server".
>
>   The service is selected by argument @protocol.  The code special-cases
>   protocol-specific argument checking, then calls a protocol-specific
>   function to do the work.  If it fails, the command fails with "Could
>   not set password".  It does when the service isn't compiled in (it's a
>   stub then).
>
>   We could make these commands conditional on the conjunction of all
>   services [currently: defined(CONFIG_VNC) || defined(CONFIG_SPICE)],
>   but I doubt it's worthwhile.

What could be worthwhile: change @protocol from 'str' to suitable
enumeration type with appropriately conditional values, so that
introspection correctly reports available functioniality.

Not possible for add_client, because that one overloads protocol
further: "spice" and "vnc" are special, anything else is interpreted as
character device name.  Character devices named "spice" or "vnc" don't
work.  Bad design.  Needs to be replaced & deprecated.

> * change
>
>   Command has other uses, namely changing media.
>   This patch inlines a stub; no functional change.

Already deprecated.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qapi/crypto.json           |  3 ++-
>  qapi/ui.json               | 45 ++++++++++++++++++++++++++++-----------------
>  ui/vnc.h                   |  2 ++
>  crypto/cipher-builtin.c    |  9 +++++++++
>  crypto/cipher-gcrypt.c     | 10 ++++++++--
>  crypto/cipher-nettle.c     | 14 +++++++++++---
>  crypto/cipher.c            | 13 +++++++++++--
>  hmp.c                      |  9 ++++++++-
>  qmp.c                      | 30 ++++--------------------------
>  tests/test-crypto-cipher.c |  2 ++
>  hmp-commands-info.hx       |  2 ++
>  11 files changed, 87 insertions(+), 52 deletions(-)
>
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 288bc056ef..09535b7be2 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -79,7 +79,8 @@
>  { 'enum': 'QCryptoCipherAlgorithm',
>    'prefix': 'QCRYPTO_CIPHER_ALG',
>    'data': ['aes-128', 'aes-192', 'aes-256',
> -           'des-rfb', '3des',
> +           { 'name': 'des-rfb', 'if': 'defined(CONFIG_VNC)' },
> +           '3des',
>             'cast5-128',
>             'serpent-128', 'serpent-192', 'serpent-256',
>             'twofish-128', 'twofish-192', 'twofish-256']}
> diff --git a/qapi/ui.json b/qapi/ui.json
> index e5d6610b4a..4b573d214b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -369,7 +369,8 @@
>    'data': { 'host': 'str',
>              'service': 'str',
>              'family': 'NetworkAddressFamily',
> -            'websocket': 'bool' } }
> +            'websocket': 'bool' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncServerInfo:
> @@ -383,7 +384,8 @@
>  ##
>  { 'struct': 'VncServerInfo',
>    'base': 'VncBasicInfo',
> -  'data': { '*auth': 'str' } }
> +  'data': { '*auth': 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncClientInfo:
> @@ -400,7 +402,8 @@
>  ##
>  { 'struct': 'VncClientInfo',
>    'base': 'VncBasicInfo',
> -  'data': { '*x509_dname': 'str', '*sasl_username': 'str' } }
> +  'data': { '*x509_dname': 'str', '*sasl_username': 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncInfo:
> @@ -441,7 +444,8 @@
>  { 'struct': 'VncInfo',
>    'data': {'enabled': 'bool', '*host': 'str',
>             '*family': 'NetworkAddressFamily',
> -           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
> +           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']},
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncPrimaryAuth:
> @@ -452,7 +456,8 @@
>  ##
>  { 'enum': 'VncPrimaryAuth',
>    'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
> -            'tls', 'vencrypt', 'sasl' ] }
> +            'tls', 'vencrypt', 'sasl' ],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncVencryptSubAuth:
> @@ -466,8 +471,8 @@
>              'tls-none',  'x509-none',
>              'tls-vnc',   'x509-vnc',
>              'tls-plain', 'x509-plain',
> -            'tls-sasl',  'x509-sasl' ] }
> -
> +            'tls-sasl',  'x509-sasl' ],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncServerInfo2:
> @@ -484,8 +489,8 @@
>  { 'struct': 'VncServerInfo2',
>    'base': 'VncBasicInfo',
>    'data': { 'auth'      : 'VncPrimaryAuth',
> -            '*vencrypt' : 'VncVencryptSubAuth' } }
> -
> +            '*vencrypt' : 'VncVencryptSubAuth' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncInfo2:
> @@ -517,7 +522,8 @@
>              'clients'   : ['VncClientInfo'],
>              'auth'      : 'VncPrimaryAuth',
>              '*vencrypt' : 'VncVencryptSubAuth',
> -            '*display'  : 'str' } }
> +            '*display'  : 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @query-vnc:
> @@ -548,8 +554,8 @@
>  #    }
>  #
>  ##
> -{ 'command': 'query-vnc', 'returns': 'VncInfo' }
> -
> +{ 'command': 'query-vnc', 'returns': 'VncInfo',
> +  'if': 'defined(CONFIG_VNC)' }
>  ##
>  # @query-vnc-servers:
>  #
> @@ -559,7 +565,8 @@
>  #
>  # Since: 2.3
>  ##
> -{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'] }
> +{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @change-vnc-password:
> @@ -573,7 +580,8 @@
>  # Notes:  An empty password in this command will set the password to the empty
>  #         string.  Existing clients are unaffected by executing this command.
>  ##
> -{ 'command': 'change-vnc-password', 'data': {'password': 'str'} }
> +{ 'command': 'change-vnc-password', 'data': {'password': 'str'},
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VNC_CONNECTED:
> @@ -602,7 +610,8 @@
>  ##
>  { 'event': 'VNC_CONNECTED',
>    'data': { 'server': 'VncServerInfo',
> -            'client': 'VncBasicInfo' } }
> +            'client': 'VncBasicInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VNC_INITIALIZED:
> @@ -629,7 +638,8 @@
>  ##
>  { 'event': 'VNC_INITIALIZED',
>    'data': { 'server': 'VncServerInfo',
> -            'client': 'VncClientInfo' } }
> +            'client': 'VncClientInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VNC_DISCONNECTED:
> @@ -655,7 +665,8 @@
>  ##
>  { 'event': 'VNC_DISCONNECTED',
>    'data': { 'server': 'VncServerInfo',
> -            'client': 'VncClientInfo' } }
> +            'client': 'VncClientInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # = Input
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 694cf32ca9..5572bfdc9e 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -291,7 +291,9 @@ struct VncState
>      bool encode_ws;
>      bool websocket;
>  
> +#ifdef CONFIG_VNC
>      VncClientInfo *info;
> +#endif
>  
>      Buffer output;
>      Buffer input;
> diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
> index d8c811fd33..647850843e 100644
> --- a/crypto/cipher-builtin.c
> +++ b/crypto/cipher-builtin.c
> @@ -35,17 +35,22 @@ struct QCryptoCipherBuiltinAES {
>      QCryptoCipherBuiltinAESContext key_tweak;
>      uint8_t iv[AES_BLOCK_SIZE];
>  };
> +
> +#ifdef CONFIG_VNC
>  typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
>  struct QCryptoCipherBuiltinDESRFB {
>      uint8_t *key;
>      size_t nkey;
>  };
> +#endif
>  
>  typedef struct QCryptoCipherBuiltin QCryptoCipherBuiltin;
>  struct QCryptoCipherBuiltin {
>      union {
>          QCryptoCipherBuiltinAES aes;
> +#ifdef CONFIG_VNC
>          QCryptoCipherBuiltinDESRFB desrfb;
> +#endif
>      } state;
>      size_t blocksize;
>      void (*free)(QCryptoCipher *cipher);
> @@ -403,7 +408,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
>  {
>      switch (alg) {
> +#ifdef CONFIG_VNC
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
> +#endif
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
>      case QCRYPTO_CIPHER_ALG_AES_256:
> @@ -449,9 +456,11 @@ static QCryptoCipherBuiltin *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>      }
>  
>      switch (alg) {
> +#ifdef CONFIG_VNC
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>          ctxt = qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);
>          break;
> +#endif
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
>      case QCRYPTO_CIPHER_ALG_AES_256:
> diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
> index 10d75da75d..c240aaee26 100644
> --- a/crypto/cipher-gcrypt.c
> +++ b/crypto/cipher-gcrypt.c
> @@ -29,7 +29,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
>  {
>      switch (alg) {
> +#ifdef CONFIG_VNC
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
> +#endif
>      case QCRYPTO_CIPHER_ALG_3DES:
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
> @@ -114,10 +116,11 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>      }
>  
>      switch (alg) {
> +#ifdef CONFIG_VNC
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>          gcryalg = GCRY_CIPHER_DES;
>          break;
> -
> +#endif
>      case QCRYPTO_CIPHER_ALG_3DES:
>          gcryalg = GCRY_CIPHER_3DES;
>          break;
> @@ -181,6 +184,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>          }
>      }
>  
> +#ifdef CONFIG_VNC
>      if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
>          /* We're using standard DES cipher from gcrypt, so we need
>           * to munge the key so that the results are the same as the
> @@ -190,7 +194,9 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>          err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);
>          g_free(rfbkey);
>          ctx->blocksize = 8;
> -    } else {
> +    } else
> +#endif /* CONFIG_VNC */
> +    {
>          if (mode == QCRYPTO_CIPHER_MODE_XTS) {
>              nkey /= 2;
>              err = gcry_cipher_setkey(ctx->handle, key, nkey);
> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
> index 3848cb3b3a..ace5ec20f6 100644
> --- a/crypto/cipher-nettle.c
> +++ b/crypto/cipher-nettle.c
> @@ -67,6 +67,7 @@ static void aes_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      aes_decrypt(&aesctx->dec, length, dst, src);
>  }
>  
> +#ifdef CONFIG_VNC
>  static void des_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                 uint8_t *dst, const uint8_t *src)
>  {
> @@ -78,6 +79,7 @@ static void des_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>  {
>      des_decrypt(ctx, length, dst, src);
>  }
> +#endif
>  
>  static void des3_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                  uint8_t *dst, const uint8_t *src)
> @@ -141,6 +143,7 @@ static void aes_decrypt_wrapper(const void *ctx, size_t length,
>      aes_decrypt(&aesctx->dec, length, dst, src);
>  }
>  
> +#ifdef CONFIG_VNC
>  static void des_encrypt_wrapper(const void *ctx, size_t length,
>                                  uint8_t *dst, const uint8_t *src)
>  {
> @@ -152,6 +155,7 @@ static void des_decrypt_wrapper(const void *ctx, size_t length,
>  {
>      des_decrypt(ctx, length, dst, src);
>  }
> +#endif
>  
>  static void des3_encrypt_wrapper(const void *ctx, size_t length,
>                                  uint8_t *dst, const uint8_t *src)
> @@ -221,7 +225,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
>  {
>      switch (alg) {
> +#ifdef CONFIG_VNC
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
> +#endif
>      case QCRYPTO_CIPHER_ALG_3DES:
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
> @@ -271,7 +277,6 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>                                                     Error **errp)
>  {
>      QCryptoCipherNettle *ctx;
> -    uint8_t *rfbkey;
>  
>      switch (mode) {
>      case QCRYPTO_CIPHER_MODE_ECB:
> @@ -292,7 +297,9 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>      ctx = g_new0(QCryptoCipherNettle, 1);
>  
>      switch (alg) {
> -    case QCRYPTO_CIPHER_ALG_DES_RFB:
> +#ifdef CONFIG_VNC
> +    case QCRYPTO_CIPHER_ALG_DES_RFB: {
> +        uint8_t *rfbkey;
>          ctx->ctx = g_new0(struct des_ctx, 1);
>          rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
>          des_set_key(ctx->ctx, rfbkey);
> @@ -305,7 +312,8 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>  
>          ctx->blocksize = DES_BLOCK_SIZE;
>          break;
> -
> +    }
> +#endif
>      case QCRYPTO_CIPHER_ALG_3DES:
>          ctx->ctx = g_new0(struct des3_ctx, 1);
>          des3_set_key(ctx->ctx, key);
> diff --git a/crypto/cipher.c b/crypto/cipher.c
> index 0aad9d6d79..80355f4530 100644
> --- a/crypto/cipher.c
> +++ b/crypto/cipher.c
> @@ -28,7 +28,9 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
>      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> +#ifdef CONFIG_VNC
>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> +#endif
>      [QCRYPTO_CIPHER_ALG_3DES] = 24,
>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> @@ -43,7 +45,9 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> +#ifdef CONFIG_VNC
>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> +#endif
>      [QCRYPTO_CIPHER_ALG_3DES] = 8,
>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> @@ -106,8 +110,11 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
>      }
>  
>      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> -                || alg == QCRYPTO_CIPHER_ALG_3DES) {
> +        if (
> +#ifdef CONFIG_VNC
> +            alg == QCRYPTO_CIPHER_ALG_DES_RFB ||
> +#endif
> +            alg == QCRYPTO_CIPHER_ALG_3DES) {
>              error_setg(errp, "XTS mode not compatible with DES-RFB/3DES");
>              return false;
>          }
> @@ -131,6 +138,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
>      return true;
>  }
>  
> +#if defined(CONFIG_VNC)
>  #if defined(CONFIG_GCRYPT) || defined(CONFIG_NETTLE)
>  static uint8_t *
>  qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,
> @@ -148,6 +156,7 @@ qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,
>      return ret;
>  }
>  #endif /* CONFIG_GCRYPT || CONFIG_NETTLE */
> +#endif /* CONFIG_VNC */
>  
>  #ifdef CONFIG_GCRYPT
>  #include "crypto/cipher-gcrypt.c"
> diff --git a/hmp.c b/hmp.c
> index cd046c6d71..5893e5bf16 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -604,6 +604,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
>      qapi_free_BlockStatsList(stats_list);
>  }
>  
> +#ifdef CONFIG_VNC
>  /* Helper for hmp_info_vnc_clients, _servers */
>  static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
>                                    const char *name)
> @@ -691,6 +692,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
>      qapi_free_VncInfo2List(info2l);
>  
>  }
> +#endif
>  
>  #ifdef CONFIG_SPICE
>  void hmp_info_spice(Monitor *mon, const QDict *qdict)
> @@ -1702,12 +1704,14 @@ void hmp_eject(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &err);
>  }
>  
> +#ifdef CONFIG_VNC
>  static void hmp_change_read_arg(void *opaque, const char *password,
>                                  void *readline_opaque)
>  {
>      qmp_change_vnc_password(password, NULL);
>      monitor_read_command(opaque, 1);
>  }
> +#endif
>  
>  void hmp_change(Monitor *mon, const QDict *qdict)
>  {
> @@ -1718,6 +1722,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>      BlockdevChangeReadOnlyMode read_only_mode = 0;
>      Error *err = NULL;
>  
> +#ifdef CONFIG_VNC
>      if (strcmp(device, "vnc") == 0) {
>          if (read_only) {
>              monitor_printf(mon,
> @@ -1732,7 +1737,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>              }
>          }
>          qmp_change("vnc", target, !!arg, arg, &err);
> -    } else {
> +    } else
> +#endif
> +    {
>          if (read_only) {
>              read_only_mode =
>                  qapi_enum_parse(&BlockdevChangeReadOnlyMode_lookup,
> diff --git a/qmp.c b/qmp.c
> index b86201e349..2c90dacb56 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -130,22 +130,6 @@ void qmp_cpu_add(int64_t id, Error **errp)
>      }
>  }
>  
> -#ifndef CONFIG_VNC
> -/* If VNC support is enabled, the "true" query-vnc command is
> -   defined in the VNC subsystem */
> -VncInfo *qmp_query_vnc(Error **errp)
> -{
> -    error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
> -    return NULL;
> -};
> -
> -VncInfo2List *qmp_query_vnc_servers(Error **errp)
> -{
> -    error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
> -    return NULL;
> -};
> -#endif
> -
>  #ifndef CONFIG_SPICE
>  /*
>   * qmp-commands.hx ensures that QMP command query-spice exists only
> @@ -403,23 +387,17 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
>          qmp_change_vnc_listen(target, errp);
>      }
>  }
> -#else
> -void qmp_change_vnc_password(const char *password, Error **errp)
> -{
> -    error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
> -}
> -static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
> -                           Error **errp)
> -{
> -    error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
> -}
>  #endif /* !CONFIG_VNC */
>  
>  void qmp_change(const char *device, const char *target,
>                  bool has_arg, const char *arg, Error **errp)
>  {
>      if (strcmp(device, "vnc") == 0) {
> +#ifdef CONFIG_VNC
>          qmp_change_vnc(target, has_arg, arg, errp);
> +#else
> +        error_setg(errp, QERR_FEATURE_DISABLED, "vnc");
> +#endif
>      } else {
>          qmp_blockdev_change_medium(true, device, false, NULL, target,
>                                     has_arg, arg, false, 0, errp);
> diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
> index 07fa2fa616..5980b27389 100644
> --- a/tests/test-crypto-cipher.c
> +++ b/tests/test-crypto-cipher.c
> @@ -149,6 +149,7 @@ static QCryptoCipherTestData test_data[] = {
>              "39f23369a9d9bacfa530e26304231461"
>              "b2eb05e2c39be9fcda6c19078c6a9d1b",
>      },
> +#ifdef CONFIG_VNC
>      {
>          .path = "/crypto/cipher/des-rfb-ecb-56",
>          .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
> @@ -165,6 +166,7 @@ static QCryptoCipherTestData test_data[] = {
>              "ffd29f1bb5596ad94ea2d8e6196b7f09"
>              "30d8ed0bf2773af36dd82a6280c20926",
>      },
> +#endif
>  #if defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)
>      {
>          /* Borrowed from linux-kernel crypto/testmgr.h */
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 4ab7fcee98..aece8c5999 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -420,6 +420,7 @@ STEXI
>  Show which guest mouse is receiving events.
>  ETEXI
>  
> +#if defined(CONFIG_VNC)
>      {
>          .name       = "vnc",
>          .args_type  = "",
> @@ -427,6 +428,7 @@ ETEXI
>          .help       = "show the vnc server status",
>          .cmd        = hmp_info_vnc,
>      },
> +#endif
>  
>  STEXI
>  @item info vnc

I should mention at this point: the resulting change to generated code
is lovely :)