Re: [PATCH 2/3] qemu: roll back if not all nbdkit backends are successful

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/29/24 2:30 AM, Peter Krempa wrote:
On Thu, Jan 25, 2024 at 14:52:10 -0600, Jonathon Jongsma wrote:
When starting nbdkit processes for the backing store of a disk, we were
returning an error if any backing store failed, but we were not cleaning
up processes that succeeded higher in the chain. Make sure that if we
return a failure status from qemuNbdkitStartStorageSource() that we roll
back any processes that had been started.

qemuNbdkitStartStorageSource is called from:

src/qemu/qemu_extdevice.c=qemuExtDevicesStart(virQEMUDriver *driver,
src/qemu/qemu_extdevice.c:        if (qemuNbdkitStartStorageSource(driver, vm, disk->src, true) < 0)
src/qemu/qemu_extdevice.c:        if (qemuNbdkitStartStorageSource(driver, vm, def->os.loader->nvram, true) < 0)

The counterpart is qemuExtDevicesStop.

src/qemu/qemu_hotplug.c=qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver,
src/qemu/qemu_hotplug.c:        if (qemuNbdkitStartStorageSource(driver, vm, disk->src, true) < 0)

This has cleanup inline.

Thus I don't see a possibility where what you describe in the commit
message can happen.

Oops it seems that i never actually responded to this comment. Perhaps because I was waiting for a resolution on patch 3.

The motivation for this patch was actually in anticipation of patch 3. In that patch, I followed the existing pattern inside of qemuDomainStorageSourceAccessModify() where we only set a 'revoke_foo' variable if the initial 'allow' call succeeds. In other words, if a function like qemuDomainStorageSourceAccessModifyNVMe() fails, we assume that it has left things in a consistent state and doesn't need to be revoked in the error path. So I followed the same pattern for this function.

If you think it would be better, I could just squash this patch with patch 3?

Jonathon



Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
  src/qemu/qemu_nbdkit.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 39f9c58a48..edbe2137d0 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -920,8 +920,11 @@ qemuNbdkitStartStorageSource(virQEMUDriver *driver,
          return qemuNbdkitStartStorageSourceOne(driver, vm, src);
for (backing = src; backing != NULL; backing = backing->backingStore) {
-        if (qemuNbdkitStartStorageSourceOne(driver, vm, backing) < 0)
+        if (qemuNbdkitStartStorageSourceOne(driver, vm, backing) < 0) {
+            /* roll back any previously-started sources */
+            qemuNbdkitStopStorageSource(src, vm, true);
              return -1;
+        }
      }
return 0;
--
2.43.0
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux