[libvirt] [PATCHv2 5/9] conf: move virtPortProfile out of unions in virDomainNetDef
Laine Stump
laine at laine.org
Tue Aug 14 07:04:41 UTC 2012
virtPortProfile is now used by 4 different types of network devices
(NETWORK, BRIDGE, DIRECT, and HOSTDEV), and it's getting cumbersome to
replicate so much code in 4 different places just because each type
has the virtPortProfile in a slightly different place. This patch puts
a single virtPortProfile in a common place (outside the type-specific
union) in both virDomainNetDef and virDomainActualNetDef, and adjusts
the parse and format code (and the few other places where it is used)
accordingly.
Note that when a <virtualport> element is found, the parse functions
verify that the interface is of a type that supports one, otherwise an
error is generated (CONFIG_UNSUPPORTED in the case of <interface>, and
INTERNAL in the case of <actual>, since the contents of <actual> are
always generated by libvirt itself).
---
src/conf/domain_conf.c | 125 +++++++++++++++-----------------------------
src/conf/domain_conf.h | 10 ++--
src/network/bridge_driver.c | 16 +++---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 19 +++----
5 files changed, 63 insertions(+), 109 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8c0969..e239909 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1018,20 +1018,18 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
VIR_FREE(def->data.bridge.brname);
- VIR_FREE(def->data.bridge.virtPortProfile);
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
VIR_FREE(def->data.direct.linkdev);
- VIR_FREE(def->data.direct.virtPortProfile);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
virDomainHostdevDefClear(&def->data.hostdev.def);
- VIR_FREE(def->data.hostdev.virtPortProfile);
break;
default:
break;
}
+ VIR_FREE(def->virtPortProfile);
virNetDevBandwidthFree(def->bandwidth);
VIR_FREE(def);
@@ -1059,14 +1057,12 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
case VIR_DOMAIN_NET_TYPE_NETWORK:
VIR_FREE(def->data.network.name);
VIR_FREE(def->data.network.portgroup);
- VIR_FREE(def->data.network.virtPortProfile);
virDomainActualNetDefFree(def->data.network.actual);
break;
case VIR_DOMAIN_NET_TYPE_BRIDGE:
VIR_FREE(def->data.bridge.brname);
VIR_FREE(def->data.bridge.ipaddr);
- VIR_FREE(def->data.bridge.virtPortProfile);
break;
case VIR_DOMAIN_NET_TYPE_INTERNAL:
@@ -1075,12 +1071,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
case VIR_DOMAIN_NET_TYPE_DIRECT:
VIR_FREE(def->data.direct.linkdev);
- VIR_FREE(def->data.direct.virtPortProfile);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
virDomainHostdevDefClear(&def->data.hostdev.def);
- VIR_FREE(def->data.hostdev.virtPortProfile);
break;
case VIR_DOMAIN_NET_TYPE_USER:
@@ -1088,6 +1082,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
break;
}
+ VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
VIR_FREE(def->ifname);
@@ -4371,6 +4366,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
int ret = -1;
xmlNodePtr save_ctxt = ctxt->node;
xmlNodePtr bandwidth_node = NULL;
+ xmlNodePtr virtPortNode;
char *type = NULL;
char *mode = NULL;
char *addrtype = NULL;
@@ -4403,15 +4399,24 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
goto error;
}
- if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- xmlNodePtr virtPortNode = virXPathNode("./virtualport", ctxt);
- if (virtPortNode &&
- (!(actual->data.bridge.virtPortProfile =
- virNetDevVPortProfileParse(virtPortNode))))
+ virtPortNode = virXPathNode("./virtualport", ctxt);
+ if (virtPortNode) {
+ if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actual->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
+ actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (!(actual->virtPortProfile
+ = virNetDevVPortProfileParse(virtPortNode))) {
+ goto error;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("<virtualport> element unsupported for type='%s'"
+ " in interface's <actual> element"), type);
goto error;
- } else if (actual->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
- xmlNodePtr virtPortNode;
+ }
+ }
+ if (actual->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
actual->data.direct.linkdev = virXPathString("string(./source[1]/@dev)", ctxt);
mode = virXPathString("string(./source[1]/@mode)", ctxt);
@@ -4425,14 +4430,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
}
actual->data.direct.mode = m;
}
-
- virtPortNode = virXPathNode("./virtualport", ctxt);
- if (virtPortNode &&
- (!(actual->data.direct.virtPortProfile =
- virNetDevVPortProfileParse(virtPortNode))))
- goto error;
} else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
- xmlNodePtr virtPortNode = virXPathNode("./virtualport", ctxt);
virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def;
hostdev->parent.type = VIR_DOMAIN_DEVICE_NET;
@@ -4453,12 +4451,6 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
hostdev, flags) < 0) {
goto error;
}
-
- if (virtPortNode &&
- (!(actual->data.hostdev.virtPortProfile =
- virNetDevVPortProfileParse(virtPortNode)))) {
- goto error;
- }
}
bandwidth_node = virXPathNode("./bandwidth", ctxt);
@@ -4518,7 +4510,6 @@ virDomainNetDefParseXML(virCapsPtr caps,
char *linkstate = NULL;
char *addrtype = NULL;
virNWFilterHashTablePtr filterparams = NULL;
- virNetDevVPortProfilePtr virtPort = NULL;
virDomainActualNetDefPtr actual = NULL;
xmlNodePtr oldnode = ctxt->node;
int ret;
@@ -4565,14 +4556,22 @@ virDomainNetDefParseXML(virCapsPtr caps,
xmlStrEqual(cur->name, BAD_CAST "source")) {
dev = virXMLPropString(cur, "dev");
mode = virXMLPropString(cur, "mode");
- } else if (!virtPort &&
- (def->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
- def->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
- def->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
- def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) &&
- xmlStrEqual(cur->name, BAD_CAST "virtualport")) {
- if (!(virtPort = virNetDevVPortProfileParse(cur)))
+ } else if (!def->virtPortProfile
+ && xmlStrEqual(cur->name, BAD_CAST "virtualport")) {
+ if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ def->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ def->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
+ def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (!(def->virtPortProfile
+ = virNetDevVPortProfileParse(cur))) {
+ goto error;
+ }
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("<virtualport> element unsupported for"
+ " <interface type='%s'>"), type);
goto error;
+ }
} else if (!network &&
(def->type == VIR_DOMAIN_NET_TYPE_SERVER ||
def->type == VIR_DOMAIN_NET_TYPE_CLIENT ||
@@ -4689,8 +4688,6 @@ virDomainNetDefParseXML(virCapsPtr caps,
network = NULL;
def->data.network.portgroup = portgroup;
portgroup = NULL;
- def->data.network.virtPortProfile = virtPort;
- virtPort = NULL;
def->data.network.actual = actual;
actual = NULL;
break;
@@ -4719,8 +4716,6 @@ virDomainNetDefParseXML(virCapsPtr caps,
def->data.bridge.ipaddr = address;
address = NULL;
}
- def->data.bridge.virtPortProfile = virtPort;
- virtPort = NULL;
break;
case VIR_DOMAIN_NET_TYPE_CLIENT:
@@ -4784,8 +4779,6 @@ virDomainNetDefParseXML(virCapsPtr caps,
def->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_VEPA;
}
- def->data.direct.virtPortProfile = virtPort;
- virtPort = NULL;
def->data.direct.linkdev = dev;
dev = NULL;
@@ -4814,8 +4807,6 @@ virDomainNetDefParseXML(virCapsPtr caps,
hostdev, flags) < 0) {
goto error;
}
- def->data.hostdev.virtPortProfile = virtPort;
- virtPort = NULL;
break;
case VIR_DOMAIN_NET_TYPE_USER:
@@ -4938,7 +4929,6 @@ cleanup:
VIR_FREE(port);
VIR_FREE(ifname);
VIR_FREE(dev);
- VIR_FREE(virtPort);
virDomainActualNetDefFree(actual);
VIR_FREE(script);
VIR_FREE(bridge);
@@ -11582,12 +11572,6 @@ virDomainActualNetDefFormat(virBufferPtr buf,
case VIR_DOMAIN_NET_TYPE_BRIDGE:
virBufferEscapeString(buf, " <source bridge='%s'/>\n",
def->data.bridge.brname);
- if (def->data.bridge.virtPortProfile) {
- virBufferAdjustIndent(buf, 6);
- if (virNetDevVPortProfileFormat(def->data.bridge.virtPortProfile, buf) < 0)
- return -1;
- virBufferAdjustIndent(buf, -6);
- }
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
@@ -11604,10 +11588,6 @@ virDomainActualNetDefFormat(virBufferPtr buf,
return ret;
}
virBufferAsprintf(buf, " mode='%s'/>\n", mode);
- virBufferAdjustIndent(buf, 8);
- if (virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf) < 0)
- goto error;
- virBufferAdjustIndent(buf, -8);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
@@ -11616,10 +11596,6 @@ virDomainActualNetDefFormat(virBufferPtr buf,
flags, true) < 0) {
return -1;
}
- if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile,
- buf) < 0) {
- return -1;
- }
virBufferAdjustIndent(buf, -8);
break;
@@ -11632,6 +11608,8 @@ virDomainActualNetDefFormat(virBufferPtr buf,
}
virBufferAdjustIndent(buf, 8);
+ if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
+ return -1;
if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
goto error;
virBufferAdjustIndent(buf, -8);
@@ -11675,10 +11653,6 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, " portgroup='%s'",
def->data.network.portgroup);
virBufferAddLit(buf, "/>\n");
- virBufferAdjustIndent(buf, 6);
- if (virNetDevVPortProfileFormat(def->data.network.virtPortProfile, buf) < 0)
- return -1;
- virBufferAdjustIndent(buf, -6);
if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) &&
(virDomainActualNetDefFormat(buf, def->data.network.actual, flags) < 0))
return -1;
@@ -11698,12 +11672,6 @@ virDomainNetDefFormat(virBufferPtr buf,
if (def->data.bridge.ipaddr)
virBufferAsprintf(buf, " <ip address='%s'/>\n",
def->data.bridge.ipaddr);
- if (def->data.bridge.virtPortProfile) {
- virBufferAdjustIndent(buf, 6);
- if (virNetDevVPortProfileFormat(def->data.bridge.virtPortProfile, buf) < 0)
- return -1;
- virBufferAdjustIndent(buf, -6);
- }
break;
case VIR_DOMAIN_NET_TYPE_SERVER:
@@ -11728,10 +11696,6 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " mode='%s'",
virNetDevMacVLanModeTypeToString(def->data.direct.mode));
virBufferAddLit(buf, "/>\n");
- virBufferAdjustIndent(buf, 6);
- if (virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf) < 0)
- return -1;
- virBufferAdjustIndent(buf, -6);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
@@ -11740,10 +11704,6 @@ virDomainNetDefFormat(virBufferPtr buf,
flags, true) < 0) {
return -1;
}
- if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile,
- buf) < 0) {
- return -1;
- }
virBufferAdjustIndent(buf, -6);
break;
@@ -11752,6 +11712,11 @@ virDomainNetDefFormat(virBufferPtr buf,
break;
}
+ virBufferAdjustIndent(buf, 6);
+ if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
+ return -1;
+ virBufferAdjustIndent(buf, -6);
+
virBufferEscapeString(buf, " <script path='%s'/>\n",
def->script);
if (def->ifname &&
@@ -15082,21 +15047,17 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface)
{
switch (iface->type) {
case VIR_DOMAIN_NET_TYPE_DIRECT:
- return iface->data.direct.virtPortProfile;
case VIR_DOMAIN_NET_TYPE_BRIDGE:
- return iface->data.bridge.virtPortProfile;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
- return iface->data.hostdev.virtPortProfile;
+ return iface->virtPortProfile;
case VIR_DOMAIN_NET_TYPE_NETWORK:
if (!iface->data.network.actual)
return NULL;
switch (iface->data.network.actual->type) {
case VIR_DOMAIN_NET_TYPE_DIRECT:
- return iface->data.network.actual->data.direct.virtPortProfile;
case VIR_DOMAIN_NET_TYPE_BRIDGE:
- return iface->data.network.actual->data.bridge.virtPortProfile;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
- return iface->data.network.actual->data.hostdev.virtPortProfile;
+ return iface->data.network.actual->virtPortProfile;
default:
return NULL;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3f25ad2..f36f7d6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -770,18 +770,16 @@ struct _virDomainActualNetDef {
union {
struct {
char *brname;
- virNetDevVPortProfilePtr virtPortProfile;
} bridge;
struct {
char *linkdev;
int mode; /* enum virMacvtapMode from util/macvtap.h */
- virNetDevVPortProfilePtr virtPortProfile;
} direct;
struct {
virDomainHostdevDef def;
- virNetDevVPortProfilePtr virtPortProfile;
} hostdev;
} data;
+ virNetDevVPortProfilePtr virtPortProfile;
virNetDevBandwidthPtr bandwidth;
};
@@ -810,7 +808,6 @@ struct _virDomainNetDef {
struct {
char *name;
char *portgroup;
- virNetDevVPortProfilePtr virtPortProfile;
/* actual has info about the currently used physical
* device (if the network is of type
* bridge/private/vepa/passthrough). This is saved in the
@@ -824,7 +821,6 @@ struct _virDomainNetDef {
struct {
char *brname;
char *ipaddr;
- virNetDevVPortProfilePtr virtPortProfile;
} bridge;
struct {
char *name;
@@ -832,13 +828,13 @@ struct _virDomainNetDef {
struct {
char *linkdev;
int mode; /* enum virMacvtapMode from util/macvtap.h */
- virNetDevVPortProfilePtr virtPortProfile;
} direct;
struct {
virDomainHostdevDef def;
- virNetDevVPortProfilePtr virtPortProfile;
} hostdev;
} data;
+ /* virtPortProfile is used by network/bridge/direct/hostdev */
+ virNetDevVPortProfilePtr virtPortProfile;
struct {
bool sndbuf_specified;
unsigned long sndbuf;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a5046f1..d9646fb 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2854,8 +2854,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
}
/* Find the most specific virtportprofile and copy it */
- if (iface->data.network.virtPortProfile) {
- virtport = iface->data.network.virtPortProfile;
+ if (iface->virtPortProfile) {
+ virtport = iface->virtPortProfile;
} else {
if (portgroup)
virtport = portgroup->virtPortProfile;
@@ -2863,14 +2863,14 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virtport = netdef->virtPortProfile;
}
if (virtport) {
- if (VIR_ALLOC(iface->data.network.actual->data.direct.virtPortProfile) < 0) {
+ if (VIR_ALLOC(iface->data.network.actual->virtPortProfile) < 0) {
virReportOOMError();
goto cleanup;
}
/* There are no pointers in a virtualPortProfile, so a shallow copy
* is sufficient
*/
- *iface->data.network.actual->data.direct.virtPortProfile = *virtport;
+ *iface->data.network.actual->virtPortProfile = *virtport;
}
/* If there is only a single device, just return it (caller will detect
@@ -2935,8 +2935,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
}
}
} else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
- iface->data.network.actual->data.direct.virtPortProfile &&
- (iface->data.network.actual->data.direct.virtPortProfile->virtPortType
+ iface->data.network.actual->virtPortProfile &&
+ (iface->data.network.actual->virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
/* pick first dev with 0 usageCount */
@@ -3068,8 +3068,8 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
if ((dev->usageCount > 0) &&
((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
- iface->data.network.actual->data.direct.virtPortProfile &&
- (iface->data.network.actual->data.direct.virtPortProfile->virtPortType
+ iface->data.network.actual->virtPortProfile &&
+ (iface->data.network.actual->virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("network '%s' claims dev='%s' is already in use by a different domain"),
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9bf89bb..21d8699 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4763,7 +4763,6 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
}
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
VIR_FREE(net->data.direct.linkdev);
- VIR_FREE(net->data.direct.virtPortProfile);
memset(net, 0, sizeof(*net));
@@ -4783,6 +4782,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
net->data.ethernet.dev = brname;
net->data.ethernet.ipaddr = ipaddr;
}
+ VIR_FREE(net->virtPortProfile);
net->info.bootIndex = bootIndex;
}
for (i = 0 ; i < def->ngraphics ; i++) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e128e58..339906e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1340,6 +1340,11 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
return -1;
}
+ if (!virNetDevVPortProfileEqual(olddev->virtPortProfile, dev->virtPortProfile)) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot change <virtualport> settings"));
+ }
+
switch (olddev->type) {
case VIR_DOMAIN_NET_TYPE_USER:
break;
@@ -1367,8 +1372,7 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
case VIR_DOMAIN_NET_TYPE_NETWORK:
if (STRNEQ_NULLABLE(olddev->data.network.name, dev->data.network.name) ||
- STRNEQ_NULLABLE(olddev->data.network.portgroup, dev->data.network.portgroup) ||
- !virNetDevVPortProfileEqual(olddev->data.network.virtPortProfile, dev->data.network.virtPortProfile)) {
+ STRNEQ_NULLABLE(olddev->data.network.portgroup, dev->data.network.portgroup)) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("cannot modify network device configuration"));
return -1;
@@ -1377,13 +1381,7 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
break;
case VIR_DOMAIN_NET_TYPE_BRIDGE:
- /* allow changing brname, but not portprofile */
- if (!virNetDevVPortProfileEqual(olddev->data.bridge.virtPortProfile,
- dev->data.bridge.virtPortProfile)) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("cannot modify bridge network device configuration"));
- return -1;
- }
+ /* allow changing brname */
break;
case VIR_DOMAIN_NET_TYPE_INTERNAL:
@@ -1396,8 +1394,7 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
case VIR_DOMAIN_NET_TYPE_DIRECT:
if (STRNEQ_NULLABLE(olddev->data.direct.linkdev, dev->data.direct.linkdev) ||
- olddev->data.direct.mode != dev->data.direct.mode ||
- !virNetDevVPortProfileEqual(olddev->data.direct.virtPortProfile, dev->data.direct.virtPortProfile)) {
+ olddev->data.direct.mode != dev->data.direct.mode) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("cannot modify direct network device configuration"));
return -1;
--
1.7.11.2
More information about the libvir-list
mailing list