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

[Wangrui (K)]

> -----Original Message-----
> From: sendmail [mailto:justsendmailnothingelse at gmail.com] On Behalf Of
> Laine Stump
> Sent: Tuesday, May 13, 2014 10:11 PM
> To: Matthew Rosato; libvir-list at 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 at linux.vnet.ibm.com>
> >> 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.

Thanks for fix the issue that I reported.
I think it can be considered that all types of netdevs are brought up until migration finish, not only macvtap.

> 
> 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
> 
> 

I haven't seen the open BZ until Laine mentioned.
Next time I will search the open BZs before report a new issue.

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