Se/retry failed deletes

internal/server/storage/drivers/driver_truenas_utils.go

@@ -1,6 +1,7 @@
 package drivers
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -12,6 +13,7 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/lxc/incus/v6/shared/api"
+	"github.com/lxc/incus/v6/shared/logger"
 	"github.com/lxc/incus/v6/shared/revert"
 	"github.com/lxc/incus/v6/shared/subprocess"
 	"github.com/lxc/incus/v6/shared/util"
@@ -493,6 +495,36 @@ func (d *truenas) deleteSnapshot(snapshot string, recursive bool, options ...str
 	return d.deleteDataset(snapshot, recursive, options...)
 }
 
+// tryDeleteDataset attempts to delete a dataset, repeating if busy until success, or the context is ended
+func (d *truenas) tryDeleteBusyDataset(ctx context.Context, dataset string, recursive bool, options ...string) error {
+	for {
+		if ctx.Err() != nil {
+			return fmt.Errorf("Failed to delete dataset for %q: %w", dataset, ctx.Err())
+		}
+
+		// we sometimes we recieve a "busy" error when deleting... which I think is a race, although iSCSI should've finished with the zvol by the time
+		// deleteIscsiShare returns, maybe it hasn't yet... so we retry... in general if incus is calling deleteDataset it shouldn't be busy.
+		err := d.deleteDataset(dataset, recursive, options...)
+		if err == nil {
+			return nil
+		}
+
+		/*
+			Error -32001
+			Method call error
+			[EBUSY] Failed to delete dataset: cannot destroy '<dataset>': dataset is busy)
+		*/
+		if !strings.Contains(err.Error(), "[EBUSY]") {
+			return err
+		}
+
+		d.logger.Warn("Error while trying to delete dataset, will retry", logger.Ctx{"dataset": dataset, "err": err})
+
+		// was busy, lets try again.
+		time.Sleep(500 * time.Millisecond)
+	}
+}
+
 func (d *truenas) deleteDataset(dataset string, recursive bool, options ...string) error {
 	args := []string{d.getDatasetOrSnapshot(dataset), "delete"}
 
@@ -630,8 +662,10 @@ func (d *truenas) deleteDatasetRecursive(dataset string) error {
 		return err
 	}
 
-	// Delete the dataset (and any snapshots left).
-	err = d.deleteDataset(dataset, true)
+	// Try delete the dataset (and any snapshots left), waiting up to 5 seconds if its busy
+	ctx, cancel := context.WithTimeout(d.state.ShutdownCtx, 5*time.Second)
+	defer cancel()
+	err = d.tryDeleteBusyDataset(ctx, dataset, true)
 	if err != nil {
 		return err
 	}

internal/server/storage/drivers/driver_truenas_volumes.go

@@ -1034,15 +1034,24 @@ func (d *truenas) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool
 	return nil
 }
 
+// getTempSnapshotVolName returns a derived volume name for the server specific clone of the specified snapshot volume
+func (d *truenas) getTempSnapshotVolName(vol Volume) string {
+	parent, snapshotOnlyName, _ := api.GetParentAndSnapshotName(vol.Name())
+	parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig)
+	parentDataset := d.dataset(parentVol, false)
+
+	// serverName to allow other cluster members to mount the same snapshot at the same time.
+	dataset := fmt.Sprintf("%s_%s_%s%s", parentDataset, snapshotOnlyName, d.state.ServerName, tmpVolSuffix)
+
+	return dataset
+}
+
 // GetVolumeDiskPath returns the location of a root disk block device.
 func (d *truenas) GetVolumeDiskPath(vol Volume) (string, error) {
 	var dataset string
 
 	if vol.IsSnapshot() {
-		parent, snapshotOnlyName, _ := api.GetParentAndSnapshotName(vol.Name())
-		parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig)
-		parentDataset := d.dataset(parentVol, false)
-		dataset = fmt.Sprintf("%s_%s%s", parentDataset, snapshotOnlyName, tmpVolSuffix)
+		dataset = d.getTempSnapshotVolName(vol)
 	} else {
 		dataset = d.dataset(vol, false)
 	}
@@ -1545,22 +1554,21 @@ func (d *truenas) MountVolumeSnapshot(snapVol Volume, op *operations.Operation)
 	}
 
 	srcSnapshot := d.dataset(snapVol, false)
-
-	parent, snapshotOnlyName, _ := api.GetParentAndSnapshotName(snapVol.Name())
-	parentVol := NewVolume(d, d.Name(), snapVol.volType, snapVol.contentType, parent, snapVol.config, snapVol.poolConfig)
-	parentDataset := d.dataset(parentVol, false)
-	cloneDataset := fmt.Sprintf("%s_%s%s", parentDataset, snapshotOnlyName, tmpVolSuffix)
+	cloneDataset := d.getTempSnapshotVolName(snapVol)
 
 	// Create a temporary clone from the snapshot.
 	err = d.cloneSnapshot(srcSnapshot, cloneDataset)
-
+	if err != nil {
+		return err
+	}
 	reverter.Add(func() { _ = d.deleteDatasetRecursive(cloneDataset) })
 
 	// and share the clone
 	err = d.createIscsiShare(cloneDataset, snapVol.contentType != ContentTypeFS) // ro if not FS
 	if err != nil {
 		return err
 	}
+	reverter.Add(func() { _ = d.deleteIscsiShare(cloneDataset) })
 
 	// and then activate
 	volDevPath, err := d.activateIscsiDataset(cloneDataset)
@@ -1659,17 +1667,14 @@ func (d *truenas) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation
 		l.Debug("Unmounted TrueNAS snapshot volume filesystem", logger.Ctx{"path": mountPath})
 	}
 
-	parent, snapshotOnlyName, _ := api.GetParentAndSnapshotName(snapVol.Name())
-	parentVol := NewVolume(d, d.Name(), snapVol.volType, snapVol.contentType, parent, snapVol.config, snapVol.poolConfig)
-	parentDataset := d.dataset(parentVol, false)
-	cloneDataset := fmt.Sprintf("%s_%s%s", parentDataset, snapshotOnlyName, tmpVolSuffix)
+	cloneDataset := d.getTempSnapshotVolName(snapVol)
 
 	l.Debug("Deleting temporary TrueNAS snapshot volume")
 
-	// Deactivate
-	err = d.deactivateIscsiDataset(cloneDataset)
+	// Deactivate & Delete iSCSI share
+	err = d.deleteIscsiShare(cloneDataset)
 	if err != nil {
-		return false, fmt.Errorf("Could not deactivate temporary snapshot volume: %w", err)
+		return false, fmt.Errorf("Could not delete iscsi target for temporary snapshot volume: %w", err)
 	}
 
 	// Destroy clone