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@oracle.com> --- 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))