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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt; John