From 1f48a4f400b32f2b1ef09c1fa026be667bd2b881 Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 12 Oct 2026 21:53:57 +0300 Subject: [PATCH] tests: Cover SCSI commands on local and iSCSI targets Add shared sg3_utils scenarios for scst_local and iSCSI exports: inquiry and VPD identity, capacity, LUN visibility, write protection and invalid LBA handling. Require the expected SCSI status and raw sense for rejected commands, then verify that valid reads still work. Use temporary fileio backends with 512-byte sectors. Keep fixture identifiers within vdisk's 20-character USN limit. Match each exported device to its fixture by identity and geometry before submitting commands. Carry binary responses over the existing supervised command transports and retain fixture ownership through successful cleanup. Add framework checks for response parsing, device selection and command interruption. Document tool prerequisites and the scsi label for both transports through the common Make targets. --- Makefile | 2 +- tests/ginkgo/AGENTS.md | 9 +- tests/ginkgo/README.md | 13 +- tests/ginkgo/internal/dataio/command.go | 68 +++++ tests/ginkgo/internal/dataio/command.py | 32 +++ tests/ginkgo/internal/dataio/command_test.go | 54 ++++ tests/ginkgo/internal/dataio/device.go | 192 +++++++++++++ tests/ginkgo/internal/dataio/device_test.go | 70 +++++ tests/ginkgo/internal/dataio/scsi.go | 283 +++++++++++++++++++ tests/ginkgo/internal/dataio/scsi_test.go | 104 +++++++ tests/ginkgo/internal/dataio/specs.go | 29 ++ tests/ginkgo/iscsi/dataio_test.go | 22 ++ tests/ginkgo/iscsi/fixture_test.go | 45 ++- tests/ginkgo/suite/dataio_test.go | 39 +++ 14 files changed, 950 insertions(+), 12 deletions(-) create mode 100644 tests/ginkgo/internal/dataio/command.go create mode 100644 tests/ginkgo/internal/dataio/command.py create mode 100644 tests/ginkgo/internal/dataio/command_test.go create mode 100644 tests/ginkgo/internal/dataio/device.go create mode 100644 tests/ginkgo/internal/dataio/device_test.go create mode 100644 tests/ginkgo/internal/dataio/scsi.go create mode 100644 tests/ginkgo/internal/dataio/scsi_test.go create mode 100644 tests/ginkgo/internal/dataio/specs.go create mode 100644 tests/ginkgo/iscsi/dataio_test.go create mode 100644 tests/ginkgo/suite/dataio_test.go diff --git a/Makefile b/Makefile index 5decc826a..adc71c7e5 100644 --- a/Makefile +++ b/Makefile @@ -142,7 +142,7 @@ help: @echo " ginkgo-config: validate GINKGO_CONFIG (JSON file)" @echo " ginkgo-test: load built SCST modules, then run tests (root)" @echo " ginkgo-test-build: build and load SCST, then run tests (root)" - @echo " GINKGO_LABEL selects test groups: local, iscsi, basic, stress" + @echo " GINKGO_LABEL selects test groups: local, iscsi, basic, stress, scsi" @echo "" @echo "Build variables:" @echo " WERROR=y|n : treat warnings as errors (default: n)" diff --git a/tests/ginkgo/AGENTS.md b/tests/ginkgo/AGENTS.md index 35a0f565d..a3be00502 100644 --- a/tests/ginkgo/AGENTS.md +++ b/tests/ginkgo/AGENTS.md @@ -2,16 +2,19 @@ Use [README.md](README.md) for setup, configuration, commands and recovery. `suite/` owns local tests, `iscsi/` owns target/SSH-initiator tests, and -`internal/` owns shared configuration, execution and fixtures. +`internal/` owns shared configuration, execution, fixtures and data checks. ## Interfaces and lifecycle - Management operations and assertions use `scstadmin`, never direct sysfs or Perl-library calls. A rejection covers the whole management chain, not necessarily a kernel check. Check operation errors even with exit status 0. -- Initiator management uses `iscsiadm` over SSH. +- Initiator management uses `iscsiadm` over SSH; sg3_utils uses only + fixture-owned exports. Preserve device identity and geometry checks before + writes. - Use shared command runners with contexts, timeouts, logs and failure handling. - Execution failures must not satisfy assertions for expected rejections. + Execution failures must not satisfy assertions for expected rejections; + SCSI rejection tests must check status and sense. - Keep ownership and cleanup explicit for configuration, backing files, sessions, node/iface records, the daemon and modules. Restore changed global settings; never use global initiator logout, deletion or service restarts diff --git a/tests/ginkgo/README.md b/tests/ginkgo/README.md index 11be96ad8..96fc2a3c2 100644 --- a/tests/ginkgo/README.md +++ b/tests/ginkgo/README.md @@ -1,7 +1,8 @@ # SCST component tests -Ginkgo/Gomega tests exercise SCST management through `scstadmin` on -`scst_local` and TCP iSCSI with a separate initiator controlled over SSH. +Ginkgo/Gomega tests exercise management through `scstadmin` and SCSI commands +through sg3_utils. Transports are `scst_local` and TCP iSCSI with a separate +initiator controlled over SSH. ## Setup and run @@ -44,7 +45,7 @@ runtime directories under `/var/lib/scst` remain. ## Select tests The default is `GINKGO_LABEL='local && basic'`. Combine `local` or `iscsi` -with `basic` (management) or other labels from the listing below. +with `basic` (management) or `scsi` (protocol checks). `GINKGO_FOCUS` additionally filters spec names by regexp. ```sh @@ -96,6 +97,12 @@ and set both `disposable_host` flags to `true`. The suite owns its target daemon and initiator sessions/node/iface records. It preserves existing records and leaves initiator services and modules loaded. +## SCSI commands + +Install Python 3, util-linux and sg3_utils on the command host: the target for +`local`, the initiator for `iscsi`. Cases check identity, capacity, LUN +visibility and rejection status/sense on temporary fileio exports. + ## Checks and recovery `make ginkgo-check` runs framework unit tests, vet and suite compilation; diff --git a/tests/ginkgo/internal/dataio/command.go b/tests/ginkgo/internal/dataio/command.go new file mode 100644 index 000000000..e1ca8d49e --- /dev/null +++ b/tests/ginkgo/internal/dataio/command.go @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "context" + _ "embed" + "encoding/json" + "fmt" + "time" + + "scst-component-tests/internal/admin" +) + +type RunFunc func(context.Context, time.Duration, ...string) (admin.Result, error) + +//go:embed command.py +var binaryCommand string + +type Reply struct { + ExitCode int `json:"exit_code"` + Data []byte `json:"data"` + Stderr string `json:"stderr"` +} + +func invoke(ctx context.Context, run RunFunc, timeout time.Duration, request any) (Reply, error) { + var response Reply + data, err := json.Marshal(request) + if err != nil { + return response, err + } + r, err := run(ctx, timeout, "python3", "-c", binaryCommand, string(data)) + if err != nil { + return response, fmt.Errorf("data command execution: %w\n%s", err, r.String()) + } + if r.ExitCode != 0 { + return response, fmt.Errorf("data command helper: %s", r.String()) + } + var wire struct { + ExitCode *int `json:"exit_code"` + Data *[]byte `json:"data"` + Stderr *string `json:"stderr"` + } + if err := json.Unmarshal([]byte(r.Stdout), &wire); err != nil { + return response, fmt.Errorf("data command response: %w", err) + } + if wire.ExitCode == nil || wire.Data == nil || wire.Stderr == nil { + return response, fmt.Errorf("incomplete data command response") + } + response = Reply{*wire.ExitCode, *wire.Data, *wire.Stderr} + return response, nil +} + +func command(ctx context.Context, run RunFunc, timeout time.Duration, input []byte, argv ...string) (Reply, error) { + return invoke(ctx, run, timeout, struct { + Args []string `json:"argv"` + Input []byte `json:"input,omitempty"` + }{argv, input}) +} + +func success(r Reply, err error) ([]byte, error) { + if err != nil { + return nil, err + } + if r.ExitCode != 0 { + return nil, fmt.Errorf("data command exit %d: %s", r.ExitCode, r.Stderr) + } + return r.Data, nil +} diff --git a/tests/ginkgo/internal/dataio/command.py b/tests/ginkgo/internal/dataio/command.py new file mode 100644 index 000000000..0ef13bce1 --- /dev/null +++ b/tests/ginkgo/internal/dataio/command.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: GPL-2.0-only +"""Carry binary sg output over the existing argv transport. + +The caller supervises the entire process group, including the child. A child +signal is propagated so that the command runner latches uncertain execution. +""" +import base64 +import json +import os +import signal +import subprocess +import sys +import tempfile + + +def main(): + request = json.loads(sys.argv[1]) + with tempfile.TemporaryDirectory(prefix="scst-dataio-") as directory: + argv = request["argv"] + result = subprocess.run( + argv, input=base64.b64decode(request.get("input", "")), + stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=directory, + env=dict(os.environ, LC_ALL="C")) + if result.returncode < 0: + os.kill(os.getpid(), signal.SIGKILL) + print(json.dumps({"exit_code": result.returncode, + "data": base64.b64encode(result.stdout).decode("ascii"), + "stderr": result.stderr.decode("utf-8", errors="replace")})) + + +if __name__ == "__main__": + main() diff --git a/tests/ginkgo/internal/dataio/command_test.go b/tests/ginkgo/internal/dataio/command_test.go new file mode 100644 index 000000000..319ad7170 --- /dev/null +++ b/tests/ginkgo/internal/dataio/command_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "bytes" + "context" + "os/exec" + "testing" + "time" + + "scst-component-tests/internal/admin" +) + +func TestBinaryCommandPreservesOutputAndExit(t *testing.T) { + if _, err := exec.LookPath("python3"); err != nil { + t.Skip("python3 not available") + } + run := func(ctx context.Context, _ time.Duration, argv ...string) (admin.Result, error) { + out, err := exec.CommandContext(ctx, argv[0], argv[1:]...).Output() + return admin.Result{Stdout: string(out)}, err + } + payload := []byte{0, 0xff, 0x80, '\n'} + r, err := command(context.Background(), run, time.Second, payload, "python3", "-c", + "import sys; sys.stdout.buffer.write(sys.stdin.buffer.read()); sys.stderr.write('sense'); sys.exit(7)") + if err != nil || r.ExitCode != 7 || r.Stderr != "sense" || !bytes.Equal(r.Data, payload) { + t.Fatalf("binary response: %+v %v", r, err) + } + for _, output := range []string{"null", "{}", `{"exit_code":0,"stderr":""}`} { + fake := func(context.Context, time.Duration, ...string) (admin.Result, error) { + return admin.Result{Stdout: output}, nil + } + if _, err := command(context.Background(), fake, time.Second, nil, "unused"); err == nil { + t.Fatal("accepted missing reply") + } + } +} + +func TestChildInterruptionBlocksFurtherCommands(t *testing.T) { + if _, err := exec.LookPath("python3"); err != nil { + t.Skip("python3 not available") + } + for name, script := range map[string]string{ + "signal": "import os,signal; os.kill(os.getpid(), signal.SIGTERM)", + "timeout": "import time; time.sleep(5)", + } { + t.Run(name, func(t *testing.T) { + client := &admin.Client{} + _, err := command(context.Background(), client.RunProgram, 500*time.Millisecond, nil, "python3", "-c", script) + if err == nil || client.Blocked() == nil { + t.Fatalf("lost supervision failure: %v, latch %v", err, client.Blocked()) + } + }) + } +} diff --git a/tests/ginkgo/internal/dataio/device.go b/tests/ginkgo/internal/dataio/device.go new file mode 100644 index 000000000..235346e1c --- /dev/null +++ b/tests/ginkgo/internal/dataio/device.go @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "context" + "crypto/sha256" + "encoding/json" + "fmt" + "os" + "regexp" + "strings" + "time" + + "scst-component-tests/internal/testcfg" +) + +type Profile struct { + Handler string + ReadOnly bool + ExtraLUNs bool +} + +type Backend struct { + Handler, Attributes, Serial string + Bytes uint64 + BlockSize uint32 +} + +type block struct { + HCTL string `json:"hctl"` + Path string `json:"path"` + Type string `json:"type"` + Size uint64 `json:"size"` + Logical uint32 `json:"log-sec"` + ReadOnly bool `json:"ro"` + Serial string `json:"serial"` + Filesystem *string `json:"fstype"` + Mountpoints []*string `json:"mountpoints"` + Children []block `json:"children"` +} + +func blocks(output string) ([]block, error) { + var list struct { + Devices []block `json:"blockdevices"` + } + if err := json.Unmarshal([]byte(output), &list); err != nil { + return nil, err + } + if list.Devices == nil { + return nil, fmt.Errorf("missing lsblk device list") + } + return list.Devices, nil +} + +func unused(b block) error { + if b.Type != "disk" || len(b.Children) != 0 || b.Filesystem != nil && *b.Filesystem != "" { + return fmt.Errorf("%s must be a plain disk without partitions, holders or filesystem signatures", b.Path) + } + for _, mount := range b.Mountpoints { + if mount != nil && *mount != "" { + return fmt.Errorf("%s is mounted at %s", b.Path, *mount) + } + } + return nil +} + +func listBlocks(ctx context.Context, run RunFunc, timeout time.Duration, path string) ([]block, error) { + args := []string{"lsblk", "--json", "--bytes", "--tree", "--output", "PATH,TYPE,SIZE,LOG-SEC,RO,MOUNTPOINTS,FSTYPE,SERIAL,HCTL"} + if path != "" { + args = append(args, "--", path) + } + r, err := run(ctx, timeout, args...) + if err != nil { + return nil, err + } + if r.ExitCode != 0 { + return nil, fmt.Errorf("lsblk: %s", r.String()) + } + return blocks(r.Stdout) +} + +// PrepareBackend owns only newly created files. The caller records them before +// any fallible operation and removes them only after successful SCST cleanup. +func PrepareBackend(ctx context.Context, cfg testcfg.Config, artifacts, id string, options Profile, run RunFunc, ownFile func(string)) (Backend, error) { + b := Backend{Handler: options.Handler, Serial: id, Bytes: 16 * 1024 * 1024, BlockSize: 512} + path := "" + switch options.Handler { + case "vdisk_fileio": + dir := cfg.BackingDir + if dir == "" { + dir = artifacts + } + f, err := os.CreateTemp(dir, id+"-*.img") + if err != nil { + return b, err + } + path = f.Name() + ownFile(path) + err = f.Truncate(int64(b.Bytes)) + closeErr := f.Close() + if err != nil { + return b, err + } + if closeErr != nil { + return b, closeErr + } + default: + return b, fmt.Errorf("unsupported data handler %q", options.Handler) + } + if strings.ContainsAny(path, " \t\r\n,;\"{}\\") { + return b, fmt.Errorf("backing path cannot be represented in scstadmin attributes") + } + b.Attributes = fmt.Sprintf("filename=%s,blocksize=%d,thin_provisioned=0,t10_dev_id=%s", path, b.BlockSize, id) + if options.Handler == "vdisk_fileio" { + b.Attributes += ",o_direct=0" + } + return b, nil +} + +// Endpoint addresses only a fixture-owned exported disk, never its backing file. +type Endpoint struct { + Run RunFunc + Timeout time.Duration + Backend Backend + ReadOnly bool + Artifacts, Name, Path string +} + +func (e *Endpoint) Discover(ctx context.Context) error { + devices, err := listBlocks(ctx, e.Run, e.Timeout, "") + if err != nil { + return err + } + var matches []block + var visit func([]block) + visit = func(list []block) { + for _, d := range list { + if d.Serial == e.Backend.Serial && regexp.MustCompile(`^\d+:\d+:\d+:0$`).MatchString(d.HCTL) { + matches = append(matches, d) + } + visit(d.Children) + } + } + visit(devices) + if len(matches) != 1 { + return fmt.Errorf("expected one exported disk with serial %s, found %d", e.Backend.Serial, len(matches)) + } + d := matches[0] + if err := unused(d); err != nil { + return err + } + if !regexp.MustCompile(`^/dev/[a-zA-Z0-9/_-]+$`).MatchString(d.Path) || d.Size != e.Backend.Bytes || d.Logical != e.Backend.BlockSize || d.ReadOnly != e.ReadOnly { + return fmt.Errorf("exported device geometry/access mismatch: %+v", d) + } + e.Path = d.Path + if _, err := e.sg(ctx, "sg_turs"); err != nil { + return err + } + return e.Identity(ctx) +} + +func (e *Endpoint) Check(ctx context.Context) error { + path := e.Path + if err := e.Discover(ctx); err != nil { + return err + } + if path != "" && e.Path != path { + return fmt.Errorf("exported device path changed from %s to %s", path, e.Path) + } + return nil +} + +func (e *Endpoint) sg(ctx context.Context, args ...string) ([]byte, error) { + return success(command(ctx, e.Run, e.Timeout, nil, append(args, e.Path)...)) +} + +// DeviceID also supplies the vdisk USN, which is limited to 20 characters. +func DeviceID(name string) string { return fmt.Sprintf("%x", sha256.Sum256([]byte(name)))[:20] } + +func RequireTools(ctx context.Context, run RunFunc, timeout time.Duration) error { + tools := []string{"python3", "lsblk", "sg_inq", "sg_readcap", "sg_luns", "sg_raw", "sg_turs", "sg_sync"} + for _, tool := range tools { + r, err := run(ctx, timeout, tool, "--version") + if err != nil { + return err + } + if r.ExitCode != 0 { + return fmt.Errorf("required tool %s: %s", tool, r.String()) + } + } + return nil +} diff --git a/tests/ginkgo/internal/dataio/device_test.go b/tests/ginkgo/internal/dataio/device_test.go new file mode 100644 index 000000000..3259ec98b --- /dev/null +++ b/tests/ginkgo/internal/dataio/device_test.go @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "context" + "encoding/json" + "testing" + "time" + + "scst-component-tests/internal/admin" +) + +func TestDeviceIDFitsVDiskUSN(t *testing.T) { + seen := make(map[string]string) + for _, name := range []string{ + "scsttest_0123456789abcdef_1", + "scsttest_0123456789abcdef_2", + "scsttest_0123456789abcdef_1_io", + } { + id := DeviceID(name) + if len(id) == 0 || len(id) > 20 { + t.Fatalf("fixture identifier %q exceeds the 20-character vdisk USN limit", id) + } + if previous, exists := seen[id]; exists { + t.Fatalf("fixtures %q and %q share identifier %q", previous, name, id) + } + seen[id] = name + } +} + +func TestRejectUnsafeDisk(t *testing.T) { + mounted, fs := "/mnt/data", "xfs" + for _, b := range []block{ + {Type: "part"}, {Type: "disk", Children: []block{{Type: "part"}}}, + {Type: "disk", Mountpoints: []*string{&mounted}}, {Type: "disk", Filesystem: &fs}, + } { + if unused(b) == nil { + t.Fatalf("accepted disk in use: %+v", b) + } + } + if err := unused(block{Type: "disk", Mountpoints: []*string{nil}}); err != nil { + t.Fatal(err) + } +} + +func TestDiscoverRejectsAmbiguousOrWrongDiskBeforeSG(t *testing.T) { + good := block{Path: "/dev/sdz", Type: "disk", HCTL: "1:0:0:0", Serial: "owned", Size: 192 << 20, Logical: 512} + for name, disks := range map[string][]block{ + "absent": {}, "duplicate": {good, good}, + "wrong LUN": {func() block { b := good; b.HCTL = "1:0:0:7"; return b }()}, + "wrong size": {func() block { b := good; b.Size--; return b }()}, + "wrong sector": {func() block { b := good; b.Logical = 4096; return b }()}, + "readonly": {func() block { b := good; b.ReadOnly = true; return b }()}, + "other serial": {func() block { b := good; b.Serial = "other"; return b }()}, + } { + t.Run(name, func(t *testing.T) { + run := func(_ context.Context, _ time.Duration, argv ...string) (admin.Result, error) { + if argv[0] != "lsblk" { + t.Fatalf("unsafe device reached SCSI utility: %v", argv) + } + data, err := json.Marshal(map[string]any{"blockdevices": disks}) + return admin.Result{Stdout: string(data)}, err + } + e := Endpoint{Run: run, Backend: Backend{Serial: "owned", Bytes: 192 << 20, BlockSize: 512}} + if e.Discover(context.Background()) == nil { + t.Fatal("accepted wrong disk") + } + }) + } +} diff --git a/tests/ginkgo/internal/dataio/scsi.go b/tests/ginkgo/internal/dataio/scsi.go new file mode 100644 index 000000000..76b30ba7f --- /dev/null +++ b/tests/ginkgo/internal/dataio/scsi.go @@ -0,0 +1,283 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "bytes" + "context" + "encoding/binary" + "encoding/hex" + "fmt" + "slices" + "strings" +) + +func vpd(data []byte, page byte) ([]byte, error) { + if len(data) < 4 || data[0] != 0 || data[1] != page { + return nil, fmt.Errorf("invalid VPD page %02x: %x", page, data) + } + n := int(binary.BigEndian.Uint16(data[2:4])) + if len(data) < 4+n { + return nil, fmt.Errorf("truncated VPD page %02x", page) + } + return data[4 : 4+n], nil +} + +func identity83(data []byte, serial string) error { + descriptors, err := vpd(data, 0x83) + if err != nil { + return err + } + found := false + for len(descriptors) > 0 { + if len(descriptors) < 4 || int(descriptors[3])+4 > len(descriptors) { + return fmt.Errorf("truncated VPD designator") + } + n := int(descriptors[3]) + // ASCII T10 vendor identifier, associated with the logical unit. + if descriptors[0]&0xf == 2 && descriptors[1]&0x3f == 1 && n >= 8 { + found = found || strings.TrimRight(string(descriptors[12:4+n]), "\x00 ") == serial + } + descriptors = descriptors[4+n:] + } + if !found { + return fmt.Errorf("VPD 83 lacks the fixture's logical-unit identifier %s", serial) + } + return nil +} + +func (e *Endpoint) Identity(ctx context.Context) error { + data, err := e.sg(ctx, "sg_inq", "--raw", "--len=96") + if err != nil { + return err + } + if len(data) < 36 || data[0] != 0 || int(data[4])+5 > len(data) { + return fmt.Errorf("invalid direct-access INQUIRY: %x", data) + } + data, err = e.sg(ctx, "sg_inq", "--raw", "--page=0x80") + if err != nil { + return err + } + serial, err := vpd(data, 0x80) + if err != nil { + return err + } + if strings.TrimRight(string(serial), "\x00 ") != e.Backend.Serial { + return fmt.Errorf("VPD 80 serial mismatch: %q", serial) + } + data, err = e.sg(ctx, "sg_inq", "--raw", "--page=0x83") + if err != nil { + return err + } + return identity83(data, e.Backend.Serial) +} + +func capacity(data []byte, sixteen bool) (uint64, uint32, error) { + if sixteen { + if len(data) != 32 { + return 0, 0, fmt.Errorf("READ CAPACITY(16) returned %d bytes", len(data)) + } + return binary.BigEndian.Uint64(data[:8]), binary.BigEndian.Uint32(data[8:12]), nil + } + if len(data) != 8 { + return 0, 0, fmt.Errorf("READ CAPACITY(10) returned %d bytes", len(data)) + } + return uint64(binary.BigEndian.Uint32(data[:4])), binary.BigEndian.Uint32(data[4:]), nil +} + +func (e *Endpoint) Capacity(ctx context.Context) error { + for _, sixteen := range []bool{false, true} { + flag := "--10" + if sixteen { + flag = "--16" + } + data, err := e.sg(ctx, "sg_readcap", "--raw", flag) + if err != nil { + return err + } + last, block, err := capacity(data, sixteen) + if err != nil { + return err + } + want := e.Backend.Bytes/uint64(e.Backend.BlockSize) - 1 + if !sixteen && want > 0xffffffff { + want = 0xffffffff + } + if last != want || block != e.Backend.BlockSize { + return fmt.Errorf("READ CAPACITY %s: last LBA %d, block %d; want %d, %d", flag, last, block, want, e.Backend.BlockSize) + } + } + return nil +} + +func luns(data []byte) ([]int, error) { + if len(data) < 8 { + return nil, fmt.Errorf("short REPORT LUNS header") + } + n := int(binary.BigEndian.Uint32(data[:4])) + if n%8 != 0 || n > len(data)-8 { + return nil, fmt.Errorf("truncated/misaligned REPORT LUNS") + } + var result []int + for data = data[8 : 8+n]; len(data) > 0; data = data[8:] { + // These fixtures use peripheral addressing and simple LUN numbers. + if data[0] != 0 || !bytes.Equal(data[2:8], make([]byte, 6)) { + return nil, fmt.Errorf("unexpected LUN encoding: %x", data[:8]) + } + result = append(result, int(data[1])) + } + slices.Sort(result) + return result, nil +} + +func (e *Endpoint) LUNs(ctx context.Context) error { + data, err := e.sg(ctx, "sg_luns", "--raw", "--maxlen=256") + if err != nil { + return err + } + got, err := luns(data) + if err != nil { + return err + } + if !slices.Equal(got, []int{0, 7}) { + return fmt.Errorf("REPORT LUNS = %v, want only accessible LUNs [0 7]", got) + } + return nil +} + +// Decode raw sense from sg_raw --verbose. A utility/OS error without a SCSI +// CHECK CONDITION and the exact key/ASC/ASCQ cannot satisfy a negative test. +func sense(stderr string) ([3]byte, error) { + var result [3]byte + if !strings.Contains(strings.ToLower(stderr), "scsi status: check condition") { + return result, fmt.Errorf("missing CHECK CONDITION: %s", stderr) + } + _, raw, ok := strings.Cut(stderr, "Raw sense data (in hex)") + if !ok { + return result, fmt.Errorf("missing raw sense: %s", stderr) + } + _, raw, _ = strings.Cut(raw, "\n") + var data []byte + for _, line := range strings.Split(raw, "\n") { + fields := strings.Fields(line) + if len(fields) == 0 { + if len(data) != 0 { + break + } + continue + } + var row []byte + for _, f := range fields { + if len(f) != 2 { + break + } + b, err := hex.DecodeString(f) + if err != nil { + break + } + row = append(row, b[0]) + } + if len(row) != len(fields) { + break + } + data = append(data, row...) + } + if len(data) < 8 || int(data[7])+8 > len(data) { + return result, fmt.Errorf("truncated raw sense: %x", data) + } + switch data[0] & 0x7f { + case 0x70: + if len(data) < 14 || data[7] < 6 { + return result, fmt.Errorf("short fixed sense") + } + return [3]byte{data[2] & 0xf, data[12], data[13]}, nil + case 0x72: + return [3]byte{data[1] & 0xf, data[2], data[3]}, nil + default: + return result, fmt.Errorf("unexpected/deferred sense response: %x", data[0]) + } +} + +func (e *Endpoint) raw(ctx context.Context, write bool, lba uint64, input []byte) (Reply, error) { + cdb := make([]byte, 16) + cdb[0] = 0x88 // READ(16), one logical block. + if write { + cdb[0] = 0x8a + } + binary.BigEndian.PutUint64(cdb[2:10], lba) + binary.BigEndian.PutUint32(cdb[10:14], 1) + // O_RDONLY permits opening a read-only LUN. Root SG_IO still submits the + // WRITE CDB; its rejection must carry target sense, not an open() error. + args := []string{"sg_raw", "--readonly", "--verbose", "--binary"} + if write { + args = append(args, fmt.Sprintf("--send=%d", len(input))) + } else { + args = append(args, fmt.Sprintf("--request=%d", e.Backend.BlockSize)) + } + args = append(args, e.Path) + for _, b := range cdb { + args = append(args, fmt.Sprintf("%02x", b)) + } + return command(ctx, e.Run, e.Timeout, input, args...) +} + +func rejection(r Reply, err error, exit int, want [3]byte) error { + if err != nil { + return err + } + if r.ExitCode != exit { + return fmt.Errorf("sg_raw exit %d, want %d: %s", r.ExitCode, exit, r.Stderr) + } + got, err := sense(r.Stderr) + if err != nil { + return err + } + if got != want { + return fmt.Errorf("sense key/ASC/ASCQ %x, want %x: %s", got, want, r.Stderr) + } + return nil +} + +func (e *Endpoint) ReadOnlyWrite(ctx context.Context) error { + if err := e.Check(ctx); err != nil { + return err + } + before, err := success(e.raw(ctx, false, 0, nil)) + if err != nil { + return err + } + if len(before) != int(e.Backend.BlockSize) { + return fmt.Errorf("short baseline READ") + } + payload := bytes.Repeat([]byte{0xa5}, len(before)) + if bytes.Equal(payload, before) { + payload[0] ^= 0xff + } + r, err := e.raw(ctx, true, 0, payload) + // The read-only LUN check uses scst_sense_data_protect (07/00/00). + if err := rejection(r, err, 7, [3]byte{7, 0, 0}); err != nil { + return err + } + after, err := success(e.raw(ctx, false, 0, nil)) + if err != nil { + return err + } + if !bytes.Equal(before, after) { + return fmt.Errorf("rejected WRITE changed data") + } + return nil +} + +func (e *Endpoint) OutOfRange(ctx context.Context) error { + r, err := e.raw(ctx, false, e.Backend.Bytes/uint64(e.Backend.BlockSize), nil) + if err := rejection(r, err, 22, [3]byte{5, 0x21, 0}); err != nil { + return err + } + data, err := success(e.raw(ctx, false, 0, nil)) + if err != nil { + return err + } + if len(data) != int(e.Backend.BlockSize) { + return fmt.Errorf("short valid READ after rejected CDB") + } + return nil +} diff --git a/tests/ginkgo/internal/dataio/scsi_test.go b/tests/ginkgo/internal/dataio/scsi_test.go new file mode 100644 index 000000000..5341360a5 --- /dev/null +++ b/tests/ginkgo/internal/dataio/scsi_test.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + "encoding/binary" + "strings" + "testing" +) + +func TestVPDIdentity(t *testing.T) { + // Target-port identifiers must not substitute for the LU identifier. + page := []byte{0, 0x83, 0, 15, 2, 1, 0, 11} + page = append(page, []byte("SCST_FIOabc")...) + if err := identity83(page, "abc"); err != nil { + t.Fatal(err) + } + port := append([]byte(nil), page...) + port[5] = 0x11 + for name, data := range map[string][]byte{ + "short header": page[:3], "truncated page": page[:len(page)-1], "port only": port, + "wrong page": {0, 0x80, 0, 0}, "empty": {0, 0x83, 0, 0}, + "short descriptor": {0, 0x83, 0, 3, 2, 1, 0}, + } { + t.Run(name, func(t *testing.T) { + if identity83(data, "abc") == nil { + t.Fatal("accepted invalid identity") + } + }) + } + if identity83(page, "other") == nil { + t.Fatal("accepted another disk") + } +} + +func TestCapacityAndLUNs(t *testing.T) { + ten := []byte{0, 0, 1, 0xff, 0, 0, 2, 0} + last, block, err := capacity(ten, false) + if err != nil || last != 511 || block != 512 { + t.Fatalf("capacity10: %d %d %v", last, block, err) + } + sixteen := make([]byte, 32) + binary.BigEndian.PutUint64(sixteen, 0x100000001) + binary.BigEndian.PutUint32(sixteen[8:], 4096) + last, block, err = capacity(sixteen, true) + if err != nil || last != 0x100000001 || block != 4096 { + t.Fatalf("capacity16: %d %d %v", last, block, err) + } + if _, _, err := capacity(sixteen[:31], true); err == nil { + t.Fatal("accepted truncated capacity") + } + report := make([]byte, 24) + report[3], report[9] = 16, 7 + got, err := luns(report) + if err != nil || len(got) != 2 || got[0] != 0 || got[1] != 7 { + t.Fatalf("LUNs: %v %v", got, err) + } + for _, data := range [][]byte{report[:7], report[:23], {0, 0, 0, 1, 0, 0, 0, 0, 0}} { + if _, err := luns(data); err == nil { + t.Fatal("accepted malformed LUN list") + } + } + report[8] = 0xc0 + if _, err := luns(report); err == nil { + t.Fatal("accepted unexpected LUN encoding") + } +} + +const fixedSense = `SCSI Status: Check Condition + +Sense Information: +Fixed format, current; Sense key: Data Protect +Additional sense: No additional sense information + Raw sense data (in hex), sb_len=18, calculated_len=18 + 70 00 07 00 00 00 00 0a 00 00 00 00 00 00 00 00 + 00 00 + +` +const descriptorSense = `SCSI Status: Check Condition + Raw sense data (in hex), sb_len=8, calculated_len=8 + 72 05 21 00 00 00 00 00 +` + +func TestNegativeCDBRequiresActualSense(t *testing.T) { + if err := rejection(Reply{ExitCode: 7, Stderr: fixedSense}, nil, 7, [3]byte{7, 0, 0}); err != nil { + t.Fatal(err) + } + if err := rejection(Reply{ExitCode: 22, Stderr: descriptorSense}, nil, 22, [3]byte{5, 0x21, 0}); err != nil { + t.Fatal(err) + } + for name, reply := range map[string]Reply{ + "no sense": {ExitCode: 7, Stderr: "open: Read-only filesystem"}, + "wrong exit": {ExitCode: 1, Stderr: fixedSense}, + "wrong sense": {ExitCode: 7, Stderr: descriptorSense}, + "wrong status": {ExitCode: 7, Stderr: strings.ReplaceAll(fixedSense, "Check Condition", "Good")}, + "deferred sense": {ExitCode: 7, Stderr: strings.ReplaceAll(fixedSense, "70 00 07", "71 00 07")}, + "short sense": {ExitCode: 7, Stderr: strings.ReplaceAll(fixedSense, " 00 00\n", "")}, + } { + t.Run(name, func(t *testing.T) { + if rejection(reply, nil, 7, [3]byte{7, 0, 0}) == nil { + t.Fatal("accepted unrelated/incomplete failure") + } + }) + } +} diff --git a/tests/ginkgo/internal/dataio/specs.go b/tests/ginkgo/internal/dataio/specs.go new file mode 100644 index 000000000..5bda863d9 --- /dev/null +++ b/tests/ginkgo/internal/dataio/specs.go @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0-only +package dataio + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Register shares protocol assertions across transports; +// each suite owns its target/session lifecycle and supplies the exported disk. +func Register(create func(SpecContext, Profile) *Endpoint) bool { + return Describe("Exported SCSI devices", func() { + It("returns INQUIRY and VPD identity", Label("scsi"), func(ctx SpecContext) { + Expect(create(ctx, Profile{Handler: "vdisk_fileio"}).Identity(ctx)).To(Succeed()) + }) + It("reports matching READ CAPACITY(10) and (16) geometry", Label("scsi"), func(ctx SpecContext) { + Expect(create(ctx, Profile{Handler: "vdisk_fileio"}).Capacity(ctx)).To(Succeed()) + }) + It("reports only the initiator's accessible LUNs", Label("scsi"), func(ctx SpecContext) { + Expect(create(ctx, Profile{Handler: "vdisk_fileio", ExtraLUNs: true}).LUNs(ctx)).To(Succeed()) + }) + It("rejects a read-only WRITE with DATA PROTECT and preserves data", Label("scsi", "negative"), func(ctx SpecContext) { + Expect(create(ctx, Profile{Handler: "vdisk_fileio", ReadOnly: true}).ReadOnlyWrite(ctx)).To(Succeed()) + }) + It("rejects an out-of-range READ with ILLEGAL REQUEST and remains usable", Label("scsi", "negative"), func(ctx SpecContext) { + Expect(create(ctx, Profile{Handler: "vdisk_fileio"}).OutOfRange(ctx)).To(Succeed()) + }) + }) +} diff --git a/tests/ginkgo/iscsi/dataio_test.go b/tests/ginkgo/iscsi/dataio_test.go new file mode 100644 index 000000000..798b203dc --- /dev/null +++ b/tests/ginkgo/iscsi/dataio_test.go @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0-only +package iscsi + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "scst-component-tests/internal/dataio" + "scst-component-tests/internal/testcfg" +) + +var _ = dataio.Register(func(ctx SpecContext, profile dataio.Profile) *dataio.Endpoint { + timeout := testcfg.Duration(cfg.CommandTimeout) + Expect(dataio.RequireTools(ctx, peer.Run, timeout)).To(Succeed()) + f := newProfileFixture(ctx, &profile) + f.login(ctx) + e := &dataio.Endpoint{Run: peer.Run, Timeout: timeout, + Backend: f.Backend, ReadOnly: profile.ReadOnly, Artifacts: artifactDir, Name: f.Device} + Eventually(func() error { return e.Discover(ctx) }, testcfg.Duration(cfg.SettleTimeout), + testcfg.Duration(cfg.PollInterval)).WithContext(ctx).Should(Succeed()) + return e +}) diff --git a/tests/ginkgo/iscsi/fixture_test.go b/tests/ginkgo/iscsi/fixture_test.go index 5c506d7f1..3f8d2ae2f 100644 --- a/tests/ginkgo/iscsi/fixture_test.go +++ b/tests/ginkgo/iscsi/fixture_test.go @@ -14,18 +14,24 @@ import ( . "github.com/onsi/gomega" "scst-component-tests/internal/admin" + "scst-component-tests/internal/dataio" "scst-component-tests/internal/initiator" "scst-component-tests/internal/testcfg" ) type fixture struct { + Backend dataio.Backend + LUNs map[int]string + BackingFiles []string Target, Initiator, Iface, Device string ifaceOwned, nodeOwned, targetOwned, deviceOwned bool } var sequence int -func newFixture(ctx context.Context) *fixture { +func newFixture(ctx context.Context) *fixture { return newProfileFixture(ctx, nil) } + +func newProfileFixture(ctx context.Context, profile *dataio.Profile) *fixture { GinkgoHelper() if pending.Load() != 0 { AbortSuite("previous cleanup failed; inspect both hosts") @@ -46,6 +52,11 @@ func newFixture(ctx context.Context) *fixture { Iface: fmt.Sprintf("scst_%s_%d", runID, sequence), Device: fmt.Sprintf("%s_%s_%d", cfg.NamePrefix, runID, sequence), } + f.Backend.Handler = "vdisk_nullio" + if profile != nil { + f.Backend.Handler = profile.Handler + } + f.LUNs = map[int]string{0: f.Device} data := must(json.MarshalIndent(f, "", " ")) Expect(os.WriteFile(filepath.Join(artifactDir, f.Device+".json"), data, 0600)).To(Succeed()) pending.Add(1) @@ -76,15 +87,36 @@ func newFixture(ctx context.Context) *fixture { } { f.update(ctx, setting[0], setting[1]) } + attrs := "dummy=1,size_mb=8" + if profile != nil { + f.Backend = must(dataio.PrepareBackend(ctx, cfg, artifactDir, dataio.DeviceID(f.Device), *profile, + cli.RunProgram, func(path string) { f.BackingFiles = append(f.BackingFiles, path) })) + attrs = f.Backend.Attributes + } f.deviceOwned = true - do(ctx, "-open_dev", f.Device, "-handler", "vdisk_nullio", "-attributes", "dummy=1,size_mb=8") + do(ctx, "-open_dev", f.Device, "-handler", f.Backend.Handler, "-attributes", attrs) + if profile != nil { + do(ctx, "-set_dev_attr", f.Device, "-attributes", "usn="+f.Backend.Serial+",vend_specific_id="+f.Backend.Serial) + } f.targetOwned = true do(ctx, "-add_target", f.Target, "-driver", "iscsi") do(ctx, "-set_tgt_attr", f.Target, "-driver", "iscsi", "-attributes", "per_portal_acl=0") f.at(ctx, "", "-add_group", "access") - f.at(ctx, "access", "-add_lun", "0", "-device", f.Device) + lunAttrs := "read_only=0" + if profile != nil && profile.ReadOnly { + lunAttrs = "read_only=1" + } + f.at(ctx, "access", "-add_lun", "0", "-device", f.Device, "-attributes", lunAttrs) + if profile != nil && profile.ExtraLUNs { + f.at(ctx, "access", "-add_lun", "7", "-device", f.Device) + f.LUNs[7] = f.Device + f.at(ctx, "", "-add_group", "hidden") + f.at(ctx, "hidden", "-add_lun", "13", "-device", f.Device) + } f.at(ctx, "access", "-add_init", f.Initiator) do(ctx, "-enable_target", f.Target, "-driver", "iscsi") + data = must(json.MarshalIndent(f, "", " ")) + Expect(os.WriteFile(filepath.Join(artifactDir, f.Device+".json"), data, 0600)).To(Succeed()) return f } @@ -146,7 +178,7 @@ func (f *fixture) login(ctx context.Context) { g.Expect(sessions["iscsi/"+f.Target]).To(HaveLen(1)) // The public CLI shows the group's LUN map; this is not a data I/O check. for _, s := range sessions["iscsi/"+f.Target] { - g.Expect(s.LUNs).To(Equal(map[int]string{0: f.Device})) + g.Expect(s.LUNs).To(Equal(f.LUNs)) } }, testcfg.Duration(cfg.SettleTimeout), testcfg.Duration(cfg.PollInterval)).WithContext(ctx).Should(Succeed()) } @@ -193,9 +225,12 @@ func (f *fixture) cleanup(ctx context.Context) { } if f.deviceOwned { if handler, present := must(cli.Devices(ctx))[f.Device]; present { - Expect(handler).To(Equal("vdisk_nullio")) + Expect(handler).To(Equal(f.Backend.Handler)) do(ctx, "-close_dev", f.Device, "-handler", handler) } } Expect(empty(ctx)).To(Succeed()) + for _, path := range f.BackingFiles { + Expect(os.Remove(path)).To(Succeed()) + } } diff --git a/tests/ginkgo/suite/dataio_test.go b/tests/ginkgo/suite/dataio_test.go new file mode 100644 index 000000000..c8bf5202c --- /dev/null +++ b/tests/ginkgo/suite/dataio_test.go @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0-only +package suite + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "scst-component-tests/internal/dataio" + "scst-component-tests/internal/testcfg" +) + +var _ = dataio.Register(func(ctx SpecContext, profile dataio.Profile) *dataio.Endpoint { + timeout := testcfg.Duration(cfg.CommandTimeout) + Expect(dataio.RequireTools(ctx, cli.RunProgram, timeout)).To(Succeed()) + f := newFixture() + backend := must(dataio.PrepareBackend(ctx, cfg, artifactDir, dataio.DeviceID(f.prefix), profile, + cli.RunProgram, func(path string) { f.backingFiles = append(f.backingFiles, path) })) + d := f.openDevice(ctx, "io", backend.Handler, backend.Attributes) + f.setDevice(ctx, d, "usn="+backend.Serial+",vend_specific_id="+backend.Serial) + session := f.name("session") + target := f.target(ctx, "target", "session_name="+session) + f.at(ctx, target, "", "-add_group", "access") + attrs := "read_only=0" + if profile.ReadOnly { + attrs = "read_only=1" + } + f.at(ctx, target, "access", "-add_lun", "0", "-device", d, "-attributes", attrs) + if profile.ExtraLUNs { + f.at(ctx, target, "access", "-add_lun", "7", "-device", d) + f.at(ctx, target, "", "-add_group", "hidden") + f.at(ctx, target, "hidden", "-add_lun", "13", "-device", d) + } + f.at(ctx, target, "access", "-add_init", session) + e := &dataio.Endpoint{Run: cli.RunProgram, Timeout: timeout, + Backend: backend, ReadOnly: profile.ReadOnly, Artifacts: artifactDir, Name: f.name("io")} + Eventually(func() error { return e.Discover(ctx) }, testcfg.Duration(cfg.SettleTimeout), + testcfg.Duration(cfg.PollInterval)).WithContext(ctx).Should(Succeed()) + return e +})