Prevent custom block volume sharing

doc/api-extensions.md

@@ -2424,3 +2424,9 @@ The OVN driver will allocate IP addresses from the subnets specified in the upli
 
 Adds the ability to explicitly specify a trust token when creating a certificate
 and joining an existing cluster.
+
+## `shared_custom_block_volumes`
+
+This adds a 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/config_options.txt

@@ -4754,6 +4754,15 @@ prior to creating the storage pool.
 
 <!-- config group storage-btrfs-pool-conf end -->
 <!-- config group storage-btrfs-volume-conf start -->
+```{config:option} security.shared storage-btrfs-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-btrfs-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -4909,6 +4918,15 @@ If not set, `ext4` is assumed.
 
 ```
 
+```{config:option} security.shared storage-ceph-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-ceph-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -5049,6 +5067,15 @@ when creating a missing OSD pool.
 
 <!-- config group storage-cephfs-pool-conf end -->
 <!-- config group storage-cephfs-volume-conf start -->
+```{config:option} security.shared storage-cephfs-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-cephfs-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -5188,6 +5215,15 @@ to be placed on the socket I/O.
 
 <!-- config group storage-dir-pool-conf end -->
 <!-- config group storage-dir-volume-conf start -->
+```{config:option} security.shared storage-dir-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-dir-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -5374,6 +5410,15 @@ If not set, `ext4` is assumed.
 The size must be at least 4096 bytes, and a multiple of 512 bytes.
 ```
 
+```{config:option} security.shared storage-lvm-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-lvm-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -5552,6 +5597,15 @@ If not set, `ext4` is assumed.
 
 ```
 
+```{config:option} security.shared storage-powerflex-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-powerflex-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"
@@ -5697,6 +5751,15 @@ If not set, `ext4` is assumed.
 
 ```
 
+```{config:option} security.shared storage-zfs-volume-conf
+:condition: "custom block volume"
+:defaultdesc: "same as `volume.security.shared` or `false`"
+:shortdesc: "Enable volume sharing"
+:type: "bool"
+Enabling this option allows sharing the volume across multiple instances despite the possibility of data loss.
+
+```
+
 ```{config:option} security.shifted storage-zfs-volume-conf
 :condition: "custom volume"
 :defaultdesc: "same as `volume.security.shifted` or `false`"

doc/explanation/storage.md

@@ -132,7 +132,7 @@ Storage volumes can be of the following types:
 
 `custom`
 : You can add one or more custom storage volumes to hold data that you want to store separately from your instances.
-  Custom storage volumes can be shared between instances, and they are retained until you delete them.
+  Custom storage volumes of content type `filesystem` or `iso` can be shared between instances, and they are retained until you delete them.
 
   You can also use custom storage volumes to hold your backups or images.
 
@@ -154,7 +154,8 @@ Each storage volume uses one of the following content types:
   You can create a custom storage volume of type `block` by using the `--type=block` flag.
 
   Custom storage volumes of content type `block` can only be attached to virtual machines.
-  They should not be shared between instances, because simultaneous access can lead to data corruption.
+  By default, they can only be attached to one instance at a time, because simultaneous access can lead to data corruption.
+  Sharing a custom storage volumes of content type `block` is made possible through the usage of the `security.shared` configuration key.
 
 `iso`
 : This content type is used for custom ISO volumes.

doc/howto/storage_volumes.md

@@ -53,6 +53,7 @@ The following restrictions apply:
 - To avoid data corruption, storage volumes of {ref}`content type <storage-content-types>` `block` should never be attached to more than one virtual machine at a time.
 - Storage volumes of {ref}`content type <storage-content-types>` `iso` are always read-only, and can therefore be attached to more than one virtual machine at a time without corrupting data.
 - File system storage volumes can't be attached to virtual machines while they're running.
+- Custom block storage volumes that don't have `security.shared` enabled cannot be attached to more than one instance at the same time and neither can be attached to profiles.
 
 For custom storage volumes with the content type `filesystem`, use the following command, where `<location>` is the path for accessing the storage volume inside the instance (for example, `/data`):
 

lxd/db/instances.go

@@ -216,8 +216,8 @@ func (c *ClusterTx) GetInstancesByMemberAddress(ctx context.Context, offlineThre
 	return memberAddressInstances, nil
 }
 
-// ErrInstanceListStop used as return value from InstanceList's instanceFunc when prematurely stopping the search.
-var ErrInstanceListStop = fmt.Errorf("search stopped")
+// ErrListStop used as return value from InstanceList's instanceFunc when prematurely stopping the search.
+var ErrListStop = fmt.Errorf("search stopped")
 
 // InstanceList loads all instances across all projects and for each instance runs the instanceFunc passing in the
 // instance and it's project and profiles. Accepts optional filter arguments to specify a subset of instances.

lxd/device/config/devices.go

@@ -141,7 +141,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) (removedList Devices, addedList Devices, updatedList Devices, changedKeys []string) {
 	rmlist := map[string]Device{}
 	addlist := map[string]Device{}
 	updatelist := map[string]Device{}

lxd/device/disk.go

@@ -157,6 +157,36 @@ func (d *disk) sourceIsLocalPath(source string) bool {
 	return true
 }
 
+// Check that unshared custom storage block volumes are not added to profiles or multiple instances.
+func (d *disk) checkBlockVolSharing(instanceType instancetype.Type, projectName string, volume *api.StorageVolume) error {
+	// Skip the checks if the volume is set to be shared or is not a block volume.
+	if volume.ContentType != cluster.StoragePoolVolumeContentTypeNameBlock || shared.IsTrue(volume.Config["security.shared"]) {
+		return nil
+	}
+
+	if instanceType == instancetype.Any {
+		return fmt.Errorf("Cannot add custom storage block volume to profiles if security.shared is false or unset")
+	}
+
+	err := storagePools.VolumeUsedByInstanceDevices(d.state, d.pool.Name(), projectName, volume, true, func(inst db.InstanceArgs, project api.Project, usedByDevices []string) error {
+		// Don't count the current instance.
+		if d.inst != nil && d.inst.Project().Name == inst.Project && d.inst.Name() == inst.Name {
+			return nil
+		}
+
+		return db.ErrListStop
+	})
+	if err != nil {
+		if err == db.ErrListStop {
+			return fmt.Errorf("Cannot add custom storage block volume to more than one instance if security.shared is false or unset")
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 // validateConfig checks the supplied config for correctness.
 func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	if !instanceSupported(instConf.Type(), instancetype.Container, instancetype.VM) {
@@ -358,7 +388,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 	}
 
 	if d.config["source"] == "" && d.config["path"] != "/" {
-		return fmt.Errorf(`Disk entry is missing the required "source" or "path" property`)
+		return fmt.Errorf(`Non root disk devices require the "source" property`)
 	}
 
 	if d.config["path"] == "/" && d.config["source"] != "" {
@@ -421,6 +451,7 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 		return fmt.Errorf("Missing source path %q for disk %q", d.config["source"], d.name)
 	}
 
+	// Check if validating a storage volume disk.
 	if d.config["pool"] != "" {
 		if d.config["shift"] != "" {
 			return fmt.Errorf(`The "shift" property cannot be used with custom storage volumes (set "security.shifted=true" on the volume instead)`)
@@ -430,38 +461,51 @@ 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 dbCustomVolume *db.StorageVolume
+		var storageProjectName string
+
+		// Check if validating an instance or a custom storage volume attached to a profile.
+		if (d.inst != nil && !d.inst.IsSnapshot()) || (d.inst == nil && instConf.Type() == instancetype.Any && !instancetype.IsRootDiskDevice(d.config)) {
 			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"] != "/" {
+			if !instancetype.IsRootDiskDevice(d.config) {
 				// Derive the effective storage project name from the instance config's project.
-				storageProjectName, err := project.StorageVolumeProject(d.state.DB.Cluster, instConf.Project().Name, cluster.StoragePoolVolumeTypeCustom)
+				storageProjectName, err = project.StorageVolumeProject(d.state.DB.Cluster, instConf.Project().Name, cluster.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, cluster.StoragePoolVolumeTypeCustom, d.config["source"], true)
+					dbCustomVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, cluster.StoragePoolVolumeTypeCustom, d.config["source"], true)
 					return err
 				})
 				if err != nil {
 					return fmt.Errorf("Failed loading custom volume: %w", err)
 				}
 
+				err := d.checkBlockVolSharing(instConf.Type(), storageProjectName, &dbCustomVolume.StorageVolume)
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		// 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.Status() == "Pending" {
+				return fmt.Errorf("Pool %q is pending", d.config["pool"])
+			}
+
+			// Custom volume validation.
+			if dbCustomVolume != nil {
 				// Check storage volume is available to mount on this cluster member.
-				remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(d.state, d.config["pool"], storageProjectName, &dbVolume.StorageVolume)
+				remoteInstance, err := storagePools.VolumeUsedByExclusiveRemoteInstancesWithProfiles(d.state, d.config["pool"], storageProjectName, &dbCustomVolume.StorageVolume)
 				if err != nil {
 					return fmt.Errorf("Failed checking if custom volume is exclusively attached to another instance: %w", err)
 				}
@@ -471,20 +515,15 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 				}
 
 				// Check that block volumes are *only* attached to VM instances.
-				contentType, err := storagePools.VolumeContentTypeNameToContentType(dbVolume.ContentType)
-				if err != nil {
-					return err
-				}
-
-				if contentType == cluster.StoragePoolVolumeContentTypeBlock {
+				if dbCustomVolume.ContentType == cluster.StoragePoolVolumeContentTypeNameBlock {
 					if instConf.Type() == instancetype.Container {
 						return fmt.Errorf("Custom block volumes cannot be used on containers")
 					}
 
 					if d.config["path"] != "" {
 						return fmt.Errorf("Custom block volumes cannot have a path defined")
 					}
-				} else if contentType == cluster.StoragePoolVolumeContentTypeISO {
+				} else if dbCustomVolume.ContentType == cluster.StoragePoolVolumeContentTypeNameISO {
 					if instConf.Type() == instancetype.Container {
 						return fmt.Errorf("Custom ISO volumes cannot be used on containers")
 					}