On Thu, Feb 19, 2015 at 4:18 PM, Laine Stump <laine@laine.org> wrote:
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.
Agreed! Thanks
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).
Will do!
}
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.
Will do!
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.
Will do!
/* 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).
Agreed. Much better. Thanks a lot for the review Laine. I'll send the updated v3 today.