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 <[email protected]>
_______________________________________________
Devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to