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

[John Ferlan]

On 02/15/2018 11:43 AM, 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é 
> ---
>  src/lxc/lxc_container.c  |  7 ---
>  src/lxc/lxc_controller.c | 10 +-
>  src/lxc/lxc_driver.c | 40 
>  3 files changed, 49 insertions(+), 8 deletions(-)
> 


[...]

> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 961baa344c..7d6568cdf8 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -3968,10 +3968,22 @@ 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:
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unexpected net type %d"), actualType);
> +goto cleanup;
>  }
>  /* Set bandwidth or warn if requested and not supported. */
>  actualBandwidth = virDomainNetGetActualBandwidth(net);
> @@ -4011,6 +4023,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;

Something Coverity noted long ago, but I've just held onto the patch...
At this point (e.g. when ret != 0 in cleanup:), @veth can only defined
if DIRECT, BRIDGE, NETWORK, or ETHERNET, so it's not happy with the
switch and cases. The way I worked around this in Coverity was:

if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT)
ignore_value(virNetDevMacVLanDelete(veth));
else /* BRIDGE, NETWORK, ETHERNET */
ignore_value(virNetDevVethDelete(veth));

I don't think that's perfect either, but it sufficed.

IDC if you keep the code as is, but figured I'd note it.

> @@ -4446,13 +4467,24 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,

Since they'd all be included here - if you adjust the comment to
indicate "It'd be nice to support DIRECT" instead of just "this" (or
somehow indicate that DIRECT is possible, but the others really aren't).

>   * 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;

[...]

I'll leave it up to you for deciding on the comment above...

Reviewed-by: John Ferlan 

John

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

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

[Daniel P . Berrangé]

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é 
---
 src/lxc/lxc_container.c  |  7 ---
 src/lxc/lxc_controller.c | 10 +-
 src/lxc/lxc_driver.c | 40 
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 96fceaf1b8..ec641c3cab 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,11 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def,
 }
 break;
 
+case VIR_DOMAIN_CAPABILITIES_POLICY_LAST:
 default:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Unsupported capabilities policy: %s"),
-   virDomainCapabilitiesPolicyTypeToString(policy));
+   _("Unsupported capabilities policy: %d"),
+   policy);
 }
 }
 
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c5e67df938..f9f26570cd 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -395,8 +395,16 @@ 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;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unexpected net type %d"),
+   ctrl->def->nets[i]->type);
+goto cleanup;
 }
 }
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 961baa344c..7d6568cdf8 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3968,10 +3968,22 @@ 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:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unexpected net type %d"), actualType);
+goto cleanup;
 }
 /* Set bandwidth or warn if requested and not supported. */
 actualBandwidth = virDomainNetGetActualBandwidth(net);
@@ -4011,6 +4023,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 +4467,24 @@ 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:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unexpected net typ