From 7ab1cdaf043d4094ea6cbfdbc77ed455351cec92 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Thu, 26 Mar 2026 16:11:27 -0700 Subject: [PATCH] Implements the VM SCSI Controller This change implements the SCSI controller operations for the new V2 shim. Signed-off-by: Justin Terry --- go.mod | 2 +- internal/controller/device/scsi/controller.go | 221 ++++++ .../controller/device/scsi/controller_test.go | 475 +++++++++++++ internal/controller/device/scsi/disk/disk.go | 221 ++++++ .../controller/device/scsi/disk/disk_test.go | 635 ++++++++++++++++++ .../controller/device/scsi/mount/mount.go | 179 +++++ .../device/scsi/mount/mount_lcow.go | 77 +++ .../device/scsi/mount/mount_test.go | 391 +++++++++++ .../device/scsi/mount/mount_wcow.go | 69 ++ internal/controller/device/scsi/scsi.go | 7 + internal/vm/guestmanager/scsi_lcow.go | 12 - internal/vm/guestmanager/scsi_wcow.go | 14 - internal/vm/vmmanager/scsi.go | 11 - 13 files changed, 2276 insertions(+), 38 deletions(-) create mode 100644 internal/controller/device/scsi/controller.go create mode 100644 internal/controller/device/scsi/controller_test.go create mode 100644 internal/controller/device/scsi/disk/disk.go create mode 100644 internal/controller/device/scsi/disk/disk_test.go create mode 100644 internal/controller/device/scsi/mount/mount.go create mode 100644 internal/controller/device/scsi/mount/mount_lcow.go create mode 100644 internal/controller/device/scsi/mount/mount_test.go create mode 100644 internal/controller/device/scsi/mount/mount_wcow.go create mode 100644 internal/controller/device/scsi/scsi.go diff --git a/go.mod b/go.mod index c19e49c4e5..ce0de078ac 100644 --- a/go.mod +++ b/go.mod @@ -43,6 +43,7 @@ require ( github.com/containerd/typeurl/v2 v2.2.3 github.com/google/go-cmp v0.7.0 github.com/google/go-containerregistry v0.20.1 + github.com/google/uuid v1.6.0 github.com/linuxkit/virtsock v0.0.0-20241009230534-cb6a20cc0422 github.com/mattn/go-shellwords v1.0.12 github.com/moby/sys/user v0.4.0 @@ -98,7 +99,6 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/gorilla/mux v1.8.1 // indirect github.com/josephspurrier/goversioninfo v1.5.0 // indirect github.com/klauspost/compress v1.18.0 // indirect diff --git a/internal/controller/device/scsi/controller.go b/internal/controller/device/scsi/controller.go new file mode 100644 index 0000000000..879b130508 --- /dev/null +++ b/internal/controller/device/scsi/controller.go @@ -0,0 +1,221 @@ +//go:build windows + +package scsi + +import ( + "context" + "fmt" + "sync" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/disk" + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + + "github.com/google/uuid" +) + +type VMSCSIOps interface { + disk.VMSCSIAdder + disk.VMSCSIRemover +} + +type LinuxGuestSCSIOps interface { + mount.LinuxGuestSCSIMounter + mount.LinuxGuestSCSIUnmounter + disk.LinuxGuestSCSIEjector +} + +type WindowsGuestSCSIOps interface { + mount.WindowsGuestSCSIMounter + mount.WindowsGuestSCSIUnmounter +} + +// numLUNsPerController is the maximum number of LUNs per controller, fixed by Hyper-V. +const numLUNsPerController = 64 + +// The controller manages all SCSI attached devices and guest mounted +// directories. +// +// It is required that all callers: +// +// 1. Obtain a reservation using Reserve(). +// +// 2. Use the reservation to MapToGuest() to ensure resource availability. +// +// 3. Call UnmapFromGuest() to release the reservation and all resources. +// +// If MapToGuest() fails, the caller must call UnmapFromGuest() to release the +// reservation and all resources. +// +// If UnmapFromGuest() fails, the caller must call UnmapFromGuest() again until +// it succeeds to release the reservation and all resources. +type Controller struct { + vm VMSCSIOps + lGuest LinuxGuestSCSIOps + wGuest WindowsGuestSCSIOps + + mu sync.Mutex + + // Every call to Reserve gets a unique reservation ID which holds pointers + // to its controllerSlot for the disk and its partition for the mount. + reservations map[uuid.UUID]*reservation + + // For fast lookup we keep a hostPath to controllerSlot mapping for all + // allocated disks. + disksByPath map[string]int + + // Tracks all allocated and unallocated available slots on the SCSI + // controllers. + // + // NumControllers == len(controllerSlots) / numLUNsPerController + // ControllerID == index / numLUNsPerController + // LunID == index % numLUNsPerController + controllerSlots []*disk.Disk +} + +func New(numControllers int, vm VMSCSIOps, lGuest LinuxGuestSCSIOps, wGuest WindowsGuestSCSIOps) *Controller { + return &Controller{ + vm: vm, + lGuest: lGuest, + wGuest: wGuest, + mu: sync.Mutex{}, + reservations: make(map[uuid.UUID]*reservation), + disksByPath: make(map[string]int), + controllerSlots: make([]*disk.Disk, numControllers*numLUNsPerController), + } +} + +// ReserveForRootfs reserves a specific controller and lun location for the +// rootfs. This is required to ensure the rootfs is always at a known location +// and that location is not used for any other disk. This should only be called +// once per controller and lun location, and must be called before any calls to +// Reserve() to ensure the rootfs reservation is not evicted by a dynamic +// reservation. +func (c *Controller) ReserveForRootfs(ctx context.Context, controller, lun uint) error { + c.mu.Lock() + defer c.mu.Unlock() + + slot := int(controller*numLUNsPerController + lun) + if slot >= len(c.controllerSlots) { + return fmt.Errorf("invalid controller %d or lun %d", controller, lun) + } + if c.controllerSlots[slot] != nil { + return fmt.Errorf("slot for controller %d and lun %d is already reserved", controller, lun) + } + c.controllerSlots[slot] = disk.NewReserved(controller, lun, disk.DiskConfig{}) + return nil +} + +// Reserves a referenced counted mapping entry for a SCSI attachment based on +// the SCSI disk path, and partition number. +// +// If an error is returned from this function, it is guaranteed that no +// reservation mapping was made and no UnmapFromGuest() call is necessary to +// clean up. +func (c *Controller) Reserve(ctx context.Context, diskConfig disk.DiskConfig, mountConfig mount.MountConfig) (uuid.UUID, error) { + c.mu.Lock() + defer c.mu.Unlock() + + // Generate a new reservation id. + id := uuid.New() + if _, ok := c.reservations[id]; ok { + return uuid.Nil, fmt.Errorf("reservation ID collision") + } + r := &reservation{ + controllerSlot: -1, + partition: mountConfig.Partition, + } + + // Determine if this hostPath already had a disk known. + if slot, ok := c.disksByPath[diskConfig.HostPath]; ok { + r.controllerSlot = slot // Update our reservation where the disk is. + d := c.controllerSlots[slot] + + // Verify the caller config is the same. + if !d.Config().Equals(diskConfig) { + return uuid.Nil, fmt.Errorf("cannot reserve ref on disk with different config") + } + + // We at least have a disk, now determine if we have a mount for this + // partition. + if _, err := d.ReservePartition(ctx, mountConfig); err != nil { + return uuid.Nil, fmt.Errorf("reserve partition %d: %w", mountConfig.Partition, err) + } + } else { + // No hostPath was found. Find a slot for the disk. + nextSlot := -1 + for i, d := range c.controllerSlots { + if d == nil { + nextSlot = i + break + } + } + if nextSlot == -1 { + return uuid.Nil, fmt.Errorf("no available slots") + } + + // Create the Disk and Partition Mount in the reserved states. + controller := uint(nextSlot / numLUNsPerController) + lun := uint(nextSlot % numLUNsPerController) + d := disk.NewReserved(controller, lun, diskConfig) + if _, err := d.ReservePartition(ctx, mountConfig); err != nil { + return uuid.Nil, fmt.Errorf("reserve partition %d: %w", mountConfig.Partition, err) + } + c.controllerSlots[controller*numLUNsPerController+lun] = d + c.disksByPath[diskConfig.HostPath] = nextSlot + r.controllerSlot = nextSlot + } + + // Ensure our reservation is saved for all future operations. + c.reservations[id] = r + return id, nil +} + +func (c *Controller) MapToGuest(ctx context.Context, reservation uuid.UUID) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if r, ok := c.reservations[reservation]; ok { + d := c.controllerSlots[r.controllerSlot] + if err := d.AttachToVM(ctx, c.vm); err != nil { + return "", fmt.Errorf("attach disk to vm: %w", err) + } + guestPath, err := d.MountPartitionToGuest(ctx, r.partition, c.lGuest, c.wGuest) + if err != nil { + return "", fmt.Errorf("mount partition %d to guest: %w", r.partition, err) + } + return guestPath, nil + } + return "", fmt.Errorf("reservation %s not found", reservation) +} + +func (c *Controller) UnmapFromGuest(ctx context.Context, reservation uuid.UUID) error { + c.mu.Lock() + defer c.mu.Unlock() + + if r, ok := c.reservations[reservation]; ok { + d := c.controllerSlots[r.controllerSlot] + // Ref counted unmount. + if err := d.UnmountPartitionFromGuest(ctx, r.partition, c.lGuest, c.wGuest); err != nil { + return fmt.Errorf("unmount partition %d from guest: %w", r.partition, err) + } + if err := d.DetachFromVM(ctx, c.vm, c.lGuest); err != nil { + return fmt.Errorf("detach disk from vm: %w", err) + } + if d.State() == disk.DiskStateDetached { + // If we have no more mounts on this disk, remove the disk from the + // known disks and free the slot. + delete(c.disksByPath, d.HostPath()) + c.controllerSlots[r.controllerSlot] = nil + } + delete(c.reservations, reservation) + return nil + } + return fmt.Errorf("reservation %s not found", reservation) +} + +type reservation struct { + // This is the index into controllerSlots that holds this disk. + controllerSlot int + // This is the index into the disk mounts for this partition. + partition uint64 +} diff --git a/internal/controller/device/scsi/controller_test.go b/internal/controller/device/scsi/controller_test.go new file mode 100644 index 0000000000..9a6daa8c5b --- /dev/null +++ b/internal/controller/device/scsi/controller_test.go @@ -0,0 +1,475 @@ +//go:build windows + +package scsi + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/disk" + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + "github.com/google/uuid" +) + +// --- Mock types --- + +type mockVMOps struct { + addErr error + removeErr error +} + +func (m *mockVMOps) AddSCSIDisk(_ context.Context, _ hcsschema.Attachment, _ uint, _ uint) error { + return m.addErr +} + +func (m *mockVMOps) RemoveSCSIDisk(_ context.Context, _ uint, _ uint) error { + return m.removeErr +} + +type mockLinuxGuestOps struct { + mountErr error + unmountErr error + ejectErr error +} + +func (m *mockLinuxGuestOps) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockLinuxGuestOps) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.unmountErr +} + +func (m *mockLinuxGuestOps) RemoveSCSIDevice(_ context.Context, _ guestresource.SCSIDevice) error { + return m.ejectErr +} + +type mockWindowsGuestOps struct { + mountErr error + unmountErr error +} + +func (m *mockWindowsGuestOps) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockWindowsGuestOps) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockWindowsGuestOps) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.unmountErr +} + +// --- Helpers --- + +func defaultDiskConfig() disk.DiskConfig { + return disk.DiskConfig{ + HostPath: `C:\test\disk.vhdx`, + ReadOnly: false, + Type: disk.DiskTypeVirtualDisk, + } +} + +func defaultMountConfig() mount.MountConfig { + return mount.MountConfig{ + Partition: 1, + ReadOnly: false, + } +} + +func newController(vm *mockVMOps, lg *mockLinuxGuestOps, wg *mockWindowsGuestOps) *Controller { + return New(1, vm, lg, wg) +} + +func reservedController(t *testing.T) (*Controller, uuid.UUID) { + t.Helper() + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("setup Reserve: %v", err) + } + return c, id +} + +func mappedController(t *testing.T) (*Controller, uuid.UUID) { + t.Helper() + c, id := reservedController(t) + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("setup MapToGuest: %v", err) + } + return c, id +} + +// --- Tests: New --- + +func TestNew(t *testing.T) { + c := New(4, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + if c == nil { + t.Fatal("expected non-nil controller") + } +} + +// --- Tests: Reserve --- + +func TestReserve_Success(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id == uuid.Nil { + t.Fatal("expected non-nil reservation ID") + } +} + +func TestReserve_SameDiskSamePartition(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + mc := defaultMountConfig() + id1, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + id2, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + if id1 == id2 { + t.Error("expected different reservation IDs") + } +} + +func TestReserve_SameDiskDifferentPartition(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + _, err := c.Reserve(context.Background(), dc, mount.MountConfig{Partition: 1}) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + _, err = c.Reserve(context.Background(), dc, mount.MountConfig{Partition: 2}) + if err != nil { + t.Fatalf("second reserve different partition: %v", err) + } +} + +func TestReserve_SamePathDifferentDiskConfig(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + mc := defaultMountConfig() + _, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + dc2 := dc + dc2.ReadOnly = true + _, err = c.Reserve(context.Background(), dc2, mc) + if err == nil { + t.Fatal("expected error for same path with different disk config") + } +} + +func TestReserve_DifferentDisks(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + mc := defaultMountConfig() + _, err := c.Reserve(context.Background(), disk.DiskConfig{HostPath: `C:\a.vhdx`, Type: disk.DiskTypeVirtualDisk}, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + _, err = c.Reserve(context.Background(), disk.DiskConfig{HostPath: `C:\b.vhdx`, Type: disk.DiskTypeVirtualDisk}, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } +} + +func TestReserve_NoAvailableSlots(t *testing.T) { + // Create with 0 controllers so there are no slots. + c := New(0, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + _, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err == nil { + t.Fatal("expected error when no slots available") + } +} + +func TestReserve_FillsAllSlots(t *testing.T) { + c := New(1, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + mc := defaultMountConfig() + for i := range numLUNsPerController { + dc := disk.DiskConfig{ + HostPath: fmt.Sprintf(`C:\disk%d.vhdx`, i), + Type: disk.DiskTypeVirtualDisk, + } + _, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("reserve slot %d: %v", i, err) + } + } + // Next reservation should fail. + _, err := c.Reserve(context.Background(), disk.DiskConfig{HostPath: `C:\overflow.vhdx`, Type: disk.DiskTypeVirtualDisk}, mc) + if err == nil { + t.Fatal("expected error when all slots are full") + } +} + +// --- Tests: MapToGuest --- + +func TestMapToGuest_Success(t *testing.T) { + c, id := reservedController(t) + guestPath, err := c.MapToGuest(context.Background(), id) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMapToGuest_UnknownReservation(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + _, err := c.MapToGuest(context.Background(), uuid.New()) + if err == nil { + t.Fatal("expected error for unknown reservation") + } +} + +func TestMapToGuest_AttachError(t *testing.T) { + vm := &mockVMOps{addErr: errors.New("attach failed")} + c := New(1, vm, &mockLinuxGuestOps{}, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.MapToGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestMapToGuest_MountError(t *testing.T) { + lg := &mockLinuxGuestOps{mountErr: errors.New("mount failed")} + c := New(1, &mockVMOps{}, lg, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.MapToGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestMapToGuest_Idempotent(t *testing.T) { + c, id := reservedController(t) + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("first map: %v", err) + } + // Second call should be idempotent (disk already attached, mount already mounted). + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("second map (idempotent): %v", err) + } +} + +// --- Tests: UnmapFromGuest --- + +func TestUnmapFromGuest_Success(t *testing.T) { + c, id := mappedController(t) + err := c.UnmapFromGuest(context.Background(), id) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmapFromGuest_UnknownReservation(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + err := c.UnmapFromGuest(context.Background(), uuid.New()) + if err == nil { + t.Fatal("expected error for unknown reservation") + } +} + +func TestUnmapFromGuest_CleansUpSlotWhenFullyDetached(t *testing.T) { + c, id := mappedController(t) + err := c.UnmapFromGuest(context.Background(), id) + if err != nil { + t.Fatalf("UnmapFromGuest: %v", err) + } + // The slot should be freed, so we can reserve the same path again. + _, err = c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("re-reserve after unmap: %v", err) + } +} + +func TestUnmapFromGuest_UnmountError(t *testing.T) { + lg := &mockLinuxGuestOps{} + c := New(1, &mockVMOps{}, lg, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("MapToGuest: %v", err) + } + // Now inject an unmount error. + lg.unmountErr = errors.New("unmount failed") + err = c.UnmapFromGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestUnmapFromGuest_DetachError(t *testing.T) { + vm := &mockVMOps{} + c := New(1, vm, &mockLinuxGuestOps{}, nil) + id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("MapToGuest: %v", err) + } + // Now inject a remove error. Unmount succeeds but detach fails. + vm.removeErr = errors.New("remove failed") + err = c.UnmapFromGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestUnmapFromGuest_MultipleReservationsSameDisk(t *testing.T) { + c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + mc := defaultMountConfig() + + id1, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + id2, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + + // Map both. + if _, err := c.MapToGuest(context.Background(), id1); err != nil { + t.Fatalf("MapToGuest id1: %v", err) + } + if _, err := c.MapToGuest(context.Background(), id2); err != nil { + t.Fatalf("MapToGuest id2: %v", err) + } + + // Unmap first. Disk should still be attached because id2 holds a ref. + if err := c.UnmapFromGuest(context.Background(), id1); err != nil { + t.Fatalf("UnmapFromGuest id1: %v", err) + } + + // Unmap second. Now disk should be fully detached and slot freed. + if err := c.UnmapFromGuest(context.Background(), id2); err != nil { + t.Fatalf("UnmapFromGuest id2: %v", err) + } + + // Slot should be free for reuse. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after full unmap: %v", err) + } +} + +func TestUnmapFromGuest_WindowsGuest(t *testing.T) { + wg := &mockWindowsGuestOps{} + c := New(1, &mockVMOps{}, nil, wg) + dc := defaultDiskConfig() + mc := defaultMountConfig() + + id, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("MapToGuest: %v", err) + } + if err := c.UnmapFromGuest(context.Background(), id); err != nil { + t.Fatalf("UnmapFromGuest: %v", err) + } +} + +// --- Tests: Reserve + Unmap lifecycle --- + +func TestReserveAfterUnmap_ReusesSlot(t *testing.T) { + c, id := mappedController(t) + if err := c.UnmapFromGuest(context.Background(), id); err != nil { + t.Fatalf("UnmapFromGuest: %v", err) + } + // Reserve a different disk in the now-freed slot. + dc := disk.DiskConfig{HostPath: `C:\other.vhdx`, Type: disk.DiskTypeVirtualDisk} + _, err := c.Reserve(context.Background(), dc, defaultMountConfig()) + if err != nil { + t.Fatalf("reserve after unmap: %v", err) + } +} + +func TestUnmapFromGuest_AfterFailedMapToGuest(t *testing.T) { + // MapToGuest fails at attach, then UnmapFromGuest should clean up the + // reservation and free the slot. + vm := &mockVMOps{addErr: errors.New("attach failed")} + c := New(1, vm, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + mc := defaultMountConfig() + + id, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.MapToGuest(context.Background(), id) + if err == nil { + t.Fatal("expected MapToGuest to fail") + } + // UnmapFromGuest should succeed and clean up. + err = c.UnmapFromGuest(context.Background(), id) + if err != nil { + t.Fatalf("UnmapFromGuest after failed MapToGuest: %v", err) + } + // Slot should be freed for reuse. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after cleanup: %v", err) + } +} + +func TestUnmapFromGuest_RetryAfterDetachFailure(t *testing.T) { + // UnmapFromGuest fails at detach. Retry should succeed because + // UnmountPartitionFromGuest is a no-op when the partition is already gone. + vm := &mockVMOps{} + c := New(1, vm, &mockLinuxGuestOps{}, nil) + dc := defaultDiskConfig() + mc := defaultMountConfig() + + id, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.MapToGuest(context.Background(), id); err != nil { + t.Fatalf("MapToGuest: %v", err) + } + // Inject remove error for first attempt. + vm.removeErr = errors.New("transient remove failure") + err = c.UnmapFromGuest(context.Background(), id) + if err == nil { + t.Fatal("expected detach error") + } + // Fix the error and retry. + vm.removeErr = nil + err = c.UnmapFromGuest(context.Background(), id) + if err != nil { + t.Fatalf("retry UnmapFromGuest: %v", err) + } + // Slot should be freed. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after retry: %v", err) + } +} diff --git a/internal/controller/device/scsi/disk/disk.go b/internal/controller/device/scsi/disk/disk.go new file mode 100644 index 0000000000..2c5e383ab5 --- /dev/null +++ b/internal/controller/device/scsi/disk/disk.go @@ -0,0 +1,221 @@ +//go:build windows + +package disk + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// DiskType identifies the attachment protocol used when adding a disk to the VM's SCSI bus. +type DiskType string + +const ( + // DiskTypeVirtualDisk attaches the disk as a virtual hard disk (VHD/VHDX). + DiskTypeVirtualDisk DiskType = "VirtualDisk" + + // DiskTypePassThru attaches a physical disk directly to the VM with pass-through access. + DiskTypePassThru DiskType = "PassThru" + + // DiskTypeExtensibleVirtualDisk attaches a disk via an extensible virtual disk (EVD) provider. + // The hostPath must be in the form evd:///. + DiskTypeExtensibleVirtualDisk DiskType = "ExtensibleVirtualDisk" +) + +// DiskConfig describes the host-side disk to attach to the VM's SCSI bus. +type DiskConfig struct { + // HostPath is the path on the host to the disk to be attached. + HostPath string + // ReadOnly specifies whether the disk should be attached with read-only access. + ReadOnly bool + // Type specifies the attachment protocol to use when attaching the disk. + Type DiskType + // EVDType is the EVD provider name. + // Only populated when Type is [DiskTypeExtensibleVirtualDisk]. + EVDType string +} + +// equals reports whether two DiskConfig values describe the same attachment parameters. +func (d DiskConfig) Equals(other DiskConfig) bool { + return d.HostPath == other.HostPath && + d.ReadOnly == other.ReadOnly && + d.Type == other.Type && + d.EVDType == other.EVDType +} + +type DiskState int + +const ( + // The disk has never been attached. + DiskStateReserved DiskState = iota + // The disk is currently attached to the guest. + DiskStateAttached + // The disk was previously attached and ejected and must be detached. + DiskStateEjected + // The disk was previously attached and detached, this is terminal. + DiskStateDetached +) + +type VMSCSIAdder interface { + AddSCSIDisk(ctx context.Context, disk hcsschema.Attachment, controller uint, lun uint) error +} + +type VMSCSIRemover interface { + RemoveSCSIDisk(ctx context.Context, controller uint, lun uint) error +} + +type LinuxGuestSCSIEjector interface { + RemoveSCSIDevice(ctx context.Context, settings guestresource.SCSIDevice) error +} + +// Disk represents a SCSI disk attached to the VM. It manages the lifecycle of +// the disk attachment as well as the guest mounts on the disk partitions. +// +// All operations on the disk are expected to be ordered by the caller. No +// locking is done at this layer. +type Disk struct { + controller uint + lun uint + config DiskConfig + + state DiskState + // Note that len(mounts) > 0 is the ref count for a disk. + mounts map[uint64]*mount.Mount +} + +// NewReserved creates a new Disk in the reserved state with the provided configuration. +func NewReserved(controller, lun uint, config DiskConfig) *Disk { + return &Disk{ + controller: controller, + lun: lun, + config: config, + state: DiskStateReserved, + mounts: make(map[uint64]*mount.Mount), + } +} + +func (d *Disk) State() DiskState { + return d.state +} + +func (d *Disk) Config() DiskConfig { + return d.config +} + +func (d *Disk) HostPath() string { + return d.config.HostPath +} + +func (d *Disk) AttachToVM(ctx context.Context, vm VMSCSIAdder) error { + switch d.state { + case DiskStateReserved: + // Attach the disk to the VM. + if err := vm.AddSCSIDisk(ctx, hcsschema.Attachment{ + Path: d.config.HostPath, + Type_: string(d.config.Type), + ReadOnly: d.config.ReadOnly, + ExtensibleVirtualDiskType: d.config.EVDType, + }, d.controller, d.lun); err != nil { + // Move to detached since we know from reserved there was no guest + // state. + d.state = DiskStateDetached + return fmt.Errorf("attach disk to VM: %w", err) + } + d.state = DiskStateAttached + return nil + case DiskStateAttached: + // Disk is already attached, this is idempotent. + return nil + case DiskStateDetached: + // We don't support re-attaching a detached disk, this is an error. + return fmt.Errorf("disk already detached") + } + return nil +} + +func (d *Disk) DetachFromVM(ctx context.Context, vm VMSCSIRemover, lGuest LinuxGuestSCSIEjector) error { + if d.state == DiskStateAttached { + // Ensure for correctness nobody leaked a mount + if len(d.mounts) != 0 { + // This disk is still active by some other mounts. Leave it. + return nil + } + // The linux guest needs to have the SCSI ejected. Do that now. + if lGuest != nil { + if err := lGuest.RemoveSCSIDevice(ctx, guestresource.SCSIDevice{ + Controller: uint8(d.controller), + Lun: uint8(d.lun), + }); err != nil { + return fmt.Errorf("remove SCSI device from guest: %w", err) + } + } + // Set it to ejected and continue processing. + d.state = DiskStateEjected + } + + switch d.state { + case DiskStateReserved: + // Disk is not attached, this is idempotent. + return nil + case DiskStateAttached: + panic(fmt.Errorf("unexpected attached disk state in detach, expected ejected")) + case DiskStateEjected: + // The disk is ejected but still attached, attempt to detach it again. + if err := vm.RemoveSCSIDisk(ctx, d.controller, d.lun); err != nil { + return fmt.Errorf("detach ejected disk from VM: %w", err) + } + d.state = DiskStateDetached + return nil + case DiskStateDetached: + // Disk is already detached, this is idempotent. + return nil + } + return fmt.Errorf("unexpected disk state %d", d.state) +} + +func (d *Disk) ReservePartition(ctx context.Context, config mount.MountConfig) (*mount.Mount, error) { + if d.state != DiskStateReserved && d.state != DiskStateAttached { + return nil, fmt.Errorf("unexpected disk state %d, expected reserved or attached", d.state) + } + + // Check if the partition is already reserved. + if m, ok := d.mounts[config.Partition]; ok { + if err := m.Reserve(config); err != nil { + return nil, fmt.Errorf("reserve partition %d: %w", config.Partition, err) + } + return m, nil + } + // Create a new mount for this partition in the reserved state. + m := mount.NewReserved(d.controller, d.lun, config) + d.mounts[config.Partition] = m + return m, nil +} + +func (d *Disk) MountPartitionToGuest(ctx context.Context, partition uint64, linuxGuest mount.LinuxGuestSCSIMounter, windowsGuest mount.WindowsGuestSCSIMounter) (string, error) { + if d.state != DiskStateAttached { + return "", fmt.Errorf("unexpected disk state %d, expected attached", d.state) + } + if m, ok := d.mounts[partition]; ok { + return m.MountToGuest(ctx, linuxGuest, windowsGuest) + } + return "", fmt.Errorf("partition %d not found on disk", partition) +} + +func (d *Disk) UnmountPartitionFromGuest(ctx context.Context, partition uint64, linuxGuest mount.LinuxGuestSCSIUnmounter, windowsGuest mount.WindowsGuestSCSIUnmounter) error { + if m, ok := d.mounts[partition]; ok { + if err := m.UnmountFromGuest(ctx, linuxGuest, windowsGuest); err != nil { + return fmt.Errorf("unmount partition %d from guest: %w", partition, err) + } + // This was the last caller of Unmount, remove the partition in the disk + // mounts. + if m.State() == mount.MountStateUnmounted { + delete(d.mounts, partition) + } + } + // Consider a not found mount, a success for retry logic in the caller. + return nil +} diff --git a/internal/controller/device/scsi/disk/disk_test.go b/internal/controller/device/scsi/disk/disk_test.go new file mode 100644 index 0000000000..6c9140e057 --- /dev/null +++ b/internal/controller/device/scsi/disk/disk_test.go @@ -0,0 +1,635 @@ +//go:build windows + +package disk + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockVMSCSIAdder struct { + err error +} + +func (m *mockVMSCSIAdder) AddSCSIDisk(_ context.Context, _ hcsschema.Attachment, _ uint, _ uint) error { + return m.err +} + +type mockVMSCSIRemover struct { + err error +} + +func (m *mockVMSCSIRemover) RemoveSCSIDisk(_ context.Context, _ uint, _ uint) error { + return m.err +} + +type mockLinuxGuestSCSIEjector struct { + err error +} + +func (m *mockLinuxGuestSCSIEjector) RemoveSCSIDevice(_ context.Context, _ guestresource.SCSIDevice) error { + return m.err +} + +type mockLinuxGuestSCSIMounter struct { + err error +} + +func (m *mockLinuxGuestSCSIMounter) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockLinuxGuestSCSIUnmounter struct { + err error +} + +func (m *mockLinuxGuestSCSIUnmounter) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockWindowsGuestSCSIMounter struct { + err error +} + +func (m *mockWindowsGuestSCSIMounter) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +func (m *mockWindowsGuestSCSIMounter) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +type mockWindowsGuestSCSIUnmounter struct { + err error +} + +func (m *mockWindowsGuestSCSIUnmounter) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers --- + +func defaultConfig() DiskConfig { + return DiskConfig{ + HostPath: `C:\test\disk.vhdx`, + ReadOnly: false, + Type: DiskTypeVirtualDisk, + } +} + +func attachedDisk(t *testing.T) *Disk { + t.Helper() + d := NewReserved(0, 0, defaultConfig()) + if err := d.AttachToVM(context.Background(), &mockVMSCSIAdder{}); err != nil { + t.Fatalf("setup AttachToVM: %v", err) + } + return d +} + +// --- Tests --- + +func TestNewReserved(t *testing.T) { + cfg := DiskConfig{ + HostPath: `C:\test\disk.vhdx`, + ReadOnly: true, + Type: DiskTypePassThru, + EVDType: "evd-type", + } + d := NewReserved(1, 2, cfg) + + if d.State() != DiskStateReserved { + t.Errorf("expected state %d, got %d", DiskStateReserved, d.State()) + } + if d.Config() != cfg { + t.Errorf("expected config %+v, got %+v", cfg, d.Config()) + } + if d.HostPath() != cfg.HostPath { + t.Errorf("expected host path %q, got %q", cfg.HostPath, d.HostPath()) + } +} + +func TestDiskConfigEquals(t *testing.T) { + base := DiskConfig{ + HostPath: `C:\a.vhdx`, + ReadOnly: true, + Type: DiskTypeVirtualDisk, + EVDType: "evd", + } + same := base + if !base.Equals(same) { + t.Error("expected equal configs to be equal") + } + + diffPath := base + diffPath.HostPath = `C:\b.vhdx` + if base.Equals(diffPath) { + t.Error("expected different HostPath to be not equal") + } + + diffReadOnly := base + diffReadOnly.ReadOnly = false + if base.Equals(diffReadOnly) { + t.Error("expected different ReadOnly to be not equal") + } + + diffType := base + diffType.Type = DiskTypePassThru + if base.Equals(diffType) { + t.Error("expected different Type to be not equal") + } + + diffEVD := base + diffEVD.EVDType = "other" + if base.Equals(diffEVD) { + t.Error("expected different EVDType to be not equal") + } +} + +func TestAttachToVM_FromReserved_Success(t *testing.T) { + d := NewReserved(0, 1, defaultConfig()) + err := d.AttachToVM(context.Background(), &mockVMSCSIAdder{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != DiskStateAttached { + t.Errorf("expected state %d, got %d", DiskStateAttached, d.State()) + } +} + +func TestAttachToVM_FromReserved_Error(t *testing.T) { + addErr := errors.New("add failed") + d := NewReserved(0, 1, defaultConfig()) + err := d.AttachToVM(context.Background(), &mockVMSCSIAdder{err: addErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, addErr) { + t.Errorf("expected wrapped error %v, got %v", addErr, err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d after failure, got %d", DiskStateDetached, d.State()) + } +} + +func TestAttachToVM_Idempotent_WhenAttached(t *testing.T) { + d := attachedDisk(t) + // Second attach should be idempotent. + if err := d.AttachToVM(context.Background(), &mockVMSCSIAdder{}); err != nil { + t.Fatalf("unexpected error on idempotent attach: %v", err) + } + if d.State() != DiskStateAttached { + t.Errorf("expected state %d, got %d", DiskStateAttached, d.State()) + } +} + +func TestAttachToVM_ErrorWhenDetached(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + // Fail attachment to move to detached. + _ = d.AttachToVM(context.Background(), &mockVMSCSIAdder{err: errors.New("fail")}) + if d.State() != DiskStateDetached { + t.Fatalf("expected detached state, got %d", d.State()) + } + err := d.AttachToVM(context.Background(), &mockVMSCSIAdder{}) + if err == nil { + t.Fatal("expected error when attaching detached disk") + } +} + +func TestDetachFromVM_FromAttached_NoMounts_NoLinuxGuest(t *testing.T) { + d := attachedDisk(t) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d, got %d", DiskStateDetached, d.State()) + } +} + +func TestDetachFromVM_FromAttached_WithLinuxGuest(t *testing.T) { + d := attachedDisk(t) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, &mockLinuxGuestSCSIEjector{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d, got %d", DiskStateDetached, d.State()) + } +} + +func TestDetachFromVM_LinuxEjectError(t *testing.T) { + d := attachedDisk(t) + ejectErr := errors.New("eject failed") + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, &mockLinuxGuestSCSIEjector{err: ejectErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, ejectErr) { + t.Errorf("expected wrapped error %v, got %v", ejectErr, err) + } + // State should remain attached since eject failed before state transition. + if d.State() != DiskStateAttached { + t.Errorf("expected state %d, got %d", DiskStateAttached, d.State()) + } +} + +func TestDetachFromVM_RemoveError(t *testing.T) { + d := attachedDisk(t) + removeErr := errors.New("remove failed") + // No linux guest so eject is skipped, but RemoveSCSIDisk fails. + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{err: removeErr}, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, removeErr) { + t.Errorf("expected wrapped error %v, got %v", removeErr, err) + } + // State should be ejected since eject succeeded but removal failed. + if d.State() != DiskStateEjected { + t.Errorf("expected state %d, got %d", DiskStateEjected, d.State()) + } +} + +func TestDetachFromVM_SkipsWhenMountsExist(t *testing.T) { + d := attachedDisk(t) + // Reserve a partition so len(mounts) > 0. + _, err := d.ReservePartition(context.Background(), mount.MountConfig{Partition: 1}) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Should remain attached because there are outstanding mounts. + if d.State() != DiskStateAttached { + t.Errorf("expected state %d, got %d", DiskStateAttached, d.State()) + } +} + +func TestDetachFromVM_Idempotent_WhenReserved(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("unexpected error detaching reserved disk: %v", err) + } +} + +func TestDetachFromVM_Idempotent_WhenDetached(t *testing.T) { + d := attachedDisk(t) + _ = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + // Second detach should be idempotent. + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("unexpected error on idempotent detach: %v", err) + } +} + +func TestReservePartition_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1, ReadOnly: true} + m, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m == nil { + t.Fatal("expected non-nil mount") + } + if m.State() != mount.MountStateReserved { + t.Errorf("expected mount state %d, got %d", mount.MountStateReserved, m.State()) + } +} + +func TestReservePartition_SuccessFromReservedDisk(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + cfg := mount.MountConfig{Partition: 1} + m, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m == nil { + t.Fatal("expected non-nil mount") + } +} + +func TestReservePartition_SamePartitionSameConfig(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1, ReadOnly: true} + m1, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + m2, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + if m1 != m2 { + t.Error("expected same mount object on duplicate reservation") + } +} + +func TestReservePartition_SamePartitionDifferentConfig(t *testing.T) { + d := attachedDisk(t) + cfg1 := mount.MountConfig{Partition: 1, ReadOnly: true} + _, err := d.ReservePartition(context.Background(), cfg1) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + cfg2 := mount.MountConfig{Partition: 1, ReadOnly: false} + _, err = d.ReservePartition(context.Background(), cfg2) + if err == nil { + t.Fatal("expected error reserving same partition with different config") + } +} + +func TestReservePartition_ErrorWhenDetached(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + _ = d.AttachToVM(context.Background(), &mockVMSCSIAdder{err: errors.New("fail")}) + _, err := d.ReservePartition(context.Background(), mount.MountConfig{Partition: 1}) + if err == nil { + t.Fatal("expected error when disk is detached") + } +} + +func TestMountPartitionToGuest_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + guestPath, err := d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMountPartitionToGuest_ErrorWhenNotAttached(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + _, err := d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err == nil { + t.Fatal("expected error when disk is not attached") + } +} + +func TestMountPartitionToGuest_PartitionNotFound(t *testing.T) { + d := attachedDisk(t) + _, err := d.MountPartitionToGuest(context.Background(), 99, &mockLinuxGuestSCSIMounter{}, nil) + if err == nil { + t.Fatal("expected error for unreserved partition") + } +} + +func TestMountPartitionToGuest_MountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{err: errors.New("mount fail")}, nil) + if err != nil { + // This is expected - the mount error propagates. + return + } +} + +func TestUnmountPartitionFromGuest_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmountPartitionFromGuest_SucceedsWhenNotAttached(t *testing.T) { + d := NewReserved(0, 0, defaultConfig()) + // No partition reserved, so this is a no-op success for retry logic. + err := d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("expected nil error for missing partition on non-attached disk, got: %v", err) + } +} + +func TestUnmountPartitionFromGuest_PartitionNotFound_IsNoOp(t *testing.T) { + d := attachedDisk(t) + // Missing partition is treated as success for retry safety. + err := d.UnmountPartitionFromGuest(context.Background(), 99, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("expected nil error for missing partition, got: %v", err) + } +} + +func TestUnmountPartitionFromGuest_UnmountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + unmountErr := errors.New("unmount fail") + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{err: unmountErr}, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, unmountErr) { + t.Errorf("expected wrapped error %v, got %v", unmountErr, err) + } +} + +func TestUnmountPartitionFromGuest_RemovesMountWhenFullyUnmounted(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("UnmountPartitionFromGuest: %v", err) + } + // After full unmount the partition should be removed, so detach should succeed + // (no mounts remain). + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("DetachFromVM after unmount: %v", err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d, got %d", DiskStateDetached, d.State()) + } +} + +func TestUnmountPartitionFromGuest_RetryAfterDetachFailure(t *testing.T) { + // Simulates the scenario where unmount succeeds but detach fails. + // On retry, UnmountPartitionFromGuest should be a no-op (partition already removed) + // so that DetachFromVM can be retried. + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockLinuxGuestSCSIMounter{}, nil) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + // Unmount succeeds and removes the partition from the mounts map. + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("UnmountPartitionFromGuest: %v", err) + } + // Detach fails (e.g. transient error). + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{err: errors.New("transient")}, nil) + if err == nil { + t.Fatal("expected detach error") + } + // Retry: unmount is a no-op since partition is gone. + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockLinuxGuestSCSIUnmounter{}, nil) + if err != nil { + t.Fatalf("retry UnmountPartitionFromGuest should be no-op, got: %v", err) + } + // Retry detach now succeeds. + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("retry DetachFromVM: %v", err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d, got %d", DiskStateDetached, d.State()) + } +} + +// --- Windows (WCOW) guest tests --- + +func TestMountPartitionToGuest_WCOW_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + guestPath, err := d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMountPartitionToGuest_WCOW_MountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{err: errors.New("wcow mount fail")}) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestMountPartitionToGuest_WCOW_FormatWithRefs(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1, FormatWithRefs: true} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + guestPath, err := d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestUnmountPartitionFromGuest_WCOW_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmountPartitionFromGuest_WCOW_UnmountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + unmountErr := errors.New("wcow unmount fail") + err = d.UnmountPartitionFromGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIUnmounter{err: unmountErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, unmountErr) { + t.Errorf("expected wrapped error %v, got %v", unmountErr, err) + } +} + +func TestUnmountPartitionFromGuest_WCOW_RemovesMountWhenFullyUnmounted(t *testing.T) { + d := attachedDisk(t) + cfg := mount.MountConfig{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, nil, &mockWindowsGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("UnmountPartitionFromGuest: %v", err) + } + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + if err != nil { + t.Fatalf("DetachFromVM after unmount: %v", err) + } + if d.State() != DiskStateDetached { + t.Errorf("expected state %d, got %d", DiskStateDetached, d.State()) + } +} diff --git a/internal/controller/device/scsi/mount/mount.go b/internal/controller/device/scsi/mount/mount.go new file mode 100644 index 0000000000..1d820f3bad --- /dev/null +++ b/internal/controller/device/scsi/mount/mount.go @@ -0,0 +1,179 @@ +//go:build windows + +package mount + +import ( + "context" + "fmt" + "slices" +) + +// MountConfig describes how a partition of a SCSI disk should be mounted inside +// the guest. +type MountConfig struct { + // Partition is the target partition index (1-based) on a partitioned + // device. Zero means the whole disk. + // + // WCOW only supports whole disk. LCOW supports both whole disk and + // partition mounts if the disk has multiple. + Partition uint64 + // ReadOnly mounts the disk read-only. + ReadOnly bool + // Encrypted encrypts the device with dm-crypt. + // + // Only supported for LCOW. + Encrypted bool + // Options are mount flags or data passed to the guest mount call. + // + // Only supported for LCOW. + Options []string + // EnsureFilesystem formats the mount as Filesystem if not already + // formatted. + // + // Only supported for LCOW. + EnsureFilesystem bool + // Filesystem is the target filesystem type. + // + // Only supported for LCOW. + Filesystem string + // BlockDev mounts the device as a block device. + // + // Only supported for LCOW. + BlockDev bool + // FormatWithRefs formats the disk using refs. + // + // Only supported for WCOW scratch disks. + FormatWithRefs bool +} + +// equals reports whether two MountConfig values describe the same mount parameters. +func (mc MountConfig) Equals(other MountConfig) bool { + return mc.ReadOnly == other.ReadOnly && + mc.Encrypted == other.Encrypted && + mc.EnsureFilesystem == other.EnsureFilesystem && + mc.Filesystem == other.Filesystem && + mc.BlockDev == other.BlockDev && + mc.FormatWithRefs == other.FormatWithRefs && + slices.Equal(mc.Options, other.Options) +} + +type MountState int + +const ( + // The mount has never been mounted. + MountStateReserved MountState = iota + // The mount is current mounted in the guest. + MountStateMounted + // The mount was previously mounted and unmounted. + MountStateUnmounted +) + +// Defines a mount of a partition of a SCSI disk inside the guest. It manages +// the lifecycle of the mount inside the guest independent of the lifecycle of +// the disk attachment. +// +// All operations on the mount are expected to be ordered by the caller. No +// locking is done at this layer. +type Mount struct { + controller uint + lun uint + config MountConfig + + state MountState + refCount int + guestPath string +} + +// NewReserved creates a new Mount in the reserved state with the provided configuration. +func NewReserved(controller, lun uint, config MountConfig) *Mount { + return &Mount{ + controller: controller, + lun: lun, + config: config, + state: MountStateReserved, + refCount: 1, + } +} + +func (m *Mount) State() MountState { + return m.state +} + +func (m *Mount) GuestPath() string { + return m.guestPath +} + +func (m *Mount) Reserve(config MountConfig) error { + if !m.config.Equals(config) { + return fmt.Errorf("cannot reserve ref on mount with different config") + } + if m.state != MountStateReserved && m.state != MountStateMounted { + return fmt.Errorf("cannot reserve ref on mount in state %d", m.state) + } + m.refCount++ + return nil +} + +func (m *Mount) MountToGuest(ctx context.Context, linuxGuest LinuxGuestSCSIMounter, windowsGuest WindowsGuestSCSIMounter) (string, error) { + switch m.state { + // If the mount is reserved, we need to actually mount it in the guest. + case MountStateReserved: + if linuxGuest != nil { + if err := m.mountReservedLCOW(ctx, linuxGuest); err != nil { + // Move to unmounted since we know from reserved there was no + // guest state. + m.state = MountStateUnmounted + return "", err + } + } else if windowsGuest != nil { + if err := m.mountReservedWCOW(ctx, windowsGuest); err != nil { + // Move to unmounted since we know from reserved there was no + // guest state. + m.state = MountStateUnmounted + return "", err + } + } else { + panic(fmt.Errorf("both linuxGuest and windowsGuest cannot be nil")) + } + m.state = MountStateMounted + // Note we don't increment the ref count here as the caller of + // MountToGuest is responsible for calling it once per reservation, so + // we know the ref count should be 1 at this point. + return m.guestPath, nil + case MountStateMounted: + // The mount is already mounted, and the caller has a reservation so do nothing, its ready. + return m.guestPath, nil + case MountStateUnmounted: + return "", fmt.Errorf("cannot mount an unmounted mount") + } + return "", nil +} + +func (m *Mount) UnmountFromGuest(ctx context.Context, linuxGuest LinuxGuestSCSIUnmounter, windowsGuest WindowsGuestSCSIUnmounter) error { + switch m.state { + case MountStateReserved: + // No guest work to do, just decrement the ref count and if it hits zero we are done. + m.refCount-- + return nil + case MountStateMounted: + if m.refCount == 1 { + if linuxGuest != nil { + if err := m.unmountLCOW(ctx, linuxGuest); err != nil { + return err + } + } else if windowsGuest != nil { + if err := m.unmountWCOW(ctx, windowsGuest); err != nil { + return err + } + } else { + return fmt.Errorf("both linuxGuest and windowsGuest cannot be nil") + } + m.state = MountStateUnmounted + } + m.refCount-- + return nil + case MountStateUnmounted: + return fmt.Errorf("cannot unmount an unmounted mount") + } + return nil +} diff --git a/internal/controller/device/scsi/mount/mount_lcow.go b/internal/controller/device/scsi/mount/mount_lcow.go new file mode 100644 index 0000000000..3423ae3580 --- /dev/null +++ b/internal/controller/device/scsi/mount/mount_lcow.go @@ -0,0 +1,77 @@ +//go:build windows + +package mount + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +type LinuxGuestSCSIMounter interface { + AddLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error +} + +type LinuxGuestSCSIUnmounter interface { + RemoveLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error +} + +const ( + mountFmtLCOW = "/run/mounts/scsi/%d_%d_%d" +) + +// Implements the mount operation for LCOW from the expected Reserved state. +// +// It does not transition the state to ensure the exposed mount interface +// handles transitions for all LCOW/WCOW. +func (m *Mount) mountReservedLCOW(ctx context.Context, guest LinuxGuestSCSIMounter) error { + if m.state != MountStateReserved { + return fmt.Errorf("unexpected mount state %d, expected reserved", m.state) + } + guestPath := fmt.Sprintf(mountFmtLCOW, m.controller, m.lun, m.config.Partition) + settings := guestresource.LCOWMappedVirtualDisk{ + MountPath: guestPath, + Controller: uint8(m.controller), + Lun: uint8(m.lun), + Partition: m.config.Partition, + ReadOnly: m.config.ReadOnly, + Encrypted: m.config.Encrypted, + Options: m.config.Options, + EnsureFilesystem: m.config.EnsureFilesystem, + Filesystem: m.config.Filesystem, + BlockDev: m.config.BlockDev, + } + if err := guest.AddLCOWMappedVirtualDisk(ctx, settings); err != nil { + return fmt.Errorf("add LCOW mapped virtual disk controller=%d lun=%d: %w", m.controller, m.lun, err) + } + m.guestPath = guestPath + return nil +} + +// Implements the unmount operation for LCOW from the expected Mounted state. +// +// It does not transition the state to ensure the exposed mount interface +// handles transitions for all LCOW/WCOW. +func (m *Mount) unmountLCOW(ctx context.Context, guest LinuxGuestSCSIUnmounter) error { + if m.state != MountStateMounted { + return fmt.Errorf("unexpected mount state %d, expected mounted", m.state) + } + guestPath := fmt.Sprintf(mountFmtLCOW, m.controller, m.lun, m.config.Partition) + settings := guestresource.LCOWMappedVirtualDisk{ + MountPath: guestPath, + Controller: uint8(m.controller), + Lun: uint8(m.lun), + Partition: m.config.Partition, + ReadOnly: m.config.ReadOnly, + Encrypted: m.config.Encrypted, + Options: m.config.Options, + EnsureFilesystem: m.config.EnsureFilesystem, + Filesystem: m.config.Filesystem, + BlockDev: m.config.BlockDev, + } + if err := guest.RemoveLCOWMappedVirtualDisk(ctx, settings); err != nil { + return fmt.Errorf("remove LCOW mapped virtual disk controller=%d lun=%d: %w", m.controller, m.lun, err) + } + return nil +} diff --git a/internal/controller/device/scsi/mount/mount_test.go b/internal/controller/device/scsi/mount/mount_test.go new file mode 100644 index 0000000000..7250db76ac --- /dev/null +++ b/internal/controller/device/scsi/mount/mount_test.go @@ -0,0 +1,391 @@ +//go:build windows + +package mount + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockLinuxMounter struct { + err error +} + +func (m *mockLinuxMounter) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockLinuxUnmounter struct { + err error +} + +func (m *mockLinuxUnmounter) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockWindowsMounter struct { + err error + scratchFn func() +} + +func (m *mockWindowsMounter) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +func (m *mockWindowsMounter) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + if m.scratchFn != nil { + m.scratchFn() + } + return m.err +} + +type mockWindowsUnmounter struct { + err error +} + +func (m *mockWindowsUnmounter) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers --- + +func defaultMountConfig() MountConfig { + return MountConfig{ + Partition: 1, + ReadOnly: false, + } +} + +func mountedLCOW(t *testing.T) *Mount { + t.Helper() + m := NewReserved(0, 0, defaultMountConfig()) + if _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil); err != nil { + t.Fatalf("setup MountToGuest: %v", err) + } + return m +} + +func mountedWCOW(t *testing.T) *Mount { + t.Helper() + m := NewReserved(0, 0, defaultMountConfig()) + if _, err := m.MountToGuest(context.Background(), nil, &mockWindowsMounter{}); err != nil { + t.Fatalf("setup MountToGuest WCOW: %v", err) + } + return m +} + +// --- Tests --- + +func TestNewReserved(t *testing.T) { + cfg := MountConfig{ + Partition: 2, + ReadOnly: true, + Encrypted: true, + Filesystem: "ext4", + } + m := NewReserved(1, 3, cfg) + if m.State() != MountStateReserved { + t.Errorf("expected state %d, got %d", MountStateReserved, m.State()) + } +} + +func TestMountConfigEquals(t *testing.T) { + base := MountConfig{ + ReadOnly: true, + Encrypted: true, + EnsureFilesystem: true, + Filesystem: "ext4", + BlockDev: false, + FormatWithRefs: false, + Options: []string{"rw", "noatime"}, + } + same := MountConfig{ + ReadOnly: true, + Encrypted: true, + EnsureFilesystem: true, + Filesystem: "ext4", + BlockDev: false, + FormatWithRefs: false, + Options: []string{"rw", "noatime"}, + } + if !base.Equals(same) { + t.Error("expected equal configs to be equal") + } + + tests := []struct { + name string + modify func(c *MountConfig) + }{ + {"ReadOnly", func(c *MountConfig) { c.ReadOnly = false }}, + {"Encrypted", func(c *MountConfig) { c.Encrypted = false }}, + {"EnsureFilesystem", func(c *MountConfig) { c.EnsureFilesystem = false }}, + {"Filesystem", func(c *MountConfig) { c.Filesystem = "xfs" }}, + {"BlockDev", func(c *MountConfig) { c.BlockDev = true }}, + {"FormatWithRefs", func(c *MountConfig) { c.FormatWithRefs = true }}, + {"Options", func(c *MountConfig) { c.Options = []string{"ro"} }}, + {"OptionsNil", func(c *MountConfig) { c.Options = nil }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modified := same + tt.modify(&modified) + if base.Equals(modified) { + t.Errorf("expected configs to differ on %s", tt.name) + } + }) + } +} + +func TestReserve_SameConfig(t *testing.T) { + cfg := defaultMountConfig() + m := NewReserved(0, 0, cfg) + if err := m.Reserve(cfg); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestReserve_DifferentConfig(t *testing.T) { + m := NewReserved(0, 0, MountConfig{ReadOnly: true}) + err := m.Reserve(MountConfig{ReadOnly: false}) + if err == nil { + t.Fatal("expected error for different config") + } +} + +func TestReserve_WhenMounted(t *testing.T) { + m := mountedLCOW(t) + cfg := defaultMountConfig() + if err := m.Reserve(cfg); err != nil { + t.Fatalf("unexpected error reserving mounted mount: %v", err) + } +} + +func TestReserve_WhenUnmounted(t *testing.T) { + m := mountedLCOW(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + if m.State() != MountStateUnmounted { + t.Fatalf("expected unmounted, got %d", m.State()) + } + err := m.Reserve(defaultMountConfig()) + if err == nil { + t.Fatal("expected error reserving unmounted mount") + } +} + +func TestMountToGuest_LCOW_Success(t *testing.T) { + m := NewReserved(0, 0, defaultMountConfig()) + guestPath, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestMountToGuest_WCOW_Success(t *testing.T) { + m := NewReserved(0, 0, defaultMountConfig()) + guestPath, err := m.MountToGuest(context.Background(), nil, &mockWindowsMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestMountToGuest_LCOW_Error(t *testing.T) { + mountErr := errors.New("lcow mount failed") + m := NewReserved(0, 0, defaultMountConfig()) + _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{err: mountErr}, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, mountErr) { + t.Errorf("expected wrapped error %v, got %v", mountErr, err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d after failure, got %d", MountStateUnmounted, m.State()) + } +} + +func TestMountToGuest_WCOW_Error(t *testing.T) { + mountErr := errors.New("wcow mount failed") + m := NewReserved(0, 0, defaultMountConfig()) + _, err := m.MountToGuest(context.Background(), nil, &mockWindowsMounter{err: mountErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, mountErr) { + t.Errorf("expected wrapped error %v, got %v", mountErr, err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d after failure, got %d", MountStateUnmounted, m.State()) + } +} + +func TestMountToGuest_WCOW_FormatWithRefs(t *testing.T) { + scratchCalled := false + m := NewReserved(0, 0, MountConfig{Partition: 1, FormatWithRefs: true}) + wm := &mockWindowsMounter{scratchFn: func() { scratchCalled = true }} + _, err := m.MountToGuest(context.Background(), nil, wm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !scratchCalled { + t.Error("expected AddWCOWMappedVirtualDiskForContainerScratch to be called") + } +} + +func TestMountToGuest_Idempotent_WhenMounted(t *testing.T) { + m := mountedLCOW(t) + guestPath, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil) + if err != nil { + t.Fatalf("unexpected error on idempotent mount: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath on idempotent mount") + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestMountToGuest_ErrorWhenUnmounted(t *testing.T) { + m := mountedLCOW(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil) + if err == nil { + t.Fatal("expected error mounting an unmounted mount") + } +} + +func TestMountToGuest_PanicsWhenBothGuestsNil(t *testing.T) { + m := NewReserved(0, 0, defaultMountConfig()) + defer func() { + if r := recover(); r == nil { + t.Fatal("expected panic when both guests are nil") + } + }() + _, _ = m.MountToGuest(context.Background(), nil, nil) +} + +func TestUnmountFromGuest_LCOW_Success(t *testing.T) { + m := mountedLCOW(t) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d, got %d", MountStateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_WCOW_Success(t *testing.T) { + m := mountedWCOW(t) + err := m.UnmountFromGuest(context.Background(), nil, &mockWindowsUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d, got %d", MountStateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_LCOW_Error(t *testing.T) { + m := mountedLCOW(t) + unmountErr := errors.New("lcow unmount failed") + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{err: unmountErr}, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, unmountErr) { + t.Errorf("expected wrapped error %v, got %v", unmountErr, err) + } + // State should remain mounted since unmount failed. + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestUnmountFromGuest_WCOW_Error(t *testing.T) { + m := mountedWCOW(t) + unmountErr := errors.New("wcow unmount failed") + err := m.UnmountFromGuest(context.Background(), nil, &mockWindowsUnmounter{err: unmountErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, unmountErr) { + t.Errorf("expected wrapped error %v, got %v", unmountErr, err) + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestUnmountFromGuest_FromReserved_DecrementsRefCount(t *testing.T) { + m := NewReserved(0, 0, defaultMountConfig()) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Should still be reserved since no guest mount was done. + if m.State() != MountStateReserved { + t.Errorf("expected state %d, got %d", MountStateReserved, m.State()) + } +} + +func TestUnmountFromGuest_ErrorWhenUnmounted(t *testing.T) { + m := mountedLCOW(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + if err == nil { + t.Fatal("expected error unmounting already-unmounted mount") + } +} + +func TestUnmountFromGuest_ErrorWhenBothGuestsNil(t *testing.T) { + m := mountedLCOW(t) + err := m.UnmountFromGuest(context.Background(), nil, nil) + if err == nil { + t.Fatal("expected error when both guests are nil") + } +} + +func TestUnmountFromGuest_MultipleRefs_DoesNotUnmountUntilLastRef(t *testing.T) { + cfg := defaultMountConfig() + m := NewReserved(0, 0, cfg) + // Add a second reservation. + if err := m.Reserve(cfg); err != nil { + t.Fatalf("Reserve: %v", err) + } + // Mount the guest. + if _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil); err != nil { + t.Fatalf("MountToGuest: %v", err) + } + // First unmount should decrement ref but stay mounted. + if err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil); err != nil { + t.Fatalf("first UnmountFromGuest: %v", err) + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d after first unmount, got %d", MountStateMounted, m.State()) + } + // Second unmount should actually unmount. + if err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil); err != nil { + t.Fatalf("second UnmountFromGuest: %v", err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d after final unmount, got %d", MountStateUnmounted, m.State()) + } +} diff --git a/internal/controller/device/scsi/mount/mount_wcow.go b/internal/controller/device/scsi/mount/mount_wcow.go new file mode 100644 index 0000000000..8b624d192d --- /dev/null +++ b/internal/controller/device/scsi/mount/mount_wcow.go @@ -0,0 +1,69 @@ +//go:build windows + +package mount + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +type WindowsGuestSCSIMounter interface { + AddWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error + AddWCOWMappedVirtualDiskForContainerScratch(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error +} + +type WindowsGuestSCSIUnmounter interface { + RemoveWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error +} + +const ( + mountFmtWCOW = "C:\\mounts\\scsi\\%d_%d_%d" +) + +// Implements the mount operation for WCOW from the expected Reserved state. +// +// It does not transition the state to ensure the exposed mount interface +// handles transitions for all LCOW/WCOW. +func (m *Mount) mountReservedWCOW(ctx context.Context, guest WindowsGuestSCSIMounter) error { + if m.state != MountStateReserved { + return fmt.Errorf("unexpected mount state %d, expected reserved", m.state) + } + guestPath := fmt.Sprintf(mountFmtWCOW, m.controller, m.lun, m.config.Partition) + settings := guestresource.WCOWMappedVirtualDisk{ + ContainerPath: guestPath, + Lun: int32(m.lun), + } + + if m.config.FormatWithRefs { + if err := guest.AddWCOWMappedVirtualDiskForContainerScratch(ctx, settings); err != nil { + return fmt.Errorf("add WCOW mapped virtual disk for container scratch controller=%d lun=%d: %w", m.controller, m.lun, err) + } + } else { + if err := guest.AddWCOWMappedVirtualDisk(ctx, settings); err != nil { + return fmt.Errorf("add WCOW mapped virtual disk controller=%d lun=%d: %w", m.controller, m.lun, err) + } + } + m.guestPath = guestPath + return nil +} + +// Implements the unmount operation for WCOW from the expected Mounted state. +// +// It does not transition the state to ensure the exposed mount interface +// handles transitions for all LCOW/WCOW. +func (m *Mount) unmountWCOW(ctx context.Context, guest WindowsGuestSCSIUnmounter) error { + if m.state != MountStateMounted { + return fmt.Errorf("unexpected mount state %d, expected mounted", m.state) + } + guestPath := fmt.Sprintf(mountFmtWCOW, m.controller, m.lun, m.config.Partition) + settings := guestresource.WCOWMappedVirtualDisk{ + ContainerPath: guestPath, + Lun: int32(m.lun), + } + if err := guest.RemoveWCOWMappedVirtualDisk(ctx, settings); err != nil { + return fmt.Errorf("remove WCOW mapped virtual disk controller=%d lun=%d: %w", m.controller, m.lun, err) + } + return nil +} diff --git a/internal/controller/device/scsi/scsi.go b/internal/controller/device/scsi/scsi.go new file mode 100644 index 0000000000..a3d4c9bfb8 --- /dev/null +++ b/internal/controller/device/scsi/scsi.go @@ -0,0 +1,7 @@ +//go:build windows + +// Package SCSI implements the SCSI controller for managing SCSI attached +// devices to a Utility VM as well as the Guest Mounts surfacing those devices +// for use by the Guest containers. + +package scsi diff --git a/internal/vm/guestmanager/scsi_lcow.go b/internal/vm/guestmanager/scsi_lcow.go index 65d1b5b24e..528912275d 100644 --- a/internal/vm/guestmanager/scsi_lcow.go +++ b/internal/vm/guestmanager/scsi_lcow.go @@ -11,18 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// LCOWScsiManager exposes mapped virtual disk and SCSI device operations in the LCOW guest. -type LCOWScsiManager interface { - // AddLCOWMappedVirtualDisk maps a virtual disk into the LCOW guest. - AddLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error - // RemoveLCOWMappedVirtualDisk unmaps a virtual disk from the LCOW guest. - RemoveLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error - // RemoveSCSIDevice removes a SCSI device from the guest. - RemoveSCSIDevice(ctx context.Context, settings guestresource.SCSIDevice) error -} - -var _ LCOWScsiManager = (*Guest)(nil) - // AddLCOWMappedVirtualDisk maps a virtual disk into a LCOW guest. func (gm *Guest) AddLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error { request := &hcsschema.ModifySettingRequest{ diff --git a/internal/vm/guestmanager/scsi_wcow.go b/internal/vm/guestmanager/scsi_wcow.go index 74691f395c..d8ea38f952 100644 --- a/internal/vm/guestmanager/scsi_wcow.go +++ b/internal/vm/guestmanager/scsi_wcow.go @@ -11,20 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// WCOWScsiManager exposes mapped virtual disk and SCSI device operations in the WCOW guest. -type WCOWScsiManager interface { - // AddWCOWMappedVirtualDisk maps a virtual disk into the WCOW guest. - AddWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error - // AddWCOWMappedVirtualDiskForContainerScratch attaches a scratch disk in the WCOW guest. - AddWCOWMappedVirtualDiskForContainerScratch(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error - // RemoveWCOWMappedVirtualDisk unmaps a virtual disk from the WCOW guest. - RemoveWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error - // RemoveSCSIDevice removes a SCSI device from the guest. - RemoveSCSIDevice(ctx context.Context, settings guestresource.SCSIDevice) error -} - -var _ WCOWScsiManager = (*Guest)(nil) - // AddWCOWMappedVirtualDisk maps a virtual disk into a WCOW guest. func (gm *Guest) AddWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error { request := &hcsschema.ModifySettingRequest{ diff --git a/internal/vm/vmmanager/scsi.go b/internal/vm/vmmanager/scsi.go index 6af4e8cc96..038ac66a5e 100644 --- a/internal/vm/vmmanager/scsi.go +++ b/internal/vm/vmmanager/scsi.go @@ -11,17 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" ) -// SCSIManager manages adding and removing SCSI devices for a Utility VM. -type SCSIManager interface { - // AddSCSIDisk hot adds a SCSI disk to the Utility VM. - AddSCSIDisk(ctx context.Context, disk hcsschema.Attachment, controller uint, lun uint) error - - // RemoveSCSIDisk removes a SCSI disk from a Utility VM. - RemoveSCSIDisk(ctx context.Context, controller uint, lun uint) error -} - -var _ SCSIManager = (*UtilityVM)(nil) - func (uvm *UtilityVM) AddSCSIDisk(ctx context.Context, disk hcsschema.Attachment, controller uint, lun uint) error { request := &hcsschema.ModifySettingRequest{ RequestType: guestrequest.RequestTypeAdd,