Re: [Xen-devel] [PATCH] libxl: free ifname on migration begin phase

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 12/04/2015 05:28 PM, Joao Martins wrote:
> Commit d2e5538b1 changes virDomainDef to create ifnames
> that are autogenerated by libxl, and also clearing them up
> on domain cleanup. One place that was missing was also
> on migration, when domain xml is sent to dst libvirtd and
> would contain ifnames from the source libvirtd. This
> would lead to erronous behaviour (as seen in osstest CI)
> such as failing to migrate when a vif with the a name
> that existed on the destination host (belonging to another domain).
> 
> This patch adds an helper libxlDomainFreeIfaceNames for
> freeing autogenerated ifnames on both migration begin phase
> and domain cleanup.
> 
Apologies, this might not be the correct way. While migration happens we can't
get the interface stats, plus if the migration fails we would leave the domain
without ifname. So perhaps should be done on Prepare phase and the destination
would be responsible for clearing them up since XML is not active and might
actually disappear if migration fails. It would also be better for P2P support.
I will resubmit (and retest). Please ignore this patch.

Joao

> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> ---
>  src/libxl/libxl_domain.c    | 32 ++++++++++++++++++++++----------
>  src/libxl/libxl_domain.h    |  3 +++
>  src/libxl/libxl_migration.c |  2 ++
>  3 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index ef92974..487589d 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -728,16 +728,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
>          }
>      }
>  
> -    if ((vm->def->nnets)) {
> -        size_t i;
> -
> -        for (i = 0; i < vm->def->nnets; i++) {
> -            virDomainNetDefPtr net = vm->def->nets[i];
> -
> -            if (STRPREFIX(net->ifname, "vif"))
> -                VIR_FREE(net->ifname);
> -        }
> -    }
> +    libxlDomainFreeIfaceNames(vm->def);
>  
>      if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir, vm->def->name) > 0) {
>          if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
> @@ -923,6 +914,27 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def, 
> libxl_domain_config *d_config)
>      }
>  }
>  
> +/*
> + * Removes autogenerated interface names for the network devices in
> + * parameter def. User-provided interface names are skipped.
> + */
> +void
> +libxlDomainFreeIfaceNames(virDomainDefPtr def)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->nnets; i++) {
> +        virDomainNetDefPtr net = def->nets[i];
> +
> +        if (!net->ifname)
> +            continue;
> +
> +        if (STRPREFIX(net->ifname, "vif"))
> +            VIR_FREE(net->ifname);
> +    }
> +}
> +
> +
>  
>  /*
>   * Start a domain through libxenlight.
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> index 44b3e0b..70c139c 100644
> --- a/src/libxl/libxl_domain.h
> +++ b/src/libxl/libxl_domain.h
> @@ -135,6 +135,9 @@ int
>  libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver,
>                               virDomainObjPtr vm);
>  
> +void
> +libxlDomainFreeIfaceNames(virDomainDefPtr def);
> +
>  int
>  libxlDomainStart(libxlDriverPrivatePtr driver,
>                   virDomainObjPtr vm,
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index 0d23e5f..1ee19ae 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -249,6 +249,8 @@ libxlDomainMigrationBegin(virConnectPtr conn,
>          def = tmpdef;
>      } else {
>          def = vm->def;
> +
> +        libxlDomainFreeIfaceNames(def);
>      }
>  
>      if (!libxlDomainMigrationIsAllowed(def))
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel