[libvirt] [PATCHv2 5/9] conf: move virtPortProfile out of unions in virDomainNetDef

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