Midonet is an opensource virtual networking that over lays the IP network between hypervisors. Currently, such networks can be made with the openvswitch virtualport type. This patch, defines the schema and documentation that will serve as basis for the follow up patches that will add support to libvirt for using Midonet virtual ports for its interfaces. Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- docs/formatdomain.html.in | 34 ++++++++++++++++++++++++++++++++++ docs/schemas/networkcommon.rng | 12 ++++++++++++ 2 files changed, 46 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f6477c2..1c2bb45 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3652,6 +3652,40 @@ </devices> ...</pre> + <p> + On hosts that support Open vSwitch on the kernel side and that have the + Midonet Host Agent configured, it is also possible to connect to the + 'midonet' bridge device by adding a + <code><virtualport type='midonet'/></code> to the + interface definition. (<span class="since">Since + 1.2.13</span>). The Midonet virtualport type requires an + <code>interfaceid</code> attribute to its + <code><parameters></code> element. This interface id is the UUID + that specifies which port in the virtual network topology will be bound + to the interface. + </p> +<pre> + ... + <devices> + ... + <interface type='bridge'> + <source bridge='br0'/> + </interface> + <interface type='bridge'> + <source bridge='br1'/> + <target dev='vnet7'/> + <mac address="00:11:22:33:44:55"/> + </interface> + <interface type='bridge'> + <source bridge='midonet'/> + <virtualport type='midonet'> + <parameters interfaceid='0b2d64da-3d0e-431e-afdd-804415d6ebbb'/> + </virtualport> + </interface> + ... + </devices> + ...</pre> + <h5><a name="elementsNICSSlirp">Userspace SLIRP stack</a></h5> <p> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 162ea3d..cc8b1dc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -79,6 +79,18 @@ </element> </group> <group> + <element name="virtualport"> + <attribute name="type"> + <value>midonet</value> + </attribute> + <element name="parameters"> + <attribute name="interfaceid"> + <ref name="UUID"/> + </attribute> + </element> + </element> + </group> + <group> <!-- use this when no type attribute is present --> <element name="virtualport"> <optional> -- 2.3.0

On Thu, Feb 19, 2015 at 4:28 PM, Laine Stump <laine@laine.org> wrote:

On 02/17/2015 09:42 PM, Antoni Segura Puimedon wrote:

...

Midonet is an opensource virtual networking that over lays the IP network between hypervisors. Currently, such networks can be made with the openvswitch virtualport type.

This patch, defines the schema and documentation that will serve as basis for the follow up patches that will add support to libvirt for using Midonet virtual ports for its interfaces.

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- docs/formatdomain.html.in | 34 ++++++++++++++++++++++++++++++++++ docs/schemas/networkcommon.rng | 12 ++++++++++++ 2 files changed, 46 insertions(+)

This patch should also have the parser/formatter changes (which implies that it needs to be patch 2, with your current patch 2 placed first in the series - the parser/formatter needs the changes to the enum). It should also contain at least one new test case for qemuxml2xmltest (entry added to tests/qemuxml2xmltest.c, and data files added to tests/qemuxml2argvdata + tests/qemuxml2xmloutdata).

Agreed :-)

...

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f6477c2..1c2bb45 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3652,6 +3652,40 @@ </devices> ...</pre>

+ <p> + On hosts that support Open vSwitch on the kernel side and that

have the

...

+ Midonet Host Agent configured,

Oh, so midonet is something on top of OVS? Or a variation on it? Before we commit to this new type I want to make sure that it isn't better handled by keeping it as type='openvswitch' with some other modifier (probably not, but just want to be certain it's considered).

It just needs kernel support for OVS as it uses it's kernel module. That's the only thing it uses from OVS, as all the userspace is completely different. Thus it must really be a separate type, since it can't be operated in any similar way. Thanks for the review!

...

it is also possible to connect to the + 'midonet' bridge device by adding a + <code><virtualport type='midonet'/></code> to the + interface definition. (<span class="since">Since + 1.2.13</span>). The Midonet virtualport type requires an + <code>interfaceid</code> attribute to its + <code><parameters></code> element. This interface id is the UUID + that specifies which port in the virtual network topology will be bound + to the interface. + </p> +<pre> + ... + <devices> + ... + <interface type='bridge'> + <source bridge='br0'/> + </interface> + <interface type='bridge'> + <source bridge='br1'/> + <target dev='vnet7'/> + <mac address="00:11:22:33:44:55"/> + </interface> + <interface type='bridge'> + <source bridge='midonet'/> + <virtualport type='midonet'> + <parameters interfaceid='0b2d64da-3d0e-431e-afdd-804415d6ebbb'/> + </virtualport> + </interface> + ... + </devices> + ...</pre> + <h5><a name="elementsNICSSlirp">Userspace SLIRP stack</a></h5>

<p> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 162ea3d..cc8b1dc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -79,6 +79,18 @@ </element> </group> <group> + <element name="virtualport"> + <attribute name="type"> + <value>midonet</value> + </attribute> + <element name="parameters"> + <attribute name="interfaceid"> + <ref name="UUID"/> + </attribute> + </element> + </element> + </group> + <group> <!-- use this when no type attribute is present --> <element name="virtualport"> <optional>

Adds the port type definitions and methods that will be used to bind interfaces to the Midonet virtual ports. virtnetdevmidonet.c adds the way to bind and unbind the ports by calling into the Midonet Host Agent control command line (installed with the midolman package). Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- configure.ac | 4 ++ po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virnetdevmidonet.c | 97 ++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevmidonet.h | 37 +++++++++++++++ src/util/virnetdevvportprofile.c | 3 +- src/util/virnetdevvportprofile.h | 3 +- 8 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 src/util/virnetdevmidonet.c create mode 100644 src/util/virnetdevmidonet.h diff --git a/configure.ac b/configure.ac index b3e99e7..ddffbb2 100644 --- a/configure.ac +++ b/configure.ac @@ -425,6 +425,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], @@ -440,6 +442,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc program (see iproute2)]) +AC_DEFINE_UNQUOTED([MMCTL],["$MMCTL"], + [Location or name of the mm-ctl program]) AC_DEFINE_UNQUOTED([OVSVSCTL],["$OVSVSCTL"], [Location or name of the ovs-vsctl program]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3064037..cdfc839 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -195,6 +195,7 @@ src/util/virnetdev.c src/util/virnetdevbandwidth.c src/util/virnetdevbridge.c src/util/virnetdevmacvlan.c +src/util/virnetdevmidonet.c src/util/virnetdevopenvswitch.c src/util/virnetdevtap.c src/util/virnetdevveth.c diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..23d3f93 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -129,6 +129,7 @@ UTIL_SOURCES = \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ util/virnetdevmacvlan.c util/virnetdevmacvlan.h \ + util/virnetdevmidonet.h util/virnetdevmidonet.c \ util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \ util/virnetdevtap.h util/virnetdevtap.c \ util/virnetdevveth.h util/virnetdevveth.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 46a1613..0938cdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1738,6 +1738,11 @@ virNetDevMacVLanRestartWithVPortProfile; virNetDevMacVLanVPortProfileRegisterCallback; +# util/virnetdevmidonet.h +virNetDevMidonetBindPort; +virNetDevMidonetUnbindPort; + + # util/virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c new file mode 100644 index 0000000..57fb636 --- /dev/null +++ b/src/util/virnetdevmidonet.c @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2015 Midokura, Sarl. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#include <config.h> + +#include "virnetdevmidonet.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virerror.h" +#include "viruuid.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/** + * virNetDevMidonetBindPort: + * @ifname: the network interface name + * @virtualport: the midonet specific fields + * + * Bind an interface to a Midonet virtual port + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + + virCommandAddArgList(cmd, "--bind-port", virtportuuid, ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to bind port %s to the virtual port %s"), + ifname, virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + +/** + * virNetDevMidonetUnbindPort: + * @virtualport: the midonet specific fields + * + * Unbinds a virtual port from the host + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + virCommandAddArgList(cmd, "--unbind-port", virtportuuid, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to unbind the virtual port %s from Midonet"), + virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/util/virnetdevmidonet.h b/src/util/virnetdevmidonet.h new file mode 100644 index 0000000..6724d34 --- /dev/null +++ b/src/util/virnetdevmidonet.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 Midokura Sarl. + + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#ifndef __VIR_NETDEV_MIDONET_H__ +# define __VIR_NETDEV_MIDONET_H__ + +# include "internal.h" +# include "virnetdevvportprofile.h" + + +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +#endif /* __VIR_NETDEV_MIDONET_H__ */ + diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6ee20d3..09acb52 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -33,7 +33,8 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, "none", "802.1Qbg", "802.1Qbh", - "openvswitch") + "openvswitch", + "midonet") VIR_ENUM_IMPL(virNetDevVPortProfileOp, VIR_NETDEV_VPORT_PROFILE_OP_LAST, "create", diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index ad063c5..dc3e643 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -35,6 +35,7 @@ enum virNetDevVPortProfile { VIR_NETDEV_VPORT_PROFILE_8021QBG, VIR_NETDEV_VPORT_PROFILE_8021QBH, VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH, + VIR_NETDEV_VPORT_PROFILE_MIDONET, VIR_NETDEV_VPORT_PROFILE_LAST, }; @@ -73,7 +74,7 @@ struct _virNetDevVPortProfile { /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; - /* this member is used when virtPortType == openvswitch */ + /* this member is used when virtPortType == openvswitch|midonet */ unsigned char interfaceID[VIR_UUID_BUFLEN]; bool interfaceID_specified; /* NB - if virtPortType == NONE, any/all of the items could be used */ -- 2.3.0

On 02/23/2015 03:54 PM, Antoni Segura Puimedon wrote:

Adds the port type definitions and methods that will be used to bind interfaces to the Midonet virtual ports.

virtnetdevmidonet.c adds the way to bind and unbind the ports by calling into the Midonet Host Agent control command line (installed with the midolman package).

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- configure.ac | 4 ++ po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virnetdevmidonet.c | 97 ++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevmidonet.h | 37 +++++++++++++++ src/util/virnetdevvportprofile.c | 3 +- src/util/virnetdevvportprofile.h | 3 +- 8 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 src/util/virnetdevmidonet.c create mode 100644 src/util/virnetdevmidonet.h

ACK with a couple very minor formatting changes.

diff --git a/configure.ac b/configure.ac index b3e99e7..ddffbb2 100644 --- a/configure.ac +++ b/configure.ac @@ -425,6 +425,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], @@ -440,6 +442,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc program (see iproute2)]) +AC_DEFINE_UNQUOTED([MMCTL],["$MMCTL"], + [Location or name of the mm-ctl program]) AC_DEFINE_UNQUOTED([OVSVSCTL],["$OVSVSCTL"], [Location or name of the ovs-vsctl program])

diff --git a/po/POTFILES.in b/po/POTFILES.in index 3064037..cdfc839 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -195,6 +195,7 @@ src/util/virnetdev.c src/util/virnetdevbandwidth.c src/util/virnetdevbridge.c src/util/virnetdevmacvlan.c +src/util/virnetdevmidonet.c src/util/virnetdevopenvswitch.c src/util/virnetdevtap.c src/util/virnetdevveth.c diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..23d3f93 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -129,6 +129,7 @@ UTIL_SOURCES = \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ util/virnetdevmacvlan.c util/virnetdevmacvlan.h \ + util/virnetdevmidonet.h util/virnetdevmidonet.c \ util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \ util/virnetdevtap.h util/virnetdevtap.c \ util/virnetdevveth.h util/virnetdevveth.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 46a1613..0938cdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1738,6 +1738,11 @@ virNetDevMacVLanRestartWithVPortProfile; virNetDevMacVLanVPortProfileRegisterCallback;

+# util/virnetdevmidonet.h +virNetDevMidonetBindPort; +virNetDevMidonetUnbindPort; + + # util/virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c new file mode 100644 index 0000000..57fb636 --- /dev/null +++ b/src/util/virnetdevmidonet.c @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2015 Midokura, Sarl. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#include <config.h> + +#include "virnetdevmidonet.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virerror.h" +#include "viruuid.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/** + * virNetDevMidonetBindPort: + * @ifname: the network interface name + * @virtualport: the midonet specific fields + * + * Bind an interface to a Midonet virtual port + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport)

^^ here (and in the next function), put return type on separate line and realigned args.

+{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + + virCommandAddArgList(cmd, "--bind-port", virtportuuid, ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to bind port %s to the virtual port %s"), + ifname, virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + +/** + * virNetDevMidonetUnbindPort: + * @virtualport: the midonet specific fields + * + * Unbinds a virtual port from the host + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + virCommandAddArgList(cmd, "--unbind-port", virtportuuid, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to unbind the virtual port %s from Midonet"), + virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/util/virnetdevmidonet.h b/src/util/virnetdevmidonet.h new file mode 100644 index 0000000..6724d34 --- /dev/null +++ b/src/util/virnetdevmidonet.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 Midokura Sarl. + + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#ifndef __VIR_NETDEV_MIDONET_H__ +# define __VIR_NETDEV_MIDONET_H__ + +# include "internal.h" +# include "virnetdevvportprofile.h" + + +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +#endif /* __VIR_NETDEV_MIDONET_H__ */ + diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6ee20d3..09acb52 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -33,7 +33,8 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, "none", "802.1Qbg", "802.1Qbh", - "openvswitch") + "openvswitch", + "midonet")

VIR_ENUM_IMPL(virNetDevVPortProfileOp, VIR_NETDEV_VPORT_PROFILE_OP_LAST, "create", diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index ad063c5..dc3e643 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -35,6 +35,7 @@ enum virNetDevVPortProfile { VIR_NETDEV_VPORT_PROFILE_8021QBG, VIR_NETDEV_VPORT_PROFILE_8021QBH, VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH, + VIR_NETDEV_VPORT_PROFILE_MIDONET,

VIR_NETDEV_VPORT_PROFILE_LAST, }; @@ -73,7 +74,7 @@ struct _virNetDevVPortProfile { /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];

- /* this member is used when virtPortType == openvswitch */ + /* this member is used when virtPortType == openvswitch|midonet */ unsigned char interfaceID[VIR_UUID_BUFLEN]; bool interfaceID_specified; /* NB - if virtPortType == NONE, any/all of the items could be used */

On 02/23/2015 03:54 PM, Antoni Segura Puimedon wrote:

Midonet is an opensource virtual networking that over lays the IP network between hypervisors. Currently, such networks can be made with the openvswitch virtualport type.

This patch, defines the schema and documentation that will serve as basis for the follow up patches that will add support to libvirt for using Midonet virtual ports for its interfaces. The schema definition requires that the port profile expresses its interfaceid as part of the port profile. For that reason, this is part of the patch too.

As we discussed on IRC just now, libvirt's parsers don't enforce the schemas in the RNG files by default, so a small extra bit is necessary to make sure that an interfaceid is always present. I've squashed the following small bit into virNetDevVPortProfileCheckComplete() before pushing: diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 09acb52..547894c 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -189,6 +189,11 @@ virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport, } } break; + + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (!virtport->interfaceID_specified) + missing = "interfaceid"; + break; } if (missing) { ACK with that change along with the couple small grammar fixes I point out below. I've squashed these all in and will be pushing as soon as I look through 3/3.

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- docs/formatdomain.html.in | 34 +++++++++++++++++++++ docs/schemas/networkcommon.rng | 12 ++++++++ src/conf/netdev_vport_profile_conf.c | 3 +- .../qemuxml2argvdata/qemuxml2argv-net-midonet.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-midonet.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f6477c2..1c2bb45 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3652,6 +3652,40 @@ </devices> ...</pre>

+ <p> + On hosts that support Open vSwitch on the kernel side and that have the

s/that have/have/

+ Midonet Host Agent configured, it is also possible to connect to the + 'midonet' bridge device by adding a + <code><virtualport type='midonet'/></code> to the + interface definition. (<span class="since">Since + 1.2.13</span>). The Midonet virtualport type requires an + <code>interfaceid</code> attribute to its + <code><parameters></code> element.

s/to its/in its/

This interface id is the UUID + that specifies which port in the virtual network topology will be bound + to the interface. + </p> +<pre> + ... + <devices> + ... + <interface type='bridge'> + <source bridge='br0'/> + </interface> + <interface type='bridge'> + <source bridge='br1'/> + <target dev='vnet7'/> + <mac address="00:11:22:33:44:55"/> + </interface> + <interface type='bridge'> + <source bridge='midonet'/> + <virtualport type='midonet'> + <parameters interfaceid='0b2d64da-3d0e-431e-afdd-804415d6ebbb'/> + </virtualport> + </interface> + ... + </devices> + ...</pre> + <h5><a name="elementsNICSSlirp">Userspace SLIRP stack</a></h5>

<p> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 162ea3d..cc8b1dc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -79,6 +79,18 @@ </element> </group> <group> + <element name="virtualport"> + <attribute name="type"> + <value>midonet</value> + </attribute> + <element name="parameters"> + <attribute name="interfaceid"> + <ref name="UUID"/> + </attribute> + </element> + </element> + </group> + <group> <!-- use this when no type attribute is present --> <element name="virtualport"> <optional> diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index 8da0838..1641a3e 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -260,7 +260,8 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, virBufferAsprintf(buf, " instanceid='%s'", uuidstr); } if (virtPort->interfaceID_specified && - (type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || + (type == VIR_NETDEV_VPORT_PROFILE_MIDONET || + type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || type == VIR_NETDEV_VPORT_PROFILE_NONE)) { char uuidstr[VIR_UUID_STRING_BUFLEN];

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-midonet.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-midonet.xml new file mode 100644 index 0000000..ae5a174 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-midonet.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='bridge'> + <mac address='00:11:22:33:44:55'/> + <source bridge='midonet'/> + <virtualport type='midonet'> + <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + </virtualport> + <model type='virtio'/> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 99d4629..ba1157c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -275,6 +275,7 @@ mymain(void) DO_TEST("net-virtio-network-portgroup"); DO_TEST("net-hostdev"); DO_TEST("net-hostdev-vfio"); + DO_TEST("net-midonet");

Yay! A test case! (and you didn't forget to add it to the list in the .c file like I always do :-)

DO_TEST("net-openvswitch"); DO_TEST("sound"); DO_TEST("sound-device");

On 02/23/2015 03:54 PM, Antoni Segura Puimedon wrote:

Use the utilities introduced in the previous patches so the qemu driver is able to create tap devices that are bound (and unbound on domain destroyal) to Midonet virtual ports.

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- src/conf/domain_conf.h | 1 + src/qemu/qemu_hotplug.c | 25 ++++++++++++++++++------- src/qemu/qemu_process.c | 13 +++++++++---- src/util/virnetdevtap.c | 11 ++++++++--- 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 325afa8..cafe50f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -42,6 +42,7 @@ # include "virnetdevmacvlan.h" # include "virsysinfo.h" # include "virnetdevvportprofile.h" +# include "virnetdevmidonet.h" # include "virnetdevopenvswitch.h"

I'm not sure why virnetdevopenvswitch.h is included here, as nothing from it is used in domain_conf.h. Maybe it was there because virnetdevbandwidth was already there. At any rate, it shouldn't be there (I'll send a patch to fix that in a bit), and neither should virnetdevmidonet.h. Instead, virnetdevmidonet.h should be included in qemu_process.c and qemu_hotplug.c, where the functions are actually used. I changed that locally and will squash it in.

# include "virnetdevbandwidth.h" # include "virnetdevvlan.h" diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8691c7e..34d7988 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1141,9 +1141,15 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, }

vport = virDomainNetGetActualVirtPortProfile(net); - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + } }

virDomainNetRemoveHostdev(vm->def, net); @@ -2934,10 +2940,15 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, }

vport = virDomainNetGetActualVirtPortProfile(net); - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), - net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + }

networkReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d4e957..982f802 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5291,10 +5291,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* release the physical device (or any other resources used by * this interface in the network driver */ - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), - net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + }

So much common code. This should really go in a utility function in the new qemu_interface.c (along with lots of other code, so it's nothing to worry about here; just thought I'd mention it)

/* kick the device out of the hostdev list too */ virDomainNetRemoveHostdev(def, net); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 83b4131..f6152e9 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -26,6 +26,7 @@ #include "virnetdevtap.h" #include "virnetdev.h" #include "virnetdevbridge.h" +#include "virnetdevmidonet.h" #include "virnetdevopenvswitch.h" #include "virerror.h" #include "virfile.h" @@ -580,9 +581,13 @@ int virNetDevTapCreateInBridgePort(const char *brname, goto error;

if (virtPortProfile) { - if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, vmuuid, - virtPortProfile, virtVlan) < 0) { - goto error; + if (virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + if (virNetDevMidonetBindPort(*ifname, virtPortProfile) < 0) + goto error; + } else { + if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, vmuuid, + virtPortProfile, virtVlan) < 0) + goto error; }

As long as we're checking the virtporttype for both cases in all the other places, we may as well do it here. It may prevent accidentally doing the wrong thing if we ever add another virtporttype, and may also help persuade someone to turn the if/else if/else if/... into a switch. ACK with the #include changes and the above change to the if. I've squashed in both and am pushing. Thanks for the contribution!! (and the patience :-)

Adds the port type definitions and methods that will be used to bind interfaces to the Midonet virtual ports. virtnetdevmidonet.c adds the way to bind and unbind the ports by calling into the Midonet Host Agent control command line (installed with the midolman package). Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- configure.ac | 4 ++ src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virnetdevmidonet.c | 97 ++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevmidonet.h | 37 +++++++++++++++ src/util/virnetdevvportprofile.c | 1 + src/util/virnetdevvportprofile.h | 3 +- 7 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 src/util/virnetdevmidonet.c create mode 100644 src/util/virnetdevmidonet.h diff --git a/configure.ac b/configure.ac index b3e99e7..ddffbb2 100644 --- a/configure.ac +++ b/configure.ac @@ -425,6 +425,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], @@ -440,6 +442,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc program (see iproute2)]) +AC_DEFINE_UNQUOTED([MMCTL],["$MMCTL"], + [Location or name of the mm-ctl program]) AC_DEFINE_UNQUOTED([OVSVSCTL],["$OVSVSCTL"], [Location or name of the ovs-vsctl program]) diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..23d3f93 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -129,6 +129,7 @@ UTIL_SOURCES = \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ util/virnetdevmacvlan.c util/virnetdevmacvlan.h \ + util/virnetdevmidonet.h util/virnetdevmidonet.c \ util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \ util/virnetdevtap.h util/virnetdevtap.c \ util/virnetdevveth.h util/virnetdevveth.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 46a1613..0938cdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1738,6 +1738,11 @@ virNetDevMacVLanRestartWithVPortProfile; virNetDevMacVLanVPortProfileRegisterCallback; +# util/virnetdevmidonet.h +virNetDevMidonetBindPort; +virNetDevMidonetUnbindPort; + + # util/virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c new file mode 100644 index 0000000..57fb636 --- /dev/null +++ b/src/util/virnetdevmidonet.c @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2015 Midokura, Sarl. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#include <config.h> + +#include "virnetdevmidonet.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virerror.h" +#include "viruuid.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/** + * virNetDevMidonetBindPort: + * @ifname: the network interface name + * @virtualport: the midonet specific fields + * + * Bind an interface to a Midonet virtual port + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + + virCommandAddArgList(cmd, "--bind-port", virtportuuid, ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to bind port %s to the virtual port %s"), + ifname, virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + +/** + * virNetDevMidonetUnbindPort: + * @virtualport: the midonet specific fields + * + * Unbinds a virtual port from the host + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + virCommandAddArgList(cmd, "--unbind-port", virtportuuid, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to unbind the virtual port %s from Midonet"), + virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/util/virnetdevmidonet.h b/src/util/virnetdevmidonet.h new file mode 100644 index 0000000..b8dbeac --- /dev/null +++ b/src/util/virnetdevmidonet.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 Midokura Sarl. + + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com + */ + +#ifndef __VIR_NETDEV_MIDONET_H__ +# define __VIR_NETDEV_MIDONET_H__ + +# include "internal.h" +# include "virnetdevvportprofile.h" + + +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +#endif /* __VIR_NETDEV_MIDONET_H__ */ + diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6ee20d3..921c6aa 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -33,6 +33,7 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, "none", "802.1Qbg", "802.1Qbh", + "midonet", "openvswitch") VIR_ENUM_IMPL(virNetDevVPortProfileOp, VIR_NETDEV_VPORT_PROFILE_OP_LAST, diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index ad063c5..7c00350 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -34,6 +34,7 @@ enum virNetDevVPortProfile { VIR_NETDEV_VPORT_PROFILE_NONE, VIR_NETDEV_VPORT_PROFILE_8021QBG, VIR_NETDEV_VPORT_PROFILE_8021QBH, + VIR_NETDEV_VPORT_PROFILE_MIDONET, VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH, VIR_NETDEV_VPORT_PROFILE_LAST, @@ -73,7 +74,7 @@ struct _virNetDevVPortProfile { /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; - /* this member is used when virtPortType == openvswitch */ + /* this member is used when virtPortType == openvswitch|midonet */ unsigned char interfaceID[VIR_UUID_BUFLEN]; bool interfaceID_specified; /* NB - if virtPortType == NONE, any/all of the items could be used */ -- 2.3.0

On Thu, Feb 19, 2015 at 4:22 PM, Laine Stump <laine@laine.org> wrote:

On 02/17/2015 09:42 PM, Antoni Segura Puimedon wrote:

...

Adds the port type definitions and methods that will be used to bind interfaces to the Midonet virtual ports.

virtnetdevmidonet.c adds the way to bind and unbind the ports by calling into the Midonet Host Agent control command line (installed with the midolman package).

Thinking a bit more about the organization of the patches - I think this patch needs to be 1 (since the parser/formatter changes will use the new enum values, then the patch with schema/docs and changes to parser/formatter (i.e. conf directory) should be 2, then the one that ties it all together would be 3.

Makes sense. Thanks.

...

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- configure.ac | 4 ++ src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virnetdevmidonet.c | 97

++++++++++++++++++++++++++++++++++++++++

...

src/util/virnetdevmidonet.h | 37 +++++++++++++++ src/util/virnetdevvportprofile.c | 1 + src/util/virnetdevvportprofile.h | 3 +- 7 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 src/util/virnetdevmidonet.c create mode 100644 src/util/virnetdevmidonet.h

diff --git a/configure.ac b/configure.ac index b3e99e7..ddffbb2 100644 --- a/configure.ac +++ b/configure.ac @@ -425,6 +425,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], @@ -440,6 +442,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc program (see iproute2)]) +AC_DEFINE_UNQUOTED([MMCTL],["$MMCTL"], + [Location or name of the mm-ctl program]) AC_DEFINE_UNQUOTED([OVSVSCTL],["$OVSVSCTL"], [Location or name of the ovs-vsctl program])

diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..23d3f93 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -129,6 +129,7 @@ UTIL_SOURCES = \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ util/virnetdevmacvlan.c util/virnetdevmacvlan.h \ + util/virnetdevmidonet.h util/virnetdevmidonet.c \ util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \ util/virnetdevtap.h util/virnetdevtap.c \ util/virnetdevveth.h util/virnetdevveth.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 46a1613..0938cdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1738,6 +1738,11 @@ virNetDevMacVLanRestartWithVPortProfile; virNetDevMacVLanVPortProfileRegisterCallback;

+# util/virnetdevmidonet.h +virNetDevMidonetBindPort; +virNetDevMidonetUnbindPort; + + # util/virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c new file mode 100644 index 0000000..57fb636 --- /dev/null +++ b/src/util/virnetdevmidonet.c @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2015 Midokura, Sarl. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com> + */ + +#include <config.h> + +#include "virnetdevmidonet.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virerror.h" +#include "viruuid.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/** + * virNetDevMidonetBindPort: + * @ifname: the network interface name + * @virtualport: the midonet specific fields + * + * Bind an interface to a Midonet virtual port + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + + virCommandAddArgList(cmd, "--bind-port", virtportuuid, ifname, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to bind port %s to the virtual port %s"), + ifname, virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} + +/** + * virNetDevMidonetUnbindPort: + * @virtualport: the midonet specific fields + * + * Unbinds a virtual port from the host + * + * Returns 0 in case of success or -1 in case of failure. + */ +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char virtportuuid[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(virtualport->interfaceID, virtportuuid); + + cmd = virCommandNew(MMCTL); + virCommandAddArgList(cmd, "--unbind-port", virtportuuid, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to unbind the virtual port %s from Midonet"), + virtportuuid); + goto cleanup; + } + + ret = 0; + cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/util/virnetdevmidonet.h b/src/util/virnetdevmidonet.h new file mode 100644 index 0000000..b8dbeac --- /dev/null +++ b/src/util/virnetdevmidonet.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2015 Midokura Sarl. + + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Antoni Segura Puimedon <toni@midokura.com + */ + +#ifndef __VIR_NETDEV_MIDONET_H__ +# define __VIR_NETDEV_MIDONET_H__ + +# include "internal.h" +# include "virnetdevvportprofile.h" + + +int virNetDevMidonetBindPort(const char *ifname, + virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +#endif /* __VIR_NETDEV_MIDONET_H__ */ + diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6ee20d3..921c6aa 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -33,6 +33,7 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, "none", "802.1Qbg", "802.1Qbh", + "midonet", "openvswitch")

I'm curious why you added the new enum in the middle rather than at the end. Was it to reduce the number of lines in the diff?

No. Something sillier than that. I just went by alphabetic order :P I'll add it at the end.

We usually add new values at the end of the list. Everything *should* work when adding to the middle, but superstition causes me to anticipate bugs cause by some idiot using a hard-coded value, or storing the enum somewhere as an integer, or other assorted bad things.

Understood. I'' put new enum things at the end from now on.

...

VIR_ENUM_IMPL(virNetDevVPortProfileOp, VIR_NETDEV_VPORT_PROFILE_OP_LAST, diff --git a/src/util/virnetdevvportprofile.h

b/src/util/virnetdevvportprofile.h

...

index ad063c5..7c00350 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -34,6 +34,7 @@ enum virNetDevVPortProfile { VIR_NETDEV_VPORT_PROFILE_NONE, VIR_NETDEV_VPORT_PROFILE_8021QBG, VIR_NETDEV_VPORT_PROFILE_8021QBH, + VIR_NETDEV_VPORT_PROFILE_MIDONET, VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH,

VIR_NETDEV_VPORT_PROFILE_LAST, @@ -73,7 +74,7 @@ struct _virNetDevVPortProfile { /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];

- /* this member is used when virtPortType == openvswitch */ + /* this member is used when virtPortType == openvswitch|midonet */ unsigned char interfaceID[VIR_UUID_BUFLEN]; bool interfaceID_specified; /* NB - if virtPortType == NONE, any/all of the items could be used */

On 02/17/2015 09:42 PM, Antoni Segura Puimedon wrote:

Use the utilities introduced in the previous patches so the qemu driver is able to create tap devices that are bound (and unbound on domain destroyal) to Midonet virtual ports.

Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com> --- src/conf/domain_conf.h | 1 + src/conf/netdev_vport_profile_conf.c | 3 ++-

Changes to the parser/formatter should be in the same patch as the schema/docs changes.

src/qemu/qemu_hotplug.c | 25 ++++++++++++++++++------- src/qemu/qemu_process.c | 13 +++++++++---- src/util/virnetdevtap.c | 11 ++++++++--- 5 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 325afa8..cafe50f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -42,6 +42,7 @@ # include "virnetdevmacvlan.h" # include "virsysinfo.h" # include "virnetdevvportprofile.h" +# include "virnetdevmidonet.h" # include "virnetdevopenvswitch.h" # include "virnetdevbandwidth.h" # include "virnetdevvlan.h" diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index 8da0838..1641a3e 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -260,7 +260,8 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, virBufferAsprintf(buf, " instanceid='%s'", uuidstr); } if (virtPort->interfaceID_specified && - (type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || + (type == VIR_NETDEV_VPORT_PROFILE_MIDONET || + type == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH || type == VIR_NETDEV_VPORT_PROFILE_NONE)) { char uuidstr[VIR_UUID_STRING_BUFLEN];

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8691c7e..34d7988 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1141,9 +1141,15 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, }

vport = virDomainNetGetActualVirtPortProfile(net); - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + }

To pre-emptively prevent bugs if we ever have to add code to specifically disconnect from standard bridges (unlikely but possible), I think these if's shouldn't be nested. Instead it should be: if (vpor && vport->virtPortType == OPENVSWITCH) { ovs stuff } else if (vport && vport->virtPortType == MIDONET) { midonet stuff } (that way we can just add an "else" clause if we have to without restructuring anything else).

}

virDomainNetRemoveHostdev(vm->def, net); @@ -2934,10 +2940,15 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, }

vport = virDomainNetGetActualVirtPortProfile(net); - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), - net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + }

See the comment above. Same applies here.

networkReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d4e957..982f802 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5291,10 +5291,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* release the physical device (or any other resources used by * this interface in the network driver */ - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(net), - net->ifname)); + if (vport) { + if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + ignore_value(virNetDevMidonetUnbindPort(vport)); + } else if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + } + }

and here.

/* kick the device out of the hostdev list too */ virDomainNetRemoveHostdev(def, net); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 83b4131..f6152e9 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -26,6 +26,7 @@ #include "virnetdevtap.h" #include "virnetdev.h" #include "virnetdevbridge.h" +#include "virnetdevmidonet.h" #include "virnetdevopenvswitch.h" #include "virerror.h" #include "virfile.h" @@ -580,9 +581,13 @@ int virNetDevTapCreateInBridgePort(const char *brname, goto error;

if (virtPortProfile) { - if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, vmuuid, - virtPortProfile, virtVlan) < 0) { - goto error; + if (virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) { + if (virNetDevMidonetBindPort(*ifname, virtPortProfile) < 0) + goto error; + } else { + if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr, vmuuid, + virtPortProfile, virtVlan) < 0) + goto error; } } else { if (virNetDevBridgeAddPort(brname, *ifname) < 0)

I know you didn't create the problem in this case, but I think here the logic should be the following: if (virtPortProfile && virtPortType == OPENVSWITCH) { virNetDevOpenvswitchAddPort(blah) } else if (virtPortProfile && virtPortType == MIDONET) { virNetDevMidonetBindPort(blah) } else { virNetDevBridgeAddPort(blah) } That way if someone specifies a <virtualport> as a placeholder, but no type, they will still get attached to a standard bridge (which is almost surely what they will expect).