Merge pull request #1092 from stgraber/main

incusd/device/disk: Allow relative paths within custom volumes

doc/api-extensions.md

@@ -2542,3 +2542,7 @@ The new configuration keys are:
 
 * `instances.vm.cpu.ARCHITECTURE.baseline`
 * `instances.vm.cpu.ARCHITECTURE.flag`
+
+## `disk_volume_subpath`
+
+This introduces the ability to access the sub-path of a file system custom volume by using the `source=volume/path` syntax.

doc/reference/devices_disk.md

@@ -28,6 +28,8 @@ Storage volume
   Alternatively, you can use the [`incus storage volume attach`](incus_storage_volume_attach.md) command to {ref}`storage-attach-volume`.
   Both commands use the same mechanism to add a storage volume as a disk device.
 
+  It's possible to attach a sub-path of a custom volume to an instance using the `source=<volume_name>/<sub_path>` syntax.
+
 Path on the host
 : You can share a path on your host (either a file system or a block device) to your instance by adding it as a disk device with the host path as the `source`:
 

internal/server/device/disk.go

@@ -468,9 +468,13 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 				return err
 			}
 
+			// Parse the volume name and path.
+			volFields := strings.SplitN(d.config["source"], "/", 2)
+			volName := volFields[0]
+
 			// 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)
+				dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, volName, true)
 				return err
 			})
 			if err != nil {
@@ -536,9 +540,13 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
 				}
 
 				if dbVolume == nil {
+					// Parse the volume name and path.
+					volFields := strings.SplitN(d.config["source"], "/", 2)
+					volName := volFields[0]
+
 					// 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)
+						dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, volName, true)
 						return err
 					})
 					if err != nil {
@@ -747,8 +755,12 @@ func (d *disk) Register() error {
 			return err
 		}
 
+		// Parse the volume name and path.
+		volFields := strings.SplitN(d.config["source"], "/", 2)
+		volName := volFields[0]
+
 		// Try to mount the volume that should already be mounted to reinitialize the ref counter.
-		_, err = d.pool.MountCustomVolume(storageProjectName, d.config["source"], nil)
+		_, err = d.pool.MountCustomVolume(storageProjectName, volName, nil)
 		if err != nil {
 			return err
 		}
@@ -873,9 +885,13 @@ func (d *disk) startContainer() (*deviceConfig.RunConfig, error) {
 				return nil, err
 			}
 
+			// Parse the volume name and path.
+			volFields := strings.SplitN(d.config["source"], "/", 2)
+			volName := volFields[0]
+
 			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)
+				dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, volName, true)
 				return err
 			})
 			if err != nil {
@@ -1147,10 +1163,14 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 					return nil, err
 				}
 
+				// Parse the volume name and path.
+				volFields := strings.SplitN(d.config["source"], "/", 2)
+				volName := volFields[0]
+
 				// 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)
+					dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, volName, true)
 					return err
 				})
 				if err != nil {
@@ -1665,63 +1685,34 @@ func (d *disk) mountPoolVolume() (func(), string, *storagePools.MountInfo, error
 	var mountInfo *storagePools.MountInfo
 
 	// Deal with mounting storage volumes created via the storage api. Extract the name of the storage volume
-	// that we are supposed to attach. We assume that the only syntactically valid ways of specifying a
-	// storage volume are:
-	// - <volume_name>
-	// - <type>/<volume_name>
-	// Currently, <type> must either be empty or "custom".
-	// We do not yet support instance mounts.
+	// that we are supposed to attach.
 	if filepath.IsAbs(d.config["source"]) {
 		return nil, "", nil, fmt.Errorf(`When the "pool" property is set "source" must specify the name of a volume, not a path`)
 	}
 
-	volumeTypeName := ""
-	volumeName := filepath.Clean(d.config["source"])
-	slash := strings.Index(volumeName, "/")
-	if (slash > 0) && (len(volumeName) > slash) {
-		// Extract volume name.
-		volumeName = d.config["source"][(slash + 1):]
-		// Extract volume type.
-		volumeTypeName = d.config["source"][:slash]
-	}
-
-	var srcPath string
-
-	// Check volume type name is custom.
-	switch volumeTypeName {
-	case db.StoragePoolVolumeTypeNameContainer:
-		return nil, "", nil, fmt.Errorf("Using instance storage volumes is not supported")
-	case "":
-		// We simply received the name of a storage volume.
-		volumeTypeName = db.StoragePoolVolumeTypeNameCustom
-		fallthrough
-	case db.StoragePoolVolumeTypeNameCustom:
-		break
-	case db.StoragePoolVolumeTypeNameImage:
-		return nil, "", nil, fmt.Errorf("Using image storage volumes is not supported")
-	default:
-		return nil, "", nil, fmt.Errorf("Unknown storage type prefix %q found", volumeTypeName)
-	}
+	// Parse the volume name and path.
+	volFields := strings.SplitN(d.config["source"], "/", 2)
+	volName := volFields[0]
 
 	// Only custom volumes can be attached currently.
 	storageProjectName, err := project.StorageVolumeProject(d.state.DB.Cluster, d.inst.Project().Name, db.StoragePoolVolumeTypeCustom)
 	if err != nil {
 		return nil, "", nil, err
 	}
 
-	volStorageName := project.StorageVolume(storageProjectName, volumeName)
-	srcPath = storageDrivers.GetVolumeMountPath(d.config["pool"], storageDrivers.VolumeTypeCustom, volStorageName)
+	volStorageName := project.StorageVolume(storageProjectName, volName)
+	srcPath := storageDrivers.GetVolumeMountPath(d.config["pool"], storageDrivers.VolumeTypeCustom, volStorageName)
 
-	mountInfo, err = d.pool.MountCustomVolume(storageProjectName, volumeName, nil)
+	mountInfo, err = d.pool.MountCustomVolume(storageProjectName, volName, nil)
 	if err != nil {
-		return nil, "", nil, fmt.Errorf("Failed mounting storage volume %q of type %q on storage pool %q: %w", volumeName, volumeTypeName, d.pool.Name(), err)
+		return nil, "", nil, fmt.Errorf("Failed mounting custom storage volume %q on storage pool %q: %w", volName, d.pool.Name(), err)
 	}
 
-	revert.Add(func() { _, _ = d.pool.UnmountCustomVolume(storageProjectName, volumeName, nil) })
+	revert.Add(func() { _, _ = d.pool.UnmountCustomVolume(storageProjectName, volName, nil) })
 
 	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, volumeName, true)
+		dbVolume, err = tx.GetStoragePoolVolume(ctx, d.pool.ID(), storageProjectName, db.StoragePoolVolumeTypeCustom, volName, true)
 		return err
 	})
 	if err != nil {
@@ -1730,17 +1721,17 @@ func (d *disk) mountPoolVolume() (func(), string, *storagePools.MountInfo, error
 
 	if d.inst.Type() == instancetype.Container {
 		if dbVolume.ContentType == db.StoragePoolVolumeContentTypeNameFS {
-			err = d.storagePoolVolumeAttachShift(storageProjectName, d.pool.Name(), volumeName, db.StoragePoolVolumeTypeCustom, srcPath)
+			err = d.storagePoolVolumeAttachShift(storageProjectName, d.pool.Name(), volName, db.StoragePoolVolumeTypeCustom, srcPath)
 			if err != nil {
-				return nil, "", nil, fmt.Errorf("Failed shifting storage volume %q of type %q on storage pool %q: %w", volumeName, volumeTypeName, d.pool.Name(), err)
+				return nil, "", nil, fmt.Errorf("Failed shifting custom storage volume %q on storage pool %q: %w", volName, d.pool.Name(), err)
 			}
 		} else {
 			return nil, "", nil, fmt.Errorf("Only filesystem volumes are supported for containers")
 		}
 	}
 
 	if dbVolume.ContentType == db.StoragePoolVolumeContentTypeNameBlock || dbVolume.ContentType == db.StoragePoolVolumeContentTypeNameISO {
-		srcPath, err = d.pool.GetCustomVolumeDisk(storageProjectName, volumeName)
+		srcPath, err = d.pool.GetCustomVolumeDisk(storageProjectName, volName)
 		if err != nil {
 			return nil, "", nil, fmt.Errorf("Failed to get disk path: %w", err)
 		}
@@ -1841,6 +1832,39 @@ func (d *disk) createDevice(srcPath string) (func(), string, bool, error) {
 
 			srcPath = fmt.Sprintf("/proc/self/fd/%d", f.Fd())
 		}
+	} else if d.config["source"] != "" {
+		// Handle mounting a sub-path.
+		volFields := strings.SplitN(d.config["source"], "/", 2)
+		if len(volFields) == 2 {
+			subPath := volFields[1]
+
+			// Open file handle to parent for use with openat2 later.
+			// Has to use unix.O_PATH to support directories and sockets.
+			volPath, err := os.OpenFile(srcPath, unix.O_PATH, 0)
+			if err != nil {
+				return nil, "", false, fmt.Errorf("Failed opening volume path %q: %w", srcPath, err)
+			}
+
+			defer func() { _ = volPath.Close() }()
+
+			// Use openat2 to prevent resolving to a mount path outside of the volume.
+			fd, err := unix.Openat2(int(volPath.Fd()), subPath, &unix.OpenHow{
+				Flags:   unix.O_PATH | unix.O_CLOEXEC,
+				Resolve: unix.RESOLVE_BENEATH | unix.RESOLVE_NO_MAGICLINKS,
+			})
+			if err != nil {
+				if errors.Is(err, unix.EXDEV) {
+					return nil, "", false, fmt.Errorf("Volume sub-path %q resolves outside of the volume", subPath)
+				}
+
+				return nil, "", false, fmt.Errorf("Failed opening volume sub-path %q: %w", subPath, err)
+			}
+
+			srcPathFd := os.NewFile(uintptr(fd), subPath)
+			defer func() { _ = srcPathFd.Close() }()
+
+			srcPath = fmt.Sprintf("/proc/self/fd/%d", srcPathFd.Fd())
+		}
 	}
 
 	// Create the devices directory if missing.
@@ -2167,7 +2191,11 @@ func (d *disk) postStop() error {
 			return err
 		}
 
-		_, err = d.pool.UnmountCustomVolume(storageProjectName, d.config["source"], nil)
+		// Parse the volume name and path.
+		volFields := strings.SplitN(d.config["source"], "/", 2)
+		volName := volFields[0]
+
+		_, err = d.pool.UnmountCustomVolume(storageProjectName, volName, nil)
 		if err != nil && !errors.Is(err, storageDrivers.ErrInUse) {
 			return err
 		}

internal/version/api.go

@@ -430,6 +430,7 @@ var APIExtensions = []string{
 	"clustering_groups_config",
 	"instances_lxcfs_per_instance",
 	"clustering_groups_vm_cpu_definition",
+	"disk_volume_subpath",
 }
 
 // APIExtensionsCount returns the number of available API extensions.

test/suites/container_devices_disk.sh

@@ -5,6 +5,7 @@ test_container_devices_disk() {
   incus init testimage foo
 
   test_container_devices_disk_shift
+  test_container_devices_disk_subpath
   test_container_devices_raw_mount_options
   test_container_devices_disk_ceph
   test_container_devices_disk_cephfs
@@ -182,3 +183,46 @@ test_container_devices_disk_char() {
   incus config device remove foo char
   incus stop foo -f
 }
+
+test_container_devices_disk_subpath() {
+  POOL=$(incus profile device get default root pool)
+
+  # Create a test volume and main container
+  incus storage volume create "${POOL}" foo
+  incus launch testimage foo-main
+  incus config device add foo-main foo disk pool="${POOL}" source=foo path=/foo
+
+  # Create some entries
+  incus exec foo-main -- mkdir /foo/path1 /foo/path2
+  incus exec foo-main -- ln -s /etc /foo/path3
+  incus exec foo-main -- ln -s path1 /foo/path4
+  echo path1 | incus file push - foo-main/foo/path1/hello
+  echo path2 | incus file push - foo-main/foo/path2/hello
+
+  # Create some test containers
+  incus create testimage foo-path1
+  incus config device add foo-path1 foo disk pool="${POOL}" source=foo/path1 path=/foo
+
+  incus create testimage foo-path2
+  incus config device add foo-path2 foo disk pool="${POOL}" source=foo/path2 path=/foo
+
+  incus create testimage foo-path3
+  incus config device add foo-path3 foo disk pool="${POOL}" source=foo/path3 path=/foo
+
+  incus create testimage foo-path4
+  incus config device add foo-path4 foo disk pool="${POOL}" source=foo/path4 path=/foo
+
+  # Validation
+  incus start foo-path1
+  incus start foo-path2
+  ! incus start foo-path3 || false
+  incus start foo-path4
+
+  [ "$(incus file pull foo-path1/foo/hello -)" = "path1" ]
+  [ "$(incus file pull foo-path2/foo/hello -)" = "path2" ]
+  [ "$(incus file pull foo-path4/foo/hello -)" = "path1" ]
+
+  # Cleanup
+  incus delete -f foo-main foo-path1 foo-path2 foo-path3 foo-path4
+  incus storage volume delete "${POOL}" foo
+}