Merge pull request #1516 from afbjorklund/advanced-disks

AkihiroSuda · web-flow · commit 4761cf5e803b · 2023-07-27T16:02:31.000+09:00
Allow leaving additional disks unformatted
diff --git a/cmd/limactl/disk.go b/cmd/limactl/disk.go
@@ -245,7 +245,7 @@ func diskDeleteAction(cmd *cobra.Command, args []string) error {
 			for _, inst := range instances {
 				if len(inst.AdditionalDisks) > 0 {
 					for _, d := range inst.AdditionalDisks {
-						if d == diskName {
+						if d.Name == diskName {
 							refInstances = append(refInstances, inst.Name)
 						}
 					}
diff --git a/cmd/limactl/stop.go b/cmd/limactl/stop.go
@@ -113,7 +113,8 @@ func stopInstanceForcibly(inst *store.Instance) {
 		logrus.Infof("The %s driver process seems already stopped", inst.VMType)
 	}
 
-	for _, diskName := range inst.AdditionalDisks {
+	for _, d := range inst.AdditionalDisks {
+		diskName := d.Name
 		disk, err := store.InspectDisk(diskName)
 		if err != nil {
 			logrus.Warnf("Disk %q does not exist", diskName)
diff --git a/examples/default.yaml b/examples/default.yaml
@@ -105,8 +105,12 @@ mountType: null
 # `/mnt/lima-${VOLUME}`.
 # 🟢 Builtin default: null
 additionalDisks:
-# disks should be a list of disk name strings, for example:
+# disks should either be a list of disk name strings, for example:
 # - "data"
+# or a list of disk objects with extra parameters, for example:
+# - name: "data"
+#   format: true
+#   fsType: "ext4"
 
 ssh:
   # A localhost port of the host. Forwarded to port 22 of the guest.
diff --git a/pkg/cidata/cidata.TEMPLATE.d/boot/05-lima-disks.sh b/pkg/cidata/cidata.TEMPLATE.d/boot/05-lima-disks.sh
@@ -12,14 +12,22 @@ get_disk_var() {
 for i in $(seq 0 $((LIMA_CIDATA_DISKS - 1))); do
 	DISK_NAME="$(get_disk_var "$i" "NAME")"
 	DEVICE_NAME="$(get_disk_var "$i" "DEVICE")"
+	FORMAT_DISK="$(get_disk_var "$i" "FORMAT")"
+	FORMAT_FSTYPE="$(get_disk_var "$i" "FSTYPE")"
+	FORMAT_FSARGS="$(get_disk_var "$i" "FSARGS")"
+
+	test -n "$FORMAT_DISK" || FORMAT_DISK=true
+	test -n "$FORMAT_FSTYPE" || FORMAT_FSTYPE=ext4
 
 	# first time setup
 	if [[ ! -b "/dev/disk/by-label/lima-${DISK_NAME}" ]]; then
-		# TODO: skip if disk is tagged as "raw"
-		echo 'type=linux' | sfdisk --label gpt "/dev/${DEVICE_NAME}"
-		mkfs.ext4 -L "lima-${DISK_NAME}" "/dev/${DEVICE_NAME}1"
+		if $FORMAT_DISK; then
+			echo 'type=linux' | sfdisk --label gpt "/dev/${DEVICE_NAME}"
+			# shellcheck disable=SC2086
+			mkfs.$FORMAT_FSTYPE $FORMAT_FSARGS -L "lima-${DISK_NAME}" "/dev/${DEVICE_NAME}1"
+		fi
 	fi
 
 	mkdir -p "/mnt/lima-${DISK_NAME}"
-	mount -t ext4 "/dev/${DEVICE_NAME}1" "/mnt/lima-${DISK_NAME}"
+	mount -t $FORMAT_FSTYPE "/dev/${DEVICE_NAME}1" "/mnt/lima-${DISK_NAME}"
 done
diff --git a/pkg/cidata/cidata.TEMPLATE.d/lima.env b/pkg/cidata/cidata.TEMPLATE.d/lima.env
@@ -11,6 +11,9 @@ LIMA_CIDATA_DISKS={{ len .Disks }}
 {{- range $i, $disk := .Disks}}
 LIMA_CIDATA_DISK_{{$i}}_NAME={{$disk.Name}}
 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}}
 {{- end}}
 LIMA_CIDATA_GUEST_INSTALL_PREFIX={{ .GuestInstallPrefix }}
 {{- if .Containerd.User}}
diff --git a/pkg/cidata/cidata.go b/pkg/cidata/cidata.go
@@ -203,10 +203,21 @@ func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML, udpDNSLocalPort
 		args.MountType = "virtiofs"
 	}
 
-	for i, disk := range y.AdditionalDisks {
+	for i, d := range y.AdditionalDisks {
+		format := true
+		if d.Format != nil {
+			format = *d.Format
+		}
+		fstype := ""
+		if d.FSType != nil {
+			fstype = *d.FSType
+		}
 		args.Disks = append(args.Disks, Disk{
-			Name:   disk,
+			Name:   d.Name,
 			Device: diskDeviceNameFromOrder(i),
+			Format: format,
+			FSType: fstype,
+			FSArgs: d.FSArgs,
 		})
 	}
 
diff --git a/pkg/cidata/template.go b/pkg/cidata/template.go
@@ -48,6 +48,9 @@ type BootCmds struct {
 type Disk struct {
 	Name   string
 	Device string
+	Format bool
+	FSType string
+	FSArgs []string
 }
 type TemplateArgs struct {
 	Name                            string // instance name
diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go
@@ -436,7 +436,7 @@ sudo chown -R "${USER}" /run/host-services`
 		a.onClose = append(a.onClose, func() error {
 			var unlockMErr error
 			for _, d := range a.y.AdditionalDisks {
-				disk, inspectErr := store.InspectDisk(d)
+				disk, inspectErr := store.InspectDisk(d.Name)
 				if inspectErr != nil {
 					unlockMErr = multierror.Append(unlockMErr, inspectErr)
 					continue
diff --git a/pkg/limayaml/defaults_test.go b/pkg/limayaml/defaults_test.go
@@ -277,7 +277,7 @@ func TestFillDefault(t *testing.T) {
 		Memory: pointer.String("5GiB"),
 		Disk:   pointer.String("105GiB"),
 		AdditionalDisks: []Disk{
-			"data",
+			{Name: "data"},
 		},
 		GuestInstallPrefix: pointer.String("/opt"),
 		Containerd: Containerd{
@@ -413,7 +413,7 @@ func TestFillDefault(t *testing.T) {
 
 	y = filledDefaults
 	y.DNS = []net.IP{net.ParseIP("8.8.8.8")}
-	y.AdditionalDisks = []Disk{"overridden"}
+	y.AdditionalDisks = []Disk{{Name: "overridden"}}
 
 	expect = y
 
@@ -454,7 +454,7 @@ func TestFillDefault(t *testing.T) {
 		Memory: pointer.String("7GiB"),
 		Disk:   pointer.String("117GiB"),
 		AdditionalDisks: []Disk{
-			"test",
+			{Name: "test"},
 		},
 		GuestInstallPrefix: pointer.String("/usr"),
 		Containerd: Containerd{
diff --git a/pkg/limayaml/limayaml.go b/pkg/limayaml/limayaml.go
@@ -79,7 +79,12 @@ type Image struct {
 	Initrd *File   `yaml:"initrd,omitempty" json:"initrd,omitempty"`
 }
 
-type Disk = string
+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"`
+}
 
 type Mount struct {
 	Location   string   `yaml:"location" json:"location"` // REQUIRED
diff --git a/pkg/limayaml/load.go b/pkg/limayaml/load.go
@@ -13,16 +13,36 @@ import (
 	yamlv3 "gopkg.in/yaml.v3"
 )
 
+func unmarshalDisk(dst *Disk, b []byte) error {
+	var s string
+	if err := yaml.Unmarshal(b, &s); err == nil {
+		*dst = Disk{Name: s}
+		return nil
+	}
+	return yaml.Unmarshal(b, dst)
+}
+
+func (d *Disk) UnmarshalYAML(value *yamlv3.Node) error {
+	var v interface{}
+	if err := value.Decode(&v); err != nil {
+		return err
+	}
+	if s, ok := v.(string); ok {
+		*d = Disk{Name: s}
+	}
+	return nil
+}
+
 func unmarshalYAML(data []byte, v interface{}, comment string) error {
-	if err := yaml.UnmarshalWithOptions(data, v, yaml.DisallowDuplicateKey()); err != nil {
+	if err := yaml.UnmarshalWithOptions(data, v, yaml.DisallowDuplicateKey(), yaml.CustomUnmarshaler[Disk](unmarshalDisk)); err != nil {
 		return fmt.Errorf("failed to unmarshal YAML (%s): %w", comment, err)
 	}
 	// the go-yaml library doesn't catch all markup errors, unfortunately
 	// make sure to get a "second opinion", using the same library as "yq"
 	if err := yamlv3.Unmarshal(data, v); err != nil {
 		return fmt.Errorf("failed to unmarshal YAML (%s): %w", comment, err)
 	}
-	if err := yaml.UnmarshalWithOptions(data, v, yaml.Strict()); err != nil {
+	if err := yaml.UnmarshalWithOptions(data, v, yaml.Strict(), yaml.CustomUnmarshaler[Disk](unmarshalDisk)); err != nil {
 		logrus.WithField("comment", comment).WithError(err).Warn("Non-strict YAML is deprecated and will be unsupported in a future version of Lima")
 		// Non-strict YAML is known to be used by Rancher Desktop:
 		// https://github.com/rancher-sandbox/rancher-desktop/blob/c7ea7508a0191634adf16f4675f64c73198e8d37/src/backend/lima.ts#L114-L117
diff --git a/pkg/limayaml/load_test.go b/pkg/limayaml/load_test.go
@@ -30,3 +30,38 @@ provision:
 	_, err := Load([]byte(s), "error.yaml")
 	assert.ErrorContains(t, err, "failed to unmarshal YAML")
 }
+
+func TestLoadDiskString(t *testing.T) {
+	s := `
+additionalDisks:
+- name
+`
+	y, err := Load([]byte(s), "disk.yaml")
+	assert.NilError(t, err)
+	assert.Equal(t, len(y.AdditionalDisks), 1)
+	assert.Equal(t, y.AdditionalDisks[0].Name, "name")
+	assert.Assert(t, y.AdditionalDisks[0].Format == nil)
+	assert.Assert(t, y.AdditionalDisks[0].FSType == nil)
+	assert.Assert(t, y.AdditionalDisks[0].FSArgs == nil)
+}
+
+func TestLoadDiskStruct(t *testing.T) {
+	s := `
+additionalDisks:
+- name: "name"
+  format: false
+  fsType: "xfs"
+  fsArgs: ["-i","size=512"]
+`
+	y, err := Load([]byte(s), "disk.yaml")
+	assert.NilError(t, err)
+	assert.Assert(t, len(y.AdditionalDisks) == 1)
+	assert.Equal(t, y.AdditionalDisks[0].Name, "name")
+	assert.Assert(t, y.AdditionalDisks[0].Format != nil)
+	assert.Equal(t, *y.AdditionalDisks[0].Format, false)
+	assert.Assert(t, y.AdditionalDisks[0].FSType != nil)
+	assert.Equal(t, *y.AdditionalDisks[0].FSType, "xfs")
+	assert.Assert(t, len(y.AdditionalDisks[0].FSArgs) == 2)
+	assert.Equal(t, y.AdditionalDisks[0].FSArgs[0], "-i")
+	assert.Equal(t, y.AdditionalDisks[0].FSArgs[1], "size=512")
+}
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
@@ -587,24 +587,25 @@ func Cmdline(cfg Config) (string, []string, error) {
 	diffDisk := filepath.Join(cfg.InstanceDir, filenames.DiffDisk)
 	extraDisks := []string{}
 	if len(y.AdditionalDisks) > 0 {
-		for _, diskName := range y.AdditionalDisks {
-			d, err := store.InspectDisk(diskName)
+		for _, d := range y.AdditionalDisks {
+			diskName := d.Name
+			disk, err := store.InspectDisk(diskName)
 			if err != nil {
 				logrus.Errorf("could not load disk %q: %q", diskName, err)
 				return "", nil, err
 			}
 
-			if d.Instance != "" {
-				logrus.Errorf("could not attach disk %q, in use by instance %q", diskName, d.Instance)
+			if disk.Instance != "" {
+				logrus.Errorf("could not attach disk %q, in use by instance %q", diskName, disk.Instance)
 				return "", nil, err
 			}
-			logrus.Infof("Mounting disk %q on %q", diskName, d.MountPoint)
-			err = d.Lock(cfg.InstanceDir)
+			logrus.Infof("Mounting disk %q on %q", diskName, disk.MountPoint)
+			err = disk.Lock(cfg.InstanceDir)
 			if err != nil {
 				logrus.Errorf("could not lock disk %q: %q", diskName, err)
 				return "", nil, err
 			}
-			dataDisk := filepath.Join(d.Dir, filenames.DataDisk)
+			dataDisk := filepath.Join(disk.Dir, filenames.DataDisk)
 			extraDisks = append(extraDisks, dataDisk)
 		}
 	}
diff --git a/pkg/vz/vm_darwin.go b/pkg/vz/vm_darwin.go
@@ -437,21 +437,22 @@ func attachDisks(driver *driver.BaseDriver, vmConfig *vz.VirtualMachineConfigura
 	}
 	configurations = append(configurations, diffDisk)
 
-	for _, diskName := range driver.Yaml.AdditionalDisks {
-		d, err := store.InspectDisk(diskName)
+	for _, d := range driver.Yaml.AdditionalDisks {
+		diskName := d.Name
+		disk, err := store.InspectDisk(diskName)
 		if err != nil {
 			return fmt.Errorf("failed to run load disk %q: %q", diskName, err)
 		}
 
-		if d.Instance != "" {
-			return fmt.Errorf("failed to run attach disk %q, in use by instance %q", diskName, d.Instance)
+		if disk.Instance != "" {
+			return fmt.Errorf("failed to run attach disk %q, in use by instance %q", diskName, disk.Instance)
 		}
-		logrus.Infof("Mounting disk %q on %q", diskName, d.MountPoint)
-		err = d.Lock(driver.Instance.Dir)
+		logrus.Infof("Mounting disk %q on %q", diskName, disk.MountPoint)
+		err = disk.Lock(driver.Instance.Dir)
 		if err != nil {
 			return fmt.Errorf("failed to run lock disk %q: %q", diskName, err)
 		}
-		extraDiskPath := filepath.Join(d.Dir, filenames.DataDisk)
+		extraDiskPath := filepath.Join(disk.Dir, filenames.DataDisk)
 		// ConvertToRaw is a NOP if no conversion is needed
 		logrus.Debugf("Converting extra disk %q to a raw disk (if it is not a raw)", extraDiskPath)
 		if err = nativeimgutil.ConvertToRaw(extraDiskPath, extraDiskPath, nil, true); err != nil {

Original file line number	Diff line number	Diff line change
`@@ -245,7 +245,7 @@ func diskDeleteAction(cmd *cobra.Command, args []string) error {`
`245`	`245`	`for _, inst := range instances {`
`246`	`246`	`if len(inst.AdditionalDisks) > 0 {`
`247`	`247`	`for _, d := range inst.AdditionalDisks {`
`248`		`- if d == diskName {`
	`248`	`+ if d.Name == diskName {`
`249`	`249`	`refInstances = append(refInstances, inst.Name)`
`250`	`250`	`}`
`251`	`251`	`}`
Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,8 @@ func stopInstanceForcibly(inst *store.Instance) {`
`113`	`113`	`logrus.Infof("The %s driver process seems already stopped", inst.VMType)`
`114`	`114`	`}`
`115`	`115`
`116`		`- for _, diskName := range inst.AdditionalDisks {`
	`116`	`+ for _, d := range inst.AdditionalDisks {`
	`117`	`+ diskName := d.Name`
`117`	`118`	`disk, err := store.InspectDisk(diskName)`
`118`	`119`	`if err != nil {`
`119`	`120`	`logrus.Warnf("Disk %q does not exist", diskName)`