On Thu, Feb 08, 2024 at 15:11:25 -0600, Jonathon Jongsma wrote: > 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? Fair enough; Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx> _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
