util: put ParseDiskSpec func to common

Refer to @jlebon's suggestion, see #3477 (comment)

mantle/platform/api/gcloud/compute.go

@@ -17,7 +17,6 @@ package gcloud
 import (
 	"crypto/rand"
 	"fmt"
-	"strconv"
 	"strings"
 	"time"
 
@@ -35,39 +34,33 @@ func (a *API) vmname() string {
 	return fmt.Sprintf("%s-%x", a.options.BaseName, b)
 }
 
-// ["5G:interface=NVME"], by default the disk type is local-ssd
-func ParseDiskSpec(spec string, zone string) (*compute.AttachedDisk, error) {
-	split := strings.Split(spec, ":")
-	var diskinterface string
-	var disksize string
-	if len(split) == 1 {
-		disksize = split[0]
-	} else if len(split) == 2 {
-		disksize = split[0]
-		for _, opt := range strings.Split(split[1], ",") {
-			if strings.HasPrefix(opt, "interface=") {
-				diskinterface = strings.TrimPrefix(opt, "interface=")
-				if diskinterface == "" || (diskinterface != "NVME" && diskinterface != "SCSI") {
-					return nil, fmt.Errorf("invalid interface opt %s", opt)
-				}
-			} else {
-				return nil, fmt.Errorf("unknown disk option %s", opt)
+// ["5G:channel=nvme"], by default the disk type is local-ssd
+func ParseDisk(spec string, zone string) (*compute.AttachedDisk, error) {
+	var diskInterface string
+
+	size, diskmap, err := util.ParseDiskSpec(spec)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse disk spec %q: %w", spec, err)
+	}
+	for key, value := range diskmap {
+		switch key {
+		case "channel":
+			switch value {
+			case "nvme", "scsi":
+				diskInterface = strings.ToUpper(value)
+			default:
+				return nil, fmt.Errorf("invalid channel value: %q", value)
 			}
+		default:
+			return nil, fmt.Errorf("invalid key %q", key)
 		}
-	} else {
-		return nil, fmt.Errorf("invalid disk spec %s", spec)
-	}
-	disksize = strings.TrimSuffix(disksize, "G")
-	size, err := strconv.ParseInt(disksize, 10, 32)
-	if err != nil {
-		return nil, fmt.Errorf("invalid size opt %s: %w", disksize, err)
 	}
 
 	return &compute.AttachedDisk{
 		AutoDelete: true,
 		Boot:       false,
 		Type:       "SCRATCH",
-		Interface:  diskinterface,
+		Interface:  diskInterface,
 		InitializeParams: &compute.AttachedDiskInitializeParams{
 			DiskType:   "/zones/" + zone + "/diskTypes/local-ssd",
 			DiskSizeGb: size,
@@ -165,9 +158,9 @@ func (a *API) mkinstance(userdata, name string, keys []*agent.Key, opts platform
 	// attach aditional disk
 	for _, spec := range opts.AdditionalDisks {
 		plog.Debugf("Parsing disk spec %q\n", spec)
-		disk, err := ParseDiskSpec(spec, a.options.Zone)
+		disk, err := ParseDisk(spec, a.options.Zone)
 		if err != nil {
-			return nil, fmt.Errorf("failed to parse spec %s: %w", spec, err)
+			return nil, fmt.Errorf("failed to parse spec %q: %w", spec, err)
 		}
 		instance.Disks = append(instance.Disks, disk)
 	}

mantle/platform/qemu.go

@@ -101,46 +101,37 @@ type Disk struct {
 	nbdServCmd     exec.Cmd // command to serve the disk
 }
 
-// ParseDiskSpec converts a disk specification into a Disk. The format is:
-// <size>[:<opt1>,<opt2>,...].
-func ParseDiskSpec(spec string) (*Disk, error) {
-	split := strings.Split(spec, ":")
-	var size string
+func ParseDisk(spec string) (*Disk, error) {
 	var channel string
-	multipathed := false
 	sectorSize := 0
-	serial_opt := []string{}
-	if len(split) == 1 {
-		size = split[0]
-	} else if len(split) == 2 {
-		size = split[0]
-		for _, opt := range strings.Split(split[1], ",") {
-			if opt == "mpath" {
-				multipathed = true
-			} else if opt == "4k" {
-				sectorSize = 4096
-			} else if strings.HasPrefix(opt, "channel=") {
-				channel = strings.TrimPrefix(opt, "channel=")
-				if channel == "" {
-					return nil, fmt.Errorf("invalid channel opt %s", opt)
-				}
-			} else if strings.HasPrefix(opt, "serial=") {
-				serial := strings.TrimPrefix(opt, "serial=")
-				if serial == "" {
-					return nil, fmt.Errorf("invalid serial opt %s", opt)
-				}
-				serial_opt = []string{"serial=" + serial}
-			} else {
-				return nil, fmt.Errorf("unknown disk option %s", opt)
-			}
+	serialOpt := []string{}
+	multipathed := false
+
+	size, diskmap, err := util.ParseDiskSpec(spec)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse disk spec %q: %w", spec, err)
+	}
+
+	for key, value := range diskmap {
+		switch key {
+		case "channel":
+			channel = value
+		case "4k":
+			sectorSize = 4096
+		case "mpath":
+			multipathed = true
+		case "serial":
+			value = "serial=" + value
+			serialOpt = append(serialOpt, value)
+		default:
+			return nil, fmt.Errorf("invalid key %q", key)
 		}
-	} else {
-		return nil, fmt.Errorf("invalid disk spec %s", spec)
 	}
+
 	return &Disk{
-		Size:          size,
+		Size:          fmt.Sprintf("%dG", size),
 		Channel:       channel,
-		DeviceOpts:    serial_opt,
+		DeviceOpts:    serialOpt,
 		SectorSize:    sectorSize,
 		MultiPathDisk: multipathed,
 	}, nil
@@ -1225,7 +1216,7 @@ func (builder *QemuBuilder) AddDisk(disk *Disk) error {
 // AddDisksFromSpecs adds multiple secondary disks from their specs.
 func (builder *QemuBuilder) AddDisksFromSpecs(specs []string) error {
 	for _, spec := range specs {
-		if disk, err := ParseDiskSpec(spec); err != nil {
+		if disk, err := ParseDisk(spec); err != nil {
 			return errors.Wrapf(err, "parsing additional disk spec '%s'", spec)
 		} else if err = builder.AddDisk(disk); err != nil {
 			return errors.Wrapf(err, "adding additional disk '%s'", spec)

mantle/util/common.go

@@ -19,6 +19,8 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
+	"strings"
 	"time"
 	"unsafe"
 
@@ -120,3 +122,37 @@ func RunCmdTimeout(timeout time.Duration, cmd string, args ...string) error {
 		return fmt.Errorf("%s timed out after %s", cmd, timeout)
 	}
 }
+
+// ParseDiskSpec converts a disk specification into a Disk. The format is:
+// <size>[:<opt1>,<opt2>,...], like ["5G:channel=nvme"]
+func ParseDiskSpec(spec string) (int64, map[string]string, error) {
+	diskmap := map[string]string{}
+	split := strings.Split(spec, ":")
+	if split[0] == "" || (!strings.HasSuffix(split[0], "G")) {
+		return 0, nil, fmt.Errorf("invalid size opt %s", spec)
+	}
+	var disksize string
+	if len(split) == 1 {
+		disksize = split[0]
+	} else if len(split) == 2 {
+		disksize = split[0]
+		for _, opt := range strings.Split(split[1], ",") {
+			kvsplit := strings.SplitN(opt, "=", 2)
+			if len(kvsplit) == 0 {
+				return 0, nil, fmt.Errorf("invalid empty option found in spec %q", spec)
+			} else if len(kvsplit) == 1 {
+				diskmap[opt] = ""
+			} else {
+				diskmap[kvsplit[0]] = kvsplit[1]
+			}
+		}
+	} else {
+		return 0, nil, fmt.Errorf("invalid disk spec %s", spec)
+	}
+	disksize = strings.TrimSuffix(disksize, "G")
+	size, err := strconv.ParseInt(disksize, 10, 32)
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed to convert %q to int64: %w", disksize, err)
+	}
+	return size, diskmap, nil
+}