[libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases

[John Ferlan]

On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements, or cast away
> enum type in places where we don't wish to cover all cases.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/lxc/lxc_container.c  |  8 ++++----
>  src/lxc/lxc_controller.c |  8 +++++++-
>  src/lxc/lxc_driver.c     | 38 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 45 insertions(+), 9 deletions(-)
> 

[...]

> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index c5e67df938..7346804c18 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -395,8 +395,14 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr ctrl)
>          case VIR_DOMAIN_NET_TYPE_INTERNAL:
>          case VIR_DOMAIN_NET_TYPE_DIRECT:
>          case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Unsupported net type %s"),
> +                           virDomainNetTypeToString(ctrl->def->nets[i]->type));
> +            goto cleanup;

So this will cause an error; whereas, previously there was no error for
main startup and the values essentially ignored (with nothing added to
nicindexes). Is this the "safe" thing to do? Especially for existing
configs which may now fail... Perhaps some kind of VIR_* message printed
instead?

This certainly falls into the category of probably should have been an
error all along, but "fear" (at least in my mind) causes trepidation to
just spring an error on someone now.

Feels like one of those guest disappearing things, but you understand
the LXC controller better than I - so I'll just note it and let you decide.


> +        case VIR_DOMAIN_NET_TYPE_LAST:
>          default:
> -            break;
> +            virReportEnumRangeError(virDomainNetType, ctrl->def->nets[i]->type);
> +            goto cleanup;

I think having an error here is certainly reasonable even though it
probably is a can't get there type thing.

>          }
>      }
>  

[...]

Reviewed-by: John Ferlan <jferlan at redhat.com>

John