Merge pull request #1888 from rneugeba/qemu

cmd: Unify disk command line options for local hypervisors

docs/external-disk.md

@@ -5,38 +5,18 @@
 2. Mount the disk
 
 ## Make Disk Available
-In order to make the disk available, you need to tell `linuxkit` where the disk file or block device is. This depends on your run method.
+In order to make the disk available, you need to tell `linuxkit` where the disk file or block device is.
 
-**Note:** With all of the options below, *always* give disk options before the _prefix_.
+All local `linuxkit run` methods (currently `hyperkit`, `qemu`, and `vmware`) take two arguments:
 
-### Mac HyperKit
+* `-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.
-`linuxkit run hyperkit -disk <path_to_disk> -disk-size <size> <prefix>`
-
-* `-disk-size <size>`: size of disk in MB, e.g. `-disk-size 4096` 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`
 
-Several important points to note:
+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`. 
 
-1. if you do not provide `-disk `_path_, `linuxkit` assumes the default to be `$PWD/`_prefix_`-disk.img`.
+If the disk at `<path>`, or the default if `-disk` option is not provided, does not exist, `linuxkit` will create one of size `<size>`.
-2. If the disk at `<path>`, or the default if `-disk` option is not provided, does not exist, `linuxkit` will create one of size `<size>`.
 
-### QEMU
+**TODO:** GCP
-`linuxkit run qemu -disk <path_to_disk> -disk-size <size> <prefix>`
-
-* `-disk-size <size>`: size of disk in MB, e.g. `-disk-size 4096` 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`
-
-Several important points to note:
-
-1. if you do not provide `-disk `_path_, `linuxkit` assumes the default to be `$PWD/`_prefix_`-disk.img`.
-2. If the disk at `<path>`, or the default if `-disk` option is not provided, does not exist, `linuxkit` will create one of size `<size>`.
-
-### vmware
-`linuxkit run vmware -disk <path_to_disk> <prefix>`
-
-* `-disk <path>`: use the disk at location _path_, e.g. `-disk foo.img` will use the disk at `$PWD/foo.img`
-
-Unlike with qemu and hyperkit, `linuxkit run vmware` _currently_ will not create the disk for you if it does not exist. We intend to align the functionality in the near future. 
 
 ## Mount the Disk
 A disk created or used via `hyperkit run` will be available inside the image at `/dev/vda` with the first partition at `/dev/vda1`.

src/cmd/linuxkit/run_hyperkit.go

@@ -27,8 +27,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")
-	diskSz := flags.Int("disk-size", 0, "Size of Disk 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 used")
+	disk := flags.String("disk", "", "Path to disk image to use")
 	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")
@@ -52,6 +52,11 @@ 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
@@ -92,7 +97,7 @@ func runHyperKit(args []string) {
 		log.Fatalf("Cannot open cmdline file: %v", err)
 	}
 
-	if *diskSz != 0 && *disk == "" {
+	if diskSz != 0 && *disk == "" {
 		*disk = filepath.Join(*state, "disk.img")
 	}
 
@@ -114,7 +119,7 @@ func runHyperKit(args []string) {
 	h.VSock = true
 	h.CPUs = *cpus
 	h.Memory = *mem
-	h.DiskSize = *diskSz
+	h.DiskSize = diskSz
 
 	err = h.Run(string(cmdline))
 	if err != nil {

src/cmd/linuxkit/run_qemu.go

@@ -58,8 +58,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")
-	diskSz := flags.String("disk-size", "", "Size of disk to create, only created if it doesn't exist")
 	diskFmt := flags.String("disk-format", "qcow2", "Format of disk: raw, qcow2 etc")
 
 	// Paths and settings for UEFI firware
@@ -87,6 +87,7 @@ func runQemu(args []string) {
 		os.Exit(1)
 	}
 	path := remArgs[0]
+	prefix := path
 
 	_, err := os.Stat(path)
 	stat := err == nil
@@ -103,13 +104,23 @@ func runQemu(args []string) {
 		// if path ends in .iso they meant an ISO
 		if strings.HasSuffix(path, ".iso") {
 			*isoBoot = true
+			prefix = strings.TrimSuffix(path, ".iso")
 		}
 		// autodetect EFI ISO from our default naming
 		if strings.HasSuffix(path, "-efi.iso") {
 			*uefiBoot = true
+			prefix = strings.TrimSuffix(path, "-efi.iso")
 		}
 	}
 
+	diskSz, err := getDiskSizeMB(*diskSzFlag)
+	if err != nil {
+		log.Fatalf("Could parse disk-size %s: %v", *diskSzFlag, err)
+	}
+	if diskSz != 0 && *disk == "" {
+		*disk = prefix + "-disk.img"
+	}
+
 	// user not trying to boot off ISO or kernel, so assume booting from a disk image
 	if !*kernelBoot && !*isoBoot {
 		if *disk != "" {
@@ -126,7 +137,7 @@ func runQemu(args []string) {
 		Kernel:         *kernelBoot,
 		GUI:            *enableGUI,
 		DiskPath:       *disk,
-		DiskSize:       *diskSz,
+		DiskSize:       fmt.Sprintf("%dM", diskSz),
 		DiskFormat:     *diskFmt,
 		FWPath:         *fw,
 		Arch:           *arch,

src/cmd/linuxkit/run_vmware.go

@@ -76,8 +76,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")
-	diskSz := flags.String("disk-size", "", "Size of the disk to create, only created if it doesn't exist")
 	state := flags.String("state", "", "Path to directory to keep VM state in")
 
 	if err := flags.Parse(args); err != nil {
@@ -99,6 +99,11 @@ 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
 
@@ -131,7 +136,7 @@ func runVMware(args []string) {
 		log.Fatalf("ERROR VMware executables can not be found, ensure software is installed")
 	}
 
-	if *disk == "" && *diskSz != "" {
+	if diskSz != 0 && *disk == "" {
 		*disk = filepath.Join(*state, "disk.vmdk")
 	}
 	if *disk != "" {
@@ -147,7 +152,7 @@ func runVMware(args []string) {
 			}
 			if os.IsNotExist(err) {
 				log.Infof("Creating new VMware disk [%s]", *disk)
-				vmDiskCmd := exec.Command(vmDiskManagerPath, "-c", "-s", *diskSz, "-a", "lsilogic", "-t", "0", *disk)
+				vmDiskCmd := exec.Command(vmDiskManagerPath, "-c", "-s", fmt.Sprintf("%dMB", diskSz), "-a", "lsilogic", "-t", "0", *disk)
 				if err = vmDiskCmd.Run(); err != nil {
 					log.Fatalf("Error creating disk [%s]:  %v", *disk, err)
 				}
@@ -165,7 +170,7 @@ func runVMware(args []string) {
 
 	// 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

@@ -97,3 +97,24 @@ func stringToIntArray(l string, sep string) ([]int, error) {
 	}
 	return i, nil
 }
+
+// Parse a string which is either a number in MB, or a number with
+// either M (for Megabytes) or G (for GigaBytes) as a suffix and
+// returns the number in MB. Return 0 if string is empty.
+func getDiskSizeMB(s string) (int, error) {
+	if s == "" {
+		return 0, nil
+	}
+	sz := len(s)
+	if strings.HasSuffix(s, "G") {
+		i, err := strconv.Atoi(s[:sz-1])
+		if err != nil {
+			return 0, err
+		}
+		return i * 1024, nil
+	}
+	if strings.HasSuffix(s, "M") {
+		s = s[:sz-1]
+	}
+	return strconv.Atoi(s)
+}