From 94365a48710227a5fec836baadd12d8ff5a5fe8a Mon Sep 17 00:00:00 2001
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Thu, 25 Jan 2024 11:51:51 -0600
Subject: [PATCH] qemu: handle adding/removing nbdkit-backed disk sources

Previously we were only starting or stopping nbdkit when the guest was
started or stopped or when hotplugging/unplugging a disk. But when doing
block operations, the disk backing store sources can also be be added or
removed independently of the disk device. When this happens the nbdkit
backend was not being handled properly. For example, when doing a
blockcopy from a nbdkit-backed disk to a new disk and pivoting to that
new location, the nbdkit process did not get cleaned up properly. Add
some functionality to qemuDomainStorageSourceAccessModify() to handle
this scenario.

Since we're now starting nbdkit from the ChainAccessAllow/Revoke()
functions, we no longer need to explicitly start nbdkit in hotplug code
paths because the hotplug functions already call these allow/revoke
functions and will start/stop nbdkit if necessary.

Add a check to qemuNbdkitProcessStart() to report an error if we
are trying to start nbdkit for a disk source that already has a running
nbdkit process. This shouldn't happen, and if it does it indicates an
error in another part of our code.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c  | 10 ++++++++++
 src/qemu/qemu_hotplug.c |  5 -----
 src/qemu/qemu_nbdkit.c  |  7 +++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 38ef692b44..d7be544710 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8060,6 +8060,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver,
     bool revoke_namespace = false;
     bool revoke_nvme = false;
     bool revoke_lockspace = false;
+    bool revoke_nbdkit = false;
 
     VIR_DEBUG("src='%s' readonly=%d force_ro=%d force_rw=%d revoke=%d chain=%d",
               NULLSTR(src->path), src->readonly, force_ro, force_rw, revoke, chain);
@@ -8078,6 +8079,7 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver,
         revoke_namespace = true;
         revoke_nvme = true;
         revoke_lockspace = true;
+        revoke_nbdkit = true;
         ret = 0;
         goto revoke;
     }
@@ -8093,6 +8095,11 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver,
 
         revoke_nvme = true;
 
+        if (qemuNbdkitStartStorageSource(driver, vm, src, chain) < 0)
+            goto revoke;
+
+        revoke_nbdkit = true;
+
         if (qemuDomainNamespaceSetupDisk(vm, src, &revoke_namespace) < 0)
             goto revoke;
     }
@@ -8147,6 +8154,9 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver,
             VIR_WARN("Unable to release lock on %s", srcstr);
     }
 
+    if (revoke_nbdkit)
+        qemuNbdkitStopStorageSource(src, vm, chain);
+
  cleanup:
     src->readonly = was_readonly;
     virErrorRestore(&orig_err);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 44c76d4270..b9c502613c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1023,9 +1023,6 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver,
 
         if (qemuHotplugAttachManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE) < 0)
             goto cleanup;
-
-        if (qemuNbdkitStartStorageSource(driver, vm, disk->src, true) < 0)
-            goto cleanup;
     }
 
     ret = qemuDomainAttachDiskGeneric(vm, disk, VIR_ASYNC_JOB_NONE);
@@ -1050,8 +1047,6 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver,
 
         if (virStorageSourceChainHasManagedPR(disk->src))
             ignore_value(qemuHotplugRemoveManagedPR(vm, VIR_ASYNC_JOB_NONE));
-
-        qemuNbdkitStopStorageSource(disk->src, vm, true);
     }
     qemuDomainSecretDiskDestroy(disk);
     qemuDomainCleanupStorageSourceFD(disk->src);
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 379ee3d5d1..f099f35e1e 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -1190,6 +1190,13 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
     struct nbd_handle *nbd = NULL;
 #endif
 
+    /* don't try to start nbdkit again if we've already started it */
+    if (proc->pid > 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Attempting to start nbdkit twice"));
+        return -1;
+    }
+
     if (!(cmd = qemuNbdkitProcessBuildCommand(proc)))
         return -1;