Both of these functions returned void, but it's convenient for them to return a const char* of the char* that is passed in. This was you can call the function and use the result in the same expression/arg. --- src/util/uuid.c | 4 +++- src/util/uuid.h | 6 +++--- src/util/virmacaddr.c | 6 ++++-- src/util/virmacaddr.h | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/util/uuid.c b/src/util/uuid.c index 1efde69..be4b13a 100644 --- a/src/util/uuid.c +++ b/src/util/uuid.c @@ -184,7 +184,8 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { * * Returns 0 in case of success and -1 in case of error. */ -void virUUIDFormat(const unsigned char *uuid, char *uuidstr) +const char * +virUUIDFormat(const unsigned char *uuid, char *uuidstr) { snprintf(uuidstr, VIR_UUID_STRING_BUFLEN, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x", @@ -193,6 +194,7 @@ void virUUIDFormat(const unsigned char *uuid, char *uuidstr) uuid[8], uuid[9], uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]); uuidstr[VIR_UUID_STRING_BUFLEN-1] = '\0'; + return uuidstr; } diff --git a/src/util/uuid.h b/src/util/uuid.h index da56a92..54e0573 100644 --- a/src/util/uuid.h +++ b/src/util/uuid.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007, 2011 Red Hat, Inc. + * Copyright (C) 2007, 2011, 2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -35,7 +35,7 @@ int virUUIDParse(const char *uuidstr, unsigned char *uuid) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -void virUUIDFormat(const unsigned char *uuid, - char *uuidstr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +const char *virUUIDFormat(const unsigned char *uuid, + char *uuidstr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif /* __VIR_UUID_H__ */ diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c index c07976b..341432c 100644 --- a/src/util/virmacaddr.c +++ b/src/util/virmacaddr.c @@ -176,14 +176,16 @@ virMacAddrParse(const char* str, virMacAddrPtr addr) return -1; } -void virMacAddrFormat(const virMacAddrPtr addr, - char *str) +const char * +virMacAddrFormat(const virMacAddrPtr addr, + char *str) { snprintf(str, VIR_MAC_STRING_BUFLEN, "%02X:%02X:%02X:%02X:%02X:%02X", addr->addr[0], addr->addr[1], addr->addr[2], addr->addr[3], addr->addr[4], addr->addr[5]); str[VIR_MAC_STRING_BUFLEN-1] = '\0'; + return str; } void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h index fdac362..4c5074c 100644 --- a/src/util/virmacaddr.h +++ b/src/util/virmacaddr.h @@ -44,8 +44,8 @@ int virMacAddrCmpRaw(const virMacAddrPtr mac1, void virMacAddrSet(virMacAddrPtr dst, const virMacAddrPtr src); void virMacAddrSetRaw(virMacAddrPtr dst, const unsigned char s[VIR_MAC_BUFLEN]); void virMacAddrGetRaw(virMacAddrPtr src, unsigned char dst[VIR_MAC_BUFLEN]); -void virMacAddrFormat(const virMacAddrPtr addr, - char *str); +const char *virMacAddrFormat(const virMacAddrPtr addr, + char *str); void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], virMacAddrPtr addr); int virMacAddrParse(const char* str, -- 1.7.11.2

This patch adds three utility functions that operate on virNetDevVPortProfile objects. * virNetDevVPortProfileCheckComplete() - verifies that all attributes required for the type of the given virtport are specified. * virNetDevVPortProfileCheckNoExtras() - verifies that there are no attributes specified which are inappropriate for the type of the given virtport. * virNetDevVPortProfileMerge3() - merges 3 virtports into a single, newly allocated virtport. If any attributes are specified in more than one of the three sources, and do not exactly match, an error is logged and the function fails. These new functions depend on new fields in the virNetDevVPortProfile object that keep track of whether or not each attribute was specified. Since the higher level parse function doesn't yet set those fields, these functions are not actually usable yet (but that's okay, because they also aren't yet used - all of that functionality comes in a later patch.) Note that these three functions return 0 on success and -1 on failure. This may seem odd for the first two Check functions, since they could also easily return true/false, but since they actually log an error when the requested condition isn't met (and should result in a failure of the calling function), I thought 0/-1 was more appropriate. --- src/libvirt_private.syms | 3 + src/util/virnetdevvportprofile.c | 291 +++++++++++++++++++++++++++++++++++++++ src/util/virnetdevvportprofile.h | 16 ++- 3 files changed, 309 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 44b6652..5d46e61 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1428,8 +1428,11 @@ virNetDevVethDelete; # virnetdevvportprofile.h virNetDevVPortProfileAssociate; +virNetDevVPortProfileCheckComplete; +virNetDevVPortProfileCheckNoExtras; virNetDevVPortProfileDisassociate; virNetDevVPortProfileEqual; +virNetDevVPortProfileMerge3; virNetDevVPortProfileOpTypeFromString; virNetDevVPortProfileOpTypeToString; diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6db04f7..176190f 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -120,6 +120,297 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr return true; } +/* virNetDevVPortProfileCheckComplete() checks that all attributes + * required for the type of virtport are specified. if virtport == + * NULL or virtPortType == NONE, then the result is always 0 + * (success). If a required attribute is missing, an error is logged + * and -1 is returned. + */ +int +virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport) +{ + const char *missing = NULL; + + if (!virtport || virtport->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE) + return 0; + + switch (virtport->virtPortType) { + case VIR_NETDEV_VPORT_PROFILE_8021QBG: + if (!virtport->managerID_specified) + missing = "managerid"; + else if (!virtport->typeID_specified) + missing = "typeid"; + else if (!virtport->typeIDVersion_specified) + missing = "typeidversion"; + else if (!virtport->instanceID_specified) + missing = "instanceid"; + break; + + case VIR_NETDEV_VPORT_PROFILE_8021QBH: + if (!virtport->profileID[0]) + missing = "profileid"; + break; + + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + /* profileid is optional for openvswitch */ + if (!virtport->interfaceID_specified) { + missing = "interfaceid"; + } + break; + } + + if (missing) { + virReportError(VIR_ERR_XML_ERROR, + _("missing %s in <virtualport type='%s'>"), missing, + virNetDevVPortTypeToString(virtport->virtPortType)); + return -1; + } + + return 0; +} + +/* virNetDevVPortProfileCheckNoExtras() checks that there are no + * attributes specified in this virtport that are inappropriate for + * the type. if virtport == NULL or virtPortType == NONE, then the + * result is always 0 (success). If an extra attribute is present, + * an error is logged and -1 is returned. + */ +int +virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport) +{ + const char *extra = NULL; + + if (!virtport || virtport->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE) + return 0; + + switch (virtport->virtPortType) { + case VIR_NETDEV_VPORT_PROFILE_8021QBG: + if (virtport->profileID[0]) + extra = "profileid"; + else if (virtport->interfaceID_specified) + extra = "interfaceid"; + break; + + case VIR_NETDEV_VPORT_PROFILE_8021QBH: + if (virtport->managerID_specified) + extra = "managerid"; + else if (virtport->typeID_specified) + extra = "typeid"; + else if (virtport->typeIDVersion_specified) + extra = "typeidversion"; + else if (virtport->instanceID_specified) + extra = "instanceid"; + else if (virtport->interfaceID_specified) + extra = "interfaceid"; + break; + + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (virtport->managerID_specified) + extra = "managerid"; + else if (virtport->typeID_specified) + extra = "typeid"; + else if (virtport->typeIDVersion_specified) + extra = "typeidversion"; + else if (virtport->instanceID_specified) + extra = "instanceid"; + break; + } + + if (extra) { + virReportError(VIR_ERR_XML_ERROR, + _("extra %s unsupported in <virtualport type='%s'>"), + extra, + virNetDevVPortTypeToString(virtport->virtPortType)); + return -1; + } + + return 0; +} + +/* virNetDevVPortProfileMerge() - merge the attributes in mods into + * orig. If anything that is set in mods has already been set in orig + * *and doesn't match*, log an error and return -1, otherwise return 0. + */ +static int +virNetDevVPortProfileMerge(virNetDevVPortProfilePtr orig, + virNetDevVPortProfilePtr mods) +{ + enum virNetDevVPortProfile otype; + + if (!orig || !mods) + return 0; + + otype = orig->virtPortType; + + if (mods->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { + if (otype != VIR_NETDEV_VPORT_PROFILE_NONE && + otype != mods->virtPortType) { + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports " + "with mismatched types (%s and %s)"), + virNetDevVPortTypeToString(otype), + virNetDevVPortTypeToString(mods->virtPortType)); + return -1; + } + otype = orig->virtPortType = mods->virtPortType; + } + + if (mods->managerID_specified && + (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->managerID_specified && + (orig->managerID != mods->managerID)) { + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports " + "with mismatched managerids (%d and %d)"), + orig->managerID, mods->managerID); + return -1; + } + orig->managerID = mods->managerID; + orig->managerID_specified = true; + } + + if (mods->typeID_specified && + (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->typeID_specified && + (orig->typeID != mods->typeID)) { + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports " + "with mismatched typeids (%d and %d)"), + orig->typeID, mods->typeID); + return -1; + } + orig->typeID = mods->typeID; + orig->typeID_specified = true; + } + + if (mods->typeIDVersion_specified && + (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->typeIDVersion_specified && + (orig->typeIDVersion != mods->typeIDVersion)) { + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports with " + "mismatched typeidversions (%d and %d)"), + orig->typeIDVersion, mods->typeIDVersion); + return -1; + } + orig->typeIDVersion = mods->typeIDVersion; + orig->typeIDVersion_specified = true; + } + + if (mods->instanceID_specified && + (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->instanceID_specified && + memcmp(orig->instanceID, mods->instanceID, + sizeof(orig->instanceID))) { + char origuuid[VIR_UUID_STRING_BUFLEN]; + char modsuuid[VIR_UUID_STRING_BUFLEN]; + + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports with " + "mismatched instanceids ('%s' and '%s')"), + virUUIDFormat(orig->instanceID, origuuid), + virUUIDFormat(mods->instanceID, modsuuid)); + return -1; + } + memcpy(orig->instanceID, mods->instanceID, sizeof(orig->instanceID)); + orig->instanceID_specified = true; + } + + if (mods->interfaceID_specified && + (otype == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->interfaceID_specified && + memcmp(orig->interfaceID, mods->interfaceID, + sizeof(orig->interfaceID))) { + char origuuid[VIR_UUID_STRING_BUFLEN]; + char modsuuid[VIR_UUID_STRING_BUFLEN]; + + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports with " + "mismatched interfaceids ('%s' and '%s')"), + virUUIDFormat(orig->interfaceID, origuuid), + virUUIDFormat(mods->interfaceID, modsuuid)); + return -1; + } + memcpy(orig->interfaceID, mods->interfaceID, sizeof(orig->interfaceID)); + orig->interfaceID_specified = true; + } + + if (mods->profileID[0] && + (otype == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || + otype == VIR_NETDEV_VPORT_PROFILE_8021QBH || + otype == VIR_NETDEV_VPORT_PROFILE_NONE)) { + if (orig->profileID[0] && + STRNEQ(orig->profileID, mods->profileID)) { + virReportError(VIR_ERR_XML_ERROR, + _("attempt to merge virtualports with " + "mismatched profileids ('%s' and '%s')"), + orig->profileID, mods->profileID); + return -1; + } + if (virStrcpyStatic(orig->profileID, mods->profileID)) { + /* this should never happen - it indicates mods->profileID + * isn't properly null terminated. */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("corrupted profileid string")); + return -1; + } + } + return 0; +} + +/* virNetDevVPortProfileMerge3() - create a new virNetDevVPortProfile + * that is a combination of the three input profiles. fromInterface is + * highest priority and fromPortgroup is lowest. As lower priority + * objects' attributes are merged in, if the attribute is unset in the + * result object, it is set from the lower priority object, but if it + * is already set in the result and the lower priority object wants to + * change it, that is an error. + */ + +int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result, + virNetDevVPortProfilePtr fromInterface, + virNetDevVPortProfilePtr fromNetwork, + virNetDevVPortProfilePtr fromPortgroup) +{ + int ret = -1; + *result = NULL; + + if ((!fromInterface || (fromInterface->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)) && + (!fromNetwork || (fromNetwork->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)) && + (!fromPortgroup || (fromPortgroup->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE))) { + return ret; + } + + /* at least one of the source profiles is non-empty */ + if (VIR_ALLOC(*result) < 0) { + virReportOOMError(); + return ret; + } + + /* start with the interface's profile. There are no pointers in a + * virtualPortProfile, so a shallow copy is sufficient. + */ + if (fromInterface) + **result = *fromInterface; + + if (virNetDevVPortProfileMerge(*result, fromNetwork) < 0) + goto error; + if (virNetDevVPortProfileMerge(*result, fromPortgroup) < 0) + goto error; + + ret = 0; + +error: + if (ret < 0) + VIR_FREE(*result); + return ret; +} + #if WITH_VIRTUALPORT diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index e204c95..f5f9b12 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -60,18 +60,24 @@ VIR_ENUM_DECL(virNetDevVPortProfileOp) typedef struct _virNetDevVPortProfile virNetDevVPortProfile; typedef virNetDevVPortProfile *virNetDevVPortProfilePtr; struct _virNetDevVPortProfile { - enum virNetDevVPortProfile virtPortType; + int virtPortType; /* enum virNetDevVPortProfile */ /* these members are used when virtPortType == 802.1Qbg */ uint8_t managerID; + bool managerID_specified; uint32_t typeID; /* 24 bit valid */ + bool typeID_specified; uint8_t typeIDVersion; + bool typeIDVersion_specified; unsigned char instanceID[VIR_UUID_BUFLEN]; + bool instanceID_specified; /* this member is used when virtPortType == 802.1Qbh|openvswitch */ + /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; /* this member is used when virtPortType == openvswitch */ unsigned char interfaceID[VIR_UUID_BUFLEN]; + bool interfaceID_specified; /* NB - if virtPortType == NONE, any/all of the items could be used */ }; @@ -79,6 +85,14 @@ struct _virNetDevVPortProfile { bool virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr b); +int virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport); +int virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport); + +int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result, + virNetDevVPortProfilePtr fromInterface, + virNetDevVPortProfilePtr fromNetwork, + virNetDevVPortProfilePtr fromPortgroup); + int virNetDevVPortProfileAssociate(const char *ifname, const virNetDevVPortProfilePtr virtPort, const virMacAddrPtr macaddr, -- 1.7.11.2

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 58603a3..a3fb1e7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1003,20 +1003,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); @@ -1044,14 +1042,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: @@ -1060,12 +1056,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: @@ -1073,6 +1067,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break; } + VIR_FREE(def->virtPortProfile); VIR_FREE(def->script); VIR_FREE(def->ifname); @@ -4376,6 +4371,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; @@ -4408,15 +4404,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); @@ -4430,14 +4435,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; @@ -4458,12 +4456,6 @@ virDomainActualNetDefParseXML(xmlNodePtr node, hostdev, flags) < 0) { goto error; } - - if (virtPortNode && - (!(actual->data.hostdev.virtPortProfile = - virNetDevVPortProfileParse(virtPortNode)))) { - goto error; - } } bandwidth_node = virXPathNode("./bandwidth", ctxt); @@ -4523,7 +4515,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; @@ -4570,14 +4561,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 || @@ -4694,8 +4693,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; @@ -4724,8 +4721,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: @@ -4789,8 +4784,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; @@ -4819,8 +4812,6 @@ virDomainNetDefParseXML(virCapsPtr caps, hostdev, flags) < 0) { goto error; } - def->data.hostdev.virtPortProfile = virtPort; - virtPort = NULL; break; case VIR_DOMAIN_NET_TYPE_USER: @@ -4943,7 +4934,6 @@ cleanup: VIR_FREE(port); VIR_FREE(ifname); VIR_FREE(dev); - VIR_FREE(virtPort); virDomainActualNetDefFree(actual); VIR_FREE(script); VIR_FREE(bridge); @@ -11588,12 +11578,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: @@ -11610,10 +11594,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: @@ -11622,10 +11602,6 @@ virDomainActualNetDefFormat(virBufferPtr buf, flags, true) < 0) { return -1; } - if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile, - buf) < 0) { - return -1; - } virBufferAdjustIndent(buf, -8); break; @@ -11638,6 +11614,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); @@ -11681,10 +11659,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; @@ -11704,12 +11678,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: @@ -11734,10 +11702,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: @@ -11746,10 +11710,6 @@ virDomainNetDefFormat(virBufferPtr buf, flags, true) < 0) { return -1; } - if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile, - buf) < 0) { - return -1; - } virBufferAdjustIndent(buf, -6); break; @@ -11758,6 +11718,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 && @@ -15090,21 +15055,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 f4c43c6..dc8c009 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -769,18 +769,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; }; @@ -809,7 +807,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 @@ -823,7 +820,6 @@ struct _virDomainNetDef { struct { char *brname; char *ipaddr; - virNetDevVPortProfilePtr virtPortProfile; } bridge; struct { char *name; @@ -831,13 +827,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 369e8ed..77f6741 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4732,7 +4732,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)); @@ -4752,6 +4751,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 71ec484..8cddf7d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1329,6 +1329,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; @@ -1356,8 +1361,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; @@ -1366,13 +1370,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: @@ -1385,8 +1383,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

This function has several calls to increase the buffer indent by 6, then decrease it again, then increase, then decrease. Additionally, there were several printfs that had 6 spaces at the beginning of the line. virDomainActualNetDefFormat, which is called by virDomainNetDefFormat, had similar ugliness. This patch changes both functions to just increase the indent at the beginning, decrease it at (well, just before*) the end, and remove all of the occurences of 6/8 spaces at the beginning of lines. *The indent had to be reset before the end of the function because virDomainDeviceInfoFormat assumes a 0 indent and is called from many other places, and I didn't want to do an overhaul of every caller of that function. A separate patch to switch all of domain_conf.c would be a useful exercise, but my current goal is unrelated to that, so I'll leave it for another day. --- src/conf/domain_conf.c | 73 +++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d1a29a..e212263 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11566,21 +11566,22 @@ virDomainActualNetDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <actual type='%s'", type); + virBufferAsprintf(buf, "<actual type='%s'", type); if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV && def->data.hostdev.def.managed) { virBufferAddLit(buf, " managed='yes'"); } virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - virBufferEscapeString(buf, " <source bridge='%s'/>\n", + virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); break; case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferAddLit(buf, " <source"); + virBufferAddLit(buf, "<source"); if (def->data.direct.linkdev) virBufferEscapeString(buf, " dev='%s'", def->data.direct.linkdev); @@ -11596,12 +11597,10 @@ virDomainActualNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: - virBufferAdjustIndent(buf, 8); if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def, flags, true) < 0) { return -1; } - virBufferAdjustIndent(buf, -8); break; case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -11612,14 +11611,13 @@ virDomainActualNetDefFormat(virBufferPtr buf, return -1; } - virBufferAdjustIndent(buf, 8); if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) return -1; if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) return -1; - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </actual>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</actual>\n"); return 0; } @@ -11643,14 +11641,15 @@ virDomainNetDefFormat(virBufferPtr buf, } virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 6); virBufferAsprintf(buf, - " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", def->mac.addr[0], def->mac.addr[1], def->mac.addr[2], def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]); switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: - virBufferEscapeString(buf, " <source network='%s'", + virBufferEscapeString(buf, "<source network='%s'", def->data.network.name); virBufferEscapeString(buf, " portgroup='%s'", def->data.network.portgroup); @@ -11661,39 +11660,41 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - virBufferEscapeString(buf, " <source dev='%s'/>\n", + virBufferEscapeString(buf, "<source dev='%s'/>\n", def->data.ethernet.dev); if (def->data.ethernet.ipaddr) - virBufferAsprintf(buf, " <ip address='%s'/>\n", + virBufferAsprintf(buf, "<ip address='%s'/>\n", def->data.ethernet.ipaddr); break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - virBufferEscapeString(buf, " <source bridge='%s'/>\n", + virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); - if (def->data.bridge.ipaddr) - virBufferAsprintf(buf, " <ip address='%s'/>\n", + if (def->data.bridge.ipaddr) { + virBufferAsprintf(buf, "<ip address='%s'/>\n", def->data.bridge.ipaddr); + } break; case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: - if (def->data.socket.address) - virBufferAsprintf(buf, " <source address='%s' port='%d'/>\n", + if (def->data.socket.address) { + virBufferAsprintf(buf, "<source address='%s' port='%d'/>\n", def->data.socket.address, def->data.socket.port); - else - virBufferAsprintf(buf, " <source port='%d'/>\n", + } else { + virBufferAsprintf(buf, "<source port='%d'/>\n", def->data.socket.port); + } break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - virBufferEscapeString(buf, " <source name='%s'/>\n", + virBufferEscapeString(buf, "<source name='%s'/>\n", def->data.internal.name); break; case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferEscapeString(buf, " <source dev='%s'", + virBufferEscapeString(buf, "<source dev='%s'", def->data.direct.linkdev); virBufferAsprintf(buf, " mode='%s'", virNetDevMacVLanModeTypeToString(def->data.direct.mode)); @@ -11701,12 +11702,10 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: - virBufferAdjustIndent(buf, 6); if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def, flags, true) < 0) { return -1; } - virBufferAdjustIndent(buf, -6); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -11714,26 +11713,22 @@ virDomainNetDefFormat(virBufferPtr buf, break; } - virBufferAdjustIndent(buf, 6); if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) return -1; - virBufferAdjustIndent(buf, -6); - - virBufferEscapeString(buf, " <script path='%s'/>\n", + virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); if (def->ifname && !((flags & VIR_DOMAIN_XML_INACTIVE) && (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) { /* Skip auto-generated target names for inactive config. */ - virBufferEscapeString(buf, " <target dev='%s'/>\n", - def->ifname); + virBufferEscapeString(buf, "<target dev='%s'/>\n", def->ifname); } if (def->model) { - virBufferEscapeString(buf, " <model type='%s'/>\n", + virBufferEscapeString(buf, "<model type='%s'/>\n", def->model); if (STREQ(def->model, "virtio") && (def->driver.virtio.name || def->driver.virtio.txmode)) { - virBufferAddLit(buf, " <driver"); + virBufferAddLit(buf, "<driver"); if (def->driver.virtio.name) { virBufferAsprintf(buf, " name='%s'", virDomainNetBackendTypeToString(def->driver.virtio.name)); @@ -11754,27 +11749,25 @@ virDomainNetDefFormat(virBufferPtr buf, } } if (def->filter) { - virBufferAdjustIndent(buf, 6); if (virNWFilterFormatParamAttributes(buf, def->filterparams, def->filter) < 0) return -1; - virBufferAdjustIndent(buf, -6); } if (def->tune.sndbuf_specified) { - virBufferAddLit(buf, " <tune>\n"); - virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n", - def->tune.sndbuf); - virBufferAddLit(buf, " </tune>\n"); + virBufferAddLit(buf, "<tune>\n"); + virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n", def->tune.sndbuf); + virBufferAddLit(buf, "</tune>\n"); } - if (def->linkstate) - virBufferAsprintf(buf, " <link state='%s'/>\n", + if (def->linkstate) { + virBufferAsprintf(buf, "<link state='%s'/>\n", virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); + } - virBufferAdjustIndent(buf, 6); if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) return -1; + virBufferAdjustIndent(buf, -6); if (virDomainDeviceInfoFormat(buf, &def->info, -- 1.7.11.2

One of the original ideas behind allowing a <virtualport> in an interface definition as well as in the <network> definition *and*one or more <portgroup>s within the network, was that guest-specific parameteres (like instanceid and interfaceid) could be given in the interface's virtualport, and more general things (portid, managerid, etc) could be given in the network and/or portgroup, with all the bits brought together at guest startup time and combined into a single virtualport to be used by the guest. This was somehow overlooked in the implementation, though - it simply picks the "most specific" virtualport, and uses the entire thing, with no attempt to merge in details from the others. This patch uses virNetDevVPortProfileMerge3() to combine the three possible virtualports into one, then uses virNetDevVPortProfileCheck*() to verify that the resulting virtualport type is appropriate for the type of network, and that all the required attributes for that type are present. An example of usage is this: assuming a <network> definitions on host ABC of: <network> <name>testA</name> ... <virtualport type='openvswitch'/> ... <portgroup name='engineering'> <virtualport> <parameters profileid='eng'/> </virtualport> </portgroup> <portgroup name='sales'> <virtualport> <parameters profileid='sales'/> </virtualport> </portgroup> </network> and the same <network> on host DEF of: <network> <name>testA</name> ... <virtualport type='802.1Qbg'> <parameters typeid="1193047" typeidversion="2"/> </virtualport> ... <portgroup name='engineering'> <virtualport> <parameters managerid="11"/> </virtualport> </portgroup> <portgroup name='sales'> <virtualport> <parameters managerid="55"/> </virtualport> </portgroup> </network> and a guest <interface> definition of: <interface type='network'> <source network='testA' portgroup='sales'/> <virtualport> <parameters instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f" interfaceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f"\> </virtualport> ... </interface> If the guest was started on host ABC, the <virtualport> used would be: <virtualport type='openvswitch'> <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' profileid='sales'/> </virtualport> but if that guest was started on host DEF, the <virtualport> would be: <virtualport type='802.1Qbg'> <parameters instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f" typeid="1193047" typeidversion="2" managerid="55"/> </virtualport> Additionally, if none of the involved <virtualport>s had a specified type (this includes cases where no virtualport is given at all), --- docs/formatdomain.html.in | 72 ++++++++++++++++++++++++++++++++++++++------- docs/formatnetwork.html.in | 27 ++++++++++++----- src/network/bridge_driver.c | 61 ++++++++++++++++++++++++++++---------- 3 files changed, 127 insertions(+), 33 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..f3b3fa8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2230,11 +2230,40 @@ the network; one network may have multiple portgroups defined, with each portgroup containing slightly different configuration information for different classes of network - connections. <span class="since">Since 0.9.4</span>). Also, - similar to <code>direct</code> network connections (described - below), a connection of type <code>network</code> may specify - a <code>virtportprofile</code> element, with configuration data - to be forwarded to a vepa or 802.1Qbh compliant switch. + connections. <span class="since">Since 0.9.4</span>. + </p> + <p> + Also, similar to <code>direct</code> network connections + (described below), a connection of type <code>network</code> may + specify a <code>virtualport</code> element, with configuration + data to be forwarded to a vepa (802.1Qbg) or 802.1Qbh compliant + switch (<span class="since">Since 0.8.2</span>), or to an + OpenvSwitch virtual switch (<span class="since">Since + 0.9.11</span>). + </p> + <p> + Since the actual type of switch may vary depending on the + configuration in the <code><network></code> on the host, + it is acceptable to omit the virtualport <code>type</code> + attribute, and specify attributes from multiple different + virtualport types (and also to leave out certain attributes); at + domain startup time, a complete <code><virtualport></code> + element will be constructed by merging together the type and + attributes found in the which will be filled in from the network + or portgroup <code><virtualport></code>) + (<span class="since">Since 0.10.0</span>). For example, in order + to work properly with both an 802.1Qbh switch and an OpenvSwitch + switch, you may choose to specify no type, but both + an <code>instanceid</code> (in case the switch is 802.1Qbh) and + an <code>interfaceid</code> (in case the switch is OpenvSwitch) + (you may also omit the other attributes, such as managerid, + typeid, or profileid, to be filled in from the + network's <code><virtualport></code>). If you want to + limit a guest to connecting only to certain types of switches, + you can specify the virtualport type, but still omit some/all of + the parameters - in this case if the host's network has a + different type of virtualport, connection of the interface will + fail. </p> <pre> @@ -2248,8 +2277,8 @@ <source network='default' portgroup='engineering'/> <target dev='vnet7'/> <mac address="00:11:22:33:44:55"/> - <virtualport type='802.1Qbg'> - <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + <virtualport> + <parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> @@ -2261,7 +2290,7 @@ <p> <strong><em> This is the recommended config for general guest connectivity on - hosts with static wired networking configs + hosts with static wired networking configs. </em></strong> </p> @@ -2276,19 +2305,40 @@ configuration is whatever is used on the LAN. This provides the guest VM full incoming & outgoing net access just like a physical machine. </p> - + <p> + On Linux systems, the bridge device is normally a standard Linux + host bridge. On hosts that support OpenvSwitch, it is also + possible to connect to an openvswitch bridge device by adding + a <code><virtualport type='openvswitch'/></code> to the + interface definition. (<span class="since">Since + 0.9.11</span>). The OpenvSwitch type virtualport accepts two + parameters in its <code><parameters></code> element - + an <code>interfaceid</code> which is a standard uuid used to + uniquely identify this particular interface to OpenvSwitch (if + you do no specify one, a random interfaceid will be generated + for you when you first define the interface), and an + optional <code>profileid</code> which is sent to OpenvSwitch as + the interfaces "port-profile". + </p> <pre> ... <devices> + ... <interface type='bridge'> <source bridge='br0'/> </interface> - ... <interface type='bridge'> - <source bridge='br0'/> + <source bridge='br1'/> <target dev='vnet7'/> <mac address="00:11:22:33:44:55"/> </interface> + <interface type='bridge'> + <source bridge='ovsbr'/> + <virtualport type='openvswitch'/> + <parameters profileid='menial' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + </virtualport> + </interface> + ... </devices> ...</pre> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7e8e991..0ba4d2d 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -147,10 +147,17 @@ This network describes either 1) an existing host bridge that was configured outside of libvirt (if a <code><bridge name='xyz'/></code> element has been - specified), or 2) an interface or group of interfaces to - be used for a "direct" connection via macvtap using - macvtap's "bridge" mode (if the forward element has one or - more <code><interface></code> subelements) + specified, <span class="since">Since 0.9.4</span>), 2) an + existing OpenvSwitch bridge that was configured outside of + libvirt (if both a <code><bridge name='xyz'/></code> + element <b>and</b> a <code><virtualport + type='openvswitch'/></code> have been + specified <span class="since">Since 0.10.0</span>) 3) an + interface or group of interfaces to be used for a "direct" + connection via macvtap using macvtap's "bridge" mode (if + the forward element has one or + more <code><interface></code> + subelements, <span class="since">Since 0.9.4</span>) (see <a href="formatdomain.html#elementsNICSDirect">Direct attachment to physical interface</a> for descriptions of the various macvtap modes). libvirt doesn't attempt to @@ -337,9 +344,15 @@ default portgroup will be used. If no portgroup is given in the interface definition, and there is no default portgroup, then none will be used. Any <code><bandwidth></code> - or <code><virtualport></code> specified directly in the - domain XML will take precedence over any setting in the chosen - portgroup. + + specified directly in the domain XML will take precedence over + any setting in the chosen portgroup. if + a <code><virtualport></code> is specified in the portgroup + (and/or directly in the network definition), the multiple + virtualports will be merged, and any parameter that is specified + in more than one virtualport, and is not identical, will be + considered an error, and will prevent the interface from + starting. </p> <h3><a name="elementsAddress">Addressing</a></h3> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d9646fb..2bbfd3b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -61,6 +61,7 @@ #include "virnetdev.h" #include "virnetdevbridge.h" #include "virnetdevtap.h" +#include "virnetdevvportprofile.h" #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network" #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network" @@ -2746,6 +2747,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) virNetworkObjPtr network; virNetworkDefPtr netdef; virPortGroupDefPtr portgroup; + virNetDevVPortProfilePtr virtport = NULL; unsigned int num_virt_fns = 0; char **vfname = NULL; int ii; @@ -2820,11 +2822,33 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) goto cleanup; } + /* 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 cleanup; + } + 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 cleanup; + } + } + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) { - virNetDevVPortProfilePtr virtport = NULL; /* <forward type='bridge|private|vepa|passthrough'> are all * VIR_DOMAIN_NET_TYPE_DIRECT. @@ -2853,24 +2877,28 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) break; } - /* Find the most specific virtportprofile and copy it */ - if (iface->virtPortProfile) { - virtport = iface->virtPortProfile; - } else { - if (portgroup) - virtport = portgroup->virtPortProfile; - else - virtport = netdef->virtPortProfile; + /* 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 cleanup; } + virtport = iface->data.network.actual->virtPortProfile; if (virtport) { - if (VIR_ALLOC(iface->data.network.actual->virtPortProfile) < 0) { - virReportOOMError(); + /* make sure type is supported for macvtap connections */ + if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && + virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<virtualport type='%s'> not supported for network " + "'%s' which uses a macvtap device"), + virNetDevVPortTypeToString(virtport->virtPortType), + netdef->name); goto cleanup; } - /* There are no pointers in a virtualPortProfile, so a shallow copy - * is sufficient - */ - *iface->data.network.actual->virtPortProfile = *virtport; } /* If there is only a single device, just return it (caller will detect @@ -2974,6 +3002,9 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } } + if (virNetDevVPortProfileCheckComplete(virtport) < 0) + goto cleanup; + ret = 0; cleanup: for (ii = 0; ii < num_virt_fns; ii++) -- 1.7.11.2