Accepting request 874914 from home:jfehlig:branches:Virtualization

- libxl: Add lock process indicator to track resource locking
  fa58f571-libxl-lock-proc-indicator.patch
  bsc#1182367

OBS-URL: https://build.opensuse.org/request/show/874914
OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=879

fa58f571-libxl-lock-proc-indicator.patch

@@ -0,0 +1,109 @@
+commit fa58f571ee0b2ab72f8a5a19d35298d6de6469b3
+Author: Jim Fehlig <jfehlig@suse.com>
+Date:   Fri Feb 19 14:58:19 2021 -0700
+
+    libxl: Add lock process indicator to libxlDomainObjPrivate object
+    
+    The libvirt libxl driver has no access to FDs associated with VM disks.
+    The disks are opened by libxl.so and any related FDs are not exposed to
+    applications. The prevents using virtlockd's auto-release feature to
+    release locks when the FD is closed. Acquiring and releasing locks is
+    explicitly handled by the libxl driver.
+    
+    The current logic is structured such that locks are acquired in
+    libxlDomainStart and released in libxlDomainCleanup. This works well
+    except for migration, where the locks must be released on the source
+    host before the domain can be started on the destination host, but the
+    domain cannot be cleaned up until the migration confirmation stage.
+    When libxlDomainCleanup if finally called in the confirm stage, locks
+    are again released resulting in confusing errors from virtlockd and
+    libvirtd
+    
+    virtlockd[8095]: resource busy: Lockspace resource 'xxxxxx' is not locked
+    libvirtd[8050]: resource busy: Lockspace resource 'xxxxxx' is not locked
+    libvirtd[8050]: Unable to release lease on testvm
+    
+    The error is also encountered in some error cases, e.g. when
+    libxlDomainStart fails before acquiring locks and libxlDomainCleanup
+    is still used for cleanup.
+    
+    In lieu of a mechanism to check if a lock has been acquired, this patch
+    takes an easy approach to fixing the unnecessary lock releases by adding
+    an indicator to the libxlDomainPrivate object that can be set when the
+    lock is acquired and cleared when the lock is released. libxlDomainCleanup
+    can then skip releasing the lock in cases where it was previously released
+    or never acquired in the first place.
+    
+    Signed-off-by: Jim Fehlig <jfehlig@suse.com>
+    Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
+
+Index: libvirt-7.0.0/src/libxl/libxl_domain.c
+===================================================================
+--- libvirt-7.0.0.orig/src/libxl/libxl_domain.c
++++ libvirt-7.0.0/src/libxl/libxl_domain.c
+@@ -864,10 +864,14 @@ libxlDomainCleanup(libxlDriverPrivatePtr
+     virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
+                                     vm->def, hostdev_flags, NULL);
+ 
+-    VIR_FREE(priv->lockState);
+-    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));
++    if (priv->lockProcessRunning) {
++        VIR_FREE(priv->lockState);
++        if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0)
++            VIR_WARN("Unable to release lease on %s", vm->def->name);
++        else
++            priv->lockProcessRunning = false;
++        VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
++    }
+ 
+     libxlLoggerCloseFile(cfg->logger, vm->def->id);
+     vm->def->id = -1;
+@@ -1431,6 +1435,7 @@ libxlDomainStart(libxlDriverPrivatePtr d
+                                   priv->lockState) < 0)
+         goto destroy_dom;
+     VIR_FREE(priv->lockState);
++    priv->lockProcessRunning = true;
+ 
+     /* Always enable domain death events */
+     if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW))
+Index: libvirt-7.0.0/src/libxl/libxl_domain.h
+===================================================================
+--- libvirt-7.0.0.orig/src/libxl/libxl_domain.h
++++ libvirt-7.0.0/src/libxl/libxl_domain.h
+@@ -68,6 +68,7 @@ struct _libxlDomainObjPrivate {
+     virThreadPtr migrationDstReceiveThr;
+     unsigned short migrationPort;
+     char *lockState;
++    bool lockProcessRunning;
+ 
+     struct libxlDomainJobObj job;
+ 
+Index: libvirt-7.0.0/src/libxl/libxl_migration.c
+===================================================================
+--- libvirt-7.0.0.orig/src/libxl/libxl_migration.c
++++ libvirt-7.0.0/src/libxl/libxl_migration.c
+@@ -1247,9 +1247,12 @@ libxlDomainMigrationSrcPerform(libxlDriv
+     virObjectLock(vm);
+ 
+     if (ret == 0) {
+-        if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0)
++        if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) == 0) {
++            priv->lockProcessRunning = false;
++            VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
++        } else {
+             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
+@@ -1382,6 +1385,7 @@ libxlDomainMigrationSrcConfirm(libxlDriv
+                                    "xen:///system",
+                                    vm,
+                                    priv->lockState);
++        priv->lockProcessRunning = true;
+         if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) == 0) {
+             ret = 0;
+         } else {

libvirt.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Feb 24 16:20:47 UTC 2021 - James Fehlig <jfehlig@suse.com>
+
+- libxl: Add lock process indicator to track resource locking
+  fa58f571-libxl-lock-proc-indicator.patch
+  bsc#1182367
+
 -------------------------------------------------------------------
 Mon Feb 22 18:07:47 UTC 2021 - James Fehlig <jfehlig@suse.com>
 

libvirt.spec

@@ -300,6 +300,7 @@ Patch5:         c05f0066-conf-drop-empty-virDomainNetDefPostParse.patch
 Patch6:         19d4e467-conf-improve-virDomainVirtioOptionsCheckABIStability.patch
 Patch7:         bd112c9e-qemu-virtio-options-vsock.patch
 Patch8:         87a9d3a6-libxl-fix-domain-shutdown.patch
+Patch9:         fa58f571-libxl-lock-proc-indicator.patch
 # Patches pending upstream review
 Patch100:       libxl-dom-reset.patch
 Patch101:       network-don-t-use-dhcp-authoritative-on-static-netwo.patch

suse-bump-xen-version.patch

@@ -26,7 +26,7 @@ Index: libvirt-7.0.0/src/libxl/libxl_domain.c
 ===================================================================
 --- libvirt-7.0.0.orig/src/libxl/libxl_domain.c
 +++ libvirt-7.0.0/src/libxl/libxl_domain.c
-@@ -1007,8 +1007,8 @@ libxlDomainSetVcpuAffinities(libxlDriver
+@@ -1011,8 +1011,8 @@ libxlDomainSetVcpuAffinities(libxlDriver
  static int
  libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config)
  {
@@ -37,7 +37,7 @@ Index: libvirt-7.0.0/src/libxl/libxl_domain.c
      int32_t target_mem;
      int tries = 3;
      int wait_secs = 10;
-@@ -1398,7 +1398,7 @@ libxlDomainStart(libxlDriverPrivatePtr d
+@@ -1402,7 +1402,7 @@ libxlDomainStart(libxlDriverPrivatePtr d
          params.stream_version = restore_ver;
  #endif
          ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid,