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@xxxxxxxxxxxx>
> ---
> 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).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list