[libvirt] [PATCH 2/5] macvtap support for libvirt -- parse new interface XML
This part adds support to domain_conf.{c|h} for parsing the new interface XML of type 'direct'. Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
On Thu, Feb 04, 2010 at 08:02:42AM -0500, Stefan Berger wrote:
This part adds support to domain_conf.{c|h} for parsing the new interface XML of type 'direct'.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
Index: libvirt-macvtap/src/conf/domain_conf.c =================================================================== --- libvirt-macvtap.orig/src/conf/domain_conf.c +++ libvirt-macvtap/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "c-ctype.h" #include "logging.h" #include "network.h" +#include "macvtap.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -140,7 +141,8 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_N "mcast", "network", "bridge", - "internal") + "internal", + "direct")
VIR_ENUM_IMPL(virDomainChrTarget, VIR_DOMAIN_CHR_TARGET_TYPE_LAST, "null", @@ -222,6 +224,12 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOM "dynamic", "static")
+VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, + MACVTAP_MODE_PRIVATE_STR, + MACVTAP_MODE_VEPA_STR, + MACVTAP_MODE_BRIDGE_STR)
These strings should really be included here directly - other areas of the code should only ever see the parsed enum integer value, never the string form which is for the XML only.
+ + #define virDomainReportError(conn, code, fmt...) \ virReportErrorHelper(conn, VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, fmt) @@ -430,6 +438,11 @@ void virDomainNetDefFree(virDomainNetDef case VIR_DOMAIN_NET_TYPE_INTERNAL: VIR_FREE(def->data.internal.name); break; + + case VIR_DOMAIN_NET_TYPE_DIRECT: + VIR_FREE(def->data.direct.linkdev); + VIR_FREE(def->data.direct.mode); + break; }
VIR_FREE(def->ifname); @@ -1631,6 +1644,7 @@ virDomainNetDefParseXML(virConnectPtr co char *model = NULL; char *internal = NULL; char *devaddr = NULL; + char *mode = NULL;
if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); @@ -1667,9 +1681,11 @@ virDomainNetDefParseXML(virConnectPtr co (xmlStrEqual(cur->name, BAD_CAST "source"))) { bridge = virXMLPropString(cur, "bridge"); } else if ((dev == NULL) && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET) && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_DIRECT) && xmlStrEqual(cur->name, BAD_CAST "source")) { - dev = virXMLPropString(cur, "dev"); + dev = virXMLPropString(cur, "dev"); + mode = virXMLPropString(cur, "mode"); } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -1823,6 +1839,27 @@ virDomainNetDefParseXML(virConnectPtr co def->data.internal.name = internal; internal = NULL; break; + + case VIR_DOMAIN_NET_TYPE_DIRECT: + if (dev == NULL) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("No <source> 'dev' attribute specified with <interface type='direct'/>")); + goto error; + } + + if (mode != NULL) { + if (virDomainNetdevMacvtapTypeFromString(mode) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Unkown mode has been specified")); + goto error; + } + } + + def->data.direct.linkdev = dev; + dev = NULL; + def->data.direct.mode = mode; + mode = NULL; + break;
Instead of storing the string value, this should be using the result from virDomainNetdevMacvtapTypeFromString() call a few lines earlier & storing that.
Index: libvirt-macvtap/src/conf/domain_conf.h =================================================================== --- libvirt-macvtap.orig/src/conf/domain_conf.h +++ libvirt-macvtap/src/conf/domain_conf.h @@ -211,6 +211,7 @@ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE, VIR_DOMAIN_NET_TYPE_INTERNAL, + VIR_DOMAIN_NET_TYPE_DIRECT,
VIR_DOMAIN_NET_TYPE_LAST, }; @@ -244,6 +245,10 @@ struct _virDomainNetDef { struct { char *name; } internal; + struct { + char *linkdev; + char *mode; + } direct;
The 'mode' field should be an 'int' since it should hold the enum value
} data; char *ifname; virDomainDeviceInfo info; @@ -594,6 +599,15 @@ struct _virSecurityLabelDef { int type; };
+enum virDomainNetdevMacvtapType { + VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE, + VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA, + VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE, + + VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, +}; + + #define VIR_DOMAIN_CPUMASK_LEN 1024
/* Guest VM main configuration */ @@ -915,4 +929,6 @@ VIR_ENUM_DECL(virDomainGraphics) VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainSeclabel)
+VIR_ENUM_DECL(virDomainNetdevMacvtap) + #endif /* __DOMAIN_CONF_H */
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote on 02/08/2010 11:59:52 AM:
Please respond to "Daniel P. Berrange"
On Thu, Feb 04, 2010 at 08:02:42AM -0500, Stefan Berger wrote:
This part adds support to domain_conf.{c|h} for parsing the new interface XML of type 'direct'.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
Index: libvirt-macvtap/src/conf/domain_conf.c =================================================================== --- libvirt-macvtap.orig/src/conf/domain_conf.c +++ libvirt-macvtap/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "c-ctype.h" #include "logging.h" #include "network.h" +#include "macvtap.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -140,7 +141,8 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_N "mcast", "network", "bridge", - "internal") + "internal", + "direct")
VIR_ENUM_IMPL(virDomainChrTarget, VIR_DOMAIN_CHR_TARGET_TYPE_LAST, "null", @@ -222,6 +224,12 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOM "dynamic", "static")
+VIR_ENUM_IMPL(virDomainNetdevMacvtap,
VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
+ MACVTAP_MODE_PRIVATE_STR, + MACVTAP_MODE_VEPA_STR, + MACVTAP_MODE_BRIDGE_STR)
These strings should really be included here directly - other areas of the code should only ever see the parsed enum integer value, never the string form which is for the XML only.
Ok. The reason why I did not use the returned 'int's is because the translated value will be passed to the driver and linux/if_link.h unfortunately defines those as follows: enum macvlan_mode { MACVLAN_MODE_PRIVATE = 1, MACVLAN_MODE_VEPA = 2, MACVLAN_MODE_BRIDGE = 4, }; Should I use those values in the array and fill the 0 and 3 with dummy values or have another function that translates the ones returned by the XYZTypeFromString() call in the actual 'enum macvlan_mode'? Stefan
On Mon, Feb 08, 2010 at 12:33:02PM -0500, Stefan Berger wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote on 02/08/2010 11:59:52 AM:
Please respond to "Daniel P. Berrange"
On Thu, Feb 04, 2010 at 08:02:42AM -0500, Stefan Berger wrote:
This part adds support to domain_conf.{c|h} for parsing the new interface XML of type 'direct'.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
Index: libvirt-macvtap/src/conf/domain_conf.c =================================================================== --- libvirt-macvtap.orig/src/conf/domain_conf.c +++ libvirt-macvtap/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "c-ctype.h" #include "logging.h" #include "network.h" +#include "macvtap.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -140,7 +141,8 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_N "mcast", "network", "bridge", - "internal") + "internal", + "direct")
VIR_ENUM_IMPL(virDomainChrTarget, VIR_DOMAIN_CHR_TARGET_TYPE_LAST, "null", @@ -222,6 +224,12 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOM "dynamic", "static")
+VIR_ENUM_IMPL(virDomainNetdevMacvtap,
VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
+ MACVTAP_MODE_PRIVATE_STR, + MACVTAP_MODE_VEPA_STR, + MACVTAP_MODE_BRIDGE_STR)
These strings should really be included here directly - other areas of the code should only ever see the parsed enum integer value, never the string form which is for the XML only.
Ok. The reason why I did not use the returned 'int's is because the translated value will be passed to the driver and linux/if_link.h unfortunately defines those as follows:
enum macvlan_mode { MACVLAN_MODE_PRIVATE = 1, MACVLAN_MODE_VEPA = 2, MACVLAN_MODE_BRIDGE = 4, };
Should I use those values in the array and fill the 0 and 3 with dummy values or have another function that translates the ones returned by the XYZTypeFromString() call in the actual 'enum macvlan_mode'?
It is best to avoid tieing libvirt's internal parsed enum value, directly to the kernel's values. So yes, just map between the two values when needed. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange -
Stefan Berger