From 1bf6ec2ee87d1eed2c1015babc640903c5aca63c Mon Sep 17 00:00:00 2001 From: Eugene Kalinin Date: Thu, 2 Jul 2026 12:22:52 +0300 Subject: [PATCH] additionalDisks: support custom mountPoint and optional mounting Additional disks were always mounted at the hardcoded path /mnt/lima-, with no way to change that location or to attach a disk without Lima mounting it. Issue #4065 asks for both: a custom mount point and an attach-only mode so the guest can manage the disk itself (a flexibility Colima can benefit from). Add two optional, backwards-compatible fields to each additionalDisks entry: - mountPoint: absolute guest path to mount at (default /mnt/lima-) - mount: whether Lima auto-mounts the disk (default true; false attaches the disk without mounting it) A separate `mount` boolean is used rather than overloading an empty mountPoint, per the discussion in #4065. nil for either field preserves the current behavior. Values are passed to the guest via LIMA_CIDATA_DISK__MOUNT / _MOUNTPOINT and honored by 05-lima-disks.sh (non-swap disks only). mountPoint is validated to be an absolute guest path (path.IsAbs, not the host-dependent filepath.IsAbs) and neither a critical system path nor the reserved guest home directory. Fixes #4065 Signed-off-by: Eugene Kalinin --- .../boot.Linux/05-lima-disks.sh | 10 +++-- pkg/cidata/cidata.TEMPLATE.d/lima.env | 2 + pkg/cidata/cidata.go | 20 +++++++--- pkg/cidata/template.go | 12 +++--- pkg/cidata/template_test.go | 39 +++++++++++++++++++ pkg/limatmpl/embed.go | 10 +++++ pkg/limatmpl/embed_test.go | 6 +++ pkg/limatype/lima_yaml.go | 10 +++-- pkg/limayaml/validate.go | 15 +++++++ pkg/limayaml/validate_test.go | 37 ++++++++++++++++++ templates/default.yaml | 11 +++++- website/content/en/docs/dev/internals.md | 2 + 12 files changed, 155 insertions(+), 19 deletions(-) diff --git a/pkg/cidata/cidata.TEMPLATE.d/boot.Linux/05-lima-disks.sh b/pkg/cidata/cidata.TEMPLATE.d/boot.Linux/05-lima-disks.sh index 77e36b7e..2053c760 100644 --- a/pkg/cidata/cidata.TEMPLATE.d/boot.Linux/05-lima-disks.sh +++ b/pkg/cidata/cidata.TEMPLATE.d/boot.Linux/05-lima-disks.sh @@ -18,9 +18,13 @@ for i in $(seq 0 $((LIMA_CIDATA_DISKS - 1))); do FORMAT_DISK="$(get_disk_var "$i" "FORMAT")" FORMAT_FSTYPE="$(get_disk_var "$i" "FSTYPE")" FORMAT_FSARGS="$(get_disk_var "$i" "FSARGS")" + MOUNT_DISK="$(get_disk_var "$i" "MOUNT")" + MOUNT_POINT="$(get_disk_var "$i" "MOUNTPOINT")" test -n "$FORMAT_DISK" || FORMAT_DISK=true test -n "$FORMAT_FSTYPE" || FORMAT_FSTYPE=ext4 + test -n "$MOUNT_DISK" || MOUNT_DISK=true + test -n "$MOUNT_POINT" || MOUNT_POINT="/mnt/lima-${DISK_NAME}" # first time setup if [[ ! -b "/dev/disk/by-label/lima-${DISK_NAME}" ]]; then @@ -39,9 +43,9 @@ for i in $(seq 0 $((LIMA_CIDATA_DISKS - 1))); do if [ "$FORMAT_FSTYPE" == "swap" ]; then swapon "/dev/${DEVICE_NAME}1" - else - mkdir -p "/mnt/lima-${DISK_NAME}" - mount -t "$FORMAT_FSTYPE" "/dev/${DEVICE_NAME}1" "/mnt/lima-${DISK_NAME}" + elif $MOUNT_DISK; then + mkdir -p "$MOUNT_POINT" + mount -t "$FORMAT_FSTYPE" "/dev/${DEVICE_NAME}1" "$MOUNT_POINT" fi if command -v growpart >/dev/null 2>&1 && command -v resize2fs >/dev/null 2>&1; then growpart "/dev/${DEVICE_NAME}" 1 || true diff --git a/pkg/cidata/cidata.TEMPLATE.d/lima.env b/pkg/cidata/cidata.TEMPLATE.d/lima.env index 25421141..7a3afab4 100644 --- a/pkg/cidata/cidata.TEMPLATE.d/lima.env +++ b/pkg/cidata/cidata.TEMPLATE.d/lima.env @@ -19,6 +19,8 @@ LIMA_CIDATA_DISK_{{$i}}_DEVICE={{$disk.Device}} LIMA_CIDATA_DISK_{{$i}}_FORMAT={{$disk.Format}} LIMA_CIDATA_DISK_{{$i}}_FSTYPE={{$disk.FSType}} LIMA_CIDATA_DISK_{{$i}}_FSARGS={{range $j, $arg := $disk.FSArgs}}{{if $j}} {{end}}{{$arg}}{{end}} +LIMA_CIDATA_DISK_{{$i}}_MOUNT={{$disk.Mount}} +LIMA_CIDATA_DISK_{{$i}}_MOUNTPOINT={{$disk.MountPoint}} {{- end}} {{- range $dataFile := .DataFiles}} LIMA_CIDATA_DATAFILE_{{$dataFile.FileName}}_OVERWRITE={{$dataFile.Overwrite}} diff --git a/pkg/cidata/cidata.go b/pkg/cidata/cidata.go index ec3d8b51..8b832bf9 100644 --- a/pkg/cidata/cidata.go +++ b/pkg/cidata/cidata.go @@ -283,12 +283,22 @@ func templateArgs(ctx context.Context, bootScripts bool, instDir, name string, i if d.FSType != nil { fstype = *d.FSType } + mount := true + if d.Mount != nil { + mount = *d.Mount + } + mountPoint := "" + if d.MountPoint != nil { + mountPoint = *d.MountPoint + } args.Disks = append(args.Disks, Disk{ - Name: d.Name, - Device: diskDeviceNameFromOrder(i), - Format: format, - FSType: fstype, - FSArgs: d.FSArgs, + Name: d.Name, + Device: diskDeviceNameFromOrder(i), + Format: format, + FSType: fstype, + FSArgs: d.FSArgs, + Mount: mount, + MountPoint: mountPoint, }) } diff --git a/pkg/cidata/template.go b/pkg/cidata/template.go index bfad6be9..d4a15036 100644 --- a/pkg/cidata/template.go +++ b/pkg/cidata/template.go @@ -82,11 +82,13 @@ type YQProvision struct { } type Disk struct { - Name string - Device string - Format bool - FSType string - FSArgs []string + Name string + Device string + Format bool + FSType string + FSArgs []string + Mount bool + MountPoint string } type TemplateArgs struct { Debug bool diff --git a/pkg/cidata/template_test.go b/pkg/cidata/template_test.go index 890270a6..12ed5696 100644 --- a/pkg/cidata/template_test.go +++ b/pkg/cidata/template_test.go @@ -141,6 +141,45 @@ func TestTemplate(t *testing.T) { } } +func TestTemplateDisks(t *testing.T) { + args := &TemplateArgs{ + Name: "default", + User: "foo", + UID: 501, + Home: "/home/foo.guest", + Shell: "/bin/bash", + SSHPubKeys: []string{ + "ssh-rsa dummy foo@example.com", + }, + MountType: "reverse-sshfs", + CACerts: CACerts{ + RemoveDefaults: &defaultRemoveDefaults, + }, + Disks: []Disk{ + {Name: "data", Device: "vdb", Format: true, FSType: "ext4", Mount: true, MountPoint: "/data"}, + {Name: "extra", Device: "vdc", Format: false, Mount: false}, + }, + } + layout, err := ExecuteTemplateCIDataISO(args) + assert.NilError(t, err) + var found bool + for _, f := range layout { + if f.Path != "lima.env" { + continue + } + found = true + b, err := io.ReadAll(f.Reader) + assert.NilError(t, err) + env := string(b) + assert.Assert(t, strings.Contains(env, "LIMA_CIDATA_DISK_0_MOUNT=true"), env) + assert.Assert(t, strings.Contains(env, "LIMA_CIDATA_DISK_0_MOUNTPOINT=/data"), env) + assert.Assert(t, strings.Contains(env, "LIMA_CIDATA_DISK_1_MOUNT=false"), env) + // A nil MountPoint renders as empty; the guest script falls back to /mnt/lima-. + assert.Assert(t, strings.Contains(env, "LIMA_CIDATA_DISK_1_MOUNTPOINT=\n"), env) + } + assert.Assert(t, found, "lima.env not found in layout") +} + func TestTemplate9p(t *testing.T) { args := &TemplateArgs{ Name: "default", diff --git a/pkg/limatmpl/embed.go b/pkg/limatmpl/embed.go index c269d41c..07144734 100644 --- a/pkg/limatmpl/embed.go +++ b/pkg/limatmpl/embed.go @@ -403,6 +403,16 @@ func (tmpl *Template) combineAdditionalDisks() { tmpl.copyListEntryField(additionalDisks, dst, src, "fsArgs") dest.FSArgs = disk.FSArgs } + if dest.Mount == nil && disk.Mount != nil { + upgradeDiskToMap() + tmpl.copyListEntryField(additionalDisks, dst, src, "mount") + dest.Mount = disk.Mount + } + if dest.MountPoint == nil && disk.MountPoint != nil { + upgradeDiskToMap() + tmpl.copyListEntryField(additionalDisks, dst, src, "mountPoint") + dest.MountPoint = disk.MountPoint + } // TODO: Is there a good reason not to copy comments from wildcard entries? if disk.Name != "*" { tmpl.copyListEntryComments(additionalDisks, dst, src, "") diff --git a/pkg/limatmpl/embed_test.go b/pkg/limatmpl/embed_test.go index c3d5916d..b3ccb887 100644 --- a/pkg/limatmpl/embed_test.go +++ b/pkg/limatmpl/embed_test.go @@ -144,6 +144,12 @@ additionalDisks: # comment "additionalDisks: [{name: disk2},{name: disk1, format: true}]", "additionalDisks: [{name: disk1, format: true},{name: disk2}]", }, + { + "additionalDisks merge mount and mountPoint on shared name", + "additionalDisks: [{name: disk1}]", + "additionalDisks: [{name: disk2},{name: disk1, mount: false, mountPoint: /data}]", + "additionalDisks: [{name: disk1, mount: false, mountPoint: /data},{name: disk2}]", + }, { // This test fails because there are 2 spurious newlines in the merged output "TODO mounts append, but merge fields on shared mountPoint", diff --git a/pkg/limatype/lima_yaml.go b/pkg/limatype/lima_yaml.go index f3fedb2e..6de40836 100644 --- a/pkg/limatype/lima_yaml.go +++ b/pkg/limatype/lima_yaml.go @@ -161,10 +161,12 @@ type Image struct { } type Disk struct { - Name string `yaml:"name" json:"name"` // REQUIRED - Format *bool `yaml:"format,omitempty" json:"format,omitempty"` - FSType *string `yaml:"fsType,omitempty" json:"fsType,omitempty"` - FSArgs []string `yaml:"fsArgs,omitempty" json:"fsArgs,omitempty"` + Name string `yaml:"name" json:"name"` // REQUIRED + Format *bool `yaml:"format,omitempty" json:"format,omitempty"` + FSType *string `yaml:"fsType,omitempty" json:"fsType,omitempty"` + FSArgs []string `yaml:"fsArgs,omitempty" json:"fsArgs,omitempty"` + Mount *bool `yaml:"mount,omitempty" json:"mount,omitempty"` + MountPoint *string `yaml:"mountPoint,omitempty" json:"mountPoint,omitempty"` } type Mount struct { diff --git a/pkg/limayaml/validate.go b/pkg/limayaml/validate.go index 9d16371f..0c422892 100644 --- a/pkg/limayaml/validate.go +++ b/pkg/limayaml/validate.go @@ -121,6 +121,21 @@ func Validate(y *limatype.LimaYAML, warn bool) error { if err := identifiers.Validate(disk.Name); err != nil { errs = errors.Join(errs, fmt.Errorf("field `additionalDisks[%d].name is invalid`: %w", i, err)) } + if disk.MountPoint != nil { + // This is a guest path, so use path.IsAbs (forward-slash) rather than the + // host-OS-dependent filepath.IsAbs. There is no tilde-expansion for guest + // filenames, so the mount point must be an absolute path. + if !path.IsAbs(*disk.MountPoint) { + errs = errors.Join(errs, fmt.Errorf("field `additionalDisks[%d].mountPoint` must be an absolute path, got %#q", i, *disk.MountPoint)) + } + switch *disk.MountPoint { + case "/", "/bin", "/dev", "/etc", "/home", "/opt", "/sbin", "/tmp", "/usr", "/var": + errs = errors.Join(errs, fmt.Errorf("field `additionalDisks[%d].mountPoint` must not be a critical system path such as /etc or /usr", i)) + // home directory defined in "cidata.iso:/user-data" + case *y.User.Home: + errs = errors.Join(errs, fmt.Errorf("field `additionalDisks[%d].mountPoint` is the reserved internal home directory %#q", i, *y.User.Home)) + } + } } for i, f := range y.Mounts { diff --git a/pkg/limayaml/validate_test.go b/pkg/limayaml/validate_test.go index 93aafc7c..7ade8941 100644 --- a/pkg/limayaml/validate_test.go +++ b/pkg/limayaml/validate_test.go @@ -360,6 +360,43 @@ func TestValidateBlockDevicesRejectsOverlappingStorage(t *testing.T) { } } +func TestValidateAdditionalDisksMountPoint(t *testing.T) { + images := `images: [{"location": "/"}]` + + validMountPoint := ` +additionalDisks: + - name: "disk1" + mountPoint: "/data" + - name: "disk2" + mount: false + - name: "disk3" + mountPoint: "/var/lib/data" +` + y, err := Load(t.Context(), []byte(validMountPoint+"\n"+images), "lima.yaml") + assert.NilError(t, err) + assert.NilError(t, Validate(y, false)) + + for _, tc := range []struct { + mountPoint string + expected string + }{ + {"data", "field `additionalDisks[0].mountPoint` must be an absolute path, got `data`"}, + {"~/data", "field `additionalDisks[0].mountPoint` must be an absolute path, got `~/data`"}, + {"/etc", "field `additionalDisks[0].mountPoint` must not be a critical system path such as /etc or /usr"}, + {"/home", "field `additionalDisks[0].mountPoint` must not be a critical system path such as /etc or /usr"}, + } { + invalid := "additionalDisks:\n - name: \"disk1\"\n mountPoint: \"" + tc.mountPoint + "\"\n" + y, err = Load(t.Context(), []byte(invalid+"\n"+images), "lima.yaml") + assert.NilError(t, err) + assert.Error(t, Validate(y, false), tc.expected) + } + + reservedHome := "user: {home: \"/home/foo.guest\"}\nadditionalDisks:\n - name: \"disk1\"\n mountPoint: \"/home/foo.guest\"\n" + y, err = Load(t.Context(), []byte(reservedHome+"\n"+images), "lima.yaml") + assert.NilError(t, err) + assert.Error(t, Validate(y, false), "field `additionalDisks[0].mountPoint` is the reserved internal home directory `/home/foo.guest`") +} + func TestValidateParamName(t *testing.T) { images := `images: [{"location": "/"}]` validProvision := `provision: [{"script": "echo $PARAM_name $PARAM_NAME $PARAM_Name_123"}]` diff --git a/templates/default.yaml b/templates/default.yaml index de33aa53..75afee95 100644 --- a/templates/default.yaml +++ b/templates/default.yaml @@ -122,8 +122,9 @@ mountInotify: null # Lima disks to attach to the instance. The disks will be accessible from inside the # instance, labeled by name. (e.g. if the disk is named "data", it will be labeled -# "lima-data" inside the instance). The disk will be mounted inside the instance at -# `/mnt/lima-${VOLUME}`. +# "lima-data" inside the instance). By default the disk is mounted inside the instance +# at `/mnt/lima-${VOLUME}`; use "mountPoint" to change this, or set "mount" to false to +# attach the disk without mounting it (e.g. to manage it yourself inside the guest). # 🟢 Builtin default: [] additionalDisks: # disks should either be a list of disk name strings, for example: @@ -132,6 +133,12 @@ additionalDisks: # - name: "data" # format: true # fsType: "ext4" +# # Whether Lima should auto-mount the disk inside the guest. +# # 🟢 Builtin default: true +# mount: true +# # Absolute path inside the guest to mount the disk at. +# # 🟢 Builtin default: /mnt/lima-${name} +# mountPoint: "/mnt/lima-data" # Attach host block devices (currently macOS with builtin vmType: vz only). # The helper accepts disk device paths like /dev/disk4 or /dev/rdisk4s1 diff --git a/website/content/en/docs/dev/internals.md b/website/content/en/docs/dev/internals.md index f7f50cbf..224771d9 100644 --- a/website/content/en/docs/dev/internals.md +++ b/website/content/en/docs/dev/internals.md @@ -211,6 +211,8 @@ The volume label is "cidata", as defined by [cloud-init NoCloud](https://docs.cl - `LIMA_CIDATA_DISK_%d_FORMAT`: set to "true" when the N-th additional disk should be formatted on first boot. - `LIMA_CIDATA_DISK_%d_FSTYPE`: the filesystem type to format the N-th additional disk with (e.g. `ext4`). - `LIMA_CIDATA_DISK_%d_FSARGS`: extra arguments passed to mkfs for the N-th additional disk (space-separated). +- `LIMA_CIDATA_DISK_%d_MOUNT`: set to "true" when the N-th additional disk should be mounted by the guest (default "true"). +- `LIMA_CIDATA_DISK_%d_MOUNTPOINT`: the guest path to mount the N-th additional disk at. Empty means the default `/mnt/lima-`. - `LIMA_CIDATA_DATAFILE_%08d_OVERWRITE`: set to "true" if the datafile should be overwritten if it already exists. - `LIMA_CIDATA_DATAFILE_%08d_OWNER`: set to the owner of the datafile. - `LIMA_CIDATA_DATAFILE_%08d_PATH`: set to the path the datafile should be copied to.