In commit 8bebb2b735d I've refactored how the detach of disk with a
managed persistent reservations object is handled. After the commit if
any disk with a managed PR object would be removed libvirt would also
attempt to remove the shared 'pr-manager-helper' object potentially used
by other disks.

Thankfully this should not have practical impact as qemu should reject
deletion of the object if it was still used and the rest of the code is
correct.

Fix this by removing the disk from the definition earlier and checking
if the shared/managed pr-manager-helper object is still needed.

This basically splits the detach code for the managed PR object from the
unmanaged ones. The same separation will follow for the attachment code
as well as it greatly simplifies -blockdev support for this.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_hotplug.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b4bbe62c75..a14281203a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3839,6 +3839,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
     char *drivestr;
     char *objAlias = NULL;
     char *encAlias = NULL;
+    bool prManaged = priv->prDaemonRunning;
+    bool prUsed = false;

     VIR_DEBUG("Removing disk %s from domain %p %s",
               disk->info.alias, vm, vm->def->name);
@@ -3876,6 +3878,16 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
         }
     }

+    for (i = 0; i < vm->def->ndisks; i++) {
+        if (vm->def->disks[i] == disk) {
+            virDomainDiskRemove(vm->def, i);
+            break;
+        }
+    }
+
+    /* check if the last disk with managed PR was just removed */
+    prUsed = virDomainDefHasManagedPR(vm->def);
+
     qemuDomainObjEnterMonitor(driver, vm);

     qemuMonitorDriveDel(priv->mon, drivestr);
@@ -3892,12 +3904,16 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
     VIR_FREE(encAlias);

     /* If it fails, then so be it - it was a best shot */
-    if (disk->src->pr)
+    if (disk->src->pr &&
+        !virStoragePRDefIsManaged(disk->src->pr))
         ignore_value(qemuMonitorDelObject(priv->mon, disk->src->pr->mgralias));

     if (disk->src->haveTLS)
         ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias));

+    if (prManaged && !prUsed)
+        ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias()));
+
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         return -1;

@@ -3906,16 +3922,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
     event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias);
     qemuDomainEventQueue(driver, event);

-    for (i = 0; i < vm->def->ndisks; i++) {
-        if (vm->def->disks[i] == disk) {
-            virDomainDiskRemove(vm->def, i);
-            break;
-        }
-    }
-
-    /* check if the last disk with managed PR was just removed */
-    if (priv->prDaemonRunning &&
-        !virDomainDefHasManagedPR(vm->def))
+    if (prManaged && !prUsed)
         qemuProcessKillManagedPRDaemon(vm);

     qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/31/2018 03:26 PM, Peter Krempa wrote:
> In commit 8bebb2b735d I've refactored how the detach of disk with a
> managed persistent reservations object is handled. After the commit if
> any disk with a managed PR object would be removed libvirt would also
> attempt to remove the shared 'pr-manager-helper' object potentially used
> by other disks.
> 
> Thankfully this should not have practical impact as qemu should reject
> deletion of the object if it was still used and the rest of the code is
> correct.
> 
> Fix this by removing the disk from the definition earlier and checking
> if the shared/managed pr-manager-helper object is still needed.
> 
> This basically splits the detach code for the managed PR object from the
> unmanaged ones. The same separation will follow for the attachment code
> as well as it greatly simplifies -blockdev support for this.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)

ACK and SFF (safe for freeze).

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list