Support multiple disks in qemu

This changes the CLI specification for disks, as it needs to be able to
be repeated.

```
linuxkit run qemu -disk name,size=1G,format=qcow2 ...
```

Options may be omitted.

Currently other local backends may not support multiple disks, but this
can be added in future. Code for cloud backends has not changed as the
disk support is specific to the platform.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>

docs/external-disk.md

@@ -7,14 +7,15 @@
 ## Make Disk Available
 In order to make the disk available, you need to tell `linuxkit` where the disk file or block device is.
 
-All local `linuxkit run` methods (currently `hyperkit`, `qemu`, and `vmware`) take two arguments:
+All local `linuxkit run` methods (currently `hyperkit`, `qemu`, and `vmware`) take a `-disk` argument:
 
-* `-disk-size <size>`: size of disk. The default is in MB but a `G` can be appended to specify the size in GB, e.g. `-disk-size 4096` or `disk-size 4G` will provide a 4GB disk.
-* `-disk <path>`: use the disk at location _path_, e.g. `-disk foo.img` will use the disk at `$PWD/foo.img`
+* `-disk path,size=100M,format=qcow2`. For size the default is in MB but `G` can be aspecified for GB. The format can be omitted for the platform default, and is only useful on `qemu` at present.
 
-If you do not provide `-disk `_path_, `linuxkit` assumes a default, which is _prefix_`-state/disk.img` for `hyperkit` and `vmware` and _prefix_`-disk.img` for `qemu`. 
+If the _path` is specified it will use the disk at location _path_, if you do not provide `-disk `_path_, `linuxkit` assumes a default, which is _prefix_`-state/disk.img` for `hyperkit` and `vmware` and _prefix_`-disk.img` for `qemu`. 
 
-If the disk at `<path>`, or the default if `-disk` option is not provided, does not exist, `linuxkit` will create one of size `<size>`.
+If the disk at the specified or default `<path>` does not exist, `linuxkit` will create one of size `<size>`.
+
+The `-disk` specification may be repeated for multiple disks, although a limited number may be supported, and some platforms currently only support a single disk.
 
 **TODO:** GCP
 

src/cmd/linuxkit/run_hyperkit.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	log "github.com/Sirupsen/logrus"
@@ -35,8 +36,8 @@ func runHyperKit(args []string) {
 	hyperkitPath := flags.String("hyperkit", "", "Path to hyperkit binary (if not in default location)")
 	cpus := flags.Int("cpus", 1, "Number of CPUs")
 	mem := flags.Int("mem", 1024, "Amount of memory in MB")
-	diskSzFlag := flags.String("disk-size", "", "Size of Disk in MB (or GB if 'G' is appended)")
-	disk := flags.String("disk", "", "Path to disk image to use")
+	var disks Disks
+	flags.Var(&disks, "disk", "Disk config. [file=]path[,size=1G]")
 	data := flags.String("data", "", "Metadata to pass to VM (either a path to a file or a string)")
 	ipStr := flags.String("ip", "", "IP address for the VM")
 	state := flags.String("state", "", "Path to directory to keep VM state in")
@@ -61,11 +62,6 @@ func runHyperKit(args []string) {
 		log.Fatalf("Could not create state directory: %v", err)
 	}
 
-	diskSz, err := getDiskSizeMB(*diskSzFlag)
-	if err != nil {
-		log.Fatalf("Could parse disk-size %s: %v", *diskSzFlag, err)
-	}
-
 	isoPath := ""
 	if *data != "" {
 		var d []byte
@@ -106,8 +102,22 @@ func runHyperKit(args []string) {
 		log.Fatalf("Cannot open cmdline file: %v", err)
 	}
 
-	if diskSz != 0 && *disk == "" {
-		*disk = filepath.Join(*state, "disk.img")
+	for i, d := range disks {
+		id := ""
+		if i != 0 {
+			id = strconv.Itoa(i)
+		}
+		if d.Size != 0 && d.Path == "" {
+			d.Path = filepath.Join(*state, "disk"+id+".img")
+		}
+		if d.Path == "" {
+			log.Fatalf("disk specified with no size or name")
+		}
+		disks[i] = d
+	}
+
+	if len(disks) > 1 {
+		log.Fatalf("Hyperkit driver currently only supports a single disk")
 	}
 
 	// Create new HyperKit instance (w/o networking for now)
@@ -163,12 +173,14 @@ func runHyperKit(args []string) {
 	h.Initrd = prefix + "-initrd.img"
 	h.VPNKitKey = vpnKitKey
 	h.UUID = vmUUID
-	h.DiskImage = *disk
+	if len(disks) == 1 {
+		h.DiskImage = disks[0].Path
+		h.DiskSize = disks[0].Size
+	}
 	h.ISOImage = isoPath
 	h.VSock = true
 	h.CPUs = *cpus
 	h.Memory = *mem
-	h.DiskSize = diskSz
 
 	err = h.Run(string(cmdline))
 	if err != nil {

src/cmd/linuxkit/run_qemu.go

@@ -24,9 +24,7 @@ type QemuConfig struct {
 	UEFI           bool
 	Kernel         bool
 	GUI            bool
-	DiskPath       string
-	DiskSize       string
-	DiskFormat     string
+	Disks          Disks
 	FWPath         string
 	Arch           string
 	CPUs           string
@@ -58,9 +56,8 @@ func runQemu(args []string) {
 	kernelBoot := flags.Bool("kernel", false, "Boot image is kernel+initrd+cmdline 'path'-kernel/-initrd/-cmdline")
 
 	// Paths and settings for disks
-	diskSzFlag := flags.String("disk-size", "", "Size of Disk in MB (or GB if 'G' is appended)")
-	disk := flags.String("disk", "", "Path to disk image to use")
-	diskFmt := flags.String("disk-format", "qcow2", "Format of disk: raw, qcow2 etc")
+	var disks Disks
+	flags.Var(&disks, "disk", "Disk config, may be repeated. [file=]path[,size=1G][,format=qcow2]")
 
 	// Paths and settings for UEFI firware
 	fw := flags.String("fw", "/usr/share/ovmf/bios.bin", "Path to OVMF firmware for UEFI boot")
@@ -113,21 +110,28 @@ func runQemu(args []string) {
 		}
 	}
 
-	diskSz, err := getDiskSizeMB(*diskSzFlag)
-	if err != nil {
-		log.Fatalf("Couldn't parse disk-size %s: %v", *diskSzFlag, err)
-	}
-	if diskSz != 0 && *disk == "" {
-		*disk = prefix + "-disk.img"
+	for i, d := range disks {
+		id := ""
+		if i != 0 {
+			id = strconv.Itoa(i)
+		}
+		if d.Size != 0 && d.Format == "" {
+			d.Format = "qcow2"
+		}
+		if d.Size != 0 && d.Path == "" {
+			d.Path = prefix + "-disk" + id + ".img"
+		}
+		if d.Path == "" {
+			log.Fatalf("disk specified with no size or name")
+		}
+		disks[i] = d
 	}
 
 	// user not trying to boot off ISO or kernel, so assume booting from a disk image
 	if !*kernelBoot && !*isoBoot {
-		if *disk != "" {
-			// Need to add multiple disk support to do this
-			log.Fatalf("Cannot boot from disk and specify a disk as well at present")
-		}
-		*disk = path
+		// currently no way to set format, but autodetect probably works
+		d := Disks{DiskConfig{Path: path}}
+		disks = append(d, disks...)
 	}
 
 	config := QemuConfig{
@@ -136,9 +140,7 @@ func runQemu(args []string) {
 		UEFI:           *uefiBoot,
 		Kernel:         *kernelBoot,
 		GUI:            *enableGUI,
-		DiskPath:       *disk,
-		DiskSize:       fmt.Sprintf("%dM", diskSz),
-		DiskFormat:     *diskFmt,
+		Disks:          disks,
 		FWPath:         *fw,
 		Arch:           *arch,
 		CPUs:           *cpus,
@@ -163,21 +165,21 @@ func runQemuLocal(config QemuConfig) error {
 	var args []string
 	config, args = buildQemuCmdline(config)
 
-	if config.DiskPath != "" {
+	for _, d := range config.Disks {
 		// If disk doesn't exist then create one
-		if _, err := os.Stat(config.DiskPath); err != nil {
+		if _, err := os.Stat(d.Path); err != nil {
 			if os.IsNotExist(err) {
-				log.Debugf("Creating new qemu disk [%s] format %s", config.DiskPath, config.DiskFormat)
-				qemuImgCmd := exec.Command(config.QemuImgPath, "create", "-f", config.DiskFormat, config.DiskPath, config.DiskSize)
+				log.Debugf("Creating new qemu disk [%s] format %s", d.Path, d.Format)
+				qemuImgCmd := exec.Command(config.QemuImgPath, "create", "-f", d.Format, d.Path, fmt.Sprintf("%dM", d.Size))
 				log.Debugf("%v\n", qemuImgCmd.Args)
 				if err := qemuImgCmd.Run(); err != nil {
-					return fmt.Errorf("Error creating disk [%s] format %s:  %s", config.DiskPath, config.DiskFormat, err.Error())
+					return fmt.Errorf("Error creating disk [%s] format %s:  %s", d.Path, d.Format, err.Error())
 				}
 			} else {
 				return err
 			}
 		} else {
-			log.Infof("Using existing disk [%s] format %s", config.DiskPath, config.DiskFormat)
+			log.Infof("Using existing disk [%s] format %s", d.Path, d.Format)
 		}
 	}
 
@@ -246,23 +248,23 @@ func runQemuContainer(config QemuConfig) error {
 		return fmt.Errorf("Unable to find docker in the $PATH")
 	}
 
-	if config.DiskPath != "" {
+	for _, d := range config.Disks {
 		// If disk doesn't exist then create one
-		if _, err = os.Stat(config.DiskPath); err != nil {
+		if _, err = os.Stat(d.Path); err != nil {
 			if os.IsNotExist(err) {
-				log.Debugf("Creating new qemu disk [%s] format %s", config.DiskPath, config.DiskFormat)
-				imgArgs := append(dockerArgsImg, QemuImg, "qemu-img", "create", "-f", config.DiskFormat, config.DiskPath, config.DiskSize)
+				log.Debugf("Creating new qemu disk [%s] format %s", d.Path, d.Format)
+				imgArgs := append(dockerArgsImg, QemuImg, "qemu-img", "create", "-f", d.Format, d.Path, fmt.Sprintf("%dM", d.Size))
 				qemuImgCmd := exec.Command(dockerPath, imgArgs...)
 				qemuImgCmd.Stderr = os.Stderr
 				log.Debugf("%v\n", qemuImgCmd.Args)
 				if err = qemuImgCmd.Run(); err != nil {
-					return fmt.Errorf("Error creating disk [%s] format %s:  %s", config.DiskPath, config.DiskFormat, err.Error())
+					return fmt.Errorf("Error creating disk [%s] format %s:  %s", d.Path, d.Format, err.Error())
 				}
 			} else {
 				return err
 			}
 		} else {
-			log.Infof("Using existing disk [%s] format %s", config.DiskPath, config.DiskFormat)
+			log.Infof("Using existing disk [%s] format %s", d.Path, d.Format)
 		}
 	}
 
@@ -301,8 +303,17 @@ func buildQemuCmdline(config QemuConfig) (QemuConfig, []string) {
 		qemuArgs = append(qemuArgs, "-machine", "q35,accel=kvm:tcg")
 	}
 
-	if config.DiskPath != "" {
-		qemuArgs = append(qemuArgs, "-drive", "file="+config.DiskPath+",format="+config.DiskFormat+",index=0,media=disk")
+	for i, d := range config.Disks {
+		index := i
+		// hdc is CDROM in qemu
+		if i >= 2 && config.ISO {
+			index++
+		}
+		if d.Format != "" {
+			qemuArgs = append(qemuArgs, "-drive", "file="+d.Path+",format="+d.Format+",index="+strconv.Itoa(index)+",media=disk")
+		} else {
+			qemuArgs = append(qemuArgs, "-drive", "file="+d.Path+",index="+strconv.Itoa(index)+",media=disk")
+		}
 	}
 
 	if config.ISO {

src/cmd/linuxkit/run_vmware.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strconv"
 
 	log "github.com/Sirupsen/logrus"
 )
@@ -76,8 +77,8 @@ func runVMware(args []string) {
 	}
 	cpus := flags.Int("cpus", 1, "Number of CPUs")
 	mem := flags.Int("mem", 1024, "Amount of memory in MB")
-	diskSzFlag := flags.String("disk-size", "", "Size of Disk in MB (or GB if 'G' is appended)")
-	disk := flags.String("disk", "", "Path to disk image to use")
+	var disks Disks
+	flags.Var(&disks, "disk", "Disk config. [file=]path[,size=1G]")
 	state := flags.String("state", "", "Path to directory to keep VM state in")
 
 	if err := flags.Parse(args); err != nil {
@@ -99,27 +100,19 @@ func runVMware(args []string) {
 		log.Fatalf("Could not create state directory: %v", err)
 	}
 
-	diskSz, err := getDiskSizeMB(*diskSzFlag)
-	if err != nil {
-		log.Fatalf("Could parse disk-size %s: %v", *diskSzFlag, err)
-	}
-
 	var vmrunPath, vmDiskManagerPath string
 	var vmrunArgs []string
 
-	if runtime.GOOS == "windows" {
+	switch runtime.GOOS {
+	case "windows":
 		vmrunPath = "C:\\Program\\ files\\VMware Workstation\\vmrun.exe"
 		vmDiskManagerPath = "C:\\Program\\ files\\VMware Workstation\\vmware-vdiskmanager.exe"
 		vmrunArgs = []string{"-T", "ws", "start"}
-	}
-
-	if runtime.GOOS == "darwin" {
+	case "darwin":
 		vmrunPath = "/Applications/VMware Fusion.app/Contents/Library/vmrun"
 		vmDiskManagerPath = "/Applications/VMware Fusion.app/Contents/Library/vmware-vdiskmanager"
 		vmrunArgs = []string{"-T", "fusion", "start"}
-	}
-
-	if runtime.GOOS == "linux" {
+	default:
 		vmrunPath = "vmrun"
 		vmDiskManagerPath = "vmware-vdiskmanager"
 		fullVMrunPath, err := exec.LookPath(vmrunPath)
@@ -136,41 +129,65 @@ func runVMware(args []string) {
 		log.Fatalf("ERROR VMware executables can not be found, ensure software is installed")
 	}
 
-	if diskSz != 0 && *disk == "" {
-		*disk = filepath.Join(*state, "disk.vmdk")
+	for i, d := range disks {
+		id := ""
+		if i != 0 {
+			id = strconv.Itoa(i)
+		}
+		if d.Size != 0 && d.Path == "" {
+			d.Path = filepath.Join(*state, "disk"+id+".vmdk")
+		}
+		if d.Format != "" && d.Format != "vmdk" {
+			log.Fatalf("only vmdk supported for VMware driver")
+		}
+		if d.Path == "" {
+			log.Fatalf("disk specified with no size or name")
+		}
+		disks[i] = d
 	}
-	if *disk != "" {
+
+	for _, d := range disks {
 		// Check vmDiskManagerPath exist before attempting to execute
 		if _, err := os.Stat(vmDiskManagerPath); os.IsNotExist(err) {
 			log.Fatalf("ERROR VMware Disk Manager executables can not be found, ensure software is installed")
 		}
 
 		// If disk doesn't exist then create one, error if disk is unreadable
-		if _, err := os.Stat(*disk); err != nil {
+		if _, err := os.Stat(d.Path); err != nil {
 			if os.IsPermission(err) {
-				log.Fatalf("Unable to read file [%s], please check permissions", *disk)
-			}
-			if os.IsNotExist(err) {
-				log.Infof("Creating new VMware disk [%s]", *disk)
-				vmDiskCmd := exec.Command(vmDiskManagerPath, "-c", "-s", fmt.Sprintf("%dMB", diskSz), "-a", "lsilogic", "-t", "0", *disk)
+				log.Fatalf("Unable to read file [%s], please check permissions", d.Path)
+			} else if os.IsNotExist(err) {
+				log.Infof("Creating new VMware disk [%s]", d.Path)
+				vmDiskCmd := exec.Command(vmDiskManagerPath, "-c", "-s", fmt.Sprintf("%dMB", d.Size), "-a", "lsilogic", "-t", "0", d.Path)
 				if err = vmDiskCmd.Run(); err != nil {
-					log.Fatalf("Error creating disk [%s]:  %v", *disk, err)
+					log.Fatalf("Error creating disk [%s]:  %v", d.Path, err)
 				}
+			} else {
+				log.Fatalf("Unable to read file [%s]: %v", d.Path, err)
 			}
 		} else {
-			log.Infof("Using existing disk [%s]", *disk)
+			log.Infof("Using existing disk [%s]", d.Path)
 		}
 	}
 
+	if len(disks) > 1 {
+		log.Fatalf("VMware driver currently only supports a single disk")
+	}
+
+	disk := ""
+	if len(disks) == 1 {
+		disk = disks[0].Path
+	}
+
 	// Build the contents of the VMWare .vmx file
-	vmx := buildVMX(*cpus, *mem, *disk, prefix)
+	vmx := buildVMX(*cpus, *mem, disk, prefix)
 	if vmx == "" {
 		log.Fatalf("VMware .vmx file could not be generated, please confirm inputs")
 	}
 
 	// Create the .vmx file
 	vmxPath := filepath.Join(*state, "linuxkit.vmx")
-	err = ioutil.WriteFile(vmxPath, []byte(vmx), 0644)
+	err := ioutil.WriteFile(vmxPath, []byte(vmx), 0644)
 	if err != nil {
 		log.Fatalf("Error writing .vmx file: %v", err)
 	}

src/cmd/linuxkit/util.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -118,3 +119,48 @@ func getDiskSizeMB(s string) (int, error) {
 	}
 	return strconv.Atoi(s)
 }
+
+// DiskConfig is the config for a disk
+type DiskConfig struct {
+	Path   string
+	Size   int
+	Format string
+}
+
+// Disks is the type for a list of DiskConfig
+type Disks []DiskConfig
+
+func (l *Disks) String() string {
+	return fmt.Sprint(*l)
+}
+
+// Set is used by flag to configure value from CLI
+func (l *Disks) Set(value string) error {
+	d := DiskConfig{}
+	s := strings.Split(value, ",")
+	for _, p := range s {
+		c := strings.SplitN(p, "=", 2)
+		switch len(c) {
+		case 1:
+			// assume it is a filename even if no file=x
+			d.Path = c[0]
+		case 2:
+			switch c[0] {
+			case "file":
+				d.Path = c[1]
+			case "size":
+				size, err := getDiskSizeMB(c[1])
+				if err != nil {
+					return err
+				}
+				d.Size = size
+			case "format":
+				d.Format = c[1]
+			default:
+				return fmt.Errorf("Unknown disk config: %s", c[0])
+			}
+		}
+	}
+	*l = append(*l, d)
+	return nil
+}

test/cases/010_platforms/000_qemu/030_run_qcow/test.sh

@@ -19,5 +19,5 @@ trap clean_up EXIT
 
 moby build -output qcow2 -name "${NAME}" test.yml
 [ -f "${NAME}.qcow2" ] || exit 1
-linuxkit run qemu -disk-format qcow2 "${NAME}.qcow2" | grep -q "Welcome to LinuxKit"
+linuxkit run qemu "${NAME}.qcow2" | grep -q "Welcome to LinuxKit"
 exit 0

test/cases/040_packages/013_mkimage/test.sh

@@ -19,7 +19,7 @@ trap clean_up EXIT
 # Test code goes here
 moby build -output kernel+initrd run.yml
 moby build -output kernel+initrd mkimage.yml
-linuxkit run qemu -disk-size 200 -disk-format qcow2 -disk disk.qcow2 -kernel mkimage
+linuxkit run qemu -disk disk.qcow2,size=200M,format=qcow2 -kernel mkimage
 linuxkit run qemu disk.qcow2
 
 exit 0