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>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 38ef692b440ce43be497d16232667f25f9ff33c8..d7be544710060e9bb324abdafac79c2fbac3f644 100644 (file)
--- 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 44c76d4270a47ab91c69655026fedb1c8543b5ac..b9c502613c236e8cdad7d80c6989df18025090ce 100644 (file)
--- 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 379ee3d5d13f885e3ced61757b4504738d5dcbde..f099f35e1e7e78bd40a3b8aaf15fe56a59d33c74 100644 (file)
--- 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;