[libvirt] [PATCH v2] Attach vm-id to Open vSwitch interfaces.
This patch will allow OpenFlow controllers to identify which interface belongs to a particular VM by using the Domain UUID. ovs-vsctl get Interface vnet0 external_ids {attached-mac="52:54:00:8C:55:2C", iface-id="83ce45d6-3639-096e-ab3c-21f66a05f7fa", iface-status=active, vm-id="142a90a7-0acc-ab92-511c-586f12da8851"} V2 changes: Replaced vm-uuid with vm-id. There was a discussion in Open vSwitch mailinglist that we should stick with the same DB key postfixes for the sake of consistency (e.g iface-id, vm-id ...). --- src/lxc/lxc_driver.c | 3 ++- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 3 ++- src/uml/uml_conf.c | 3 ++- src/util/virnetdevopenvswitch.c | 17 ++++++++++++++--- src/util/virnetdevopenvswitch.h | 1 + src/util/virnetdevtap.c | 3 ++- src/util/virnetdevtap.h | 1 + 8 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d9cbd9e..3a55983 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1197,7 +1197,8 @@ static int lxcSetupInterfaceBridged(virConnectPtr conn, goto cleanup; if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ret = virNetDevOpenvswitchAddPort(brname, parentVeth, net->mac, vport); + ret = virNetDevOpenvswitchAddPort(brname, parentVeth, net->mac, + vm->uuid, vport); else ret = virNetDevBridgeAddPort(brname, parentVeth); if (ret < 0) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index cf75d26..d82212f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1766,7 +1766,7 @@ networkStartNetworkVirtual(struct network_driver *driver, } if (virNetDevTapCreateInBridgePort(network->def->bridge, &macTapIfName, network->def->mac, - NULL, NULL, + NULL, NULL, NULL, VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) { VIR_FREE(macTapIfName); goto err0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 867c460..87eebce 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -244,7 +244,8 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; } - err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, &tapfd, + err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, + def->uuid, &tapfd, virDomainNetGetActualVirtPortProfile(net), tap_create_flags); virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0); diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 397d332..eae67f9 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -138,7 +138,8 @@ umlConnectTapDevice(virConnectPtr conn, template_ifname = true; } - if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, NULL, + if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, + vm->uuid, NULL, virDomainNetGetActualVirtPortProfile(net), VIR_NETDEV_TAP_CREATE_IFUP) < 0) { if (template_ifname) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e427c94..1a61345 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -36,6 +36,7 @@ * @brname: the bridge name * @ifname: the network interface name * @macaddr: the mac address of the virtual interface + * @vmuuid: the Domain UUID that has this interface * @ovsport: the ovs specific fields * * Add an interface to the OVS bridge @@ -44,24 +45,31 @@ */ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, const unsigned char *macaddr, + const unsigned char *vmuuid, virNetDevVPortProfilePtr ovsport) { int ret = -1; virCommandPtr cmd = NULL; char macaddrstr[VIR_MAC_STRING_BUFLEN]; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + char ifuuidstr[VIR_UUID_STRING_BUFLEN]; + char vmuuidstr[VIR_UUID_STRING_BUFLEN]; char *attachedmac_ex_id = NULL; char *ifaceid_ex_id = NULL; char *profile_ex_id = NULL; + char *vmid_ex_id = NULL; virMacAddrFormat(macaddr, macaddrstr); - virUUIDFormat(ovsport->u.openvswitch.interfaceID, uuidstr); + virUUIDFormat(ovsport->u.openvswitch.interfaceID, ifuuidstr); + virUUIDFormat(vmuuid, vmuuidstr); if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0) goto cleanup; if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", - uuidstr) < 0) + ifuuidstr) < 0) + goto cleanup; + if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", + vmuuidstr) < 0) goto cleanup; if (ovsport->u.openvswitch.profileID[0] != '\0') { if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", @@ -75,6 +83,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, brname, ifname, "--", "set", "Interface", ifname, attachedmac_ex_id, "--", "set", "Interface", ifname, ifaceid_ex_id, + "--", "set", "Interface", ifname, vmid_ex_id, "--", "set", "Interface", ifname, "external-ids:iface-status=active", NULL); @@ -83,6 +92,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, brname, ifname, "--", "set", "Interface", ifname, attachedmac_ex_id, "--", "set", "Interface", ifname, ifaceid_ex_id, + "--", "set", "Interface", ifname, vmid_ex_id, "--", "set", "Interface", ifname, profile_ex_id, "--", "set", "Interface", ifname, "external-ids:iface-status=active", @@ -100,6 +110,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cleanup: VIR_FREE(attachedmac_ex_id); VIR_FREE(ifaceid_ex_id); + VIR_FREE(vmid_ex_id); VIR_FREE(profile_ex_id); virCommandFree(cmd); return ret; diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index bca4c3e..8141780 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -32,6 +32,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, const unsigned char *macaddr, + const unsigned char *vmuuid, virNetDevVPortProfilePtr ovsport) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index fb0a8d2..84a8a1c 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -277,6 +277,7 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, + const unsigned char *vmuuid, int *tapfd, virNetDevVPortProfilePtr virtPortProfile, unsigned int flags) @@ -307,7 +308,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, goto error; if (virtPortProfile) { - if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, + if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, vmuuid, virtPortProfile) < 0) { goto error; } diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 971b166..d9a3593 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -47,6 +47,7 @@ typedef enum { int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, + const unsigned char *vmuuid, int *tapfd, virNetDevVPortProfilePtr virtPortProfile, unsigned int flags) -- 1.7.9
On 03/07/2012 02:15 AM, Ansis Atteka wrote:
This patch will allow OpenFlow controllers to identify which interface belongs to a particular VM by using the Domain UUID.
ovs-vsctl get Interface vnet0 external_ids {attached-mac="52:54:00:8C:55:2C", iface-id="83ce45d6-3639-096e-ab3c-21f66a05f7fa", iface-status=active, vm-id="142a90a7-0acc-ab92-511c-586f12da8851"}
V2 changes: Replaced vm-uuid with vm-id. There was a discussion in Open vSwitch mailinglist that we should stick with the same DB key postfixes for the sake of consistency (e.g iface-id, vm-id ...).
This all looks good, and simply adding the vmuuid argument to the callchain that goes down to virNetDevOpenvswitchAddPort seems like the simplest, most consistent way to get the information down to that function. ACK, and pushed.
--- src/lxc/lxc_driver.c | 3 ++- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 3 ++- src/uml/uml_conf.c | 3 ++- src/util/virnetdevopenvswitch.c | 17 ++++++++++++++--- src/util/virnetdevopenvswitch.h | 1 + src/util/virnetdevtap.c | 3 ++- src/util/virnetdevtap.h | 1 + 8 files changed, 25 insertions(+), 8 deletions(-)
"external-ids:iface-status=active", @@ -100,6 +110,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cleanup: VIR_FREE(attachedmac_ex_id); VIR_FREE(ifaceid_ex_id); + VIR_FREE(vmid_ex_id); VIR_FREE(profile_ex_id); virCommandFree(cmd); return ret;
Hmm. I just now noticed the odd indentation here. I'll take care of that in a separate small patch. I'm also sending a patch to call virReportOOMError() when the virAsprintf() calls in that function fail.
The indentation on the final lines of the function was off by four spaces, making me wonder for a second if there was something missing. (There wasn't.) --- Pushing under trivial rule. src/util/virnetdevopenvswitch.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e427c94..9d6d924 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -95,14 +95,14 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, ifname, brname); goto cleanup; } - ret = 0; - cleanup: - VIR_FREE(attachedmac_ex_id); - VIR_FREE(ifaceid_ex_id); - VIR_FREE(profile_ex_id); - virCommandFree(cmd); - return ret; + ret = 0; +cleanup: + VIR_FREE(attachedmac_ex_id); + VIR_FREE(ifaceid_ex_id); + VIR_FREE(profile_ex_id); + virCommandFree(cmd); + return ret; } /** -- 1.7.7.6
OOM conditions silently returned failure. --- src/util/virnetdevopenvswitch.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e2d5124..61bb9e1 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -64,17 +64,17 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0) - goto cleanup; + goto out_of_memory; if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", ifuuidstr) < 0) - goto cleanup; + goto out_of_memory; if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", vmuuidstr) < 0) - goto cleanup; + goto out_of_memory; if (ovsport->u.openvswitch.profileID[0] != '\0') { if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", ovsport->u.openvswitch.profileID) < 0) - goto cleanup; + goto out_of_memory; } cmd = virCommandNew(OVSVSCTL); @@ -114,6 +114,10 @@ cleanup: VIR_FREE(profile_ex_id); virCommandFree(cmd); return ret; + +out_of_memory: + virReportOOMError(); + goto cleanup; } /** -- 1.7.7.6
On 03/08/2012 11:58 AM, Laine Stump wrote:
OOM conditions silently returned failure. --- src/util/virnetdevopenvswitch.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e2d5124..61bb9e1 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -64,17 +64,17 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0) - goto cleanup; + goto out_of_memory;
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/08/2012 02:00 PM, Eric Blake wrote:
On 03/08/2012 11:58 AM, Laine Stump wrote:
OOM conditions silently returned failure. --- src/util/virnetdevopenvswitch.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e2d5124..61bb9e1 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -64,17 +64,17 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0) - goto cleanup; + goto out_of_memory; ACK.
Pushed. Thanks!
participants (3)
-
Ansis Atteka -
Eric Blake -
Laine Stump