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é <berra...@redhat.com>
> ---
> 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 <jfer...@redhat.com>
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
