[{"id":1821522,"web_url":"http://patchwork.ozlabs.org/comment/1821522/","msgid":"<87mv2maf91.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-12-13T14:12:26","subject":"Re: [Qemu-devel] [PATCH v3 36/50] qapi: add conditions to VNC\n\ttype/commands/events on the schema","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Cc: Daniel for his opinion on QCryptoCipherAlgorithm member des-rfb.\n\nMarc-André Lureau <marcandre.lureau@redhat.com> writes:\n\n> Add #if defined(CONFIG_VNC) in generated code, and adjust the\n> qmp/hmp code accordingly.\n>\n> Commands made conditional:\n>\n> * query-vnc, query-vnc-servers, change-vnc-password\n>\n>   Before the patch, the commands for !CONFIG_VNC are stubs that fail\n>   like this:\n>\n>     {\"error\": {\"class\": \"GenericError\",\n>                \"desc\": \"The feature 'vnc' is not enabled\"}}\n>\n>   Afterwards, they fail like this:\n>\n>     {\"error\": {\"class\": \"CommandNotFound\",\n>                \"desc\": \"The command FOO has not been found\"}}\n>\n>   I call that an improvement, because it lets clients distinguish\n>   between command unavailable (class CommandNotFound) and command failed\n>   (class GenericError).\n\nMoreover, query-qmp-schema no longer reports the command as available.\nThat's the point of this series!  Let's spell it out in the commit\nmessage.\n\nSame for events etc.\n\nHow this affects HMP is worth describing, too.\n\n> Events made conditional:\n>\n> * VNC_CONNECTED, VNC_INITIALIZED, VNC_DISCONNECTED\n>\n> Enum made conditional:\n\nThe enum isn't made conditional, only one of its values is.  Suggest \"\nEnumeration values made conditional:\".\n\n> * QCryptoCipherAlgorithm\n>\n>     # @des-rfb: RFB specific variant of single DES. Do not use except in VNC.\n\nDaniel, is this okay?\n\n> Occurrences of VNC (case insensitive) in the schema that aren't\n> covered by this change:\n>\n> * add_client\n>\n>   Command has other uses, including \"socket bases character devices\".\n>   These are unconditional as far as I can tell.\n>\n> * set_password, expire_password\n>\n>   In theory, these commands could be used for managing any service's\n>   password.  In practice, they're used for VNC and SPICE services.\n>   They're documented for \"remote display session\" / \"remote display\n>   server\".\n>\n>   The service is selected by argument @protocol.  The code special-cases\n>   protocol-specific argument checking, then calls a protocol-specific\n>   function to do the work.  If it fails, the command fails with \"Could\n>   not set password\".  It does when the service isn't compiled in (it's a\n>   stub then).\n>\n>   We could make these commands conditional on the conjunction of all\n>   services [currently: defined(CONFIG_VNC) || defined(CONFIG_SPICE)],\n>   but I doubt it's worthwhile.\n\nWhat could be worthwhile: change @protocol from 'str' to suitable\nenumeration type with appropriately conditional values, so that\nintrospection correctly reports available functioniality.\n\nNot possible for add_client, because that one overloads protocol\nfurther: \"spice\" and \"vnc\" are special, anything else is interpreted as\ncharacter device name.  Character devices named \"spice\" or \"vnc\" don't\nwork.  Bad design.  Needs to be replaced & deprecated.\n\n> * change\n>\n>   Command has other uses, namely changing media.\n>   This patch inlines a stub; no functional change.\n\nAlready deprecated.\n\n> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>\n> ---\n>  qapi/crypto.json           |  3 ++-\n>  qapi/ui.json               | 45 ++++++++++++++++++++++++++++-----------------\n>  ui/vnc.h                   |  2 ++\n>  crypto/cipher-builtin.c    |  9 +++++++++\n>  crypto/cipher-gcrypt.c     | 10 ++++++++--\n>  crypto/cipher-nettle.c     | 14 +++++++++++---\n>  crypto/cipher.c            | 13 +++++++++++--\n>  hmp.c                      |  9 ++++++++-\n>  qmp.c                      | 30 ++++--------------------------\n>  tests/test-crypto-cipher.c |  2 ++\n>  hmp-commands-info.hx       |  2 ++\n>  11 files changed, 87 insertions(+), 52 deletions(-)\n>\n> diff --git a/qapi/crypto.json b/qapi/crypto.json\n> index 288bc056ef..09535b7be2 100644\n> --- a/qapi/crypto.json\n> +++ b/qapi/crypto.json\n> @@ -79,7 +79,8 @@\n>  { 'enum': 'QCryptoCipherAlgorithm',\n>    'prefix': 'QCRYPTO_CIPHER_ALG',\n>    'data': ['aes-128', 'aes-192', 'aes-256',\n> -           'des-rfb', '3des',\n> +           { 'name': 'des-rfb', 'if': 'defined(CONFIG_VNC)' },\n> +           '3des',\n>             'cast5-128',\n>             'serpent-128', 'serpent-192', 'serpent-256',\n>             'twofish-128', 'twofish-192', 'twofish-256']}\n> diff --git a/qapi/ui.json b/qapi/ui.json\n> index e5d6610b4a..4b573d214b 100644\n> --- a/qapi/ui.json\n> +++ b/qapi/ui.json\n> @@ -369,7 +369,8 @@\n>    'data': { 'host': 'str',\n>              'service': 'str',\n>              'family': 'NetworkAddressFamily',\n> -            'websocket': 'bool' } }\n> +            'websocket': 'bool' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncServerInfo:\n> @@ -383,7 +384,8 @@\n>  ##\n>  { 'struct': 'VncServerInfo',\n>    'base': 'VncBasicInfo',\n> -  'data': { '*auth': 'str' } }\n> +  'data': { '*auth': 'str' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncClientInfo:\n> @@ -400,7 +402,8 @@\n>  ##\n>  { 'struct': 'VncClientInfo',\n>    'base': 'VncBasicInfo',\n> -  'data': { '*x509_dname': 'str', '*sasl_username': 'str' } }\n> +  'data': { '*x509_dname': 'str', '*sasl_username': 'str' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncInfo:\n> @@ -441,7 +444,8 @@\n>  { 'struct': 'VncInfo',\n>    'data': {'enabled': 'bool', '*host': 'str',\n>             '*family': 'NetworkAddressFamily',\n> -           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }\n> +           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']},\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncPrimaryAuth:\n> @@ -452,7 +456,8 @@\n>  ##\n>  { 'enum': 'VncPrimaryAuth',\n>    'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',\n> -            'tls', 'vencrypt', 'sasl' ] }\n> +            'tls', 'vencrypt', 'sasl' ],\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncVencryptSubAuth:\n> @@ -466,8 +471,8 @@\n>              'tls-none',  'x509-none',\n>              'tls-vnc',   'x509-vnc',\n>              'tls-plain', 'x509-plain',\n> -            'tls-sasl',  'x509-sasl' ] }\n> -\n> +            'tls-sasl',  'x509-sasl' ],\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncServerInfo2:\n> @@ -484,8 +489,8 @@\n>  { 'struct': 'VncServerInfo2',\n>    'base': 'VncBasicInfo',\n>    'data': { 'auth'      : 'VncPrimaryAuth',\n> -            '*vencrypt' : 'VncVencryptSubAuth' } }\n> -\n> +            '*vencrypt' : 'VncVencryptSubAuth' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VncInfo2:\n> @@ -517,7 +522,8 @@\n>              'clients'   : ['VncClientInfo'],\n>              'auth'      : 'VncPrimaryAuth',\n>              '*vencrypt' : 'VncVencryptSubAuth',\n> -            '*display'  : 'str' } }\n> +            '*display'  : 'str' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @query-vnc:\n> @@ -548,8 +554,8 @@\n>  #    }\n>  #\n>  ##\n> -{ 'command': 'query-vnc', 'returns': 'VncInfo' }\n> -\n> +{ 'command': 'query-vnc', 'returns': 'VncInfo',\n> +  'if': 'defined(CONFIG_VNC)' }\n>  ##\n>  # @query-vnc-servers:\n>  #\n> @@ -559,7 +565,8 @@\n>  #\n>  # Since: 2.3\n>  ##\n> -{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'] }\n> +{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @change-vnc-password:\n> @@ -573,7 +580,8 @@\n>  # Notes:  An empty password in this command will set the password to the empty\n>  #         string.  Existing clients are unaffected by executing this command.\n>  ##\n> -{ 'command': 'change-vnc-password', 'data': {'password': 'str'} }\n> +{ 'command': 'change-vnc-password', 'data': {'password': 'str'},\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VNC_CONNECTED:\n> @@ -602,7 +610,8 @@\n>  ##\n>  { 'event': 'VNC_CONNECTED',\n>    'data': { 'server': 'VncServerInfo',\n> -            'client': 'VncBasicInfo' } }\n> +            'client': 'VncBasicInfo' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VNC_INITIALIZED:\n> @@ -629,7 +638,8 @@\n>  ##\n>  { 'event': 'VNC_INITIALIZED',\n>    'data': { 'server': 'VncServerInfo',\n> -            'client': 'VncClientInfo' } }\n> +            'client': 'VncClientInfo' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # @VNC_DISCONNECTED:\n> @@ -655,7 +665,8 @@\n>  ##\n>  { 'event': 'VNC_DISCONNECTED',\n>    'data': { 'server': 'VncServerInfo',\n> -            'client': 'VncClientInfo' } }\n> +            'client': 'VncClientInfo' },\n> +  'if': 'defined(CONFIG_VNC)' }\n>  \n>  ##\n>  # = Input\n> diff --git a/ui/vnc.h b/ui/vnc.h\n> index 694cf32ca9..5572bfdc9e 100644\n> --- a/ui/vnc.h\n> +++ b/ui/vnc.h\n> @@ -291,7 +291,9 @@ struct VncState\n>      bool encode_ws;\n>      bool websocket;\n>  \n> +#ifdef CONFIG_VNC\n>      VncClientInfo *info;\n> +#endif\n>  \n>      Buffer output;\n>      Buffer input;\n> diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c\n> index d8c811fd33..647850843e 100644\n> --- a/crypto/cipher-builtin.c\n> +++ b/crypto/cipher-builtin.c\n> @@ -35,17 +35,22 @@ struct QCryptoCipherBuiltinAES {\n>      QCryptoCipherBuiltinAESContext key_tweak;\n>      uint8_t iv[AES_BLOCK_SIZE];\n>  };\n> +\n> +#ifdef CONFIG_VNC\n>  typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;\n>  struct QCryptoCipherBuiltinDESRFB {\n>      uint8_t *key;\n>      size_t nkey;\n>  };\n> +#endif\n>  \n>  typedef struct QCryptoCipherBuiltin QCryptoCipherBuiltin;\n>  struct QCryptoCipherBuiltin {\n>      union {\n>          QCryptoCipherBuiltinAES aes;\n> +#ifdef CONFIG_VNC\n>          QCryptoCipherBuiltinDESRFB desrfb;\n> +#endif\n>      } state;\n>      size_t blocksize;\n>      void (*free)(QCryptoCipher *cipher);\n> @@ -403,7 +408,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,\n>                               QCryptoCipherMode mode)\n>  {\n>      switch (alg) {\n> +#ifdef CONFIG_VNC\n>      case QCRYPTO_CIPHER_ALG_DES_RFB:\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_AES_128:\n>      case QCRYPTO_CIPHER_ALG_AES_192:\n>      case QCRYPTO_CIPHER_ALG_AES_256:\n> @@ -449,9 +456,11 @@ static QCryptoCipherBuiltin *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>      }\n>  \n>      switch (alg) {\n> +#ifdef CONFIG_VNC\n>      case QCRYPTO_CIPHER_ALG_DES_RFB:\n>          ctxt = qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);\n>          break;\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_AES_128:\n>      case QCRYPTO_CIPHER_ALG_AES_192:\n>      case QCRYPTO_CIPHER_ALG_AES_256:\n> diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c\n> index 10d75da75d..c240aaee26 100644\n> --- a/crypto/cipher-gcrypt.c\n> +++ b/crypto/cipher-gcrypt.c\n> @@ -29,7 +29,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,\n>                               QCryptoCipherMode mode)\n>  {\n>      switch (alg) {\n> +#ifdef CONFIG_VNC\n>      case QCRYPTO_CIPHER_ALG_DES_RFB:\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_3DES:\n>      case QCRYPTO_CIPHER_ALG_AES_128:\n>      case QCRYPTO_CIPHER_ALG_AES_192:\n> @@ -114,10 +116,11 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>      }\n>  \n>      switch (alg) {\n> +#ifdef CONFIG_VNC\n>      case QCRYPTO_CIPHER_ALG_DES_RFB:\n>          gcryalg = GCRY_CIPHER_DES;\n>          break;\n> -\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_3DES:\n>          gcryalg = GCRY_CIPHER_3DES;\n>          break;\n> @@ -181,6 +184,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>          }\n>      }\n>  \n> +#ifdef CONFIG_VNC\n>      if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {\n>          /* We're using standard DES cipher from gcrypt, so we need\n>           * to munge the key so that the results are the same as the\n> @@ -190,7 +194,9 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>          err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);\n>          g_free(rfbkey);\n>          ctx->blocksize = 8;\n> -    } else {\n> +    } else\n> +#endif /* CONFIG_VNC */\n> +    {\n>          if (mode == QCRYPTO_CIPHER_MODE_XTS) {\n>              nkey /= 2;\n>              err = gcry_cipher_setkey(ctx->handle, key, nkey);\n> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c\n> index 3848cb3b3a..ace5ec20f6 100644\n> --- a/crypto/cipher-nettle.c\n> +++ b/crypto/cipher-nettle.c\n> @@ -67,6 +67,7 @@ static void aes_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,\n>      aes_decrypt(&aesctx->dec, length, dst, src);\n>  }\n>  \n> +#ifdef CONFIG_VNC\n>  static void des_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,\n>                                 uint8_t *dst, const uint8_t *src)\n>  {\n> @@ -78,6 +79,7 @@ static void des_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,\n>  {\n>      des_decrypt(ctx, length, dst, src);\n>  }\n> +#endif\n>  \n>  static void des3_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,\n>                                  uint8_t *dst, const uint8_t *src)\n> @@ -141,6 +143,7 @@ static void aes_decrypt_wrapper(const void *ctx, size_t length,\n>      aes_decrypt(&aesctx->dec, length, dst, src);\n>  }\n>  \n> +#ifdef CONFIG_VNC\n>  static void des_encrypt_wrapper(const void *ctx, size_t length,\n>                                  uint8_t *dst, const uint8_t *src)\n>  {\n> @@ -152,6 +155,7 @@ static void des_decrypt_wrapper(const void *ctx, size_t length,\n>  {\n>      des_decrypt(ctx, length, dst, src);\n>  }\n> +#endif\n>  \n>  static void des3_encrypt_wrapper(const void *ctx, size_t length,\n>                                  uint8_t *dst, const uint8_t *src)\n> @@ -221,7 +225,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,\n>                               QCryptoCipherMode mode)\n>  {\n>      switch (alg) {\n> +#ifdef CONFIG_VNC\n>      case QCRYPTO_CIPHER_ALG_DES_RFB:\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_3DES:\n>      case QCRYPTO_CIPHER_ALG_AES_128:\n>      case QCRYPTO_CIPHER_ALG_AES_192:\n> @@ -271,7 +277,6 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>                                                     Error **errp)\n>  {\n>      QCryptoCipherNettle *ctx;\n> -    uint8_t *rfbkey;\n>  \n>      switch (mode) {\n>      case QCRYPTO_CIPHER_MODE_ECB:\n> @@ -292,7 +297,9 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>      ctx = g_new0(QCryptoCipherNettle, 1);\n>  \n>      switch (alg) {\n> -    case QCRYPTO_CIPHER_ALG_DES_RFB:\n> +#ifdef CONFIG_VNC\n> +    case QCRYPTO_CIPHER_ALG_DES_RFB: {\n> +        uint8_t *rfbkey;\n>          ctx->ctx = g_new0(struct des_ctx, 1);\n>          rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);\n>          des_set_key(ctx->ctx, rfbkey);\n> @@ -305,7 +312,8 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,\n>  \n>          ctx->blocksize = DES_BLOCK_SIZE;\n>          break;\n> -\n> +    }\n> +#endif\n>      case QCRYPTO_CIPHER_ALG_3DES:\n>          ctx->ctx = g_new0(struct des3_ctx, 1);\n>          des3_set_key(ctx->ctx, key);\n> diff --git a/crypto/cipher.c b/crypto/cipher.c\n> index 0aad9d6d79..80355f4530 100644\n> --- a/crypto/cipher.c\n> +++ b/crypto/cipher.c\n> @@ -28,7 +28,9 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {\n>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,\n>      [QCRYPTO_CIPHER_ALG_AES_192] = 24,\n>      [QCRYPTO_CIPHER_ALG_AES_256] = 32,\n> +#ifdef CONFIG_VNC\n>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,\n> +#endif\n>      [QCRYPTO_CIPHER_ALG_3DES] = 24,\n>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,\n>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,\n> @@ -43,7 +45,9 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {\n>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,\n>      [QCRYPTO_CIPHER_ALG_AES_192] = 16,\n>      [QCRYPTO_CIPHER_ALG_AES_256] = 16,\n> +#ifdef CONFIG_VNC\n>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,\n> +#endif\n>      [QCRYPTO_CIPHER_ALG_3DES] = 8,\n>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,\n>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,\n> @@ -106,8 +110,11 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,\n>      }\n>  \n>      if (mode == QCRYPTO_CIPHER_MODE_XTS) {\n> -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB\n> -                || alg == QCRYPTO_CIPHER_ALG_3DES) {\n> +        if (\n> +#ifdef CONFIG_VNC\n> +            alg == QCRYPTO_CIPHER_ALG_DES_RFB ||\n> +#endif\n> +            alg == QCRYPTO_CIPHER_ALG_3DES) {\n>              error_setg(errp, \"XTS mode not compatible with DES-RFB/3DES\");\n>              return false;\n>          }\n> @@ -131,6 +138,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,\n>      return true;\n>  }\n>  \n> +#if defined(CONFIG_VNC)\n>  #if defined(CONFIG_GCRYPT) || defined(CONFIG_NETTLE)\n>  static uint8_t *\n>  qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,\n> @@ -148,6 +156,7 @@ qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,\n>      return ret;\n>  }\n>  #endif /* CONFIG_GCRYPT || CONFIG_NETTLE */\n> +#endif /* CONFIG_VNC */\n>  \n>  #ifdef CONFIG_GCRYPT\n>  #include \"crypto/cipher-gcrypt.c\"\n> diff --git a/hmp.c b/hmp.c\n> index cd046c6d71..5893e5bf16 100644\n> --- a/hmp.c\n> +++ b/hmp.c\n> @@ -604,6 +604,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)\n>      qapi_free_BlockStatsList(stats_list);\n>  }\n>  \n> +#ifdef CONFIG_VNC\n>  /* Helper for hmp_info_vnc_clients, _servers */\n>  static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,\n>                                    const char *name)\n> @@ -691,6 +692,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)\n>      qapi_free_VncInfo2List(info2l);\n>  \n>  }\n> +#endif\n>  \n>  #ifdef CONFIG_SPICE\n>  void hmp_info_spice(Monitor *mon, const QDict *qdict)\n> @@ -1702,12 +1704,14 @@ void hmp_eject(Monitor *mon, const QDict *qdict)\n>      hmp_handle_error(mon, &err);\n>  }\n>  \n> +#ifdef CONFIG_VNC\n>  static void hmp_change_read_arg(void *opaque, const char *password,\n>                                  void *readline_opaque)\n>  {\n>      qmp_change_vnc_password(password, NULL);\n>      monitor_read_command(opaque, 1);\n>  }\n> +#endif\n>  \n>  void hmp_change(Monitor *mon, const QDict *qdict)\n>  {\n> @@ -1718,6 +1722,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)\n>      BlockdevChangeReadOnlyMode read_only_mode = 0;\n>      Error *err = NULL;\n>  \n> +#ifdef CONFIG_VNC\n>      if (strcmp(device, \"vnc\") == 0) {\n>          if (read_only) {\n>              monitor_printf(mon,\n> @@ -1732,7 +1737,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)\n>              }\n>          }\n>          qmp_change(\"vnc\", target, !!arg, arg, &err);\n> -    } else {\n> +    } else\n> +#endif\n> +    {\n>          if (read_only) {\n>              read_only_mode =\n>                  qapi_enum_parse(&BlockdevChangeReadOnlyMode_lookup,\n> diff --git a/qmp.c b/qmp.c\n> index b86201e349..2c90dacb56 100644\n> --- a/qmp.c\n> +++ b/qmp.c\n> @@ -130,22 +130,6 @@ void qmp_cpu_add(int64_t id, Error **errp)\n>      }\n>  }\n>  \n> -#ifndef CONFIG_VNC\n> -/* If VNC support is enabled, the \"true\" query-vnc command is\n> -   defined in the VNC subsystem */\n> -VncInfo *qmp_query_vnc(Error **errp)\n> -{\n> -    error_setg(errp, QERR_FEATURE_DISABLED, \"vnc\");\n> -    return NULL;\n> -};\n> -\n> -VncInfo2List *qmp_query_vnc_servers(Error **errp)\n> -{\n> -    error_setg(errp, QERR_FEATURE_DISABLED, \"vnc\");\n> -    return NULL;\n> -};\n> -#endif\n> -\n>  #ifndef CONFIG_SPICE\n>  /*\n>   * qmp-commands.hx ensures that QMP command query-spice exists only\n> @@ -403,23 +387,17 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,\n>          qmp_change_vnc_listen(target, errp);\n>      }\n>  }\n> -#else\n> -void qmp_change_vnc_password(const char *password, Error **errp)\n> -{\n> -    error_setg(errp, QERR_FEATURE_DISABLED, \"vnc\");\n> -}\n> -static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,\n> -                           Error **errp)\n> -{\n> -    error_setg(errp, QERR_FEATURE_DISABLED, \"vnc\");\n> -}\n>  #endif /* !CONFIG_VNC */\n>  \n>  void qmp_change(const char *device, const char *target,\n>                  bool has_arg, const char *arg, Error **errp)\n>  {\n>      if (strcmp(device, \"vnc\") == 0) {\n> +#ifdef CONFIG_VNC\n>          qmp_change_vnc(target, has_arg, arg, errp);\n> +#else\n> +        error_setg(errp, QERR_FEATURE_DISABLED, \"vnc\");\n> +#endif\n>      } else {\n>          qmp_blockdev_change_medium(true, device, false, NULL, target,\n>                                     has_arg, arg, false, 0, errp);\n> diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c\n> index 07fa2fa616..5980b27389 100644\n> --- a/tests/test-crypto-cipher.c\n> +++ b/tests/test-crypto-cipher.c\n> @@ -149,6 +149,7 @@ static QCryptoCipherTestData test_data[] = {\n>              \"39f23369a9d9bacfa530e26304231461\"\n>              \"b2eb05e2c39be9fcda6c19078c6a9d1b\",\n>      },\n> +#ifdef CONFIG_VNC\n>      {\n>          .path = \"/crypto/cipher/des-rfb-ecb-56\",\n>          .alg = QCRYPTO_CIPHER_ALG_DES_RFB,\n> @@ -165,6 +166,7 @@ static QCryptoCipherTestData test_data[] = {\n>              \"ffd29f1bb5596ad94ea2d8e6196b7f09\"\n>              \"30d8ed0bf2773af36dd82a6280c20926\",\n>      },\n> +#endif\n>  #if defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)\n>      {\n>          /* Borrowed from linux-kernel crypto/testmgr.h */\n> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx\n> index 4ab7fcee98..aece8c5999 100644\n> --- a/hmp-commands-info.hx\n> +++ b/hmp-commands-info.hx\n> @@ -420,6 +420,7 @@ STEXI\n>  Show which guest mouse is receiving events.\n>  ETEXI\n>  \n> +#if defined(CONFIG_VNC)\n>      {\n>          .name       = \"vnc\",\n>          .args_type  = \"\",\n> @@ -427,6 +428,7 @@ ETEXI\n>          .help       = \"show the vnc server status\",\n>          .cmd        = hmp_info_vnc,\n>      },\n> +#endif\n>  \n>  STEXI\n>  @item info vnc\n\nI should mention at this point: the resulting change to generated code\nis lovely :)","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yxdvj05Vbz9s74\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Dec 2017 01:13:18 +1100 (AEDT)","from localhost ([::1]:35741 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1eP7mc-0000iu-PL\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Dec 2017 09:13:14 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:51616)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1eP7mH-0000ic-Ed\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:12:55 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1eP7mE-0007nc-8Q\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:12:53 -0500","from mx1.redhat.com ([209.132.183.28]:3126)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1eP7mD-0007m3-Up\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:12:50 -0500","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id DC882C07012F\n\tfor <qemu-devel@nongnu.org>; Wed, 13 Dec 2017 14:12:48 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-74.ams2.redhat.com\n\t[10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 7021471927;\n\tWed, 13 Dec 2017 14:12:28 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid D489C1138658; Wed, 13 Dec 2017 15:12:26 +0100 (CET)"],"From":"Markus Armbruster <armbru@redhat.com>","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau <marcandre.lureau@redhat.com>","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-37-marcandre.lureau@redhat.com>","Date":"Wed, 13 Dec 2017 15:12:26 +0100","In-Reply-To":"<20170911110623.24981-37-marcandre.lureau@redhat.com> (\n\t=?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Mon,\n\t11 Sep 2017 \t13:06:09 +0200\")","Message-ID":"<87mv2maf91.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 13 Dec 2017 14:12:48 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 36/50] qapi: add conditions to VNC\n\ttype/commands/events on the schema","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org,\n\t\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1821526,"web_url":"http://patchwork.ozlabs.org/comment/1821526/","msgid":"<20171213142001.GF28379@redhat.com>","list_archive_url":null,"date":"2017-12-13T14:20:01","subject":"Re: [Qemu-devel] [PATCH v3 36/50] qapi: add conditions to VNC\n\ttype/commands/events on the schema","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Wed, Dec 13, 2017 at 03:12:26PM +0100, Markus Armbruster wrote:\n> Cc: Daniel for his opinion on QCryptoCipherAlgorithm member des-rfb.\n\n> > Enum made conditional:\n> \n> The enum isn't made conditional, only one of its values is.  Suggest \"\n> Enumeration values made conditional:\".\n> \n> > * QCryptoCipherAlgorithm\n> >\n> >     # @des-rfb: RFB specific variant of single DES. Do not use except in VNC.\n> \n> Daniel, is this okay?\n\nI don't think we should touch the crypto/ code at all here.\n\nAlthough the VNC server is the only intended user of the des-rfb choice,\nI don't really think we should make this conditional on CONFIG_VNC. It\nisn't reducing the amount of code we build in any meaningful way and is\nlittering the crypto code with '#ifdef CONFIG_VNC' conditionals, which\nharms readability and I think it is a code layering violation.\n\nSo please drop the following changes:\n\n> >  qapi/crypto.json           |  3 ++-\n> >  crypto/cipher-builtin.c    |  9 +++++++++\n> >  crypto/cipher-gcrypt.c     | 10 ++++++++--\n> >  crypto/cipher-nettle.c     | 14 +++++++++++---\n> >  crypto/cipher.c            | 13 +++++++++++--\n> >  tests/test-crypto-cipher.c |  2 ++\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yxf4Y0by8z9t2f\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 14 Dec 2017 01:21:01 +1100 (AEDT)","from localhost ([::1]:35765 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1eP7u7-0003Lg-3e\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Dec 2017 09:20:59 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:53270)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1eP7tS-0003Jb-94\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:20:19 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1eP7tP-0005bG-0z\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:20:18 -0500","from mx1.redhat.com ([209.132.183.28]:6801)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>) id 1eP7tO-0005Xp-Pv\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:20:14 -0500","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 663E06014F\n\tfor <qemu-devel@nongnu.org>; Wed, 13 Dec 2017 14:20:13 +0000 (UTC)","from redhat.com (unknown [10.33.36.56])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id AA7767BA3E;\n\tWed, 13 Dec 2017 14:20:04 +0000 (UTC)"],"Date":"Wed, 13 Dec 2017 14:20:01 +0000","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Markus Armbruster <armbru@redhat.com>","Message-ID":"<20171213142001.GF28379@redhat.com>","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-37-marcandre.lureau@redhat.com>\n\t<87mv2maf91.fsf@dusky.pond.sub.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87mv2maf91.fsf@dusky.pond.sub.org>","User-Agent":"Mutt/1.9.1 (2017-09-22)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tWed, 13 Dec 2017 14:20:13 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 36/50] qapi: add conditions to VNC\n\ttype/commands/events on the schema","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau <marcandre.lureau@redhat.com>,\n\tGerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org, \n\t\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]