Re: [libvirt] [PATCH] network: Defer online of macvtap during qemu migration

-----Original Message----- From: sendmail [mailto:justsendmailnothingelse@gmail.com] On Behalf Of Laine Stump Sent: Tuesday, May 13, 2014 10:11 PM To: Matthew Rosato; libvir-list(a)redhat.com Cc: Wangrui (K) Subject: Re: [libvirt] [PATCH] network: Defer online of macvtap during qemu migration On 05/13/2014 04:31 PM, Matthew Rosato wrote: > On 05/05/2014 12:33 PM, Matthew Rosato wrote: >> On 05/05/2014 12:26 PM, Matthew Rosato wrote: >>> When generating macvtaps via virNetDevMacVLanCreateWithVPortProfile, >>> the macvtap device is unconditionally set to the up state. However, >>> during migration, this results in a case where both the source and >>> target system are simultaneously up with the same MAC address. This >>> patch defers bringing the target macvtap up until later in the >>> migration to shrink this window. >>> >>> Signed-off-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com&gt; >> Forgot to mention that this patch is associated with what Wangrui >> reported here: >> >> http://www.redhat.com/archives/libvir-list/2014-March/msg01054.html >> >> and follows Viktor's suggested solution mentioned here: >> >> http://www.redhat.com/archives/libvir-list/2014-March/msg01654.html >> >> > Ping.

Review coming up. Sorry for the delay. (I'm looking to see if the fix can be simplified to bring up the macvtap interfaces *always*, rather than just during migration.) BTW, there is an open BZ for this: https://bugzilla.redhat.com/show_bug.cgi?id=1081461

> >>> --- >>> src/qemu/qemu_migration.c | 18 ++++++++++++++++++ >>> src/util/virnetdevmacvlan.c | 11 ++++++++--- >>> 2 files changed, 26 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >>> index a9f7fea..aee803a 100644 >>> --- a/src/qemu/qemu_migration.c >>> +++ b/src/qemu/qemu_migration.c >>> @@ -56,6 +56,7 @@ >>> #include "virhook.h" >>> #include "virstring.h" >>> #include "virtypedparam.h" >>> +#include "virnetdev.h" >>> >>> #define VIR_FROM_THIS VIR_FROM_QEMU >>> >>> @@ -4468,6 +4469,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, >>> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); >>> virCapsPtr caps = NULL; >>> unsigned short port; >>> + virDomainNetDefPtr net; >>> + size_t i; >>> >>> VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " >>> "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", >>> @@ -4574,6 +4577,21 @@ qemuMigrationFinish(virQEMUDriverPtr driver, >>> } >>> >>> if (!(flags & VIR_MIGRATE_PAUSED) && !(flags & VIR_MIGRATE_OFFLINE)) { >>> + /* Macvtaps were previously left offline, bring them online now */ >>> + for (i = 0; i < vm->def->nnets; i++) { >>> + net = vm->def->nets[i]; >>> + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { >>> + if (virNetDevSetOnline(net->ifname, true) < 0) { >>> + ignore_value(virNetDevVPortProfileDisassociate(net->ifname, >>> + virDomainNetGetActualVirtPortProfile(net), >>> + &net->mac, >>> + virDomainNetGetActualDirectDev(net), >>> + -1, >>> + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)); >>> + ignore_value(virNetDevMacVLanDelete(net->ifname)); >>> + } >>> + } >>> + } >>> /* run 'cont' on the destination, which allows migration on qemu >>> * >= 0.10.6 to work properly. This isn't strictly necessary on >>> * older qemu's, but it also doesn't hurt anything there >>> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c >>> index 7bbf540..3da845b 100644 >>> --- a/src/util/virnetdevmacvlan.c >>> +++ b/src/util/virnetdevmacvlan.c >>> @@ -898,9 +898,14 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, >>> goto link_del_exit; >>> } >>> >>> - if (virNetDevSetOnline(cr_ifname, true) < 0) { >>> - rc = -1; >>> - goto disassociate_exit; >>> + /* If this device is being created as part of an inbound >>> + * migration, leave the device offline for now. >>> + */ >>> + if (vmOp != VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) { >>> + if (virNetDevSetOnline(cr_ifname, true) < 0) { >>> + rc = -1; >>> + goto disassociate_exit; >>> + } >>> } >>> >>> if (withTap) { >>> >> -- >> libvir-list mailing list >> libvir-list(a)redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list >> >> >> > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list >