Skip to content

Commit b744a5b

Browse files
committed
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]>
1 parent 6b58ded commit b744a5b

File tree

6 files changed

+65
-33
lines changed

6 files changed

+65
-33
lines changed

pkg/driver/qemu/qemu.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ func minimumQemuVersion() (hardMin, softMin semver.Version) {
8787
return hardMin, softMin
8888
}
8989

90-
// EnsureDisk also ensures the kernel and the initrd.
90+
// EnsureDisk just renames baseDisk to diffDisk, unless baseDisk is an ISO9660 image.
91+
// Note that "diffDisk" is a misnomer, it is actually created as a full disk since Lima v2.1.
9192
func EnsureDisk(ctx context.Context, cfg Config) error {
9293
diffDisk := filepath.Join(cfg.InstanceDir, filenames.DiffDisk)
9394
if _, err := os.Stat(diffDisk); err == nil || !errors.Is(err, os.ErrNotExist) {
@@ -115,10 +116,14 @@ func EnsureDisk(ctx context.Context, cfg Config) error {
115116
if baseDiskInfo.Format == "" {
116117
return fmt.Errorf("failed to inspect the format of %q", baseDisk)
117118
}
118-
args := []string{"create", "-f", "qcow2"}
119119
if !isBaseDiskISO {
120-
args = append(args, "-F", baseDiskInfo.Format, "-b", baseDisk)
120+
// "diffdisk" is a misnomer, it is actually created as a full disk since Lima v2.1.
121+
if err = os.Rename(baseDisk, diffDisk); err != nil {
122+
return err
123+
}
124+
return nil
121125
}
126+
args := []string{"create", "-f", "qcow2"}
122127
args = append(args, diffDisk, strconv.Itoa(int(diskSize)))
123128
cmd := exec.CommandContext(ctx, "qemu-img", args...)
124129
if out, err := cmd.CombinedOutput(); err != nil {
@@ -719,9 +724,15 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er
719724
extraDisks = append(extraDisks, dataDisk)
720725
}
721726

722-
isBaseDiskCDROM, err := iso9660util.IsISO9660(baseDisk)
723-
if err != nil {
724-
return "", nil, err
727+
var baseDiskExists, isBaseDiskCDROM bool
728+
if _, err := os.Stat(baseDisk); !errors.Is(err, os.ErrNotExist) {
729+
baseDiskExists = true
730+
}
731+
if baseDiskExists {
732+
isBaseDiskCDROM, err = iso9660util.IsISO9660(baseDisk)
733+
if err != nil {
734+
return "", nil, err
735+
}
725736
}
726737
if isBaseDiskCDROM {
727738
args = appendArgsIfNoConflict(args, "-boot", "order=d,splash-time=0,menu=on")
@@ -731,7 +742,8 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er
731742
}
732743
if diskSize, _ := units.RAMInBytes(*cfg.LimaYAML.Disk); diskSize > 0 {
733744
args = append(args, "-drive", fmt.Sprintf("file=%s,if=virtio,discard=on", diffDisk))
734-
} else if !isBaseDiskCDROM {
745+
} else if baseDiskExists && !isBaseDiskCDROM { // FIXME: How does this happen? Is this even a valid case?
746+
logrus.Errorf("weird configuration, how does this happen?: diskSize /* %d */ <= 0 && baseDiskExists && !isBaseDiskCDROM", diskSize)
735747
baseDiskInfo, err := qemuimgutil.GetInfo(ctx, baseDisk)
736748
if err != nil {
737749
return "", nil, fmt.Errorf("failed to get the information of %q: %w", baseDisk, err)

pkg/driver/vz/vm_darwin.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,15 @@ func attachDisks(ctx context.Context, inst *limatype.Instance, vmConfig *vz.Virt
462462
baseDiskPath := filepath.Join(inst.Dir, filenames.BaseDisk)
463463
diffDiskPath := filepath.Join(inst.Dir, filenames.DiffDisk)
464464
ciDataPath := filepath.Join(inst.Dir, filenames.CIDataISO)
465-
isBaseDiskCDROM, err := iso9660util.IsISO9660(baseDiskPath)
466-
if err != nil {
467-
return err
465+
var (
466+
isBaseDiskCDROM bool
467+
err error
468+
)
469+
if _, err := os.Stat(baseDiskPath); !errors.Is(err, os.ErrNotExist) {
470+
isBaseDiskCDROM, err = iso9660util.IsISO9660(baseDiskPath)
471+
if err != nil {
472+
return err
473+
}
468474
}
469475
var configurations []vz.StorageDeviceConfiguration
470476

pkg/driverutil/disk.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
1919
)
2020

21+
// EnsureDiskRaw just converts baseDisk (can be qcow2) to diffDisk (raw), unless baseDisk is an ISO9660 image.
22+
// Note that "diffDisk" is a misnomer, it is actually created as a full disk since Lima v2.1.
2123
func EnsureDiskRaw(ctx context.Context, inst *limatype.Instance) error {
2224
diffDisk := filepath.Join(inst.Dir, filenames.DiffDisk)
2325
if _, err := os.Stat(diffDisk); err == nil || !errors.Is(err, os.ErrNotExist) {
@@ -33,26 +35,33 @@ func EnsureDiskRaw(ctx context.Context, inst *limatype.Instance) error {
3335
if diskSize == 0 {
3436
return nil
3537
}
36-
isBaseDiskISO, err := iso9660util.IsISO9660(baseDisk)
37-
if err != nil {
38-
return err
39-
}
40-
if isBaseDiskISO {
41-
// Create an empty data volume (sparse)
42-
diffDiskF, err := os.Create(diffDisk)
38+
var isBaseDiskISO bool
39+
if _, err := os.Stat(baseDisk); !errors.Is(err, os.ErrNotExist) {
40+
isBaseDiskISO, err = iso9660util.IsISO9660(baseDisk)
4341
if err != nil {
4442
return err
4543
}
44+
if isBaseDiskISO {
45+
// Create an empty data volume (sparse)
46+
diffDiskF, err := os.Create(diffDisk)
47+
if err != nil {
48+
return err
49+
}
4650

47-
err = diskUtil.MakeSparse(ctx, diffDiskF, 0)
48-
if err != nil {
49-
diffDiskF.Close()
50-
return fmt.Errorf("failed to create sparse diff disk %q: %w", diffDisk, err)
51+
err = diskUtil.MakeSparse(ctx, diffDiskF, 0)
52+
if err != nil {
53+
diffDiskF.Close()
54+
return fmt.Errorf("failed to create sparse diff disk %q: %w", diffDisk, err)
55+
}
56+
return diffDiskF.Close()
5157
}
52-
return diffDiskF.Close()
5358
}
54-
if err = diskUtil.ConvertToRaw(ctx, baseDisk, diffDisk, &diskSize, false); err != nil {
59+
// "diffdisk" is a misnomer, it is actually created as a full disk since Lima v2.1.
60+
if err := diskUtil.ConvertToRaw(ctx, baseDisk, diffDisk, &diskSize, false); err != nil {
5561
return fmt.Errorf("failed to convert %q to a raw disk %q: %w", baseDisk, diffDisk, err)
5662
}
57-
return err
63+
if err := os.RemoveAll(baseDisk); err != nil {
64+
return err
65+
}
66+
return nil
5867
}

pkg/instance/start.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/lima-vm/lima/v2/pkg/imgutil/proxyimgutil"
3030
"github.com/lima-vm/lima/v2/pkg/limatype"
3131
"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
32+
"github.com/lima-vm/lima/v2/pkg/limayaml"
3233
"github.com/lima-vm/lima/v2/pkg/registry"
3334
"github.com/lima-vm/lima/v2/pkg/store"
3435
"github.com/lima-vm/lima/v2/pkg/usrlocal"
@@ -66,11 +67,12 @@ func Prepare(ctx context.Context, inst *limatype.Instance, guestAgent string) (*
6667
return nil, err
6768
}
6869

69-
// Check if the instance has been created (the base disk already exists)
70-
baseDisk := filepath.Join(inst.Dir, filenames.BaseDisk)
71-
_, err = os.Stat(baseDisk)
72-
created := err == nil
70+
// Check if the instance has been created
71+
created := limayaml.IsExistingInstanceDir(inst.Dir)
7372

73+
// baseDisk is usually immediately renamed to diffDisk (misnomer) by the VM driver,
74+
// except when baseDisk is an ISO9660 image.
75+
baseDisk := filepath.Join(inst.Dir, filenames.BaseDisk)
7476
kernel := filepath.Join(inst.Dir, filenames.Kernel)
7577
kernelCmdline := filepath.Join(inst.Dir, filenames.KernelCmdline)
7678
initrd := filepath.Join(inst.Dir, filenames.Initrd)

pkg/limatype/filenames/filenames.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ const (
3636
CIDataISO = "cidata.iso"
3737
CIDataISODir = "cidata"
3838
CloudConfig = "cloud-config.yaml"
39-
BaseDisk = "basedisk"
40-
DiffDisk = "diffdisk"
39+
BaseDisk = "basedisk" // usually immediately converted/renamed to diffDisk by the VM driver
40+
DiffDisk = "diffdisk" // misnomer; actually a full disk since Lima v2.1
4141
Kernel = "kernel"
4242
KernelCmdline = "kernel.cmdline"
4343
Initrd = "initrd"

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,11 @@ Ansible:
4444
- `ansible-inventory.yaml`: the Ansible node inventory. See [ansible](#ansible).
4545

4646
disk:
47-
- `basedisk`: the base image
48-
- `diffdisk`: the diff image (QCOW2)
47+
- `basedisk`: the historical base image. Can be missing since Lima v2.1.
48+
- The main `limactl` process prepares this `basedisk`, however, a [VM driver](./drivers.md) may convert and rename `basedisk` to `diffdisk` immediately.
49+
- `diffdisk`: the image, historically a QCOW2 diff from `basedisk`.
50+
- `diffdisk` is a misnomer; it does not necessarily have a reference to `basedisk`.
51+
Notably, when a `basedisk` is an ISO9660 image, or the VM driver does not support differencing, `diffdisk` is an independent image.
4952

5053
kernel:
5154
- `kernel`: the kernel
@@ -190,4 +193,4 @@ The volume label is "cidata", as defined by [cloud-init NoCloud](https://docs.cl
190193

191194
![](/images/internals/lima-sequence-diagram.png)
192195

193-
(based on Lima 0.8.3)
196+
(based on Lima 0.8.3)

0 commit comments

Comments
 (0)