12:16 a.m.
Although it's been possible (ever since openvswitch was added to
libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
(by adding <virtualport type='openvswitch'>), the virtualport in the
network would always have a default random interfaceid included, which
would be re-used for all interfaces using that network, which doesn't
really work at all. The alternative was to not specify openvswitch in
the <network> definition, but to do it in the guest's <interface>
definition instead - this of course goes against the principle of not
having host-specific config embedded in guest config.
This patch series enhances the functionality of <virtualport>
elements, to allow omitting some attributes (and even the type), and
to merge the interface, network, and portgroup virtualports rather
than simply picking one. This not only makes openvswitch <network>s
more practical (because the network can specify type='openvswitch'
without also specifying an interfaceid), but also makes <virtualport>
in networks and portgroups more useful in general - for example, an
interface can specify an interfaceid (used only by openvswitch) *and*
an instanceid (used only by 802.1Qbh), while the network's virtualport
specifies only the type, and the portgroups specify the managerid,
typeid, profileid, or whatever is appropriate for the type of switch
used by the network.
The result is that the guest config can be completely devoid of
knowledge about the type of switch being used on the hardware, but can
still enjoy full configurability for whatever switch ends up being
used.
12:16 a.m.
New subject: [libvirt] [PATCH 1/9] util: make return value of virUUIDFormat and virMacAddrFormat useful
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
11:24 a.m.
New subject: [libvirt] [PATCH 1/9] util: make return value of virUUIDFormat and virMacAddrFormat useful
On Aug 5, 2012, at 12:16 AM, Laine Stump <laine(a)laine.org> wrote:
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)
{
The comment above the function says it returns 0 on success and -1 on error. Probably
should be updated to match what it does now.
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
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:43 a.m.
New subject: [libvirt] [PATCH 1/9] util: make return value of virUUIDFormat and virMacAddrFormat useful
On 08/05/2012 12:24 PM, Doug Goldstein wrote:
On Aug 5, 2012, at 12:16 AM, Laine Stump <laine(a)laine.org>
wrote:
> 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)
> {
The comment above the function says it returns 0 on success and -1 on error. Probably
should be updated to match what it does now.
Good point, I didn't notice that. Just because it was wrong before,
doesn't mean it should continue to be wrong :-)
12:16 a.m.
New subject: [libvirt] [PATCH 2/9] util: add openvswitch case to virNetDevVPortProfileEqual
This function was overlooked when openvswitch support was
added. Fortunately it's only use for update-device, which is
relatively new and seldom-used.
---
src/util/virnetdevvportprofile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 506240b..ac5aa24 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -107,6 +107,12 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a,
virNetDevVPortProfilePtr
return false;
break;
+ case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
+ if (STRNEQ(a->profileID, b->profileID) ||
+ memcmp(a->interfaceID, b->interfaceID, VIR_UUID_BUFLEN) != 0)
+ return false;
+ break;
+
default:
break;
}
--
1.7.11.2
6:04 p.m.
New subject: [libvirt] [PATCH 2/9] util: add openvswitch case to virNetDevVPortProfileEqual
On Sun, Aug 5, 2012 at 12:16 AM, Laine Stump <laine(a)laine.org> wrote:
This function was overlooked when openvswitch support was
added. Fortunately it's only use for update-device, which is
relatively new and seldom-used.
---
src/util/virnetdevvportprofile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 506240b..ac5aa24 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -107,6 +107,12 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a,
virNetDevVPortProfilePtr
return false;
break;
+ case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
+ if (STRNEQ(a->profileID, b->profileID) ||
+ memcmp(a->interfaceID, b->interfaceID, VIR_UUID_BUFLEN) != 0)
+ return false;
+ break;
+
default:
break;
}
--
1.7.11.2
--
If I understand your commit message correctly, it appears this was
omitted in a previous patchset and needed to be part of the
switch/case. The change looks good so ACK.
--
Doug Goldstein
12:16 a.m.
New subject: [libvirt] [PATCH 3/9] util: eliminate union in virNetDevVPortProfile
virNetDevVPortProfile has (had) a type field that can be set to one of
several values, and a union of several structs, one for each
type. When a domain's interface object is of type "network", the
domain config may not know beforehand which type of virtualport is
going to be provided in the actual device handed down from the network
driver at runtime, but may want to set some values in the virtualport
that may or may not be used, depending on the type. To support this
usage, this patch replaces the union of structs with toplevel fields
in the struct, making it possible for all of the fields to be set at
the same time.
---
src/conf/netdev_vport_profile_conf.c | 38 ++++++++++++++++++------------------
src/util/virnetdevopenvswitch.c | 8 ++++----
src/util/virnetdevvportprofile.c | 24 +++++++++++------------
src/util/virnetdevvportprofile.h | 29 ++++++++++++---------------
4 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c
index d008042..31ee9b4 100644
--- a/src/conf/netdev_vport_profile_conf.c
+++ b/src/conf/netdev_vport_profile_conf.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-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
@@ -100,7 +100,7 @@ virNetDevVPortProfileParse(xmlNodePtr node)
goto error;
}
- virtPort->u.virtPort8021Qbg.managerID = (uint8_t)val;
+ virtPort->managerID = (uint8_t)val;
if (virStrToLong_ui(virtPortTypeID, NULL, 0, &val)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -114,7 +114,7 @@ virNetDevVPortProfileParse(xmlNodePtr node)
goto error;
}
- virtPort->u.virtPort8021Qbg.typeID = (uint32_t)val;
+ virtPort->typeID = (uint32_t)val;
if (virStrToLong_ui(virtPortTypeIDVersion, NULL, 0, &val)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -128,17 +128,17 @@ virNetDevVPortProfileParse(xmlNodePtr node)
goto error;
}
- virtPort->u.virtPort8021Qbg.typeIDVersion = (uint8_t)val;
+ virtPort->typeIDVersion = (uint8_t)val;
if (virtPortInstanceID != NULL) {
if (virUUIDParse(virtPortInstanceID,
- virtPort->u.virtPort8021Qbg.instanceID)) {
+ virtPort->instanceID)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("cannot parse instanceid parameter as a
uuid"));
goto error;
}
} else {
- if (virUUIDGenerate(virtPort->u.virtPort8021Qbg.instanceID)) {
+ if (virUUIDGenerate(virtPort->instanceID)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("cannot generate a random uuid for
instanceid"));
goto error;
@@ -156,7 +156,7 @@ virNetDevVPortProfileParse(xmlNodePtr node)
case VIR_NETDEV_VPORT_PROFILE_8021QBH:
if (virtPortProfileID != NULL) {
- if (virStrcpyStatic(virtPort->u.virtPort8021Qbh.profileID,
+ if (virStrcpyStatic(virtPort->profileID,
virtPortProfileID) != NULL) {
virtPort->virtPortType = VIR_NETDEV_VPORT_PROFILE_8021QBH;
} else {
@@ -173,13 +173,13 @@ virNetDevVPortProfileParse(xmlNodePtr node)
case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
if (virtPortInterfaceID != NULL) {
if (virUUIDParse(virtPortInterfaceID,
- virtPort->u.openvswitch.interfaceID)) {
+ virtPort->interfaceID)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("cannot parse interfaceid parameter as a
uuid"));
goto error;
}
} else {
- if (virUUIDGenerate(virtPort->u.openvswitch.interfaceID)) {
+ if (virUUIDGenerate(virtPort->interfaceID)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("cannot generate a random uuid for
interfaceid"));
goto error;
@@ -187,14 +187,14 @@ virNetDevVPortProfileParse(xmlNodePtr node)
}
/* profileid is not mandatory for Open vSwitch */
if (virtPortProfileID != NULL) {
- if (virStrcpyStatic(virtPort->u.openvswitch.profileID,
+ if (virStrcpyStatic(virtPort->profileID,
virtPortProfileID) == NULL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("profileid parameter too long"));
goto error;
}
} else {
- virtPort->u.openvswitch.profileID[0] = '\0';
+ virtPort->profileID[0] = '\0';
}
break;
@@ -234,33 +234,33 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
switch (virtPort->virtPortType) {
case VIR_NETDEV_VPORT_PROFILE_8021QBG:
- virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID,
+ virUUIDFormat(virtPort->instanceID,
uuidstr);
virBufferAsprintf(buf,
" <parameters managerid='%d'
typeid='%d' "
"typeidversion='%d'
instanceid='%s'/>\n",
- virtPort->u.virtPort8021Qbg.managerID,
- virtPort->u.virtPort8021Qbg.typeID,
- virtPort->u.virtPort8021Qbg.typeIDVersion,
+ virtPort->managerID,
+ virtPort->typeID,
+ virtPort->typeIDVersion,
uuidstr);
break;
case VIR_NETDEV_VPORT_PROFILE_8021QBH:
virBufferAsprintf(buf,
" <parameters profileid='%s'/>\n",
- virtPort->u.virtPort8021Qbh.profileID);
+ virtPort->profileID);
break;
case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
- virUUIDFormat(virtPort->u.openvswitch.interfaceID,
+ virUUIDFormat(virtPort->interfaceID,
uuidstr);
- if (virtPort->u.openvswitch.profileID[0] == '\0') {
+ if (virtPort->profileID[0] == '\0') {
virBufferAsprintf(buf, " <parameters
interfaceid='%s'/>\n",
uuidstr);
} else {
virBufferAsprintf(buf, " <parameters interfaceid='%s'
"
"profileid='%s'/>\n", uuidstr,
- virtPort->u.openvswitch.profileID);
+ virtPort->profileID);
}
break;
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 7e0beef..b57532b 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -59,7 +59,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
char *vmid_ex_id = NULL;
virMacAddrFormat(macaddr, macaddrstr);
- virUUIDFormat(ovsport->u.openvswitch.interfaceID, ifuuidstr);
+ virUUIDFormat(ovsport->interfaceID, ifuuidstr);
virUUIDFormat(vmuuid, vmuuidstr);
if (virAsprintf(&attachedmac_ex_id,
"external-ids:attached-mac=\"%s\"",
@@ -71,14 +71,14 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char
*ifname,
if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"",
vmuuidstr) < 0)
goto out_of_memory;
- if (ovsport->u.openvswitch.profileID[0] != '\0') {
+ if (ovsport->profileID[0] != '\0') {
if (virAsprintf(&profile_ex_id,
"external-ids:port-profile=\"%s\"",
- ovsport->u.openvswitch.profileID) < 0)
+ ovsport->profileID) < 0)
goto out_of_memory;
}
cmd = virCommandNew(OVSVSCTL);
- if (ovsport->u.openvswitch.profileID[0] == '\0') {
+ if (ovsport->profileID[0] == '\0') {
virCommandAddArgList(cmd, "--", "--may-exist",
"add-port",
brname, ifname,
"--", "set", "Interface", ifname,
attachedmac_ex_id,
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index ac5aa24..6db04f7 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -95,15 +95,15 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a,
virNetDevVPortProfilePtr
break;
case VIR_NETDEV_VPORT_PROFILE_8021QBG:
- if (a->u.virtPort8021Qbg.managerID != b->u.virtPort8021Qbg.managerID ||
- a->u.virtPort8021Qbg.typeID != b->u.virtPort8021Qbg.typeID ||
- a->u.virtPort8021Qbg.typeIDVersion !=
b->u.virtPort8021Qbg.typeIDVersion ||
- memcmp(a->u.virtPort8021Qbg.instanceID,
b->u.virtPort8021Qbg.instanceID, VIR_UUID_BUFLEN) != 0)
+ if (a->managerID != b->managerID ||
+ a->typeID != b->typeID ||
+ a->typeIDVersion != b->typeIDVersion ||
+ memcmp(a->instanceID, b->instanceID, VIR_UUID_BUFLEN) != 0)
return false;
break;
case VIR_NETDEV_VPORT_PROFILE_8021QBH:
- if (STRNEQ(a->u.virtPort8021Qbh.profileID,
b->u.virtPort8021Qbh.profileID))
+ if (STRNEQ(a->profileID, b->profileID))
return false;
break;
@@ -643,8 +643,8 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
int rc = -1;
int op = PORT_REQUEST_ASSOCIATE;
struct ifla_port_vsi portVsi = {
- .vsi_mgr_id = virtPort->u.virtPort8021Qbg.managerID,
- .vsi_type_version = virtPort->u.virtPort8021Qbg.typeIDVersion,
+ .vsi_mgr_id = virtPort->managerID,
+ .vsi_type_version = virtPort->typeIDVersion,
};
bool nltarget_kernel = false;
int vlanid;
@@ -664,9 +664,9 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
if (vlanid < 0)
vlanid = 0;
- portVsi.vsi_type_id[2] = virtPort->u.virtPort8021Qbg.typeID >> 16;
- portVsi.vsi_type_id[1] = virtPort->u.virtPort8021Qbg.typeID >> 8;
- portVsi.vsi_type_id[0] = virtPort->u.virtPort8021Qbg.typeID;
+ portVsi.vsi_type_id[2] = virtPort->typeID >> 16;
+ portVsi.vsi_type_id[1] = virtPort->typeID >> 8;
+ portVsi.vsi_type_id[0] = virtPort->typeID;
switch (virtPortOp) {
case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE:
@@ -690,7 +690,7 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
vlanid,
NULL,
&portVsi,
- virtPort->u.virtPort8021Qbg.instanceID,
+ virtPort->instanceID,
NULL,
vf,
op,
@@ -755,7 +755,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname,
nltarget_kernel,
macaddr,
vlanid,
- virtPort->u.virtPort8021Qbh.profileID,
+ virtPort->profileID,
NULL,
vm_uuid,
hostuuid,
diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h
index 6cce1bb..e204c95 100644
--- a/src/util/virnetdevvportprofile.h
+++ b/src/util/virnetdevvportprofile.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-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
@@ -61,21 +61,18 @@ typedef struct _virNetDevVPortProfile virNetDevVPortProfile;
typedef virNetDevVPortProfile *virNetDevVPortProfilePtr;
struct _virNetDevVPortProfile {
enum virNetDevVPortProfile virtPortType;
- union {
- struct {
- uint8_t managerID;
- uint32_t typeID; /* 24 bit valid */
- uint8_t typeIDVersion;
- unsigned char instanceID[VIR_UUID_BUFLEN];
- } virtPort8021Qbg;
- struct {
- char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
- } virtPort8021Qbh;
- struct {
- unsigned char interfaceID[VIR_UUID_BUFLEN];
- char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
- } openvswitch;
- } u;
+ /* these members are used when virtPortType == 802.1Qbg */
+ uint8_t managerID;
+ uint32_t typeID; /* 24 bit valid */
+ uint8_t typeIDVersion;
+ unsigned char instanceID[VIR_UUID_BUFLEN];
+
+ /* this member is used when virtPortType == 802.1Qbh|openvswitch */
+ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
+
+ /* this member is used when virtPortType == openvswitch */
+ unsigned char interfaceID[VIR_UUID_BUFLEN];
+ /* NB - if virtPortType == NONE, any/all of the items could be used */
};
--
1.7.11.2
12:16 a.m.
New subject: [libvirt] [PATCH 4/9] util: utility functions for virNetDevVPortProfile
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
2:19 p.m.
New subject: [libvirt] [PATCH 4/9] util: utility functions for virNetDevVPortProfile
On 08/05/2012 01:16 AM, Laine Stump wrote:
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
[...]
+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;
Note that the above statement should be "return 0;" instead of "return
ret;". I've already squashed that fix into my local copy. (No matter how
long I wait to send patches, I always find something to fix after the fact.)
12:16 a.m.
New subject: [libvirt] [PATCH 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 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
12:16 a.m.
New subject: [libvirt] [PATCH 6/9] conf: make error returns from virDomainActualNetDefFormat consistent
There was an error: label that simply did "return ret", but ret was
defaulted to -1, and was never used other than setting it manually to
0 just before a non-error return. Aside from this, some of the error
return paths used "goto error" and others used "return ret".
This patch removes ret and the error: label, and makes all error
returns just consistently do "return -1".
---
src/conf/domain_conf.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a3fb1e7..6d1a29a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11553,7 +11553,6 @@ virDomainActualNetDefFormat(virBufferPtr buf,
virDomainActualNetDefPtr def,
unsigned int flags)
{
- int ret = -1;
const char *type;
const char *mode;
@@ -11564,7 +11563,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected net type %d"), def->type);
- return ret;
+ return -1;
}
virBufferAsprintf(buf, " <actual type='%s'", type);
@@ -11591,7 +11590,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected source mode %d"),
def->data.direct.mode);
- return ret;
+ return -1;
}
virBufferAsprintf(buf, " mode='%s'/>\n", mode);
break;
@@ -11610,21 +11609,18 @@ virDomainActualNetDefFormat(virBufferPtr buf,
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected net type %s"), type);
- goto error;
+ return -1;
}
virBufferAdjustIndent(buf, 8);
if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
return -1;
if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
- goto error;
+ return -1;
virBufferAdjustIndent(buf, -8);
virBufferAddLit(buf, " </actual>\n");
-
- ret = 0;
-error:
- return ret;
+ return 0;
}
static int
--
1.7.11.2
12:16 a.m.
New subject: [libvirt] [PATCH 7/9] conf: simplify Buffer Indentation in virDomainNetDefFormat
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
12:17 a.m.
New subject: [libvirt] [PATCH 8/9] conf: support partially-specified <virtualport> in parser and formatter
Until now, all attributes in a <virtualport> parameter list that were
acceptable for a particular type, were also required. There were no
optional attributes.
One of the aims of supporting <virtualport> in libvirt's virtual
networks and portgroups is to allow specifying the group-wide
parameters in the network's virtualport, and merge that with the
interface's virtualport, which will have the instance-specific info
(i.e. the interfaceid or instanceid).
Additionally, the guest's interface XML shouldn't need to know what
type of network connection will be used prior to runtime - it could be
openvswitch, 802.1Qbh, 802.1Qbg, or none of the above - but should
still be able to specify instance-specific info just in case it turns
out to be applicable.
Finally, up to now, the parser for virtualport has always generated a
random instanceid/interfaceid when appropriate, making it impossible
to leave it blank (which is what's required for virtualports within a
network/portprofile definition).
This patch modifies the parser and formatter of the <virtualport>
element in the following ways:
* because most of the attributes in a virNetDevVPortProfile are fixed
size binary data with no reserved values, there is no way to embed a
"this value wasn't specified" sentinel into the existing data. To
solve this problem, the new *_specified fields in the
virNetDevVPortProfile object that were added in a previous patch of
this series are now set when the corresponding attribute is present
during the parse.
* allow parsing/formatting a <virtualport> that has no type set. In
this case, all fields are settable, but all are also optional.
* add a GENERATE_MISSING_DEFAULTS flag to the parser - if this flag is
set and an instanceid/interfaceid is expected but not provided, a
random one will be generated. This was previously the default
behavior, but is now done only for virtualports inside an
<interface> definition, not for those in <network> or <portgroup>.
* add a REQUIRE_ALL_ATTRIBUTES flag to the parser - if this flag is
set the parser will call the new
virNetDevVPortProfileCheckComplete() functions at the end of the
parser to check for any missing attributes (based on type), and
return failure if anything is missing. This used to be default
behavior. Now it is only used for the virtualport defined inside an
interface's <actual> element (by the time you've figured out the
contents of <actual>, you should have all the necessary data to fill
in the entire virtualport)
* add a REQUIRE_TYPE flag to the parser - if this flag is set, the
parser will return an error if the virtualport has no type
attribute. This also was previously the default behavior, but isn't
needed in the case of the virtualport for a type='network' interface
(i.e. the exact type isn't yet known), or the virtualport of a
portgroup (i.e. the portgroup just has modifiers for the network's
virtualport, which *does* require a type) - in those cases, the
check will be done at domain startup, once the final virtualport is
assembled (this is handled in the next patch).
---
docs/schemas/networkcommon.rng | 114 ++++++--
src/conf/domain_conf.c | 25 +-
src/conf/netdev_vport_profile_conf.c | 308 +++++++++++----------
src/conf/netdev_vport_profile_conf.h | 17 +-
src/conf/network_conf.c | 7 +-
tests/networkxml2xmlin/openvswitch-net.xml | 16 ++
tests/networkxml2xmlin/vepa-net.xml | 6 +-
tests/networkxml2xmlout/openvswitch-net.xml | 16 ++
tests/networkxml2xmlout/vepa-net.xml | 6 +-
tests/networkxml2xmltest.c | 1 +
.../qemuxml2argv-net-virtio-network-portgroup.xml | 14 +
11 files changed, 344 insertions(+), 186 deletions(-)
create mode 100644 tests/networkxml2xmlin/openvswitch-net.xml
create mode 100644 tests/networkxml2xmlout/openvswitch-net.xml
diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng
index 2328892..f2c3330 100644
--- a/docs/schemas/networkcommon.rng
+++ b/docs/schemas/networkcommon.rng
@@ -15,22 +15,30 @@
<attribute name="type">
<value>802.1Qbg</value>
</attribute>
- <element name="parameters">
- <attribute name="managerid">
- <ref name="uint8range"/>
- </attribute>
- <attribute name="typeid">
- <ref name="uint24range"/>
- </attribute>
- <attribute name="typeidversion">
- <ref name="uint8range"/>
- </attribute>
- <optional>
- <attribute name="instanceid">
- <ref name="UUID"/>
- </attribute>
- </optional>
- </element>
+ <optional>
+ <element name="parameters">
+ <optional>
+ <attribute name="managerid">
+ <ref name="uint8range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="typeid">
+ <ref name="uint24range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="typeidversion">
+ <ref name="uint8range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="instanceid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</element>
</group>
<group>
@@ -38,11 +46,75 @@
<attribute name="type">
<value>802.1Qbh</value>
</attribute>
- <element name="parameters">
- <attribute name="profileid">
- <ref name="virtualPortProfileID"/>
- </attribute>
- </element>
+ <optional>
+ <element name="parameters">
+ <optional>
+ <attribute name="profileid">
+ <ref name="virtualPortProfileID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
+ </element>
+ </group>
+ <group>
+ <element name="virtualport">
+ <attribute name="type">
+ <value>openvswitch</value>
+ </attribute>
+ <optional>
+ <element name="parameters">
+ <optional>
+ <attribute name="profileid">
+ <ref name="virtualPortProfileID"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="interfaceid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
+ </element>
+ </group>
+ <group>
+ <!-- use this when no type attribute is present -->
+ <element name="virtualport">
+ <optional>
+ <element name="parameters">
+ <optional>
+ <attribute name="managerid">
+ <ref name="uint8range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="typeid">
+ <ref name="uint24range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="typeidversion">
+ <ref name="uint8range"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="instanceid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="profileid">
+ <ref name="virtualPortProfileID"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="interfaceid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</element>
</group>
</choice>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e212263..ce3019d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4409,8 +4409,13 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actual->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ /* the virtualport in <actual> should always already
+ * have an instanceid/interfaceid if its required,
+ * so don't let the parser generate one */
if (!(actual->virtPortProfile
- = virNetDevVPortProfileParse(virtPortNode))) {
+ = virNetDevVPortProfileParse(virtPortNode,
+ VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES |
+ VIR_VPORT_XML_REQUIRE_TYPE))) {
goto error;
}
} else {
@@ -4563,12 +4568,20 @@ virDomainNetDefParseXML(virCapsPtr caps,
mode = virXMLPropString(cur, "mode");
} 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->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!(def->virtPortProfile
- = virNetDevVPortProfileParse(cur))) {
+ = virNetDevVPortProfileParse(cur,
+
VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS))) {
+ goto error;
+ }
+ } else if (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,
+
VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS|
+
VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES|
+ VIR_VPORT_XML_REQUIRE_TYPE))) {
goto error;
}
} else {
diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c
index 31ee9b4..3569d34 100644
--- a/src/conf/netdev_vport_profile_conf.c
+++ b/src/conf/netdev_vport_profile_conf.c
@@ -37,7 +37,7 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST,
virNetDevVPortProfilePtr
-virNetDevVPortProfileParse(xmlNodePtr node)
+virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags)
{
char *virtPortType;
char *virtPortManagerID = NULL;
@@ -54,22 +54,22 @@ virNetDevVPortProfileParse(xmlNodePtr node)
return NULL;
}
- virtPortType = virXMLPropString(node, "type");
- if (!virtPortType) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing virtualportprofile type"));
+ if ((virtPortType = virXMLPropString(node, "type")) &&
+ (virtPort->virtPortType = virNetDevVPortTypeFromString(virtPortType)) <= 0)
{
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown virtualport type %s"), virtPortType);
goto error;
}
- if ((virtPort->virtPortType = virNetDevVPortTypeFromString(virtPortType)) <= 0)
{
- virReportError(VIR_ERR_XML_ERROR,
- _("unknown virtualportprofile type %s"), virtPortType);
+ if ((virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE) &&
+ (flags & VIR_VPORT_XML_REQUIRE_TYPE)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing required virtualport type"));
goto error;
}
while (cur != NULL) {
if (xmlStrEqual(cur->name, BAD_CAST "parameters")) {
-
virtPortManagerID = virXMLPropString(cur, "managerid");
virtPortTypeID = virXMLPropString(cur, "typeid");
virtPortTypeIDVersion = virXMLPropString(cur, "typeidversion");
@@ -78,132 +78,119 @@ virNetDevVPortProfileParse(xmlNodePtr node)
virtPortInterfaceID = virXMLPropString(cur, "interfaceid");
break;
}
-
cur = cur->next;
}
- switch (virtPort->virtPortType) {
- case VIR_NETDEV_VPORT_PROFILE_8021QBG:
- if (virtPortManagerID != NULL && virtPortTypeID != NULL
&&
- virtPortTypeIDVersion != NULL) {
- unsigned int val;
-
- if (virStrToLong_ui(virtPortManagerID, NULL, 0, &val)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot parse value of managerid
parameter"));
- goto error;
- }
-
- if (val > 0xff) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("value of managerid out of range"));
- goto error;
- }
-
- virtPort->managerID = (uint8_t)val;
-
- if (virStrToLong_ui(virtPortTypeID, NULL, 0, &val)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot parse value of typeid
parameter"));
- goto error;
- }
+ if (virtPortManagerID) {
+ unsigned int val;
- if (val > 0xffffff) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("value for typeid out of range"));
- goto error;
- }
-
- virtPort->typeID = (uint32_t)val;
+ if (virStrToLong_ui(virtPortManagerID, NULL, 0, &val)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("cannot parse value of managerid parameter"));
+ goto error;
+ }
+ if (val > 0xff) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("value of managerid out of range"));
+ goto error;
+ }
+ virtPort->managerID = (uint8_t)val;
+ virtPort->managerID_specified = true;
+ }
- if (virStrToLong_ui(virtPortTypeIDVersion, NULL, 0, &val)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot parse value of typeidversion
parameter"));
- goto error;
- }
+ if (virtPortTypeID) {
+ unsigned int val;
- if (val > 0xff) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("value of typeidversion out of
range"));
- goto error;
- }
+ if (virStrToLong_ui(virtPortTypeID, NULL, 0, &val)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("cannot parse value of typeid parameter"));
+ goto error;
+ }
+ if (val > 0xffffff) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("value for typeid out of range"));
+ goto error;
+ }
+ virtPort->typeID = (uint32_t)val;
+ virtPort->typeID_specified = true;
+ }
- virtPort->typeIDVersion = (uint8_t)val;
-
- if (virtPortInstanceID != NULL) {
- if (virUUIDParse(virtPortInstanceID,
- virtPort->instanceID)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot parse instanceid parameter as a
uuid"));
- goto error;
- }
- } else {
- if (virUUIDGenerate(virtPort->instanceID)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot generate a random uuid for
instanceid"));
- goto error;
- }
- }
+ if (virtPortTypeIDVersion) {
+ unsigned int val;
- virtPort->virtPortType = VIR_NETDEV_VPORT_PROFILE_8021QBG;
+ if (virStrToLong_ui(virtPortTypeIDVersion, NULL, 0, &val)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("cannot parse value of typeidversion
parameter"));
+ goto error;
+ }
+ if (val > 0xff) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("value of typeidversion out of range"));
+ goto error;
+ }
+ virtPort->typeIDVersion = (uint8_t)val;
+ virtPort->typeIDVersion_specified = true;
+ }
- } else {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("a parameter is missing for 802.1Qbg
description"));
+ if (virtPortInstanceID) {
+ if (virUUIDParse(virtPortInstanceID, virtPort->instanceID) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("cannot parse instanceid parameter as a uuid"));
goto error;
}
- break;
-
- case VIR_NETDEV_VPORT_PROFILE_8021QBH:
- if (virtPortProfileID != NULL) {
- if (virStrcpyStatic(virtPort->profileID,
- virtPortProfileID) != NULL) {
- virtPort->virtPortType = VIR_NETDEV_VPORT_PROFILE_8021QBH;
- } else {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("profileid parameter too long"));
- goto error;
- }
- } else {
+ virtPort->instanceID_specified = true;
+ }
+
+ if (virtPortProfileID &&
+ !virStrcpyStatic(virtPort->profileID, virtPortProfileID)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("profileid parameter too long"));
+ goto error;
+ }
+
+ if (virtPortInterfaceID) {
+ if (virUUIDParse(virtPortInterfaceID, virtPort->interfaceID) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("profileid parameter is missing for 802.1Qbh
description"));
+ _("cannot parse interfaceid parameter as a uuid"));
goto error;
}
- break;
- case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
- if (virtPortInterfaceID != NULL) {
- if (virUUIDParse(virtPortInterfaceID,
- virtPort->interfaceID)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot parse interfaceid parameter as a
uuid"));
- goto error;
- }
- } else {
- if (virUUIDGenerate(virtPort->interfaceID)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("cannot generate a random uuid for
interfaceid"));
+ virtPort->interfaceID_specified = true;
+ }
+
+ /* generate default instanceID/interfaceID if appropriate */
+ if (flags & VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS) {
+ if (!virtPort->instanceID_specified &&
+ (virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
+ virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ if (virUUIDGenerate(virtPort->instanceID) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot generate a random uuid for
instanceid"));
goto error;
}
+ virtPort->instanceID_specified = true;
}
- /* profileid is not mandatory for Open vSwitch */
- if (virtPortProfileID != NULL) {
- if (virStrcpyStatic(virtPort->profileID,
- virtPortProfileID) == NULL) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("profileid parameter too long"));
+ if (!virtPort->interfaceID_specified &&
+ (virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH ||
+ virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ if (virUUIDGenerate(virtPort->interfaceID) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot generate a random uuid for
interfaceid"));
goto error;
}
- } else {
- virtPort->profileID[0] = '\0';
+ virtPort->interfaceID_specified = true;
}
- break;
+ }
- default:
- virReportError(VIR_ERR_XML_ERROR,
- _("unexpected virtualport type %d"),
virtPort->virtPortType);
+ /* check for required/unsupported attributes */
+
+ if ((flags & VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES) &&
+ (virNetDevVPortProfileCheckComplete(virtPort) < 0)) {
goto error;
}
+ if (virNetDevVPortProfileCheckNoExtras(virtPort) < 0)
+ goto error;
+
cleanup:
VIR_FREE(virtPortManagerID);
VIR_FREE(virtPortTypeID);
@@ -224,53 +211,76 @@ int
virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
virBufferPtr buf)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
+ enum virNetDevVPortProfile type;
+ bool noParameters;
- if (!virtPort || virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)
+ if (!virtPort)
return 0;
- virBufferAsprintf(buf, "<virtualport type='%s'>\n",
- virNetDevVPortTypeToString(virtPort->virtPortType));
-
- switch (virtPort->virtPortType) {
- case VIR_NETDEV_VPORT_PROFILE_8021QBG:
- virUUIDFormat(virtPort->instanceID,
- uuidstr);
- virBufferAsprintf(buf,
- " <parameters managerid='%d'
typeid='%d' "
- "typeidversion='%d'
instanceid='%s'/>\n",
- virtPort->managerID,
- virtPort->typeID,
- virtPort->typeIDVersion,
- uuidstr);
- break;
-
- case VIR_NETDEV_VPORT_PROFILE_8021QBH:
- virBufferAsprintf(buf,
- " <parameters profileid='%s'/>\n",
- virtPort->profileID);
- break;
-
- case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
- virUUIDFormat(virtPort->interfaceID,
- uuidstr);
- if (virtPort->profileID[0] == '\0') {
- virBufferAsprintf(buf, " <parameters
interfaceid='%s'/>\n",
- uuidstr);
+ noParameters = !(virtPort->managerID_specified ||
+ virtPort->typeID_specified ||
+ virtPort->typeIDVersion_specified ||
+ virtPort->instanceID_specified ||
+ virtPort->profileID[0] ||
+ virtPort->interfaceID_specified);
+
+ type = virtPort->virtPortType;
+ if (type == VIR_NETDEV_VPORT_PROFILE_NONE) {
+ if (noParameters)
+ return 0;
+ virBufferAddLit(buf, "<virtualport>\n");
+ } else {
+ if (noParameters) {
+ virBufferAsprintf(buf, "<virtualport type='%s'/>\n",
+ virNetDevVPortTypeToString(type));
+ return 0;
} else {
- virBufferAsprintf(buf, " <parameters interfaceid='%s'
"
- "profileid='%s'/>\n", uuidstr,
- virtPort->profileID);
+ virBufferAsprintf(buf, "<virtualport type='%s'>\n",
+ virNetDevVPortTypeToString(type));
}
+ }
+ virBufferAddLit(buf, " <parameters");
- break;
+ if (virtPort->managerID_specified &&
+ (type == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ virBufferAsprintf(buf, " managerid='%d'",
virtPort->managerID);
+ }
+ if (virtPort->typeID_specified &&
+ (type == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ virBufferAsprintf(buf, " typeid='%d'", virtPort->typeID);
+ }
+ if (virtPort->typeIDVersion_specified &&
+ (type == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ virBufferAsprintf(buf, " typeidversion='%d'",
+ virtPort->typeIDVersion);
+ }
+ if (virtPort->instanceID_specified &&
+ (type == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
- default:
- virReportError(VIR_ERR_XML_ERROR,
- _("unexpected virtualport type %d"),
virtPort->virtPortType);
- return -1;
+ virUUIDFormat(virtPort->instanceID, uuidstr);
+ virBufferAsprintf(buf, " instanceid='%s'", uuidstr);
+ }
+ if (virtPort->interfaceID_specified &&
+ (type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(virtPort->interfaceID, uuidstr);
+ virBufferAsprintf(buf, " interfaceid='%s'", uuidstr);
+ }
+ if (virtPort->profileID[0] &&
+ (type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH ||
+ type == VIR_NETDEV_VPORT_PROFILE_8021QBH ||
+ type == VIR_NETDEV_VPORT_PROFILE_NONE)) {
+ virBufferAsprintf(buf, " profileid='%s'",
virtPort->profileID);
}
+ virBufferAddLit(buf, "/>\n");
virBufferAddLit(buf, "</virtualport>\n");
return 0;
}
diff --git a/src/conf/netdev_vport_profile_conf.h b/src/conf/netdev_vport_profile_conf.h
index 367bdf3..caf3519 100644
--- a/src/conf/netdev_vport_profile_conf.h
+++ b/src/conf/netdev_vport_profile_conf.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-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
@@ -28,8 +28,21 @@
# include "buf.h"
# include "xml.h"
+typedef enum {
+ /* generate random defaults for interfaceID/interfaceID
+ * when appropriate
+ */
+ VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS = (1<<0),
+ /* fail if any attribute required for the specified
+ * type is missing
+ */
+ VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES = (1<<1),
+ /* fail if no type is specified */
+ VIR_VPORT_XML_REQUIRE_TYPE = (1<<2),
+} virNetDevVPortXMLFlags;
+
virNetDevVPortProfilePtr
-virNetDevVPortProfileParse(xmlNodePtr node);
+virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags);
int
virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index eb92d93..ae61847 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -901,8 +901,9 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
virtPortNode = virXPathNode("./virtualport", ctxt);
if (virtPortNode &&
- (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode))))
+ (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) {
goto error;
+ }
bandwidth_node = virXPathNode("./bandwidth", ctxt);
if (bandwidth_node &&
@@ -1011,8 +1012,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
virtPortNode = virXPathNode("./virtualport", ctxt);
if (virtPortNode &&
- (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode))))
+ (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode,
+
VIR_VPORT_XML_REQUIRE_TYPE)))) {
goto error;
+ }
nPortGroups = virXPathNodeSet("./portgroup", ctxt, &portGroupNodes);
if (nPortGroups < 0)
diff --git a/tests/networkxml2xmlin/openvswitch-net.xml
b/tests/networkxml2xmlin/openvswitch-net.xml
new file mode 100644
index 0000000..8aa1897
--- /dev/null
+++ b/tests/networkxml2xmlin/openvswitch-net.xml
@@ -0,0 +1,16 @@
+<network>
+ <name>openvswitch-net</name>
+ <uuid>81ff0d90-c92e-6742-64da-4a736edb9a8b</uuid>
+ <forward mode='bridge'/>
+ <virtualport type='openvswitch'/>
+ <portgroup name='bob' default='yes'>
+ <virtualport>
+ <parameters profileid='bob-profile'/>
+ </virtualport>
+ </portgroup>
+ <portgroup name='alice'>
+ <virtualport>
+ <parameters profileid='alice-profile'/>
+ </virtualport>
+ </portgroup>
+</network>
diff --git a/tests/networkxml2xmlin/vepa-net.xml b/tests/networkxml2xmlin/vepa-net.xml
index b1a40c6..030c1d1 100644
--- a/tests/networkxml2xmlin/vepa-net.xml
+++ b/tests/networkxml2xmlin/vepa-net.xml
@@ -7,16 +7,16 @@
<interface dev="eth3"/>
</forward>
<virtualport type="802.1Qbg">
- <parameters managerid="11" typeid="1193047"
typeidversion="2"
instanceid="b153fa89-1b87-9719-ec12-99e0054fb844"/>
+ <parameters managerid="11" typeid="1193047"
typeidversion="2"/>
</virtualport>
<portgroup name="bob" default="yes">
<virtualport type="802.1Qbg">
- <parameters managerid="12" typeid="2193047"
typeidversion="3"
instanceid="5d00e0ba-e15c-959c-fbb6-b595b0655735"/>
+ <parameters typeid="2193047" typeidversion="3"/>
</virtualport>
</portgroup>
<portgroup name="alice">
<virtualport type="802.1Qbg">
- <parameters managerid="13" typeid="3193047"
typeidversion="4"
instanceid="70bf45f9-01a8-f5ee-3c0f-e25a0a2e44a6"/>
+ <parameters managerid="13"/>
</virtualport>
</portgroup>
</network>
diff --git a/tests/networkxml2xmlout/openvswitch-net.xml
b/tests/networkxml2xmlout/openvswitch-net.xml
new file mode 100644
index 0000000..8aa1897
--- /dev/null
+++ b/tests/networkxml2xmlout/openvswitch-net.xml
@@ -0,0 +1,16 @@
+<network>
+ <name>openvswitch-net</name>
+ <uuid>81ff0d90-c92e-6742-64da-4a736edb9a8b</uuid>
+ <forward mode='bridge'/>
+ <virtualport type='openvswitch'/>
+ <portgroup name='bob' default='yes'>
+ <virtualport>
+ <parameters profileid='bob-profile'/>
+ </virtualport>
+ </portgroup>
+ <portgroup name='alice'>
+ <virtualport>
+ <parameters profileid='alice-profile'/>
+ </virtualport>
+ </portgroup>
+</network>
diff --git a/tests/networkxml2xmlout/vepa-net.xml b/tests/networkxml2xmlout/vepa-net.xml
index af13d0f..4d35a8a 100644
--- a/tests/networkxml2xmlout/vepa-net.xml
+++ b/tests/networkxml2xmlout/vepa-net.xml
@@ -7,16 +7,16 @@
<interface dev='eth3'/>
</forward>
<virtualport type='802.1Qbg'>
- <parameters managerid='11' typeid='1193047'
typeidversion='2' instanceid='b153fa89-1b87-9719-ec12-99e0054fb844'/>
+ <parameters managerid='11' typeid='1193047'
typeidversion='2'/>
</virtualport>
<portgroup name='bob' default='yes'>
<virtualport type='802.1Qbg'>
- <parameters managerid='12' typeid='2193047'
typeidversion='3' instanceid='5d00e0ba-e15c-959c-fbb6-b595b0655735'/>
+ <parameters typeid='2193047' typeidversion='3'/>
</virtualport>
</portgroup>
<portgroup name='alice'>
<virtualport type='802.1Qbg'>
- <parameters managerid='13' typeid='3193047'
typeidversion='4' instanceid='70bf45f9-01a8-f5ee-3c0f-e25a0a2e44a6'/>
+ <parameters managerid='13'/>
</virtualport>
</portgroup>
</network>
diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
index 8641c41..5a5531a 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -104,6 +104,7 @@ mymain(void)
DO_TEST("host-bridge-net");
DO_TEST("vepa-net");
DO_TEST("bandwidth-network");
+ DO_TEST("openvswitch-net");
DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE);
return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml
index 3eb5c88..6d379a0 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml
@@ -29,6 +29,20 @@
</virtualport>
<model type='virtio'/>
</interface>
+ <interface type='network'>
+ <mac address='10:11:22:33:44:55'/>
+ <source network='blue' portgroup='sam'/>
+ <virtualport>
+ <parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'
interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
+ </virtualport>
+ </interface>
+ <interface type='network'>
+ <mac address='22:11:22:33:44:55'/>
+ <source network='blue' portgroup='sam'/>
+ <virtualport type='802.1Qbh'>
+ <parameters profileid='testhis99'/>
+ </virtualport>
+ </interface>
<memballoon model='virtio'/>
</devices>
</domain>
--
1.7.11.2
12:17 a.m.
New subject: [libvirt] [PATCH 9/9] network: merge relevant virtualports rather than choosing one
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
2:11 p.m.
New subject: [libvirt] [PATCH 9.5/9] network: fix bug in virtualport merging patch
The original version of the virtualport merging patch added in a call
to virNetDevVPortProfileCheckComplete() (which might fail) after the
use count for newly allocated device was incremented. If that function
did fail, the use count would be wrong which, in the worst case, would
make that physical device unusable until libvirtd was restarted.
This followup patch (which I will squash into the original before
pushing), increases the scope of the device pointer, and delays
incrementing the use count until we are *really* assured of success.
---
src/network/bridge_driver.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2bbfd3b..9e4f79a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2748,6 +2748,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virNetworkDefPtr netdef;
virPortGroupDefPtr portgroup;
virNetDevVPortProfilePtr virtport = NULL;
+ virNetworkForwardIfDefPtr dev = NULL;
unsigned int num_virt_fns = 0;
char **vfname = NULL;
int ii;
@@ -2911,8 +2912,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
netdef->name);
goto cleanup;
} else {
- virNetworkForwardIfDefPtr dev = NULL;
-
/* pick an interface from the pool */
/* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
@@ -2995,16 +2994,18 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virReportOOMError();
goto cleanup;
}
- /* we are now assured of success, so mark the allocation */
- dev->usageCount++;
- VIR_DEBUG("Using physical device %s, usageCount %d",
- dev->dev, dev->usageCount);
}
}
if (virNetDevVPortProfileCheckComplete(virtport) < 0)
goto cleanup;
+ if (dev) {
+ /* we are now assured of success, so mark the allocation */
+ dev->usageCount++;
+ VIR_DEBUG("Using physical device %s, usageCount %d",
+ dev->dev, dev->usageCount);
+ }
ret = 0;
cleanup:
for (ii = 0; ii < num_virt_fns; ii++)
--
1.7.11.2
9:35 a.m.
New subject: [libvirt] [PATCH 0/9] network: properly support openvswitch in <network>
I just looked at this series and it looks great to me Laine. I think the
new flexibility this series introduces will be really nice to have, and
it will make starting guests across hosts with differing network
types easier as well.
Thanks!
Kyle
On Aug 5, 2012, at 12:16 AM, Laine Stump wrote:
Although it's been possible (ever since openvswitch was added to
libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
(by adding <virtualport type='openvswitch'>), the virtualport in the
network would always have a default random interfaceid included, which
would be re-used for all interfaces using that network, which doesn't
really work at all. The alternative was to not specify openvswitch in
the <network> definition, but to do it in the guest's <interface>
definition instead - this of course goes against the principle of not
having host-specific config embedded in guest config.
This patch series enhances the functionality of <virtualport>
elements, to allow omitting some attributes (and even the type), and
to merge the interface, network, and portgroup virtualports rather
than simply picking one. This not only makes openvswitch <network>s
more practical (because the network can specify type='openvswitch'
without also specifying an interfaceid), but also makes <virtualport>
in networks and portgroups more useful in general - for example, an
interface can specify an interfaceid (used only by openvswitch) *and*
an instanceid (used only by 802.1Qbh), while the network's virtualport
specifies only the type, and the portgroups specify the managerid,
typeid, profileid, or whatever is appropriate for the type of switch
used by the network.
The result is that the guest config can be completely devoid of
knowledge about the type of switch being used on the hardware, but can
still enjoy full configurability for whatever switch ends up being
used.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:43 p.m.
On Sat, Aug 4, 2012 at 10:16 PM, Laine Stump <laine(a)laine.org> wrote:
Although it's been possible (ever since openvswitch was added to
libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
(by adding <virtualport type='openvswitch'>), the virtualport in the
network would always have a default random interfaceid included, which
would be re-used for all interfaces using that network, which doesn't
really work at all. The alternative was to not specify openvswitch in
the <network> definition, but to do it in the guest's <interface>
definition instead - this of course goes against the principle of not
having host-specific config embedded in guest config.
This patch series enhances the functionality of <virtualport>
elements, to allow omitting some attributes (and even the type), and
to merge the interface, network, and portgroup virtualports rather
than simply picking one. This not only makes openvswitch <network>s
more practical (because the network can specify type='openvswitch'
without also specifying an interfaceid), but also makes <virtualport>
in networks and portgroups more useful in general - for example, an
interface can specify an interfaceid (used only by openvswitch) *and*
an instanceid (used only by 802.1Qbh), while the network's virtualport
specifies only the type, and the portgroups specify the managerid,
typeid, profileid, or whatever is appropriate for the type of switch
used by the network.
The result is that the guest config can be completely devoid of
knowledge about the type of switch being used on the hardware, but can
still enjoy full configurability for whatever switch ends up being
used.
If I understand correctly, then these patch series should enable
following configuration to work:
The domain XML:
...
<interface type='network'>
<mac address='52:54:00:25:0c:bb'/>
<source network='ovsnet'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</interface>
...
The network XML:
<network>
<name>ovsnet</name>
<uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
<bridge name='ovs'/>
<forward mode='bridge'/>
<mac address='52:54:00:44:A4:D8'/>
<virtualport type='openvswitch'/>
</network>
And then I would expect that libvirt would auto generate the
interface IDs for each interface that gets added to this ovsnet
network, but instead I am seeing the following error:
2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing interfaceid in
<virtualport type='openvswitch'>
I guess, it would be desirable to auto-generate interface
IDs, if the network was of type openvswitch? Otherwise
domain XML still needs to know what kind of type is
the underlying bridge in the network XML.
Though, how would this work in Actual Config, once the
network type gets switched back from OVS to Linux
Bridge? Would the interface ID be automatically removed
from all relevant Domain XMLs?
Thanks,
Ansis
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
These patches look good to me too.
4:18 p.m.
On 08/08/2012 03:43 PM, Ansis Atteka wrote:
On Sat, Aug 4, 2012 at 10:16 PM, Laine Stump <laine(a)laine.org
<mailto:laine@laine.org>> wrote:
Although it's been possible (ever since openvswitch was added to
libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
(by adding <virtualport type='openvswitch'>), the virtualport in the
network would always have a default random interfaceid included, which
would be re-used for all interfaces using that network, which doesn't
really work at all. The alternative was to not specify openvswitch in
the <network> definition, but to do it in the guest's <interface>
definition instead - this of course goes against the principle of not
having host-specific config embedded in guest config.
This patch series enhances the functionality of <virtualport>
elements, to allow omitting some attributes (and even the type), and
to merge the interface, network, and portgroup virtualports rather
than simply picking one. This not only makes openvswitch <network>s
more practical (because the network can specify type='openvswitch'
without also specifying an interfaceid), but also makes <virtualport>
in networks and portgroups more useful in general - for example, an
interface can specify an interfaceid (used only by openvswitch) *and*
an instanceid (used only by 802.1Qbh), while the network's virtualport
specifies only the type, and the portgroups specify the managerid,
typeid, profileid, or whatever is appropriate for the type of switch
used by the network.
The result is that the guest config can be completely devoid of
knowledge about the type of switch being used on the hardware, but can
still enjoy full configurability for whatever switch ends up being
used.
If I understand correctly, then these patch series should enable
following configuration to work:
The domain XML:
...
<interface type='network'>
<mac address='52:54:00:25:0c:bb'/>
<source network='ovsnet'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</interface>
...
The network XML:
<network>
<name>ovsnet</name>
<uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
<bridge name='ovs'/>
<forward mode='bridge'/>
<mac address='52:54:00:44:A4:D8'/>
<virtualport type='openvswitch'/>
</network>
And then I would expect that libvirt would auto generate the
interface IDs for each interface that gets added to this ovsnet
network,
That was (at some point anyway) my intent - if the interface has an
interface id use it, if not then generate one, but...
but instead I am seeing the following error:
2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing
interfaceid in <virtualport type='openvswitch'>
Urg. In the end I forgot that part, so when it checks for completeness,
it fails. I'll make a patch to fix that.
Thanks for catching that bug.
(one issue about this - since it's not known until runtime that the
network is ovs, the interfaceid won't be generated until then, and at
that time it's not reasonable to update the interfaceid in the guest's
persistent config. So, if you're configured this way, the guest will get
a different new interfaceid every time it is restarted.)
I guess, it would be desirable to auto-generate interface
IDs, if the network was of type openvswitch? Otherwise
domain XML still needs to know what kind of type is
the underlying bridge in the network XML.
Agreed. It's okay if it *optionally* puts in that info (just in case the
network is ovs), but it shouldn't require it.
Though, how would this work in Actual Config, once the
network type gets switched back from OVS to Linux
Bridge? Would the interface ID be automatically removed
from all relevant Domain XMLs?
Kind of. More exactly, the interface ID would be ignored when not
relevant. Similarly, you could also have an instanceid (used by
802.1Qbg), and if the actual network type was ovs, the instanceid would
be ignored. The only thing you *can't* do is to specify <virtualport
type='xyz'> in the interface if the network is actually some other type.
7:06 p.m.
On Wed, Aug 8, 2012 at 2:18 PM, Laine Stump <laine(a)laine.org> wrote:
On 08/08/2012 03:43 PM, Ansis Atteka wrote:
On Sat, Aug 4, 2012 at 10:16 PM, Laine Stump <laine(a)laine.org> wrote:
> Although it's been possible (ever since openvswitch was added to
> libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
> (by adding <virtualport type='openvswitch'>), the virtualport in the
> network would always have a default random interfaceid included, which
> would be re-used for all interfaces using that network, which doesn't
> really work at all. The alternative was to not specify openvswitch in
> the <network> definition, but to do it in the guest's <interface>
> definition instead - this of course goes against the principle of not
> having host-specific config embedded in guest config.
>
> This patch series enhances the functionality of <virtualport>
> elements, to allow omitting some attributes (and even the type), and
> to merge the interface, network, and portgroup virtualports rather
> than simply picking one. This not only makes openvswitch <network>s
> more practical (because the network can specify type='openvswitch'
> without also specifying an interfaceid), but also makes <virtualport>
> in networks and portgroups more useful in general - for example, an
> interface can specify an interfaceid (used only by openvswitch) *and*
> an instanceid (used only by 802.1Qbh), while the network's virtualport
> specifies only the type, and the portgroups specify the managerid,
> typeid, profileid, or whatever is appropriate for the type of switch
> used by the network.
>
> The result is that the guest config can be completely devoid of
> knowledge about the type of switch being used on the hardware, but can
> still enjoy full configurability for whatever switch ends up being
> used.
>
If I understand correctly, then these patch series should enable
following configuration to work:
The domain XML:
...
<interface type='network'>
<mac address='52:54:00:25:0c:bb'/>
<source network='ovsnet'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</interface>
...
The network XML:
<network>
<name>ovsnet</name>
<uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
<bridge name='ovs'/>
<forward mode='bridge'/>
<mac address='52:54:00:44:A4:D8'/>
<virtualport type='openvswitch'/>
</network>
And then I would expect that libvirt would auto generate the
interface IDs for each interface that gets added to this ovsnet
network,
That was (at some point anyway) my intent - if the interface has an
interface id use it, if not then generate one, but...
but instead I am seeing the following error:
2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing interfaceid in
<virtualport type='openvswitch'>
Urg. In the end I forgot that part, so when it checks for completeness, it
fails. I'll make a patch to fix that.
Thanks for catching that bug.
(one issue about this - since it's not known until runtime that the
network is ovs, the interfaceid won't be generated until then, and at that
time it's not reasonable to update the interfaceid in the guest's
persistent config. So, if you're configured this way, the guest will get a
different new interfaceid every time it is restarted.)
That will not work well from the OpenFlow Controller
perspective.
Interface ID must not change across guest restarts,
otherwise OpenFlow Controller will loose the track
on which interface was which.
I guess, it would be desirable to auto-generate interface
IDs, if the network was of type openvswitch? Otherwise
domain XML still needs to know what kind of type is
the underlying bridge in the network XML.
Agreed. It's okay if it *optionally* puts in that info (just in case the
network is ovs), but it shouldn't require it.
Though, how would this work in Actual Config, once the
network type gets switched back from OVS to Linux
Bridge? Would the interface ID be automatically removed
from all relevant Domain XMLs?
Kind of. More exactly, the interface ID would be ignored when not
relevant. Similarly, you could also have an instanceid (used by 802.1Qbg),
and if the actual network type was ovs, the instanceid would be ignored.
The only thing you *can't* do is to specify <virtualport type='xyz'> in
the
interface if the network is actually some other type.
8:35 p.m.
New subject: [libvirt] [PATCH 0/9] network: properly support openvswitch in <network>
On Aug 8, 2012, at 7:06 PM, Ansis Atteka wrote:
On Wed, Aug 8, 2012 at 2:18 PM, Laine Stump <laine(a)laine.org> wrote:
On 08/08/2012 03:43 PM, Ansis Atteka wrote:
>
>
> On Sat, Aug 4, 2012 at 10:16 PM, Laine Stump <laine(a)laine.org> wrote:
> Although it's been possible (ever since openvswitch was added to
> libvirt in 0.9.11) for a libvirt network to use an openvswitch bridge
> (by adding <virtualport type='openvswitch'>), the virtualport in the
> network would always have a default random interfaceid included, which
> would be re-used for all interfaces using that network, which doesn't
> really work at all. The alternative was to not specify openvswitch in
> the <network> definition, but to do it in the guest's <interface>
> definition instead - this of course goes against the principle of not
> having host-specific config embedded in guest config.
>
> This patch series enhances the functionality of <virtualport>
> elements, to allow omitting some attributes (and even the type), and
> to merge the interface, network, and portgroup virtualports rather
> than simply picking one. This not only makes openvswitch <network>s
> more practical (because the network can specify type='openvswitch'
> without also specifying an interfaceid), but also makes <virtualport>
> in networks and portgroups more useful in general - for example, an
> interface can specify an interfaceid (used only by openvswitch) *and*
> an instanceid (used only by 802.1Qbh), while the network's virtualport
> specifies only the type, and the portgroups specify the managerid,
> typeid, profileid, or whatever is appropriate for the type of switch
> used by the network.
>
> The result is that the guest config can be completely devoid of
> knowledge about the type of switch being used on the hardware, but can
> still enjoy full configurability for whatever switch ends up being
> used.
> If I understand correctly, then these patch series should enable
> following configuration to work:
>
> The domain XML:
> ...
> <interface type='network'>
> <mac address='52:54:00:25:0c:bb'/>
> <source network='ovsnet'/>
> <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
> </interface>
> ...
>
> The network XML:
> <network>
> <name>ovsnet</name>
> <uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
> <bridge name='ovs'/>
> <forward mode='bridge'/>
> <mac address='52:54:00:44:A4:D8'/>
> <virtualport type='openvswitch'/>
> </network>
>
>
> And then I would expect that libvirt would auto generate the
> interface IDs for each interface that gets added to this ovsnet
> network,
That was (at some point anyway) my intent - if the interface has an interface id use it,
if not then generate one, but...
> but instead I am seeing the following error:
>
> 2012-08-08 19:22:16.149+0000: 10840: error : virNetDevVPortProfileCheckComplete:165 :
XML error: missing interfaceid in <virtualport type='openvswitch'>
Urg. In the end I forgot that part, so when it checks for completeness, it fails.
I'll make a patch to fix that.
Thanks for catching that bug.
(one issue about this - since it's not known until runtime that the network is ovs,
the interfaceid won't be generated until then, and at that time it's not
reasonable to update the interfaceid in the guest's persistent config. So, if
you're configured this way, the guest will get a different new interfaceid every time
it is restarted.)
That will not work well from the OpenFlow Controller
perspective.
Interface ID must not change across guest restarts,
otherwise OpenFlow Controller will loose the track
on which interface was which.
I agree with Ansis here. If this UUID changes each time the VM is started,
it's effectively useless for something external to libvirt/OVS to utilize to recognize
the interface. It needs to remain the same for the life of the interface on the VM.
>
> I guess, it would be desirable to auto-generate interface
> IDs, if the network was of type openvswitch? Otherwise
> domain XML still needs to know what kind of type is
> the underlying bridge in the network XML.
Agreed. It's okay if it *optionally* puts in that info (just in case the network is
ovs), but it shouldn't require it.
>
> Though, how would this work in Actual Config, once the
> network type gets switched back from OVS to Linux
> Bridge? Would the interface ID be automatically removed
> from all relevant Domain XMLs?
Kind of. More exactly, the interface ID would be ignored when not relevant. Similarly,
you could also have an instanceid (used by 802.1Qbg), and if the actual network type was
ovs, the instanceid would be ignored. The only thing you *can't* do is to specify
<virtualport type='xyz'> in the interface if the network is actually some
other type.
4:28 p.m.
On 08/08/2012 09:35 PM, Kyle Mestery (kmestery) wrote:
On Aug 8, 2012, at 7:06 PM, Ansis Atteka wrote:
> On Wed, Aug 8, 2012 at 2:18 PM, Laine Stump <laine(a)laine.org> wrote:
> On 08/08/2012 03:43 PM, Ansis Atteka wrote:
>> If I understand correctly, then these patch series should enable
>> following configuration to work:
>>
>> The domain XML:
>> ...
>> <interface type='network'>
>> <mac address='52:54:00:25:0c:bb'/>
>> <source network='ovsnet'/>
>> <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
>> </interface>
>> ...
>>
>> The network XML:
>> <network>
>> <name>ovsnet</name>
>> <uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
>> <bridge name='ovs'/>
>> <forward mode='bridge'/>
>> <mac address='52:54:00:44:A4:D8'/>
>> <virtualport type='openvswitch'/>
>> </network>
>>
>>
>> And then I would expect that libvirt would auto generate the
>> interface IDs for each interface that gets added to this ovsnet
>> network,
> That was (at some point anyway) my intent - if the interface has an interface id use
it, if not then generate one, but...
>
>
>> but instead I am seeing the following error:
>>
>> 2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing interfaceid in <virtualport
type='openvswitch'>
> Urg. In the end I forgot that part, so when it checks for completeness, it fails.
I'll make a patch to fix that.
>
> Thanks for catching that bug.
>
> (one issue about this - since it's not known until runtime that the network is
ovs, the interfaceid won't be generated until then, and at that time it's not
reasonable to update the interfaceid in the guest's persistent config. So, if
you're configured this way, the guest will get a different new interfaceid every time
it is restarted.)
> That will not work well from the OpenFlow Controller
> perspective.
>
> Interface ID must not change across guest restarts,
> otherwise OpenFlow Controller will loose the track
> on which interface was which.
>
I agree with Ansis here. If this UUID changes each time the VM is started, it's
effectively useless for something external to libvirt/OVS to utilize to recognize the
interface. It needs to remain the same for the life of the interface on the VM.
The problem is that there is currently no method in libvirt to feed
config changes from the network driver back out to the guest's config.
At least until someone can come up with an elegant way to do this, if
someone wants the interfaceid to remain constant across
reboots/migrations of the guest, it must be present in the guest's
interface config from the beginning.
The current setup provides a way to do that - if the guest <interface>
contains an empty <virtualport/> element, that will automatically have
both an interfaceid (openvswitch) and an instanceid (802.1Qbg/VEPA)
generated for it. Since the virtualport has no type, it's not forcing
the guest to use a particular type of connection, but if the connection
happens to be openvswitch, the interfaceid that's been generated will be
used.
Note that if we *do* figure out how to channel a
network-driver-generated interfaceid back to the guest config, the
current patches will be compatible with that, so that shouldn't
necessarily stop us from getting these patches in.
(I'm starting to wonder if every interface should just always have a
unique uuid associated with it, no matter what else it's configured for.
So far there have been two places where it could have been used
(interfaceid and instanceid), and it's possible more will come up in the
future. If we were to add that, what would we then do with the
interfaceid and instanceid, though? We must continue to support them
because libvirt guarantees 100% backward compatibility.)
1:39 p.m.
On Mon, Aug 13, 2012 at 2:28 PM, Laine Stump <laine(a)laine.org> wrote:
On 08/08/2012 09:35 PM, Kyle Mestery (kmestery) wrote:
> On Aug 8, 2012, at 7:06 PM, Ansis Atteka wrote:
>> On Wed, Aug 8, 2012 at 2:18 PM, Laine Stump <laine(a)laine.org> wrote:
>> On 08/08/2012 03:43 PM, Ansis Atteka wrote:
>>> If I understand correctly, then these patch series should enable
>>> following configuration to work:
>>>
>>> The domain XML:
>>> ...
>>> <interface type='network'>
>>> <mac address='52:54:00:25:0c:bb'/>
>>> <source network='ovsnet'/>
>>> <address type='pci' domain='0x0000'
bus='0x00' slot='0x03'
function='0x0'/>
>>> </interface>
>>> ...
>>>
>>> The network XML:
>>> <network>
>>> <name>ovsnet</name>
>>> <uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
>>> <bridge name='ovs'/>
>>> <forward mode='bridge'/>
>>> <mac address='52:54:00:44:A4:D8'/>
>>> <virtualport type='openvswitch'/>
>>> </network>
>>>
>>>
>>> And then I would expect that libvirt would auto generate the
>>> interface IDs for each interface that gets added to this ovsnet
>>> network,
>> That was (at some point anyway) my intent - if the interface has an
interface id use it, if not then generate one, but...
>>
>>
>>> but instead I am seeing the following error:
>>>
>>> 2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing interfaceid in
<virtualport type='openvswitch'>
>> Urg. In the end I forgot that part, so when it checks for completeness,
it fails. I'll make a patch to fix that.
>>
>> Thanks for catching that bug.
>>
>> (one issue about this - since it's not known until runtime that the
network is ovs, the interfaceid won't be generated until then, and at that
time it's not reasonable to update the interfaceid in the guest's
persistent config. So, if you're configured this way, the guest will get a
different new interfaceid every time it is restarted.)
>> That will not work well from the OpenFlow Controller
>> perspective.
>>
>> Interface ID must not change across guest restarts,
>> otherwise OpenFlow Controller will loose the track
>> on which interface was which.
>>
> I agree with Ansis here. If this UUID changes each time the VM is
started, it's effectively useless for something external to libvirt/OVS to
utilize to recognize the interface. It needs to remain the same for the
life of the interface on the VM.
The problem is that there is currently no method in libvirt to feed
config changes from the network driver back out to the guest's config.
At least until someone can come up with an elegant way to do this, if
someone wants the interfaceid to remain constant across
reboots/migrations of the guest, it must be present in the guest's
interface config from the beginning.
The current setup provides a way to do that - if the guest <interface>
contains an empty <virtualport/> element, that will
automatically have
both an interfaceid (openvswitch) and an instanceid (802.1Qbg/VEPA)
generated for it. Since the virtualport has no type, it's not forcing
the guest to use a particular type of connection, but if the connection
happens to be openvswitch, the interfaceid that's been generated will be
used.
Note that if we *do* figure out how to channel a
network-driver-generated interfaceid back to the guest config, the
current patches will be compatible with that, so that shouldn't
necessarily stop us from getting these patches in.
(I'm starting to wonder if every interface should just always have a
unique uuid associated with it, no matter what else it's configured for.
So far there have been two places where it could have been used
(interfaceid and instanceid), and it's possible more will come up in the
future. If we were to add that, what would we then do with the
interfaceid and instanceid, though? We must continue to support them
because libvirt guarantees 100% backward compatibility.)
What you are suggesting works completely fine with
standalone bridge (i.e. one that
does not use OpenFlow
Controller). But what I am more concerned is that,
if the bridge is managed by Controller and iface-id
will be regenerated each time a reboot or migration
happens, then we might receive bogus bug reports,
because external applications will incorrectly think
that this is a new interface.
Perhaps, if we can't generate persistent iface-id for
interfaces that are attached to OVS networks, then
we should rather prevent libvirt from generating one at
all for now?
Anyway, I will leave this call up to you, because, I
guess, OpenStack and others specify iface-id explicitly
in the Domain XML file, and that will still work.
I will proceed with reviewing your new patches.
Thanks,
Ansis
1:56 p.m.
On 08/14/2012 02:39 PM, Ansis Atteka wrote:
On Mon, Aug 13, 2012 at 2:28 PM, Laine Stump <laine(a)laine.org
<mailto:laine@laine.org>> wrote:
On 08/08/2012 09:35 PM, Kyle Mestery (kmestery) wrote:
> On Aug 8, 2012, at 7:06 PM, Ansis Atteka wrote:
>> On Wed, Aug 8, 2012 at 2:18 PM, Laine Stump <laine(a)laine.org
<mailto:laine@laine.org>> wrote:
>> On 08/08/2012 03:43 PM, Ansis Atteka wrote:
>>> If I understand correctly, then these patch series should enable
>>> following configuration to work:
>>>
>>> The domain XML:
>>> ...
>>> <interface type='network'>
>>> <mac address='52:54:00:25:0c:bb'/>
>>> <source network='ovsnet'/>
>>> <address type='pci' domain='0x0000'
bus='0x00'
slot='0x03' function='0x0'/>
>>> </interface>
>>> ...
>>>
>>> The network XML:
>>> <network>
>>> <name>ovsnet</name>
>>> <uuid>ad68ae68-ee26-5c65-8cff-e6c182ff3593</uuid>
>>> <bridge name='ovs'/>
>>> <forward mode='bridge'/>
>>> <mac address='52:54:00:44:A4:D8'/>
>>> <virtualport type='openvswitch'/>
>>> </network>
>>>
>>>
>>> And then I would expect that libvirt would auto generate the
>>> interface IDs for each interface that gets added to this ovsnet
>>> network,
>> That was (at some point anyway) my intent - if the interface
has an interface id use it, if not then generate one, but...
>>
>>
>>> but instead I am seeing the following error:
>>>
>>> 2012-08-08 19:22:16.149+0000: 10840: error :
virNetDevVPortProfileCheckComplete:165 : XML error: missing
interfaceid in <virtualport type='openvswitch'>
>> Urg. In the end I forgot that part, so when it checks for
completeness, it fails. I'll make a patch to fix that.
>>
>> Thanks for catching that bug.
>>
>> (one issue about this - since it's not known until runtime that
the network is ovs, the interfaceid won't be generated until then,
and at that time it's not reasonable to update the interfaceid in
the guest's persistent config. So, if you're configured this way,
the guest will get a different new interfaceid every time it is
restarted.)
>> That will not work well from the OpenFlow Controller
>> perspective.
>>
>> Interface ID must not change across guest restarts,
>> otherwise OpenFlow Controller will loose the track
>> on which interface was which.
>>
> I agree with Ansis here. If this UUID changes each time the VM
is started, it's effectively useless for something external to
libvirt/OVS to utilize to recognize the interface. It needs to
remain the same for the life of the interface on the VM.
The problem is that there is currently no method in libvirt to feed
config changes from the network driver back out to the guest's config.
At least until someone can come up with an elegant way to do this, if
someone wants the interfaceid to remain constant across
reboots/migrations of the guest, it must be present in the guest's
interface config from the beginning.
The current setup provides a way to do that - if the guest <interface>
contains an empty <virtualport/> element, that will automatically have
both an interfaceid (openvswitch) and an instanceid (802.1Qbg/VEPA)
generated for it. Since the virtualport has no type, it's not forcing
the guest to use a particular type of connection, but if the
connection
happens to be openvswitch, the interfaceid that's been generated
will be
used.
Note that if we *do* figure out how to channel a
network-driver-generated interfaceid back to the guest config, the
current patches will be compatible with that, so that shouldn't
necessarily stop us from getting these patches in.
(I'm starting to wonder if every interface should just always have a
unique uuid associated with it, no matter what else it's
configured for.
So far there have been two places where it could have been used
(interfaceid and instanceid), and it's possible more will come up
in the
future. If we were to add that, what would we then do with the
interfaceid and instanceid, though? We must continue to support them
because libvirt guarantees 100% backward compatibility.)
What you are suggesting works completely fine with
standalone bridge (i.e. one that does not use OpenFlow
Controller). But what I am more concerned is that,
if the bridge is managed by Controller and iface-id
will be regenerated each time a reboot or migration
happens, then we might receive bogus bug reports,
because external applications will incorrectly think
that this is a new interface.
So in the case that a site is using OpenFlow, they can just make sure
every guest has <virtualport/> in all their interfaces' XML.
Alternately, the more I think about the idea of each <interface> having
its own autogenerated <uuid> at the toplevel, the more I like it.
Another example of its usefulness - I've been considering how to allow
live updates to the <network> config (i.e. without requiring a
destroy/start of the network), and one of the problems is that, in the
cases where a physical interface can't be removed from the interface
pool due to a guest currently using it, the network driver has no unique
information available to provide to the user so they'll know *which*
guest is preventing their config change. If each guest interface (no
matter what type), the network driver could just snag the uuid and
attach it to the item in the physical interface pool list. Then when the
network driver later failed some operation due to a guest being
connected, it could include the uuid in the error message, and that uuid
could be cross-referenced with all the guest configs to find the
specific guest+interface causing the problem.
Anyway, if every guest interface has a uuid, then the ovs driver (or
network driver) could look first in the virtualport for a specific
interfaceid, and if it wasn't found, just use the uuid.
Perhaps, if we can't generate persistent iface-id for
interfaces that are attached to OVS networks, then
we should rather prevent libvirt from generating one at
all for now?
That was the behavior prior in the first revision of the patches.
Anyway, I will leave this call up to you, because, I
guess, OpenStack and others specify iface-id explicitly
in the Domain XML file, and that will still work.
Okay. I think we should leave it as is, and discuss adding a pervasive
uuid that's always present for every interface.
2:17 a.m.
I've fixed some bugs in these patches, rebased, and reposted:
http://www.redhat.com/archives/libvir-list/2012-August/msg00891.html
Consider this set obsolete.
4483
days inactive
4492
days old
23 comments
5 participants
participants (5)
-
Ansis Atteka -
Doug Goldstein -
Doug Goldstein -
Kyle Mestery (kmestery) -
Laine Stump