1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 00/14] Enforce presence of all switch enum cases
  4. View patch

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

Daniel P. Berrangé posted 14 patches 7 years, 2 months ago
[libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago 
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@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_container.c b/src/lxc/lxc_container.c
index 96fceaf1b8..14928e8ece 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2035,7 +2035,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
             break;
 
         case VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT:
-            switch ((virDomainCapsFeature) i) {
+            switch (i) {
             case VIR_DOMAIN_CAPS_FEATURE_SYS_BOOT: /* No use of reboot */
                 toDrop = !keepReboot && (state != VIR_TRISTATE_SWITCH_ON);
                 break;
@@ -2066,10 +2066,10 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
             }
             break;
 
+        case VIR_DOMAIN_CAPABILITIES_POLICY_LAST:
         default:
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("Unsupported capabilities policy: %s"),
-                           virDomainCapabilitiesPolicyTypeToString(policy));
+            virReportEnumRangeError(virDomainCapabilitiesPolicy, policy);
+            return -1;
         }
     }
 
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;
+        case VIR_DOMAIN_NET_TYPE_LAST:
         default:
-            break;
+            virReportEnumRangeError(virDomainNetType, ctrl->def->nets[i]->type);
+            goto cleanup;
         }
     }
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 961baa344c..3d5b2254f2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3968,10 +3968,21 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
         if (!(veth = virLXCProcessSetupInterfaceDirect(conn, vm->def, net)))
             goto cleanup;
     }   break;
-    default:
+    case VIR_DOMAIN_NET_TYPE_USER:
+    case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+    case VIR_DOMAIN_NET_TYPE_SERVER:
+    case VIR_DOMAIN_NET_TYPE_CLIENT:
+    case VIR_DOMAIN_NET_TYPE_MCAST:
+    case VIR_DOMAIN_NET_TYPE_INTERNAL:
+    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+    case VIR_DOMAIN_NET_TYPE_UDP:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("Network device type is not supported"));
         goto cleanup;
+    case VIR_DOMAIN_NET_TYPE_LAST:
+    default:
+        virReportEnumRangeError(virDomainNetType, actualType);
+        goto cleanup;
     }
     /* Set bandwidth or warn if requested and not supported. */
     actualBandwidth = virDomainNetGetActualBandwidth(net);
@@ -4011,6 +4022,15 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
             ignore_value(virNetDevMacVLanDelete(veth));
             break;
 
+        case VIR_DOMAIN_NET_TYPE_USER:
+        case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+        case VIR_DOMAIN_NET_TYPE_SERVER:
+        case VIR_DOMAIN_NET_TYPE_CLIENT:
+        case VIR_DOMAIN_NET_TYPE_MCAST:
+        case VIR_DOMAIN_NET_TYPE_INTERNAL:
+        case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+        case VIR_DOMAIN_NET_TYPE_UDP:
+        case VIR_DOMAIN_NET_TYPE_LAST:
         default:
             /* no-op */
             break;
@@ -4446,13 +4466,23 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
          * the host side. Further the container can change
          * the mac address of NIC name, so we can't easily
          * find out which guest NIC it maps to
+         */
     case VIR_DOMAIN_NET_TYPE_DIRECT:
-        */
-
-    default:
+    case VIR_DOMAIN_NET_TYPE_USER:
+    case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+    case VIR_DOMAIN_NET_TYPE_SERVER:
+    case VIR_DOMAIN_NET_TYPE_CLIENT:
+    case VIR_DOMAIN_NET_TYPE_MCAST:
+    case VIR_DOMAIN_NET_TYPE_INTERNAL:
+    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+    case VIR_DOMAIN_NET_TYPE_UDP:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("Only bridged veth devices can be detached"));
         goto cleanup;
+    case VIR_DOMAIN_NET_TYPE_LAST:
+    default:
+        virReportEnumRangeError(virDomainNetType, actualType);
+        goto cleanup;
     }
 
     virDomainAuditNet(vm, detach, NULL, "detach", true);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
Posted by John Ferlan 7 years, 2 months ago 

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@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@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago 
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@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@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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.