On 10/06/2016 09:38 AM, Michal Privoznik wrote:
> This function for some weird reason returns integer instead of
> virDomainNetType type. It is important to return the correct type
> so that we know what values we can expect.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/bhyve/bhyve_command.c | 2 +-
> src/bhyve/bhyve_process.c | 2 +-
> src/conf/domain_conf.c | 8 ++++----
> src/conf/domain_conf.h | 2 +-
> src/libxl/libxl_domain.c | 2 +-
> src/libxl/libxl_driver.c | 2 +-
> src/lxc/lxc_driver.c | 9 +++++++--
> src/qemu/qemu_command.c | 2 +-
> src/qemu/qemu_hotplug.c | 4 ++--
> src/qemu/qemu_process.c | 13 ++++++++++++-
> 10 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 9ad3f9b..55ad950 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -52,7 +52,7 @@ bhyveBuildNetArgStr(const virDomainDef *def,
> char macaddr[VIR_MAC_STRING_BUFLEN];
> char *realifname = NULL;
> char *brname = NULL;
> - int actualType = virDomainNetGetActualType(net);
> + virDomainNetType actualType = virDomainNetGetActualType(net);
>
> if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
> diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
> index 6db070f..9d80976 100644
> --- a/src/bhyve/bhyve_process.c
> +++ b/src/bhyve/bhyve_process.c
> @@ -77,7 +77,7 @@ bhyveNetCleanup(virDomainObjPtr vm)
>
> for (i = 0; i < vm->def->nnets; i++) {
> virDomainNetDefPtr net = vm->def->nets[i];
> - int actualType = virDomainNetGetActualType(net);
> + virDomainNetType actualType = virDomainNetGetActualType(net);
>
> if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> if (net->ifname) {
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8f04e6f..9343770 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -20804,7 +20804,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
> bool inSubelement,
> unsigned int flags)
> {
> - int actualType = virDomainNetGetActualType(def);
> + virDomainNetType actualType = virDomainNetGetActualType(def);
>
> if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def),
> @@ -20884,7 +20884,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> virDomainNetDefPtr def,
> unsigned int flags)
> {
> - unsigned int type;
> + virDomainNetType type;
> const char *typeStr;
>
> if (!def)
> @@ -21039,7 +21039,7 @@ virDomainNetDefFormat(virBufferPtr buf,
> char *prefix,
> unsigned int flags)
> {
> - unsigned int actualType = virDomainNetGetActualType(def);
> + virDomainNetType actualType = virDomainNetGetActualType(def);
> bool publicActual = false;
> int sourceLines = 0;
> const char *typeStr;
> @@ -24781,7 +24781,7 @@ virDomainStateReasonFromString(virDomainState state, const char *reason)
> * otherwise return the value from the NetDef.
> */
>
> -int
> +virDomainNetType
> virDomainNetGetActualType(virDomainNetDefPtr iface)
> {
> if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a70bc21..95bcf4f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2814,7 +2814,7 @@ int virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr def,
> const char *socket)
> ATTRIBUTE_NONNULL(1);
>
> -int virDomainNetGetActualType(virDomainNetDefPtr iface);
> +virDomainNetType virDomainNetGetActualType(virDomainNetDefPtr iface);
> const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface);
> int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface);
> const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface);
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index db2c1dc..3df04cf 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -937,7 +937,7 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
>
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
> - int actualType;
> + virDomainNetType actualType;
>
> /* If appropriate, grab a physical device from the configured
> * network's pool of devices, or resolve bridge device name
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b66cb1f..f87f549 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3328,7 +3328,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
> virDomainNetDefPtr net)
> {
> libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> - int actualType;
> + virDomainNetType actualType;
> libxl_device_nic nic;
> int ret = -1;
> char mac[VIR_MAC_STRING_BUFLEN];
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index a9e664c..91f67f2 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -3943,7 +3943,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
> {
> virLXCDomainObjPrivatePtr priv = vm->privateData;
> int ret = -1;
> - int actualType;
> + virDomainNetType actualType;
> virNetDevBandwidthPtr actualBandwidth;
> char *veth = NULL;
>
> @@ -4030,6 +4030,10 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
> case VIR_DOMAIN_NET_TYPE_DIRECT:
> ignore_value(virNetDevMacVLanDelete(veth));
> break;
> +
> + default:
> + /* no-op */
> + break;
Naturally this draws the attention of the [dead_error_condition] for
Coverity. Oh and if each of the other cases are added instead of using
default, then there's multiple dead error conditions.... It just
doesn't make sense <sigh>.
Even stranger still if I replace the actualType with just using
virDomainNetGetActualType(net) directly, then the Coverity warnings go
away. So there's something about assigning the variable that causes
Coverity to stick it's nose where it doesn't belong... <double sigh>
IDC if you keep things as is, I'll just add a false positive in my local
build...
Not that it matters (or is related), but the switch/case before this one
has no reason to go to cleanup - each condition could just return -1.
The only reason to go to cleanup is if "veth" exists and the only way
for that to happen is if the previous cases are successful.
> }
> }
>
> @@ -4430,7 +4434,8 @@ static int
> lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
> virDomainDeviceDefPtr dev)
> {
> - int detachidx, actualType, ret = -1;
> + int detachidx, ret = -1;
> + virDomainNetType actualType;
> virDomainNetDefPtr detach = NULL;
> virNetDevVPortProfilePtr vport = NULL;
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 578ff8b..2aebf8a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7910,7 +7910,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
> size_t vhostfdSize = 0;
> char **tapfdName = NULL;
> char **vhostfdName = NULL;
> - int actualType = virDomainNetGetActualType(net);
> + virDomainNetType actualType = virDomainNetGetActualType(net);
> virQEMUDriverConfigPtr cfg = NULL;
> virNetDevBandwidthPtr actualBandwidth;
> size_t i;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 72dd93b..bb8ea0d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -927,7 +927,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
> int vlan;
> bool releaseaddr = false;
> bool iface_connected = false;
> - int actualType;
> + virDomainNetType actualType;
> virNetDevBandwidthPtr actualBandwidth;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> virDomainCCWAddressSetPtr ccwaddrs = NULL;
> @@ -2373,7 +2373,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
> virDomainNetDefPtr newdev = dev->data.net;
> virDomainNetDefPtr *devslot = NULL;
> virDomainNetDefPtr olddev;
> - int oldType, newType;
> + virDomainNetType oldType, newType;
> bool needReconnect = false;
> bool needBridgeChange = false;
> bool needFilterChange = false;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a4bc082..aa6c36f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4628,7 +4628,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def)
>
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
> - int actualType;
> + virDomainNetType actualType;
>
> /* If appropriate, grab a physical device from the configured
> * network's pool of devices, or resolve bridge device name
> @@ -6038,6 +6038,17 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
> #endif
> 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:
> + /* No special cleanup procedure for these types. */
> + break;
Ironically Coverity doesn't complain on this one. There's no rhyme or
reason... This of course is where/why I got the idea to not use the
local variable.
ACK for the changes
John
> }
> /* release the physical device (or any other resources used by
> * this interface in the network driver
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list