Decouple diffdisk (misnomer) from basedisk

Fix issue 1706.

Now basedisk is immediately renamed/converted to diffdisk
(not a diff despite the name) by the VM driver,
except when basedisk is an ISO9660 image.

Decoupling diffdisk from basedisk will eliminate the overhead of differencing I/O
and save some disk space.

I cannot remember why I designed the disk to be split into basedisk and diffdisk.
Maybe it was to allow `limactl factory-reset` to retain the initial state,
although the command was apparently never implemented in that way.
Maybe it was to allow multiple instances to share the same basedisk,
although it was never implemented in that way, either.

Signed-off-by: Akihiro Suda <[email protected]>

Author: AkihiroSuda

pkg/driver/qemu/qemu.go

@@ -86,7 +86,8 @@ func minimumQemuVersion() (hardMin, softMin semver.Version) {
 	return hardMin, softMin
 }
 
-// EnsureDisk also ensures the kernel and the initrd.
+// EnsureDisk just renames baseDisk to diffDisk, unless baseDisk is an ISO9660 image.
+// Note that "diffDisk" is a misnomer, it is actually created as a full disk since Lima v2.0.
 func EnsureDisk(ctx context.Context, cfg Config) error {
 	diffDisk := filepath.Join(cfg.InstanceDir, filenames.DiffDisk)
 	if _, err := os.Stat(diffDisk); err == nil || !errors.Is(err, os.ErrNotExist) {
@@ -114,10 +115,14 @@ func EnsureDisk(ctx context.Context, cfg Config) error {
 	if baseDiskInfo.Format == "" {
 		return fmt.Errorf("failed to inspect the format of %q", baseDisk)
 	}
-	args := []string{"create", "-f", "qcow2"}
 	if !isBaseDiskISO {
-		args = append(args, "-F", baseDiskInfo.Format, "-b", baseDisk)
+		// "diffdisk" is a misnomer, it is actually created as a full disk since Lima v2.0.
+		if err = os.Rename(baseDisk, diffDisk); err != nil {
+			return err
+		}
+		return nil
 	}
+	args := []string{"create", "-f", "qcow2"}
 	args = append(args, diffDisk, strconv.Itoa(int(diskSize)))
 	cmd := exec.CommandContext(ctx, "qemu-img", args...)
 	if out, err := cmd.CombinedOutput(); err != nil {
@@ -718,9 +723,15 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er
 		extraDisks = append(extraDisks, dataDisk)
 	}
 
-	isBaseDiskCDROM, err := iso9660util.IsISO9660(baseDisk)
-	if err != nil {
-		return "", nil, err
+	var baseDiskExists, isBaseDiskCDROM bool
+	if _, err := os.Stat(baseDisk); !errors.Is(err, os.ErrNotExist) {
+		baseDiskExists = true
+	}
+	if baseDiskExists {
+		isBaseDiskCDROM, err = iso9660util.IsISO9660(baseDisk)
+		if err != nil {
+			return "", nil, err
+		}
 	}
 	if isBaseDiskCDROM {
 		args = appendArgsIfNoConflict(args, "-boot", "order=d,splash-time=0,menu=on")
@@ -730,7 +741,8 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er
 	}
 	if diskSize, _ := units.RAMInBytes(*cfg.LimaYAML.Disk); diskSize > 0 {
 		args = append(args, "-drive", fmt.Sprintf("file=%s,if=virtio,discard=on", diffDisk))
-	} else if !isBaseDiskCDROM {
+	} else if baseDiskExists && !isBaseDiskCDROM { // FIXME: How does this happen? Is this even a valid case?
+		logrus.Errorf("weird configuration, how does this happen?: diskSize /* %d */ <= 0 && baseDiskExists && !isBaseDiskCDROM", diskSize)
 		baseDiskInfo, err := qemuimgutil.GetInfo(ctx, baseDisk)
 		if err != nil {
 			return "", nil, fmt.Errorf("failed to get the information of %q: %w", baseDisk, err)

pkg/driver/vz/disk.go

@@ -18,6 +18,8 @@ import (
 	"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
 )
 
+// EnsureDisk just converts baseDisk (can be qcow2) to diffDisk (raw), unless baseDisk is an ISO9660 image.
+// Note that "diffDisk" is a misnomer, it is actually created as a full disk in the case of VZ.
 func EnsureDisk(ctx context.Context, inst *limatype.Instance) error {
 	diffDisk := filepath.Join(inst.Dir, filenames.DiffDisk)
 	if _, err := os.Stat(diffDisk); err == nil || !errors.Is(err, os.ErrNotExist) {
@@ -33,26 +35,33 @@ func EnsureDisk(ctx context.Context, inst *limatype.Instance) error {
 	if diskSize == 0 {
 		return nil
 	}
-	isBaseDiskISO, err := iso9660util.IsISO9660(baseDisk)
-	if err != nil {
-		return err
-	}
-	if isBaseDiskISO {
-		// Create an empty data volume (sparse)
-		diffDiskF, err := os.Create(diffDisk)
+	var isBaseDiskISO bool
+	if _, err := os.Stat(baseDisk); !errors.Is(err, os.ErrNotExist) {
+		isBaseDiskISO, err = iso9660util.IsISO9660(baseDisk)
 		if err != nil {
 			return err
 		}
+		if isBaseDiskISO {
+			// Create an empty data volume (sparse)
+			diffDiskF, err := os.Create(diffDisk)
+			if err != nil {
+				return err
+			}
 
-		err = diskUtil.MakeSparse(ctx, diffDiskF, 0)
-		if err != nil {
-			diffDiskF.Close()
-			return fmt.Errorf("failed to create sparse diff disk %q: %w", diffDisk, err)
+			err = diskUtil.MakeSparse(ctx, diffDiskF, 0)
+			if err != nil {
+				diffDiskF.Close()
+				return fmt.Errorf("failed to create sparse diff disk %q: %w", diffDisk, err)
+			}
+			return diffDiskF.Close()
 		}
-		return diffDiskF.Close()
 	}
-	if err = diskUtil.ConvertToRaw(ctx, baseDisk, diffDisk, &diskSize, false); err != nil {
+	// "diffdisk" is a misnomer, it is actually created as a full disk in the case of VZ.
+	if err := diskUtil.ConvertToRaw(ctx, baseDisk, diffDisk, &diskSize, false); err != nil {
 		return fmt.Errorf("failed to convert %q to a raw disk %q: %w", baseDisk, diffDisk, err)
 	}
-	return err
+	if err := os.RemoveAll(baseDisk); err != nil {
+		return err
+	}
+	return nil
 }

pkg/driver/vz/vm_darwin.go

@@ -463,9 +463,15 @@ func attachDisks(ctx context.Context, inst *limatype.Instance, vmConfig *vz.Virt
 	baseDiskPath := filepath.Join(inst.Dir, filenames.BaseDisk)
 	diffDiskPath := filepath.Join(inst.Dir, filenames.DiffDisk)
 	ciDataPath := filepath.Join(inst.Dir, filenames.CIDataISO)
-	isBaseDiskCDROM, err := iso9660util.IsISO9660(baseDiskPath)
-	if err != nil {
-		return err
+	var (
+		isBaseDiskCDROM bool
+		err             error
+	)
+	if _, err := os.Stat(baseDiskPath); !errors.Is(err, os.ErrNotExist) {
+		isBaseDiskCDROM, err = iso9660util.IsISO9660(baseDiskPath)
+		if err != nil {
+			return err
+		}
 	}
 	var configurations []vz.StorageDeviceConfiguration
 

pkg/instance/start.go

@@ -28,6 +28,7 @@ import (
 	"github.com/lima-vm/lima/v2/pkg/imgutil/proxyimgutil"
 	"github.com/lima-vm/lima/v2/pkg/limatype"
 	"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
+	"github.com/lima-vm/lima/v2/pkg/limayaml"
 	"github.com/lima-vm/lima/v2/pkg/registry"
 	"github.com/lima-vm/lima/v2/pkg/store"
 	"github.com/lima-vm/lima/v2/pkg/usrlocalsharelima"
@@ -65,11 +66,12 @@ func Prepare(ctx context.Context, inst *limatype.Instance, guestAgent string) (*
 		return nil, err
 	}
 
-	// Check if the instance has been created (the base disk already exists)
-	baseDisk := filepath.Join(inst.Dir, filenames.BaseDisk)
-	_, err = os.Stat(baseDisk)
-	created := err == nil
+	// Check if the instance has been created
+	created := limayaml.IsExistingInstanceDir(inst.Dir)
 
+	// baseDisk is usually immediately renamed to diffDisk (misnomer) by the VM driver,
+	// except when baseDisk is an ISO9660 image.
+	baseDisk := filepath.Join(inst.Dir, filenames.BaseDisk)
 	kernel := filepath.Join(inst.Dir, filenames.Kernel)
 	kernelCmdline := filepath.Join(inst.Dir, filenames.KernelCmdline)
 	initrd := filepath.Join(inst.Dir, filenames.Initrd)

pkg/limatype/filenames/filenames.go

@@ -36,8 +36,8 @@ const (
 	CIDataISO               = "cidata.iso"
 	CIDataISODir            = "cidata"
 	CloudConfig             = "cloud-config.yaml"
-	BaseDisk                = "basedisk"
-	DiffDisk                = "diffdisk"
+	BaseDisk                = "basedisk" // usually immediately converted/renamed to diffDisk by the VM driver
+	DiffDisk                = "diffdisk" // misnomer; actually a full disk since Lima v2.0
 	Kernel                  = "kernel"
 	KernelCmdline           = "kernel.cmdline"
 	Initrd                  = "initrd"

website/content/en/docs/dev/internals.md

@@ -44,8 +44,11 @@ Ansible:
 - `ansible-inventory.yaml`: the Ansible node inventory. See [ansible](#ansible).
 
 disk:
-- `basedisk`: the base image
-- `diffdisk`: the diff image (QCOW2)
+- `basedisk`: the historical base image. Can be missing since Lima v2.0.
+  - The main `limactl` process prepares this `basedisk`, however, a [VM driver](./drivers.md) may convert and rename `basedisk` to `diffdisk` immediately.
+- `diffdisk`: the image, historically a QCOW2 diff from `basedisk`.
+  - `diffdisk` is a misnomer; it does not necessarily have a reference to `basedisk`.
+    Notably, when a `basedisk` is an ISO9660 image, or the VM driver does not support differencing, `diffdisk` is an independent image.
 
 kernel:
 - `kernel`: the kernel