Prevent custom storage volumes of type block to be used more than once

doc/api-extensions.md

@@ -2369,3 +2369,9 @@ This includes the following new endpoints (see [RESTful API](rest-api.md) for de
 This adds a new `lvmcluster` storage driver which makes use of LVM shared VG through `lvmlockd`.
 
 With this, it's possible to have a single shared LVM pool across multiple servers so long as they all see the same backing device(s).
+
+## `shared_custom_block_volumes`
+
+This adds a new configuration key `security.shared` to custom block volumes.
+If unset or `false`, the custom block volume cannot be attached to multiple instances.
+This feature was added to prevent data loss which can happen when custom block volumes are attached to multiple instances at once.

doc/reference/storage_btrfs.md

@@ -72,6 +72,7 @@ Key                             | Type      | Default                    | Descr
 
 Key                     | Type      | Condition                 | Default                                       | Description
 :--                     | :---      | :--------                 | :------                                       | :----------
+`security.shared`       | bool      | custom block volume       | same as `volume.security.shared` or `false`   | Enable sharing the volume across multiple instances
 `security.shifted`      | bool      | custom volume             | same as `volume.security.shifted` or `false`  | {{enable_ID_shifting}}
 `security.unmapped`     | bool      | custom volume             | same as `volume.security.unmapped` or `false` | Disable ID mapping for the volume
 `size`                  | string    | appropriate driver        | same as `volume.size`                         | Size/quota of the storage volume

doc/reference/storage_ceph.md

@@ -100,6 +100,7 @@ Key                     | Type      | Condition                 | Default
 :--                     | :---      | :--------                 | :------                                        | :----------
 `block.filesystem`      | string    | block-based volume with content type `filesystem` | same as `volume.block.filesystem`              | {{block_filesystem}}
 `block.mount_options`   | string    | block-based volume with content type `filesystem` | same as `volume.block.mount_options`           | Mount options for block-backed file system volumes
+`security.shared`       | bool      | custom block volume       | same as `volume.security.shared` or `false`    | Enable sharing the volume across multiple instances
 `security.shifted`      | bool      | custom volume             | same as `volume.security.shifted` or `false`   | {{enable_ID_shifting}}
 `security.unmapped`     | bool      | custom volume             | same as `volume.security.unmapped` or `false`  | Disable ID mapping for the volume
 `size`                  | string    |                           | same as `volume.size`                          | Size/quota of the storage volume

doc/reference/storage_cephfs.md

@@ -77,6 +77,7 @@ Key                           | Type                          | Default
 
 Key                     | Type      | Condition                 | Default                                        | Description
 :--                     | :---      | :--------                 | :------                                        | :----------
+`security.shared`       | bool      | custom block volume       | same as `volume.security.shared` or `false`    | Enable sharing the volume across multiple instances
 `security.shifted`      | bool      | custom volume             | same as `volume.security.shifted` or `false`   | {{enable_ID_shifting}}
 `security.unmapped`     | bool      | custom volume             | same as `volume.security.unmapped` or `false`  | Disable ID mapping for the volume
 `size`                  | string    | appropriate driver        | same as `volume.size`                          | Size/quota of the storage volume

doc/reference/storage_dir.md

@@ -37,6 +37,7 @@ Key                           | Type                          | Default
 
 Key                     | Type      | Condition                 | Default                                        | Description
 :--                     | :---      | :--------                 | :------                                        | :----------
+`security.shared`       | bool      | custom block volume       | same as `volume.security.shared` or `false`    | Enable sharing the volume across multiple instances
 `security.shifted`      | bool      | custom volume             | same as `volume.security.shifted` or `false`   | {{enable_ID_shifting}}
 `security.unmapped`     | bool      | custom volume             | same as `volume.security.unmapped` or `false`  | Disable ID mapping for the volume
 `size`                  | string    | appropriate driver        | same as `volume.size`                          | Size/quota of the storage volume

doc/reference/storage_lvm.md

@@ -89,6 +89,7 @@ Key                   | Type   | Condition
 `lvm.stripes.size`    | string |                                                   | same as `volume.lvm.stripes.size`              | Size of stripes to use (at least 4096 bytes and multiple of 512 bytes)
 `security.shifted`    | bool   | custom volume                                     | same as `volume.security.shifted` or `false`   | {{enable_ID_shifting}}
 `security.unmapped`   | bool   | custom volume                                     | same as `volume.security.unmapped` or `false`  | Disable ID mapping for the volume
+`security.shared`     | bool   | custom block volume                               | same as `volume.security.shared` or `false`    | Enable sharing the volume across multiple instances
 `size`                | string |                                                   | same as `volume.size`                          | Size/quota of the storage volume
 `snapshots.expiry`    | string | custom volume                                     | same as `volume.snapshots.expiry`              | {{snapshot_expiry_format}}
 `snapshots.pattern`   | string | custom volume                                     | same as `volume.snapshots.pattern` or `snap%d` | {{snapshot_pattern_format}} [^*]

doc/reference/storage_zfs.md

@@ -109,6 +109,7 @@ Key                     | Type      | Condition                 | Default
 :--                     | :---      | :--------                 | :------                                        | :----------
 `block.filesystem`      | string    | block-based volume with content type `filesystem` (`zfs.block_mode` enabled) | same as `volume.block.filesystem`              | {{block_filesystem}}
 `block.mount_options`   | string    | block-based volume with content type `filesystem` (`zfs.block_mode` enabled) | same as `volume.block.mount_options`           | Mount options for block-backed file system volumes
+`security.shared`       | bool      | custom block volume       | same as `volume.security.shared` or `false`    | Enable sharing the volume across multiple instances
 `security.shifted`      | bool      | custom volume             | same as `volume.security.shifted` or `false`   | {{enable_ID_shifting}}
 `security.unmapped`     | bool      | custom volume             | same as `volume.security.unmapped` or `false`  | Disable ID mapping for the volume
 `size`                  | string    |                           | same as `volume.size`                          | Size/quota of the storage volume

internal/server/device/config/devices.go

@@ -80,6 +80,10 @@ func NewDevices(nativeSet map[string]map[string]string) Devices {
 	for devName, devConfig := range nativeSet {
 		newDev := Device{}
 		for k, v := range devConfig {
+			if v == "" {
+				continue
+			}
+
 			newDev[k] = v
 		}
 
@@ -133,7 +137,7 @@ func (list Devices) Contains(k string, d Device) bool {
 // Update returns the difference between two device sets (removed, added, updated devices) and a list of all
 // changed keys across all devices. Accepts a function to return which keys can be live updated, which prevents
 // them being removed and re-added if the device supports live updates of certain keys.
-func (list Devices) Update(newlist Devices, updateFields func(Device, Device) []string) (map[string]Device, map[string]Device, map[string]Device, []string) {
+func (list Devices) Update(newlist Devices, updateFields func(Device, Device) []string) (Devices, Devices, Devices, []string) {
 	rmlist := map[string]Device{}
 	addlist := map[string]Device{}
 	updatelist := map[string]Device{}

internal/server/device/disk.go

@@ -299,34 +299,92 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 			return fmt.Errorf("Storage volumes cannot be specified as absolute paths")
 		}
 
-		// Only perform expensive instance pool volume checks when not validating a profile and after
-		// device expansion has occurred (to avoid doing it twice during instance load).
-		if d.inst != nil && !d.inst.IsSnapshot() && len(instConf.ExpandedDevices()) > 0 {
+		var dbVolume *db.StorageVolume
+		var storageProjectName string
+
+		if d.inst != nil && !d.inst.IsSnapshot() && d.config["source"] != "" && d.config["path"] != "/" {
 			d.pool, err = storagePools.LoadByName(d.state, d.config["pool"])
 			if err != nil {
 				return fmt.Errorf("Failed to get storage pool %q: %w", d.config["pool"], err)
 			}
 
+			// Derive the effective storage project name from the instance config's project.
+			storageProjectName, err = project.StorageVolumeProject(d.state.DB.Cluster, instConf.Project().Name, db.StoragePoolVolumeTypeCustom)
+			if err != nil {
+				return err
+			}
+
+			// GetStoragePoolVolume returns a volume with an empty Location field for remote drivers.
+			err = d.state.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
+				dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, d.config["source"], true)
+				return err
+			})
+			if err != nil {
+				return fmt.Errorf("Failed loading custom volume: %w", err)
+			}
+
+			// Check that block volumes are *only* attached to VM instances.
+			contentType, err := storagePools.VolumeContentTypeNameToContentType(dbVolume.ContentType)
+			if err != nil {
+				return err
+			}
+
+			// Check that only shared custom storage block volume are added to profiles, or multiple instances.
+			if util.IsFalseOrEmpty(dbVolume.Config["security.shared"]) && contentType == db.StoragePoolVolumeContentTypeBlock {
+				if instConf.Type() == instancetype.Any {
+					return fmt.Errorf("Cannot add un-shared custom storage block volume to profile")
+				}
+
+				var usedBy []string
+
+				err = storagePools.VolumeUsedByInstanceDevices(d.state, d.pool.Name(), storageProjectName, &dbVolume.StorageVolume, true, func(inst db.InstanceArgs, project api.Project, usedByDevices []string) error {
+					usedBy = append(usedBy, inst.Name)
+
+					return nil
+				})
+				if err != nil {
+					return err
+				}
+
+				if len(usedBy) > 0 {
+					return fmt.Errorf("Cannot add un-shared custom storage block volume to more than one instance")
+				}
+			}
+		}
+
+		// Only perform expensive instance pool volume checks when not validating a profile and after
+		// device expansion has occurred (to avoid doing it twice during instance load).
+		if d.inst != nil && !d.inst.IsSnapshot() && len(instConf.ExpandedDevices()) > 0 {
+			if d.pool == nil {
+				d.pool, err = storagePools.LoadByName(d.state, d.config["pool"])
+				if err != nil {
+					return fmt.Errorf("Failed to get storage pool %q: %w", d.config["pool"], err)
+				}
+			}
+
 			if d.pool.Status() == "Pending" {
 				return fmt.Errorf("Pool %q is pending", d.config["pool"])
 			}
 
 			// Custom volume validation.
 			if d.config["source"] != "" && d.config["path"] != "/" {
-				// Derive the effective storage project name from the instance config's project.
-				storageProjectName, err := project.StorageVolumeProject(d.state.DB.Cluster, instConf.Project().Name, db.StoragePoolVolumeTypeCustom)
-				if err != nil {
-					return err
+				if storageProjectName == "" {
+					// Derive the effective storage project name from the instance config's project.
+					storageProjectName, err = project.StorageVolumeProject(d.state.DB.Cluster, instConf.Project().Name, db.StoragePoolVolumeTypeCustom)
+					if err != nil {
+						return err
+					}
 				}
 
-				// GetStoragePoolVolume returns a volume with an empty Location field for remote drivers.
-				var dbVolume *db.StorageVolume
-				err = d.state.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
-					dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, d.config["source"], true)
-					return err
-				})
-				if err != nil {
-					return fmt.Errorf("Failed loading custom volume: %w", err)
+				if dbVolume == nil {
+					// GetStoragePoolVolume returns a volume with an empty Location field for remote drivers.
+					err = d.state.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
+						dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, d.config["source"], true)
+						return err
+					})
+					if err != nil {
+						return fmt.Errorf("Failed loading custom volume: %w", err)
+					}
 				}
 
 				// Check storage volume is available to mount on this cluster member.

internal/server/storage/backend.go

@@ -5381,6 +5381,39 @@ func (b *backend) UpdateCustomVolume(projectName string, volName string, newDesc
 			}
 		}
 
+		sharedVolume, ok := changedConfig["security.shared"]
+		if ok && util.IsFalseOrEmpty(sharedVolume) {
+			var usedByProfileDevices []api.Profile
+
+			err = VolumeUsedByProfileDevices(b.state, b.name, projectName, &curVol.StorageVolume, func(profileID int64, profile api.Profile, project api.Project, usedByDevices []string) error {
+				usedByProfileDevices = append(usedByProfileDevices, profile)
+
+				return nil
+			})
+			if err != nil {
+				return err
+			}
+
+			if len(usedByProfileDevices) > 0 {
+				return fmt.Errorf("Cannot un-share custom storage block volume if attached to profile")
+			}
+
+			var usedByInstanceDevices []string
+
+			err = VolumeUsedByInstanceDevices(b.state, b.name, projectName, &curVol.StorageVolume, true, func(inst db.InstanceArgs, project api.Project, usedByDevices []string) error {
+				usedByInstanceDevices = append(usedByInstanceDevices, inst.Name)
+
+				return nil
+			})
+			if err != nil {
+				return err
+			}
+
+			if len(usedByInstanceDevices) > 1 {
+				return fmt.Errorf("Cannot un-share custom storage block volume if attached to more than one instance")
+			}
+		}
+
 		curVol := b.GetVolume(drivers.VolumeTypeCustom, contentType, volStorageName, curVol.Config)
 		if !userOnly {
 			err = b.driver.UpdateVolume(curVol, changedConfig)

internal/server/storage/drivers/driver_common.go

@@ -125,6 +125,11 @@ func (d *common) fillVolumeConfig(vol *Volume, excludedKeys ...string) error {
 			continue
 		}
 
+		// security.shared is only relevant for custom block volumes.
+		if (vol.Type() != VolumeTypeCustom || vol.ContentType() != ContentTypeBlock) && (volKey == "security.shared") {
+			continue
+		}
+
 		if vol.config[volKey] == "" {
 			vol.config[volKey] = d.config[k]
 		}

internal/server/storage/utils.go

@@ -494,6 +494,11 @@ func poolAndVolumeCommonRules(vol *drivers.Volume) map[string]func(string) error
 		rules["security.unmapped"] = validate.Optional(validate.IsBool)
 	}
 
+	// security.shared is only relevant for custom block volumes.
+	if (vol == nil) || (vol != nil && vol.Type() == drivers.VolumeTypeCustom && vol.ContentType() == drivers.ContentTypeBlock) {
+		rules["security.shared"] = validate.Optional(validate.IsBool)
+	}
+
 	return rules
 }
 

internal/version/api.go

@@ -399,6 +399,7 @@ var APIExtensions = []string{
 	"image_template_permissions",
 	"storage_bucket_backup",
 	"storage_lvm_cluster",
+	"shared_custom_block_volumes",
 }
 
 // APIExtensionsCount returns the number of available API extensions.