Re: [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Nov 22, 2022 at 10:07:48PM +0800, zhenwei pi wrote:
> Introduce cryptodev alg type in cryptodev.json, then apply this to
> related codes, and drop 'enum CryptoDevBackendAlgType'.
> 
> There are two options:
> 1, { 'enum': 'QCryptodevBackendAlgType',
>   'prefix': 'CRYPTODEV_BACKEND_ALG',
>   'data': ['sym', 'asym']}
> Then we can keep 'CRYPTODEV_BACKEND_ALG_SYM' and avoid lots of
> changes.
> 2, changes in this patch(with prefix 'QCRYPTODEV_BACKEND_ALG').
> 
> To avoid breaking the rule of QAPI, use 2 here.
> 
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  backends/cryptodev-builtin.c |  6 +++---
>  backends/cryptodev-lkcf.c    |  4 ++--
>  backends/cryptodev.c         |  6 +++---
>  hw/virtio/virtio-crypto.c    | 14 +++++++-------
>  include/sysemu/cryptodev.h   |  8 +-------
>  qapi/cryptodev.json          | 14 ++++++++++++++
>  6 files changed, 30 insertions(+), 22 deletions(-)


> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index 08895271eb..5fb7b8f43f 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -537,7 +537,7 @@ static int cryptodev_builtin_operation(
>      CryptoDevBackendBuiltinSession *sess;
>      CryptoDevBackendSymOpInfo *sym_op_info;
>      CryptoDevBackendAsymOpInfo *asym_op_info;
> -    enum CryptoDevBackendAlgType algtype = op_info->algtype;
> +    enum QCryptodevBackendAlgType algtype = op_info->algtype;

QAPI generates a typedef for every enum with the same name as
the enum itself. IOW, this should change to merely

   QCryptodevBackendAlgType algtype = op_info->algtype;


> diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
> index de3d1867c5..919bf05b75 100644
> --- a/backends/cryptodev-lkcf.c
> +++ b/backends/cryptodev-lkcf.c
> @@ -477,7 +477,7 @@ static int cryptodev_lkcf_operation(
>      CryptoDevBackendLKCF *lkcf =
>          CRYPTODEV_BACKEND_LKCF(backend);
>      CryptoDevBackendLKCFSession *sess;
> -    enum CryptoDevBackendAlgType algtype = op_info->algtype;
> +    enum QCryptodevBackendAlgType algtype = op_info->algtype;

As above


> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 81941af816..d3caded920 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -120,10 +120,10 @@ int cryptodev_backend_crypto_operation(
>  {
>      VirtIOCryptoReq *req = opaque1;
>      CryptoDevBackendOpInfo *op_info = &req->op_info;
> -    enum CryptoDevBackendAlgType algtype = req->flags;
> +    enum QCryptodevBackendAlgType algtype = req->flags;

As above.


> diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
> index af152d09db..f68a4baf13 100644
> --- a/include/sysemu/cryptodev.h
> +++ b/include/sysemu/cryptodev.h
> @@ -49,12 +49,6 @@ typedef struct CryptoDevBackendPeers CryptoDevBackendPeers;
>  typedef struct CryptoDevBackendClient
>                       CryptoDevBackendClient;
>  
> -enum CryptoDevBackendAlgType {
> -    CRYPTODEV_BACKEND_ALG_SYM,
> -    CRYPTODEV_BACKEND_ALG_ASYM,
> -    CRYPTODEV_BACKEND_ALG__MAX,
> -};
> -
>  /**
>   * CryptoDevBackendSymSessionInfo:
>   *
> @@ -181,7 +175,7 @@ typedef struct CryptoDevBackendAsymOpInfo {
>  } CryptoDevBackendAsymOpInfo;
>  
>  typedef struct CryptoDevBackendOpInfo {
> -    enum CryptoDevBackendAlgType algtype;
> +    enum QCryptodevBackendAlgType algtype;

As above, drop the 'enum' qualifier

>      uint32_t op_code;
>      uint64_t session_id;
>      union {

With the redundant 'enum' qualifiers dropped, then you may add

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With 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 :|