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

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 02:33:56PM -0500, John Ferlan wrote:
> 
> 
> 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.

LXC controller code only runs on guest startup, so reporting an error here
is good. The driver code should not have even launched the controller if it
saw these NIC types.

> 
> 
> > +        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

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