From 33cc9fd5c8d770915545728d0761ec87b69182e7 Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Thu, 25 May 2023 11:41:34 +0800 Subject: [PATCH] util: put `ParseDiskSpec` func to common Refer to @jlebon's suggestion, see https://github.com/coreos/coreos-assembler/pull/3477#discussion_r1198995514 --- mantle/platform/api/gcloud/compute.go | 49 +++++++++------------ mantle/platform/qemu.go | 61 ++++++++++++--------------- mantle/util/common.go | 36 ++++++++++++++++ 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/mantle/platform/api/gcloud/compute.go b/mantle/platform/api/gcloud/compute.go index 16e470959c..68a3b5b705 100644 --- a/mantle/platform/api/gcloud/compute.go +++ b/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) } diff --git a/mantle/platform/qemu.go b/mantle/platform/qemu.go index 07e763c7fe..9a2dbb75f7 100644 --- a/mantle/platform/qemu.go +++ b/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: -// [:,,...]. -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) diff --git a/mantle/util/common.go b/mantle/util/common.go index d78d7fa2ff..94acfac6b9 100644 --- a/mantle/util/common.go +++ b/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: +// [:,,...], 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 +}