From 3d262da304abe2df9f30738d0acf342aae5186a3 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Thu, 16 Oct 2025 00:04:46 +0900 Subject: [PATCH] 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 --- pkg/driver/qemu/qemu.go | 26 ++++++++++++----- pkg/driver/vz/disk.go | 37 +++++++++++++++--------- pkg/driver/vz/vm_darwin.go | 12 ++++++-- pkg/instance/start.go | 10 ++++--- pkg/limatype/filenames/filenames.go | 4 +-- website/content/en/docs/dev/internals.md | 7 +++-- 6 files changed, 64 insertions(+), 32 deletions(-) diff --git a/pkg/driver/qemu/qemu.go b/pkg/driver/qemu/qemu.go index 529b37471c8..c1f43eed3d7 100644 --- a/pkg/driver/qemu/qemu.go +++ b/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) diff --git a/pkg/driver/vz/disk.go b/pkg/driver/vz/disk.go index 9cb81e06c5c..a451b45ff74 100644 --- a/pkg/driver/vz/disk.go +++ b/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 } diff --git a/pkg/driver/vz/vm_darwin.go b/pkg/driver/vz/vm_darwin.go index ec45e39832f..2183defc145 100644 --- a/pkg/driver/vz/vm_darwin.go +++ b/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 diff --git a/pkg/instance/start.go b/pkg/instance/start.go index f29db3ed51d..20563bf02a2 100644 --- a/pkg/instance/start.go +++ b/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) diff --git a/pkg/limatype/filenames/filenames.go b/pkg/limatype/filenames/filenames.go index a8459d77888..7f44624e79b 100644 --- a/pkg/limatype/filenames/filenames.go +++ b/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" diff --git a/website/content/en/docs/dev/internals.md b/website/content/en/docs/dev/internals.md index 559f9ed75bd..02872aef5fb 100644 --- a/website/content/en/docs/dev/internals.md +++ b/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