Re: [libvirt PATCH] drivers: Group global feature together

[Ján Tomko <jtomko@xxxxxxxxxx>]

s/feature/features/

On a Wednesday in 2022, Andrea Bolognani wrote:
> All these features are supposed to be handled by the call to
> virDriverFeatureIsGlobal() placed right above the switch
> statement, so group them together and add a comment. If any of
> these features is actually encountered as part of the switch
> statements, that means there's a bug in the driver and we should
> error out.
>
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
> Applies on top of
>
>  [PATCH 0/8] driver: Fix handling of driver feature flags
>  https://listman.redhat.com/archives/libvir-list/2022-February/msg00644.html
>
> src/ch/ch_driver.c          | 13 +++++++------
> src/esx/esx_driver.c        | 21 +++++++++++----------
> src/libxl/libxl_driver.c    | 16 +++++++++-------
> src/lxc/lxc_driver.c        | 11 ++++++-----
> src/network/bridge_driver.c | 15 ++++++++-------
> src/openvz/openvz_driver.c  | 16 +++++++++-------
> src/qemu/qemu_driver.c      | 16 +++++++++-------
> 7 files changed, 59 insertions(+), 49 deletions(-)
>
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index ac9298c0b5..9cbd7b71df 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -928,23 +928,24 @@ chConnectSupportsFeature(virConnectPtr conn,
>         return supported;
>
>     switch ((virDrvFeature) feature) {
> +        case VIR_DRV_FEATURE_REMOTE:
> +        case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
> +        case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
> +        case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
>         case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>         case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:
> -            return 1;
> +        case VIR_DRV_FEATURE_FD_PASSING:
> +            /* Should have already been handled by virDriverFeatureIsGlobal() */
> +            return -1;

Here you return an error without reporting an error.

Would virReportEnumRangeError be reasonable to use here?

Jano

>         case VIR_DRV_FEATURE_MIGRATION_V2:
>         case VIR_DRV_FEATURE_MIGRATION_V3:
>         case VIR_DRV_FEATURE_MIGRATION_P2P:
>         case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:
> -        case VIR_DRV_FEATURE_FD_PASSING:
>         case VIR_DRV_FEATURE_XML_MIGRATABLE:
>         case VIR_DRV_FEATURE_MIGRATION_OFFLINE:
>         case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>         case VIR_DRV_FEATURE_MIGRATION_DIRECT:
>         case VIR_DRV_FEATURE_MIGRATION_V1:
> -        case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
> -        case VIR_DRV_FEATURE_REMOTE:
> -        case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
> -        case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
>         default:
>             return 0;

Attachment: signature.asc
Description: PGP signature