From 25f8b5455b735bc053b903ec7ff97996380746f573f2e327a3ee275c145c0b19 Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Thu, 14 Nov 2019 17:47:53 +0000 Subject: [PATCH] Accepting request 748738 from home:jfehlig:branches:Virtualization - libxl: Fix lock manager lock ordering 2552752f-libxl-fix-lock-manager-lock-ordering.patch bsc#1145774 OBS-URL: https://build.opensuse.org/request/show/748738 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=783 --- ...libxl-fix-lock-manager-lock-ordering.patch | 87 +++++++++++++++++++ libvirt.changes | 7 ++ libvirt.spec | 2 + libxl-set-migration-constraints.patch | 4 +- 4 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 2552752f-libxl-fix-lock-manager-lock-ordering.patch diff --git a/2552752f-libxl-fix-lock-manager-lock-ordering.patch b/2552752f-libxl-fix-lock-manager-lock-ordering.patch new file mode 100644 index 0000000..f9bc986 --- /dev/null +++ b/2552752f-libxl-fix-lock-manager-lock-ordering.patch @@ -0,0 +1,87 @@ +commit 2552752f0b6504a80f6306e5aae2c7063d24f1ab +Author: Jim Fehlig +Date: Mon Oct 14 14:01:00 2019 -0600 + + libxl: Fix lock manager lock ordering + + The ordering of lock manager locks in the libxl driver has a flaw that was + uncovered by a migration error path. In the perform phase of migration, the + source host calls virDomainLockProcessPause to release the lock before + sending the VM to the destination host. If the send fails an attempt is made + to reacquire the lock with virDomainLockProcessResume, but that too can fail + if the destination host has not finished cleaning up the failed VM and + releasing the lock it acquired when starting to receive the VM. + + This change delays calling virDomainLockProcessResume in libxlDomainStart + until the VM is successfully created, but before it is unpaused. A similar + approach is used by the qemu driver, avoiding the need to release the lock + if VM creation fails. In the migration perform phase, releasing the lock + with virDomainLockProcessPause is delayed until the VM is successfully + sent to the destination, which avoids reacquiring the lock if the send + fails. + + Signed-off-by: Jim Fehlig + Reviewed-by: Cole Robinson + +Index: libvirt-5.9.0/src/libxl/libxl_domain.c +=================================================================== +--- libvirt-5.9.0.orig/src/libxl/libxl_domain.c ++++ libvirt-5.9.0/src/libxl/libxl_domain.c +@@ -1347,13 +1347,6 @@ libxlDomainStart(libxlDriverPrivatePtr d + NULL) < 0) + goto cleanup; + +- if (virDomainLockProcessResume(driver->lockManager, +- "xen:///system", +- vm, +- priv->lockState) < 0) +- goto cleanup; +- VIR_FREE(priv->lockState); +- + if (libxlNetworkPrepareDevices(vm->def) < 0) + goto cleanup_dom; + +@@ -1436,6 +1429,13 @@ libxlDomainStart(libxlDriverPrivatePtr d + + libxlLoggerOpenFile(cfg->logger, domid, vm->def->name, config_json); + ++ if (virDomainLockProcessResume(driver->lockManager, ++ "xen:///system", ++ vm, ++ priv->lockState) < 0) ++ goto destroy_dom; ++ VIR_FREE(priv->lockState); ++ + /* Always enable domain death events */ + if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW)) + goto destroy_dom; +Index: libvirt-5.9.0/src/libxl/libxl_migration.c +=================================================================== +--- libvirt-5.9.0.orig/src/libxl/libxl_migration.c ++++ libvirt-5.9.0/src/libxl/libxl_migration.c +@@ -1240,20 +1240,16 @@ libxlDomainMigrationSrcPerform(libxlDriv + sockfd = virNetSocketDupFD(sock, true); + virObjectUnref(sock); + +- if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) +- VIR_WARN("Unable to release lease on %s", vm->def->name); +- VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); +- + /* suspend vm and send saved data to dst through socket fd */ + virObjectUnlock(vm); + ret = libxlDoMigrateSrcSend(driver, vm, flags, sockfd); + virObjectLock(vm); + +- if (ret < 0) { +- virDomainLockProcessResume(driver->lockManager, +- "xen:///system", +- vm, +- priv->lockState); ++ if (ret == 0) { ++ if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) ++ VIR_WARN("Unable to release lease on %s", vm->def->name); ++ VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); ++ } else { + /* + * Confirm phase will not be executed if perform fails. End the + * job started in begin phase. diff --git a/libvirt.changes b/libvirt.changes index 24da8af..c208021 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Nov 14 17:31:35 UTC 2019 - James Fehlig + +- libxl: Fix lock manager lock ordering + 2552752f-libxl-fix-lock-manager-lock-ordering.patch + bsc#1145774 + ------------------------------------------------------------------- Tue Nov 12 22:51:54 UTC 2019 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index efec269..3de04e6 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -338,6 +338,7 @@ Source6: libvirtd-relocation-server.xml Source99: baselibs.conf Source100: %{name}-rpmlintrc # Upstream patches +Patch0: 2552752f-libxl-fix-lock-manager-lock-ordering.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch @@ -871,6 +872,7 @@ libvirt plugin for NSS for translating domain names into IP addresses. %prep %setup -q +%patch0 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1 diff --git a/libxl-set-migration-constraints.patch b/libxl-set-migration-constraints.patch index 4fa2031..43e5913 100644 --- a/libxl-set-migration-constraints.patch +++ b/libxl-set-migration-constraints.patch @@ -255,7 +255,7 @@ Index: libvirt-5.9.0/src/libxl/libxl_migration.c { libxlDomainObjPrivatePtr priv = vm->privateData; char *hostname = NULL; -@@ -1246,7 +1267,7 @@ libxlDomainMigrationSrcPerform(libxlDriv +@@ -1242,7 +1263,7 @@ libxlDomainMigrationSrcPerform(libxlDriv /* suspend vm and send saved data to dst through socket fd */ virObjectUnlock(vm); @@ -263,7 +263,7 @@ Index: libvirt-5.9.0/src/libxl/libxl_migration.c + ret = libxlDoMigrateSrcSend(driver, vm, props, sockfd); virObjectLock(vm); - if (ret < 0) { + if (ret == 0) { Index: libvirt-5.9.0/src/libxl/libxl_migration.h =================================================================== --- libvirt-5.9.0.orig/src/libxl/libxl_migration.h