src/conf/domain_conf.c | 49 ++++--- src/conf/network_conf.c | 73 +++++++--- src/conf/network_conf.h | 2 +- src/conf/virnetworkobj.c | 24 +++- src/esx/esx_network_driver.c | 19 ++- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 309 ++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 8 ++ 8 files changed, 329 insertions(+), 156 deletions(-)
v1 patch here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
since v1:
1. Change the type declaration of _virNetworkForwardDef.type
from int to virNetworkForwardType
2. use the default case to report out of range error with
virReportEnumRangeError
Signed-off-by: Shi Lei <shilei.massclouds@gmx.com>
---
src/conf/domain_conf.c | 49 ++++---
src/conf/network_conf.c | 73 +++++++---
src/conf/network_conf.h | 2 +-
src/conf/virnetworkobj.c | 24 +++-
src/esx/esx_network_driver.c | 19 ++-
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 309 ++++++++++++++++++++++++++++---------------
src/qemu/qemu_process.c | 8 ++
8 files changed, 329 insertions(+), 156 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 178c6d2..de08788 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29975,40 +29975,49 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface)
if (!(def = virNetworkDefParseString(xml)))
goto cleanup;
- if ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_OPEN)) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
/* for these forward types, the actual net type really *is*
* NETWORK; we just keep the info from the portgroup in
* iface->data.network.actual
*/
ret = VIR_DOMAIN_NET_TYPE_NETWORK;
+ break;
- } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) &&
- def->bridge) {
-
- /* <forward type='bridge'/> <bridge name='xxx'/>
- * is VIR_DOMAIN_NET_TYPE_BRIDGE
- */
-
- ret = VIR_DOMAIN_NET_TYPE_BRIDGE;
-
- } else if (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) {
-
+ case VIR_NETWORK_FORWARD_HOSTDEV:
ret = VIR_DOMAIN_NET_TYPE_HOSTDEV;
+ break;
- } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_PRIVATE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_VEPA) ||
- (def->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ if (def->bridge) {
+ /* <forward type='bridge'/> <bridge name='xxx'/>
+ * is VIR_DOMAIN_NET_TYPE_BRIDGE
+ */
+ ret = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ break;
+ }
+
+ /* intentionally fall through to the direct case for
+ * VIR_NETWORK_FORWARD_BRIDGE with no bridge device defined
+ */
+ ATTRIBUTE_FALLTHROUGH;
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
/* <forward type='bridge|private|vepa|passthrough'> are all
* VIR_DOMAIN_NET_TYPE_DIRECT.
*/
-
ret = VIR_DOMAIN_NET_TYPE_DIRECT;
+ break;
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
cleanup:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 630a87f..526291c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1955,21 +1955,40 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto error;
}
VIR_FREE(stp);
- if (def->mtu &&
- (def->forward.type != VIR_NETWORK_FORWARD_NONE &&
- def->forward.type != VIR_NETWORK_FORWARD_NAT &&
- def->forward.type != VIR_NETWORK_FORWARD_ROUTE &&
- def->forward.type != VIR_NETWORK_FORWARD_OPEN)) {
- virReportError(VIR_ERR_XML_ERROR,
- _("mtu size only allowed in open, route, nat, "
- "and isolated mode, not in %s (network '%s')"),
- virNetworkForwardTypeToString(def->forward.type),
- def->name);
- goto error;
+ if (def->mtu) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ virReportError(VIR_ERR_XML_ERROR,
+ _("mtu size only allowed in open, route, nat, "
+ "and isolated mode, not in %s (network '%s')"),
+ virNetworkForwardTypeToString(def->forward.type),
+ def->name);
+ goto error;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto error;
+ }
}
/* Extract custom metadata */
@@ -2349,6 +2368,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
char uuidstr[VIR_UUID_STRING_BUFLEN];
size_t i;
bool shortforward;
+ bool hasbridge = false;
virBufferAddLit(buf, "<network");
if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0))
@@ -2469,22 +2489,33 @@ virNetworkDefFormatBuf(virBufferPtr buf,
virBufferAddLit(buf, "</forward>\n");
}
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
+ hasbridge = true;
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
- if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
- def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
- def->bridge || def->macTableManager) {
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto error;
+ }
+ if (hasbridge || def->bridge || def->macTableManager) {
virBufferAddLit(buf, "<bridge");
virBufferEscapeString(buf, " name='%s'", def->bridge);
- if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
- def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
+ if (hasbridge)
virBufferAsprintf(buf, " stp='%s' delay='%ld'",
def->stp ? "on" : "off", def->delay);
- }
if (def->macTableManager) {
virBufferAsprintf(buf, " macTableManager='%s'",
virNetworkBridgeMACTableManagerTypeToString(def->macTableManager));
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 54c8ed1..8fa8114 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -199,7 +199,7 @@ struct _virNetworkForwardPfDef {
typedef struct _virNetworkForwardDef virNetworkForwardDef;
typedef virNetworkForwardDef *virNetworkForwardDefPtr;
struct _virNetworkForwardDef {
- int type; /* One of virNetworkForwardType constants */
+ virNetworkForwardType type;
bool managed; /* managed attribute for hostdev mode */
int driverName; /* enum virNetworkForwardDriverNameType */
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index e00c8a7..81b53e8 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1009,19 +1009,31 @@ virNetworkLoadConfig(virNetworkObjListPtr nets,
goto error;
}
- if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
- def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
-
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
if (!def->mac_specified) {
virNetworkSetBridgeMacAddr(def);
virNetworkSaveConfig(configDir, def);
}
- } else {
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
/* Throw away MAC address for other forward types,
* which could have been generated by older libvirt RPMs */
def->mac_specified = false;
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto error;
}
if (!(obj = virNetworkObjAssignDef(nets, def, 0)))
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 04118b4..4d1b3cd 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -326,12 +326,27 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
}
/* FIXME: Add support for NAT */
- if (def->forward.type != VIR_NETWORK_FORWARD_NONE &&
- def->forward.type != VIR_NETWORK_FORWARD_BRIDGE) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ break;
+
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported forward mode '%s'"),
virNetworkForwardTypeToString(def->forward.type));
goto cleanup;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
/* Verify that specified HostPortGroups don't exist already */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1a4cf98..fc386e1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2369,6 +2369,7 @@ virNetDevMacVLanCreate;
virNetDevMacVLanCreateWithVPortProfile;
virNetDevMacVLanDelete;
virNetDevMacVLanDeleteWithVPortProfile;
+virNetDevMacVLanModeTypeFromString;
virNetDevMacVLanReleaseName;
virNetDevMacVLanReserveName;
virNetDevMacVLanRestartWithVPortProfile;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index da3c32e..a6c9111 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -485,6 +485,11 @@ networkUpdateState(virNetworkObjPtr obj,
case VIR_NETWORK_FORWARD_HOSTDEV:
/* so far no extra checks */
break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
/* Try and read dnsmasq/radvd pids of active networks */
@@ -2088,20 +2093,37 @@ networkRefreshDaemonsHelper(virNetworkObjPtr obj,
virObjectLock(obj);
def = virNetworkObjGetDef(obj);
- if (virNetworkObjIsActive(obj) &&
- ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_OPEN))) {
- /* Only the three L3 network types that are configured by
- * libvirt will have a dnsmasq or radvd daemon associated
- * with them. Here we send a SIGHUP to an existing
- * dnsmasq and/or radvd, or restart them if they've
- * disappeared.
- */
- networkRefreshDhcpDaemon(driver, obj);
- networkRefreshRadvd(driver, obj);
+ if (virNetworkObjIsActive(obj)) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
+ /* Only the three L3 network types that are configured by
+ * libvirt will have a dnsmasq or radvd daemon associated
+ * with them. Here we send a SIGHUP to an existing
+ * dnsmasq and/or radvd, or restart them if they've
+ * disappeared.
+ */
+ networkRefreshDhcpDaemon(driver, obj);
+ networkRefreshRadvd(driver, obj);
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
+ }
}
+
+ cleanup:
virObjectUnlock(obj);
return 0;
}
@@ -2128,20 +2150,37 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
virObjectLock(obj);
def = virNetworkObjGetDef(obj);
- if (virNetworkObjIsActive(obj) &&
- ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) {
- /* Only three of the L3 network types that are configured by
- * libvirt need to have iptables rules reloaded. The 4th L3
- * network type, forward='open', doesn't need this because it
- * has no iptables rules.
- */
- networkRemoveFirewallRules(def);
- if (networkAddFirewallRules(def) < 0) {
- /* failed to add but already logged */
+ if (virNetworkObjIsActive(obj)) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ /* Only three of the L3 network types that are configured by
+ * libvirt need to have iptables rules reloaded. The 4th L3
+ * network type, forward='open', doesn't need this because it
+ * has no iptables rules.
+ */
+ networkRemoveFirewallRules(def);
+ /* No need to check return value since already logged internally */
+ ignore_value(networkAddFirewallRules(def));
+ break;
+
+ case VIR_NETWORK_FORWARD_OPEN:
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
}
+
+ cleanup:
virObjectUnlock(obj);
return 0;
}
@@ -2719,9 +2758,13 @@ networkCreateInterfacePool(virNetworkDefPtr netdef)
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
case VIR_NETWORK_FORWARD_OPEN:
- case VIR_NETWORK_FORWARD_LAST:
/* by definition these will never be encountered here */
break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, netdef->forward.type);
+ goto cleanup;
}
}
@@ -2840,6 +2883,11 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
if (networkStartNetworkExternal(obj) < 0)
goto cleanup;
break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
/* finally we can call the 'started' hook script if any */
@@ -2919,6 +2967,11 @@ networkShutdownNetwork(virNetworkDriverStatePtr driver,
case VIR_NETWORK_FORWARD_HOSTDEV:
ret = networkShutdownNetworkExternal(obj);
break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ return -1;
}
/* now that we know it's stopped call the hook if present */
@@ -3273,11 +3326,11 @@ networkValidate(virNetworkDriverStatePtr driver,
/* Only the three L3 network types that are configured by libvirt
* need to have a bridge device name / mac address provided
*/
- if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
- def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
-
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
/* if no bridge name was given in the config, find a name
* unused by any other libvirt networks and assign it.
*/
@@ -3285,7 +3338,13 @@ networkValidate(virNetworkDriverStatePtr driver,
return -1;
virNetworkSetBridgeMacAddr(def);
- } else {
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
/* They are also the only types that currently support setting
* a MAC or IP address for the host-side device (bridge), DNS
* configuration, or network-wide bandwidth limits.
@@ -3331,6 +3390,12 @@ networkValidate(virNetworkDriverStatePtr driver,
return -1;
}
bandwidthAllowed = false;
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ return -1;
}
/* we support configs with a single PF defined:
@@ -3755,9 +3820,10 @@ networkUpdate(virNetworkPtr net,
/* Take care of anything that must be done before updating the
* live NetworkDef.
*/
- if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
- def->forward.type == VIR_NETWORK_FORWARD_NAT ||
- def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
+ switch (def->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
switch (section) {
case VIR_NETWORK_SECTION_FORWARD:
case VIR_NETWORK_SECTION_FORWARD_INTERFACE:
@@ -3768,14 +3834,26 @@ networkUpdate(virNetworkPtr net,
* old rules (and remember to load new ones after the
* update).
*/
- if (def->forward.type != VIR_NETWORK_FORWARD_OPEN) {
- networkRemoveFirewallRules(def);
- needFirewallRefresh = true;
- }
+ networkRemoveFirewallRules(def);
+ needFirewallRefresh = true;
break;
default:
break;
}
+ break;
+
+ case VIR_NETWORK_FORWARD_OPEN:
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, def->forward.type);
+ goto cleanup;
}
}
@@ -4440,10 +4518,11 @@ networkAllocateActualDevice(virDomainDefPtr dom,
iface->data.network.actual->trustGuestRxFilters
= netdef->trustGuestRxFilters;
- if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_OPEN)) {
+ switch (netdef->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
/* for these forward types, the actual net type really *is*
* NETWORK; we just keep the info from the portgroup in
* iface->data.network.actual
@@ -4463,46 +4542,9 @@ networkAllocateActualDevice(virDomainDefPtr dom,
if (networkPlugBandwidth(obj, iface) < 0)
goto error;
+ break;
- } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) &&
- netdef->bridge) {
-
- /* <forward type='bridge'/> <bridge name='xxx'/>
- * is VIR_DOMAIN_NET_TYPE_BRIDGE
- */
-
- iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_BRIDGE;
- if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
- netdef->bridge) < 0)
- goto error;
- iface->data.network.actual->data.bridge.macTableManager
- = netdef->macTableManager;
-
- /* merge virtualports from interface, network, and portgroup to
- * arrive at actual virtualport to use
- */
- if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
- iface->virtPortProfile,
- netdef->virtPortProfile,
- portgroup
- ? portgroup->virtPortProfile : NULL) < 0) {
- goto error;
- }
- virtport = iface->data.network.actual->virtPortProfile;
- if (virtport) {
- /* only type='openvswitch' is allowed for bridges */
- if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("<virtualport type='%s'> not supported for network "
- "'%s' which uses a bridge device"),
- virNetDevVPortTypeToString(virtport->virtPortType),
- netdef->name);
- goto error;
- }
- }
-
- } else if (netdef->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) {
-
+ case VIR_NETWORK_FORWARD_HOSTDEV: {
virDomainHostdevSubsysPCIBackendType backend;
iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV;
@@ -4575,32 +4617,67 @@ networkAllocateActualDevice(virDomainDefPtr dom,
goto error;
}
}
+ break;
+ }
- } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_VEPA) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ if (netdef->bridge) {
+ /* <forward type='bridge'/> <bridge name='xxx'/>
+ * is VIR_DOMAIN_NET_TYPE_BRIDGE
+ */
+ iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ if (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
+ netdef->bridge) < 0)
+ goto error;
+ iface->data.network.actual->data.bridge.macTableManager
+ = netdef->macTableManager;
+
+ /* merge virtualports from interface, network, and portgroup to
+ * arrive at actual virtualport to use
+ */
+ if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
+ iface->virtPortProfile,
+ netdef->virtPortProfile,
+ portgroup
+ ? portgroup->virtPortProfile : NULL) < 0) {
+ goto error;
+ }
+ virtport = iface->data.network.actual->virtPortProfile;
+ if (virtport) {
+ /* only type='openvswitch' is allowed for bridges */
+ if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("<virtualport type='%s'> not supported for network "
+ "'%s' which uses a bridge device"),
+ virNetDevVPortTypeToString(virtport->virtPortType),
+ netdef->name);
+ goto error;
+ }
+ }
+ break;
+ }
+
+ /* intentionally fall through to the direct case for
+ * VIR_NETWORK_FORWARD_BRIDGE with no bridge device defined
+ */
+ ATTRIBUTE_FALLTHROUGH;
+
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
/* <forward type='bridge|private|vepa|passthrough'> are all
* VIR_DOMAIN_NET_TYPE_DIRECT.
*/
/* Set type=direct and appropriate <source mode='xxx'/> */
iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_DIRECT;
- switch (netdef->forward.type) {
- case VIR_NETWORK_FORWARD_BRIDGE:
- iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_BRIDGE;
- break;
- case VIR_NETWORK_FORWARD_PRIVATE:
- iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_PRIVATE;
- break;
- case VIR_NETWORK_FORWARD_VEPA:
- iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_VEPA;
- break;
- case VIR_NETWORK_FORWARD_PASSTHROUGH:
- iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_PASSTHRU;
- break;
- }
+
+ /* NO need to check the value returned from virNetDevMacVLanModeTypeFromString
+ * it must be valid for these forward type(bridge|private|vepa|passthrough)
+ */
+ iface->data.network.actual->data.direct.mode =
+ virNetDevMacVLanModeTypeFromString(virNetworkForwardTypeToString(netdef->forward.type));
/* merge virtualports from interface, network, and portgroup to
* arrive at actual virtualport to use
@@ -4680,6 +4757,12 @@ networkAllocateActualDevice(virDomainDefPtr dom,
dev->device.dev) < 0)
goto error;
}
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, netdef->forward.type);
+ goto error;
}
if (virNetworkObjMacMgrAdd(obj, driver->dnsmasqStateDir,
@@ -5037,13 +5120,27 @@ networkReleaseActualDevice(virDomainDefPtr dom,
}
netdef = virNetworkObjGetDef(obj);
- if (iface->data.network.actual &&
- (netdef->forward.type == VIR_NETWORK_FORWARD_NONE ||
- netdef->forward.type == VIR_NETWORK_FORWARD_NAT ||
- netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
- netdef->forward.type == VIR_NETWORK_FORWARD_OPEN) &&
- networkUnplugBandwidth(obj, iface) < 0)
+ switch (netdef->forward.type) {
+ case VIR_NETWORK_FORWARD_NONE:
+ case VIR_NETWORK_FORWARD_NAT:
+ case VIR_NETWORK_FORWARD_ROUTE:
+ case VIR_NETWORK_FORWARD_OPEN:
+ if (iface->data.network.actual && networkUnplugBandwidth(obj, iface) < 0)
+ goto error;
+ break;
+
+ case VIR_NETWORK_FORWARD_BRIDGE:
+ case VIR_NETWORK_FORWARD_PRIVATE:
+ case VIR_NETWORK_FORWARD_VEPA:
+ case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, netdef->forward.type);
goto error;
+ }
if ((!iface->data.network.actual) ||
((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) &&
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bba157b..1946a28 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4582,6 +4582,14 @@ qemuProcessGetNetworkAddress(const char *netname,
goto cleanup;
}
break;
+
+ case VIR_NETWORK_FORWARD_HOSTDEV:
+ break;
+
+ case VIR_NETWORK_FORWARD_LAST:
+ default:
+ virReportEnumRangeError(virNetworkForwardType, netdef->forward.type);
+ goto cleanup;
}
if (dev_name) {
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 23, 2018 at 12:03:57PM +0800, Shi Lei wrote:
> v1 patch here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
>
> since v1:
> 1. Change the type declaration of _virNetworkForwardDef.type
> from int to virNetworkForwardType
Maybe I wasn't 100% clear in my response to v1, you have to do the typecast
explicitly in all the switches rather than change the type to an enum in the
struct definition, since the compiler can decide the enum to be unsigned which
means that the return value from the virNetworkForwardTypeFromString call in
virNetworkForwardDefParseXML might get usigned-wrapped, ergo the check for a
non-existent type would be a contradiction.
> 2. use the default case to report out of range error with
> virReportEnumRangeError
Another thing, it's cool that you incorporated the diff summary. However, this
shouldn't ever be part of the commit message, rather it should be mentioned as
a note in the note section below the "dashed line"
>
> Signed-off-by: Shi Lei <shilei.massclouds@gmx.com>
> ---
> src/conf/domain_conf.c | 49 ++++---
> src/conf/network_conf.c | 73 +++++++---
> src/conf/network_conf.h | 2 +-
> src/conf/virnetworkobj.c | 24 +++-
> src/esx/esx_network_driver.c | 19 ++-
> src/libvirt_private.syms | 1 +
> src/network/bridge_driver.c | 309 ++++++++++++++++++++++++++++---------------
> src/qemu/qemu_process.c | 8 ++
> 8 files changed, 329 insertions(+), 156 deletions(-)
>
...
> + if (virNetworkObjIsActive(obj)) {
> + switch (def->forward.type) {
> + case VIR_NETWORK_FORWARD_NONE:
> + case VIR_NETWORK_FORWARD_NAT:
> + case VIR_NETWORK_FORWARD_ROUTE:
> + /* Only three of the L3 network types that are configured by
> + * libvirt need to have iptables rules reloaded. The 4th L3
> + * network type, forward='open', doesn't need this because it
> + * has no iptables rules.
> + */
> + networkRemoveFirewallRules(def);
> + /* No need to check return value since already logged internally */
You can drop ^this comment, the fact that we're purposefully ignoring the return
value should tell the reader something ;).
> + ignore_value(networkAddFirewallRules(def));
> + break;
> +
Thanks,
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 23, 2018 at 4:47 PM, Erik wrote:
> On Mon, Jul 23, 2018 at 12:03:57PM +0800, Shi Lei wrote:
> > v1 patch here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
> >
> > since v1:
> > 1. Change the type declaration of _virNetworkForwardDef.type
> > from int to virNetworkForwardType
>
> Maybe I wasn't 100% clear in my response to v1, you have to do the typecast
> explicitly in all the switches rather than change the type to an enum in the
> struct definition, since the compiler can decide the enum to be unsigned which
> means that the return value from the virNetworkForwardTypeFromString call in
> virNetworkForwardDefParseXML might get usigned-wrapped, ergo the check for a
> non-existent type would be a contradiction.
OK.
>
> > 2. use the default case to report out of range error with
> > virReportEnumRangeError
>
> Another thing, it's cool that you incorporated the diff summary. However, this
> shouldn't ever be part of the commit message, rather it should be mentioned as
> a note in the note section below the "dashed line"
OK.
>
> >
> > Signed-off-by: Shi Lei <shilei.massclouds@gmx.com>
> > ---
> > src/conf/domain_conf.c | 49 ++++---
> > src/conf/network_conf.c | 73 +++++++---
> > src/conf/network_conf.h | 2 +-
> > src/conf/virnetworkobj.c | 24 +++-
> > src/esx/esx_network_driver.c | 19 ++-
> > src/libvirt_private.syms | 1 +
> > src/network/bridge_driver.c | 309 ++++++++++++++++++++++++++++---------------
> > src/qemu/qemu_process.c | 8 ++
> > 8 files changed, 329 insertions(+), 156 deletions(-)
> >
>
> ...
>
> > + if (virNetworkObjIsActive(obj)) {
> > + switch (def->forward.type) {
> > + case VIR_NETWORK_FORWARD_NONE:
> > + case VIR_NETWORK_FORWARD_NAT:
> > + case VIR_NETWORK_FORWARD_ROUTE:
> > + /* Only three of the L3 network types that are configured by
> > + * libvirt need to have iptables rules reloaded. The 4th L3
> > + * network type, forward='open', doesn't need this because it
> > + * has no iptables rules.
> > + */
> > + networkRemoveFirewallRules(def);
> > + /* No need to check return value since already logged internally */
>
> You can drop ^this comment, the fact that we're purposefully ignoring the return
> value should tell the reader something ;).
OK. I'll drop it.
>
> > + ignore_value(networkAddFirewallRules(def));
> > + break;
> > +
>
> Thanks,
> Erik
>
Thanks!
Shi Lei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.