[rhcos-4.16] kola: add support for simulating 512e disks and add support for primaryDisk key

docs/cosa/run.md

@@ -87,6 +87,7 @@ Additional disks CLI arguments support optional flags using the `--add-disk
 
 - `mpath`: enables multipathing for the disk (see below for details).
 - `4k`: sets the disk as 4Kn (4096 physical sector size)
+- `512e`: sets the disk as 512e (4096 physical sector size, 512 logical sector size)
 - `channel=CHANNEL`: set the channel type (e.g. `virtio`, `nvme`, `scsi`)
 - `serial=NAME`: sets the disk serial; this can then be used to customize the
   default `diskN` naming documented above (e.g. `serial=foobar` will make the

docs/kola/external-tests.md

@@ -191,6 +191,7 @@ Here's an example `kola.json`:
     "platforms": "qemu",
     "tags": "sometagname needs-internet skip-base-checks othertag",
     "requiredTag": "special",
+    "primaryDisk": ["20G:mpath"],
     "additionalDisks": [ "5G" ],
     "minMemory": 4096,
     "minDisk": 15,
@@ -229,7 +230,10 @@ If a test has a `requiredTag`, it is run only if the required tag is specified.
 In the example above, the test would only run if `--tag special` was provided.
 
 The `additionalDisks` key has the same semantics as the `--add-disk` argument
-to `qemuexec`. It is currently only supported on `qemu`.
+to `qemuexec`. It is currently only supported on `qemu`. The `primaryDisk` key
+also supports the same syntax and controls the primary boot disk. Only for the
+`primaryDisk` key, the size can be omitted (e.g. `:mpath`), in which case the
+qcow2 will not be resized.
 
 The `injectContainer` boolean if set will cause the framework to inject
 the ostree base image container into the target system; the path can be

mantle/cmd/kola/options.go

@@ -157,6 +157,7 @@ func init() {
 	bv(&kola.QEMUOptions.NbdDisk, "qemu-nbd-socket", false, "Present the disks over NBD socket to qemu")
 	bv(&kola.QEMUOptions.MultiPathDisk, "qemu-multipath", false, "Enable multiple paths for the main disk")
 	bv(&kola.QEMUOptions.Native4k, "qemu-native-4k", false, "Force 4k sectors for main disk")
+	bv(&kola.QEMUOptions.Disk512e, "qemu-512e", false, "Force 512e layout for main disk")
 	bv(&kola.QEMUOptions.Nvme, "qemu-nvme", false, "Use NVMe for main disk")
 	bv(&kola.QEMUOptions.Swtpm, "qemu-swtpm", true, "Create temporary software TPM")
 	ssv(&kola.QEMUOptions.BindRO, "qemu-bind-ro", nil, "Inject a host directory; this does not automatically mount in the guest")

mantle/cmd/kola/qemuexec.go

@@ -139,8 +139,12 @@ func buildDiskFromOptions() *platform.Disk {
 		channel = "nvme"
 	}
 	sectorSize := 0
+	logicalSectorSize := 0
 	if kola.QEMUOptions.Native4k {
 		sectorSize = 4096
+	} else if kola.QEMUOptions.Disk512e {
+		sectorSize = 4096
+		logicalSectorSize = 512
 	}
 	options := []string{}
 	if kola.QEMUOptions.DriveOpts != "" {
@@ -155,13 +159,14 @@ func buildDiskFromOptions() *platform.Disk {
 	// Build the disk definition. Note that if kola.QEMUOptions.DiskImage is
 	// "" we'll just end up with a blank disk image, which is what we want.
 	disk := &platform.Disk{
-		BackingFile:   kola.QEMUOptions.DiskImage,
-		Channel:       channel,
-		Size:          size,
-		SectorSize:    sectorSize,
-		DriveOpts:     options,
-		MultiPathDisk: kola.QEMUOptions.MultiPathDisk,
-		NbdDisk:       kola.QEMUOptions.NbdDisk,
+		BackingFile:       kola.QEMUOptions.DiskImage,
+		Channel:           channel,
+		Size:              size,
+		SectorSize:        sectorSize,
+		LogicalSectorSize: logicalSectorSize,
+		DriveOpts:         options,
+		MultiPathDisk:     kola.QEMUOptions.MultiPathDisk,
+		NbdDisk:           kola.QEMUOptions.NbdDisk,
 	}
 	return disk
 }

mantle/kola/harness.go

@@ -1020,6 +1020,7 @@ type externalTestMeta struct {
 	Tags                      string   `json:"tags,omitempty"                      yaml:"tags,omitempty"`
 	RequiredTag               string   `json:"requiredTag,omitempty"               yaml:"requiredTag,omitempty"`
 	AdditionalDisks           []string `json:"additionalDisks,omitempty"           yaml:"additionalDisks,omitempty"`
+	PrimaryDisk               string   `json:"primaryDisk,omitempty"               yaml:"primaryDisk,omitempty"`
 	InjectContainer           bool     `json:"injectContainer,omitempty"           yaml:"injectContainer,omitempty"`
 	MinMemory                 int      `json:"minMemory,omitempty"                 yaml:"minMemory,omitempty"`
 	MinDiskSize               int      `json:"minDisk,omitempty"                   yaml:"minDisk,omitempty"`
@@ -1233,6 +1234,7 @@ ExecStart=%s
 		Tags:          []string{"external"},
 
 		AdditionalDisks:           targetMeta.AdditionalDisks,
+		PrimaryDisk:               targetMeta.PrimaryDisk,
 		InjectContainer:           targetMeta.InjectContainer,
 		MinMemory:                 targetMeta.MinMemory,
 		MinDiskSize:               targetMeta.MinDiskSize,
@@ -1752,6 +1754,7 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig
 
 		options := platform.MachineOptions{
 			MultiPathDisk:             t.MultiPathDisk,
+			PrimaryDisk:               t.PrimaryDisk,
 			AdditionalDisks:           t.AdditionalDisks,
 			MinMemory:                 t.MinMemory,
 			MinDiskSize:               t.MinDiskSize,

mantle/kola/register/register.go

@@ -70,21 +70,26 @@ type Test struct {
 	RequiredTag          string        // if specified, test is filtered by default unless tag is provided -- defaults to none
 	Description          string        // test description
 
-	// Whether the primary disk is multipathed.
+	// Whether the primary disk is multipathed. Deprecated in favour of PrimaryDisk.
 	MultiPathDisk bool
 
 	// Sizes of additional empty disks to attach to the node, followed by
 	// comma-separated list of optional options (e.g. ["1G",
 	// "5G:mpath,foo,bar"]) -- defaults to none.
 	AdditionalDisks []string
 
+	// Size of primary disk to attach to the node, followed by
+	// comma-separated list of optional options (e.g. "20G:mpath"]).
+	PrimaryDisk string
+
 	// InjectContainer will cause the ostree base image to be injected into the target
 	InjectContainer bool
 
 	// Minimum amount of memory in MB required for test.
 	MinMemory int
 
-	// Minimum amount of primary disk in GB required for test.
+	// Minimum amount of primary disk in GB required for test. Deprecated in favour
+	// of PrimaryDisk.
 	MinDiskSize int
 
 	// Additional amount of NICs required for test.

mantle/platform/api/gcloud/compute.go

@@ -38,7 +38,7 @@ func (a *API) vmname() string {
 func ParseDisk(spec string, zone string) (*compute.AttachedDisk, error) {
 	var diskInterface string
 
-	size, diskmap, err := util.ParseDiskSpec(spec)
+	size, diskmap, err := util.ParseDiskSpec(spec, false)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse disk spec %q: %w", spec, err)
 	}

mantle/platform/machine/qemu/cluster.go

@@ -140,29 +140,33 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
 		builder.MemoryMiB = 4096 // SE needs at least 4GB
 	}
 
-	channel := "virtio"
+	var primaryDisk platform.Disk
+	if options.PrimaryDisk != "" {
+		var diskp *platform.Disk
+		if diskp, err = platform.ParseDisk(options.PrimaryDisk, true); err != nil {
+			return nil, errors.Wrapf(err, "parsing primary disk spec '%s'", options.PrimaryDisk)
+		}
+		primaryDisk = *diskp
+	}
+
 	if qc.flight.opts.Nvme || options.Nvme {
-		channel = "nvme"
+		primaryDisk.Channel = "nvme"
 	}
-	sectorSize := 0
 	if qc.flight.opts.Native4k {
-		sectorSize = 4096
+		primaryDisk.SectorSize = 4096
+	} else if qc.flight.opts.Disk512e {
+		primaryDisk.SectorSize = 4096
+		primaryDisk.LogicalSectorSize = 512
 	}
-	multiPathDisk := options.MultiPathDisk || qc.flight.opts.MultiPathDisk
-	var diskSize string
-	if options.MinDiskSize > 0 {
-		diskSize = fmt.Sprintf("%dG", options.MinDiskSize)
-	} else {
-		diskSize = qc.flight.opts.DiskSize
+	if options.MultiPathDisk || qc.flight.opts.MultiPathDisk {
+		primaryDisk.MultiPathDisk = true
 	}
-	primaryDisk := platform.Disk{
-		BackingFile:   qc.flight.opts.DiskImage,
-		Channel:       channel,
-		Size:          diskSize,
-		SectorSize:    sectorSize,
-		MultiPathDisk: multiPathDisk,
+	if options.MinDiskSize > 0 {
+		primaryDisk.Size = fmt.Sprintf("%dG", options.MinDiskSize)
+	} else if qc.flight.opts.DiskSize != "" {
+		primaryDisk.Size = qc.flight.opts.DiskSize
 	}
-
+	primaryDisk.BackingFile = qc.flight.opts.DiskImage
 	if options.OverrideBackingFile != "" {
 		primaryDisk.BackingFile = options.OverrideBackingFile
 	}

mantle/platform/machine/qemu/flight.go

@@ -41,6 +41,7 @@ type Options struct {
 	NbdDisk       bool
 	MultiPathDisk bool
 	Native4k      bool
+	Disk512e      bool
 	Nvme          bool
 
 	//Option to create a temporary software TPM - true by default

mantle/platform/platform.go

@@ -155,6 +155,7 @@ type Flight interface {
 
 type MachineOptions struct {
 	MultiPathDisk             bool
+	PrimaryDisk               string
 	AdditionalDisks           []string
 	MinMemory                 int
 	MinDiskSize               int

mantle/platform/qemu.go

@@ -89,30 +89,32 @@ type QEMUMachine interface {
 
 // Disk holds the details of a virtual disk.
 type Disk struct {
-	Size          string   // disk image size in bytes, optional suffixes "K", "M", "G", "T" allowed.
-	BackingFile   string   // raw disk image to use.
-	BackingFormat string   // qcow2, raw, etc.  If unspecified will be autodetected.
-	Channel       string   // virtio (default), nvme, scsi
-	DeviceOpts    []string // extra options to pass to qemu -device. "serial=XXXX" makes disks show up as /dev/disk/by-id/virtio-<serial>
-	DriveOpts     []string // extra options to pass to -drive
-	SectorSize    int      // if not 0, override disk sector size
-	NbdDisk       bool     // if true, the disks should be presented over nbd:unix socket
-	MultiPathDisk bool     // if true, present multiple paths
-	Wwn           uint64   // Optional World wide name for the SCSI disk. If not set or set to 0, a random one will be generated. Used only with "channel=scsi". Must be an integer
+	Size              string   // disk image size in bytes, optional suffixes "K", "M", "G", "T" allowed.
+	BackingFile       string   // raw disk image to use.
+	BackingFormat     string   // qcow2, raw, etc.  If unspecified will be autodetected.
+	Channel           string   // virtio (default), nvme, scsi
+	DeviceOpts        []string // extra options to pass to qemu -device. "serial=XXXX" makes disks show up as /dev/disk/by-id/virtio-<serial>
+	DriveOpts         []string // extra options to pass to -drive
+	SectorSize        int      // if not 0, override disk sector size
+	LogicalSectorSize int      // if not 0, override disk sector size
+	NbdDisk           bool     // if true, the disks should be presented over nbd:unix socket
+	MultiPathDisk     bool     // if true, present multiple paths
+	Wwn               uint64   // Optional World wide name for the SCSI disk. If not set or set to 0, a random one will be generated. Used only with "channel=scsi". Must be an integer
 
 	attachEndPoint string   // qemuPath to attach to
 	dstFileName    string   // the prepared file
 	nbdServCmd     exec.Cmd // command to serve the disk
 }
 
-func ParseDisk(spec string) (*Disk, error) {
+func ParseDisk(spec string, allowNoSize bool) (*Disk, error) {
 	var channel string
 	sectorSize := 0
+	logicalSectorSize := 0
 	serialOpt := []string{}
 	multipathed := false
 	var wwn uint64
 
-	size, diskmap, err := util.ParseDiskSpec(spec)
+	size, diskmap, err := util.ParseDiskSpec(spec, allowNoSize)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse disk spec %q: %w", spec, err)
 	}
@@ -123,6 +125,9 @@ func ParseDisk(spec string) (*Disk, error) {
 			channel = value
 		case "4k":
 			sectorSize = 4096
+		case "512e":
+			sectorSize = 4096
+			logicalSectorSize = 512
 		case "mpath":
 			multipathed = true
 		case "serial":
@@ -138,13 +143,18 @@ func ParseDisk(spec string) (*Disk, error) {
 		}
 	}
 
+	sizeStr := ""
+	if size > 0 {
+		sizeStr = fmt.Sprintf("%dG", size)
+	}
 	return &Disk{
-		Size:          fmt.Sprintf("%dG", size),
-		Channel:       channel,
-		DeviceOpts:    serialOpt,
-		SectorSize:    sectorSize,
-		MultiPathDisk: multipathed,
-		Wwn:           wwn,
+		Size:              sizeStr,
+		Channel:           channel,
+		DeviceOpts:        serialOpt,
+		SectorSize:        sectorSize,
+		LogicalSectorSize: logicalSectorSize,
+		MultiPathDisk:     multipathed,
+		Wwn:               wwn,
 	}, nil
 }
 
@@ -1171,7 +1181,11 @@ func (builder *QemuBuilder) addDiskImpl(disk *Disk, primary bool) error {
 		channel = "virtio"
 	}
 	if disk.SectorSize != 0 {
-		diskOpts = append(diskOpts, fmt.Sprintf("physical_block_size=%[1]d,logical_block_size=%[1]d", disk.SectorSize))
+		logical := disk.LogicalSectorSize
+		if logical == 0 {
+			logical = disk.SectorSize
+		}
+		diskOpts = append(diskOpts, fmt.Sprintf("physical_block_size=%d,logical_block_size=%d", disk.SectorSize, logical))
 	}
 	// Primary disk gets bootindex 1, all other disks have unspecified
 	// bootindex, which means lower priority.
@@ -1292,7 +1306,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 := ParseDisk(spec); err != nil {
+		if disk, err := ParseDisk(spec, false); 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

@@ -125,10 +125,14 @@ func RunCmdTimeout(timeout time.Duration, cmd string, args ...string) error {
 
 // 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) {
+func ParseDiskSpec(spec string, allowNoSize bool) (int64, map[string]string, error) {
 	diskmap := map[string]string{}
 	split := strings.Split(spec, ":")
-	if split[0] == "" || (!strings.HasSuffix(split[0], "G")) {
+	if split[0] == "" {
+		if !allowNoSize {
+			return 0, nil, fmt.Errorf("no size provided in '%s'", spec)
+		}
+	} else if !strings.HasSuffix(split[0], "G") {
 		return 0, nil, fmt.Errorf("invalid size opt %s", spec)
 	}
 	var disksize string
@@ -149,10 +153,14 @@ func ParseDiskSpec(spec string) (int64, map[string]string, error) {
 	} 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)
+	var size int64 = 0
+	if disksize != "" {
+		disksize = strings.TrimSuffix(disksize, "G")
+		var err error
+		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
 }