diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c880d3123f..69ba4e6780 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -317,6 +317,12 @@ jobs: - name: Test repo run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags admin -timeout=20m ./... + - name: Test lcow tagged packages (shimV2) + run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags lcow -timeout=10m ./... + + - name: Test wcow tagged packages (shimV2) + run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags wcow -timeout=10m ./... + - name: Run non-functional tests run: ${{ env.GOTESTSUM_CMD }} -mod=mod -gcflags=all=-d=checkptr ./internal/... ./pkg/... ./parity/... working-directory: test @@ -637,8 +643,16 @@ jobs: $LASTEXITCODE = 0 } + - run: ${{ env.GO_BUILD_CMD }} -tags lcow ./... + name: Build lcow tagged packages (shimV2) + - run: ${{ env.GO_BUILD_CMD }} -tags wcow ./... + name: Build wcow tagged packages (shimV2) + - run: ${{ env.GO_BUILD_CMD }} ./... + name: Build untagged packages - run: ${{ env.GO_BUILD_CMD }} ./cmd/containerd-shim-runhcs-v1 name: Build containerd-shim-runhcs-v1.exe + - run: ${{ env.GO_BUILD_CMD }} -tags lcow ./cmd/containerd-shim-lcow-v2 + name: Build containerd-shim-lcow-v2.exe - run: ${{ env.GO_BUILD_CMD }} ./cmd/device-util name: Build device-util.exe - run: ${{ env.GO_BUILD_CMD }} ./cmd/jobobject-util diff --git a/internal/controller/device/scsi/controller.go b/internal/controller/device/scsi/controller.go index d664398971..a89f4acea6 100644 --- a/internal/controller/device/scsi/controller.go +++ b/internal/controller/device/scsi/controller.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package scsi @@ -39,12 +39,9 @@ type Controller struct { // vm is the host-side interface for adding and removing SCSI disks. // Immutable after construction. vm VMSCSIOps - // linuxGuest is the guest-side interface for LCOW SCSI operations. + // guest is the guest-side interface for SCSI operations. // Immutable after construction. - linuxGuest LinuxGuestSCSIOps - // windowsGuest is the guest-side interface for WCOW SCSI operations. - // Immutable after construction. - windowsGuest WindowsGuestSCSIOps + guest GuestSCSIOps // reservations maps a reservation ID to its disk slot and partition. // Guarded by mu. @@ -65,11 +62,10 @@ type Controller struct { // New creates a new [Controller] for the given number of SCSI controllers and // host/guest operation interfaces. -func New(numControllers int, vm VMSCSIOps, linuxGuest LinuxGuestSCSIOps, windowsGuest WindowsGuestSCSIOps) *Controller { +func New(numControllers int, vm VMSCSIOps, guest GuestSCSIOps) *Controller { return &Controller{ vm: vm, - linuxGuest: linuxGuest, - windowsGuest: windowsGuest, + guest: guest, reservations: make(map[guid.GUID]*reservation), disksByPath: make(map[string]int), controllerSlots: make([]*disk.Disk, numControllers*numLUNsPerController), @@ -200,7 +196,7 @@ func (c *Controller) MapToGuest(ctx context.Context, id guid.GUID) (string, erro } // Mount the partition inside the guest. - guestPath, err := existingDisk.MountPartitionToGuest(ctx, r.partition, c.linuxGuest, c.windowsGuest) + guestPath, err := existingDisk.MountPartitionToGuest(ctx, r.partition, c.guest) if err != nil { return "", fmt.Errorf("mount partition %d to guest: %w", r.partition, err) } @@ -232,12 +228,12 @@ func (c *Controller) UnmapFromGuest(ctx context.Context, id guid.GUID) error { // Unmount the partition from the guest (ref-counted; only issues the // guest call when this is the last reservation on the partition). - if err := existingDisk.UnmountPartitionFromGuest(ctx, r.partition, c.linuxGuest, c.windowsGuest); err != nil { + if err := existingDisk.UnmountPartitionFromGuest(ctx, r.partition, c.guest); err != nil { return fmt.Errorf("unmount partition %d from guest: %w", r.partition, err) } // Detach the disk from the VM when no partitions remain active. - if err := existingDisk.DetachFromVM(ctx, c.vm, c.linuxGuest); err != nil { + if err := existingDisk.DetachFromVM(ctx, c.vm, c.guest); err != nil { return fmt.Errorf("detach disk from VM: %w", err) } diff --git a/internal/controller/device/scsi/controller_lcow_test.go b/internal/controller/device/scsi/controller_lcow_test.go new file mode 100644 index 0000000000..54dc712512 --- /dev/null +++ b/internal/controller/device/scsi/controller_lcow_test.go @@ -0,0 +1,73 @@ +//go:build windows && lcow + +package scsi + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock type --- + +type mockGuestOps struct { + mountErr error + unmountErr error + ejectErr error +} + +func (m *mockGuestOps) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockGuestOps) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.unmountErr +} + +func (m *mockGuestOps) RemoveSCSIDevice(_ context.Context, _ guestresource.SCSIDevice) error { + return m.ejectErr +} + +// --- Helpers used by shared tests --- + +func newMockGuestOps() *mockGuestOps { + return &mockGuestOps{} +} + +// --- LCOW-specific tests --- + +func TestMapToGuest_MountError(t *testing.T) { + guest := &mockGuestOps{mountErr: errors.New("mount failed")} + c := New(1, &mockVMOps{}, guest) + 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 TestUnmapFromGuest_UnmountError(t *testing.T) { + guest := &mockGuestOps{} + c := New(1, &mockVMOps{}, guest) + 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) + } + // Now inject an unmount error. + guest.unmountErr = errors.New("unmount failed") + err = c.UnmapFromGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} diff --git a/internal/controller/device/scsi/controller_test.go b/internal/controller/device/scsi/controller_test.go index 9f3efa8fcf..85b8fb92de 100644 --- a/internal/controller/device/scsi/controller_test.go +++ b/internal/controller/device/scsi/controller_test.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package scsi @@ -11,7 +11,6 @@ import ( "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/Microsoft/go-winio/pkg/guid" ) @@ -31,41 +30,6 @@ 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.Config { @@ -83,13 +47,13 @@ func defaultMountConfig() mount.Config { } } -func newController(vm *mockVMOps, lg *mockLinuxGuestOps, wg *mockWindowsGuestOps) *Controller { - return New(1, vm, lg, wg) +func newController(vm *mockVMOps, guest *mockGuestOps) *Controller { + return New(1, vm, guest) } func reservedController(t *testing.T) (*Controller, guid.GUID) { t.Helper() - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) if err != nil { t.Fatalf("setup Reserve: %v", err) @@ -109,7 +73,7 @@ func mappedController(t *testing.T) (*Controller, guid.GUID) { // --- Tests: New --- func TestNew(t *testing.T) { - c := New(4, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := New(4, &mockVMOps{}, newMockGuestOps()) if c == nil { t.Fatal("expected non-nil controller") } @@ -118,7 +82,7 @@ func TestNew(t *testing.T) { // --- Tests: Reserve --- func TestReserve_Success(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -129,7 +93,7 @@ func TestReserve_Success(t *testing.T) { } func TestReserve_SameDiskSamePartition(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) dc := defaultDiskConfig() mc := defaultMountConfig() id1, err := c.Reserve(context.Background(), dc, mc) @@ -146,7 +110,7 @@ func TestReserve_SameDiskSamePartition(t *testing.T) { } func TestReserve_SameDiskDifferentPartition(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) dc := defaultDiskConfig() _, err := c.Reserve(context.Background(), dc, mount.Config{Partition: 1}) if err != nil { @@ -159,7 +123,7 @@ func TestReserve_SameDiskDifferentPartition(t *testing.T) { } func TestReserve_SamePathDifferentConfig(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) dc := defaultDiskConfig() mc := defaultMountConfig() _, err := c.Reserve(context.Background(), dc, mc) @@ -175,7 +139,7 @@ func TestReserve_SamePathDifferentConfig(t *testing.T) { } func TestReserve_DifferentDisks(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) mc := defaultMountConfig() _, err := c.Reserve(context.Background(), disk.Config{HostPath: `C:\a.vhdx`, Type: disk.TypeVirtualDisk}, mc) if err != nil { @@ -189,7 +153,7 @@ func TestReserve_DifferentDisks(t *testing.T) { func TestReserve_NoAvailableSlots(t *testing.T) { // Create with 0 controllers so there are no slots. - c := New(0, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := New(0, &mockVMOps{}, newMockGuestOps()) _, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) if err == nil { t.Fatal("expected error when no slots available") @@ -197,7 +161,7 @@ func TestReserve_NoAvailableSlots(t *testing.T) { } func TestReserve_FillsAllSlots(t *testing.T) { - c := New(1, &mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := New(1, &mockVMOps{}, newMockGuestOps()) mc := defaultMountConfig() for i := range numLUNsPerController { dc := disk.Config{ @@ -230,7 +194,7 @@ func TestMapToGuest_Success(t *testing.T) { } func TestMapToGuest_UnknownReservation(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) unknownID, err := guid.NewV4() if err != nil { t.Fatalf("NewV4: %v", err) @@ -243,20 +207,7 @@ func TestMapToGuest_UnknownReservation(t *testing.T) { 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) + c := New(1, vm, newMockGuestOps()) id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) if err != nil { t.Fatalf("Reserve: %v", err) @@ -289,7 +240,7 @@ func TestUnmapFromGuest_Success(t *testing.T) { } func TestUnmapFromGuest_UnknownReservation(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) unknownID, err := guid.NewV4() if err != nil { t.Fatalf("NewV4: %v", err) @@ -313,27 +264,9 @@ func TestUnmapFromGuest_CleansUpSlotWhenFullyDetached(t *testing.T) { } } -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) + c := New(1, vm, newMockGuestOps()) id, err := c.Reserve(context.Background(), defaultDiskConfig(), defaultMountConfig()) if err != nil { t.Fatalf("Reserve: %v", err) @@ -350,7 +283,7 @@ func TestUnmapFromGuest_DetachError(t *testing.T) { } func TestUnmapFromGuest_MultipleReservationsSameDisk(t *testing.T) { - c := newController(&mockVMOps{}, &mockLinuxGuestOps{}, nil) + c := newController(&mockVMOps{}, newMockGuestOps()) dc := defaultDiskConfig() mc := defaultMountConfig() @@ -388,24 +321,6 @@ func TestUnmapFromGuest_MultipleReservationsSameDisk(t *testing.T) { } } -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) { @@ -425,7 +340,7 @@ 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) + c := New(1, vm, newMockGuestOps()) dc := defaultDiskConfig() mc := defaultMountConfig() @@ -453,7 +368,7 @@ 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) + c := New(1, vm, newMockGuestOps()) dc := defaultDiskConfig() mc := defaultMountConfig() diff --git a/internal/controller/device/scsi/controller_wcow_test.go b/internal/controller/device/scsi/controller_wcow_test.go new file mode 100644 index 0000000000..4c1065ef6d --- /dev/null +++ b/internal/controller/device/scsi/controller_wcow_test.go @@ -0,0 +1,90 @@ +//go:build windows && wcow + +package scsi + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock type --- + +type mockGuestOps struct { + mountErr error + unmountErr error +} + +func (m *mockGuestOps) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockGuestOps) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.mountErr +} + +func (m *mockGuestOps) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.unmountErr +} + +// --- Helpers used by shared tests --- + +func newMockGuestOps() *mockGuestOps { + return &mockGuestOps{} +} + +// --- WCOW-specific tests --- + +func TestMapToGuest_MountError(t *testing.T) { + guest := &mockGuestOps{mountErr: errors.New("mount failed")} + c := New(1, &mockVMOps{}, guest) + 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 TestUnmapFromGuest_UnmountError(t *testing.T) { + guest := &mockGuestOps{} + c := New(1, &mockVMOps{}, guest) + 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) + } + // Now inject an unmount error. + guest.unmountErr = errors.New("unmount failed") + err = c.UnmapFromGuest(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestUnmapFromGuest_WindowsGuest(t *testing.T) { + guest := &mockGuestOps{} + c := New(1, &mockVMOps{}, guest) + 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) + } +} diff --git a/internal/controller/device/scsi/disk/disk.go b/internal/controller/device/scsi/disk/disk.go index fbafd7f698..e2ce220f84 100644 --- a/internal/controller/device/scsi/disk/disk.go +++ b/internal/controller/device/scsi/disk/disk.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package disk @@ -12,7 +12,6 @@ import ( hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/logfields" - "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) // Disk represents a SCSI disk attached to a Hyper-V VM. It tracks the @@ -113,7 +112,7 @@ func (d *Disk) AttachToVM(ctx context.Context, vm VMSCSIAdder) error { // DetachFromVM ejects the disk from the guest and removes it from the VM's SCSI // bus. It is idempotent for a disk that was never attached or is already detached; // a failed removal is retriable by calling DetachFromVM again. -func (d *Disk) DetachFromVM(ctx context.Context, vm VMSCSIRemover, linuxGuest LinuxGuestSCSIEjector) error { +func (d *Disk) DetachFromVM(ctx context.Context, vm VMSCSIRemover, guest GuestSCSIEjector) error { ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{ logfields.Controller: d.controller, logfields.LUN: d.lun, @@ -127,18 +126,10 @@ func (d *Disk) DetachFromVM(ctx context.Context, vm VMSCSIRemover, linuxGuest Li return nil } - // LCOW guests require an explicit SCSI device removal before the host - // removes the disk from the VM bus. For WCOW, Windows handles hot-unplug - // automatically, so linuxGuest will be nil. - if linuxGuest != nil { - log.G(ctx).Debug("ejecting SCSI device from guest") - - if err := linuxGuest.RemoveSCSIDevice(ctx, guestresource.SCSIDevice{ - Controller: uint8(d.controller), - Lun: uint8(d.lun), - }); err != nil { - return fmt.Errorf("eject SCSI device controller=%d lun=%d from guest: %w", d.controller, d.lun, err) - } + // Attempt to eject the disk from the guest. + if err := d.ejectFromGuest(ctx, guest); err != nil { + // Leave the disk in StateAttached so the caller can retry this step. + return fmt.Errorf("eject from guest: %w", err) } // Advance to Ejected before attempting VM removal so that a removal @@ -209,7 +200,7 @@ func (d *Disk) ReservePartition(ctx context.Context, config mount.Config) (*moun // MountPartitionToGuest mounts the partition inside the guest, // returning the auto-generated guest path. The partition must first be reserved // via [Disk.ReservePartition]. -func (d *Disk) MountPartitionToGuest(ctx context.Context, partition uint64, linuxGuest mount.LinuxGuestSCSIMounter, windowsGuest mount.WindowsGuestSCSIMounter) (string, error) { +func (d *Disk) MountPartitionToGuest(ctx context.Context, partition uint64, guest mount.GuestSCSIMounter) (string, error) { if d.state != StateAttached { return "", fmt.Errorf("cannot mount partition on disk in state %s, expected attached", d.state) } @@ -219,21 +210,21 @@ func (d *Disk) MountPartitionToGuest(ctx context.Context, partition uint64, linu if !ok { return "", fmt.Errorf("partition %d not reserved on disk controller=%d lun=%d", partition, d.controller, d.lun) } - return existingMnt.MountToGuest(ctx, linuxGuest, windowsGuest) + return existingMnt.MountToGuest(ctx, guest) } // UnmountPartitionFromGuest unmounts the partition at the given index from the // guest. When the mount's reference count reaches zero, and it transitions to // the unmounted state, its entry is removed from the disk so a subsequent // [Disk.DetachFromVM] call sees no active mounts. -func (d *Disk) UnmountPartitionFromGuest(ctx context.Context, partition uint64, linuxGuest mount.LinuxGuestSCSIUnmounter, windowsGuest mount.WindowsGuestSCSIUnmounter) error { +func (d *Disk) UnmountPartitionFromGuest(ctx context.Context, partition uint64, guest mount.GuestSCSIUnmounter) error { existingMount, ok := d.mounts[partition] if !ok { // No mount found — treat as a no-op to support retry by callers. return nil } - if err := existingMount.UnmountFromGuest(ctx, linuxGuest, windowsGuest); err != nil { + if err := existingMount.UnmountFromGuest(ctx, guest); err != nil { return fmt.Errorf("unmount partition %d from guest: %w", partition, err) } diff --git a/internal/controller/device/scsi/disk/disk_lcow.go b/internal/controller/device/scsi/disk/disk_lcow.go index a53f0fd644..139499a960 100644 --- a/internal/controller/device/scsi/disk/disk_lcow.go +++ b/internal/controller/device/scsi/disk/disk_lcow.go @@ -1,3 +1,32 @@ -//go:build windows +//go:build windows && lcow package disk + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// GuestSCSIEjector issues a guest-side SCSI device removal for LCOW guests. +type GuestSCSIEjector interface { + // RemoveSCSIDevice removes a SCSI device from the LCOW guest. + RemoveSCSIDevice(ctx context.Context, settings guestresource.SCSIDevice) error +} + +// ejectFromGuest issues an explicit guest-side SCSI device removal for LCOW guests. +// This must be performed before the host removes the disk from the VM bus. +func (d *Disk) ejectFromGuest(ctx context.Context, guest GuestSCSIEjector) error { + log.G(ctx).Debug("ejecting SCSI device from guest") + + if err := guest.RemoveSCSIDevice(ctx, guestresource.SCSIDevice{ + Controller: uint8(d.controller), + Lun: uint8(d.lun), + }); err != nil { + return fmt.Errorf("eject SCSI device controller=%d lun=%d from guest: %w", d.controller, d.lun, err) + } + + return nil +} diff --git a/internal/controller/device/scsi/disk/disk_lcow_test.go b/internal/controller/device/scsi/disk/disk_lcow_test.go new file mode 100644 index 0000000000..3d77b426f2 --- /dev/null +++ b/internal/controller/device/scsi/disk/disk_lcow_test.go @@ -0,0 +1,211 @@ +//go:build windows && lcow + +package disk + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockGuestSCSIEjector struct { + err error +} + +func (m *mockGuestSCSIEjector) RemoveSCSIDevice(_ context.Context, _ guestresource.SCSIDevice) error { + return m.err +} + +type mockGuestSCSIMounter struct { + err error +} + +func (m *mockGuestSCSIMounter) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockGuestSCSIUnmounter struct { + err error +} + +func (m *mockGuestSCSIUnmounter) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers used by shared tests --- + +func newDefaultEjector() GuestSCSIEjector { + return &mockGuestSCSIEjector{} +} + +func newDefaultMounter() mount.GuestSCSIMounter { + return &mockGuestSCSIMounter{} +} + +func newDefaultUnmounter() mount.GuestSCSIUnmounter { + return &mockGuestSCSIUnmounter{} +} + +// --- LCOW-specific tests --- + +func TestDetachFromVM_FromAttached_WithGuest(t *testing.T) { + d := attachedDisk(t) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, &mockGuestSCSIEjector{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != StateDetached { + t.Errorf("expected state %d, got %d", StateDetached, d.State()) + } +} + +func TestDetachFromVM_EjectError(t *testing.T) { + d := attachedDisk(t) + ejectErr := errors.New("eject failed") + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, &mockGuestSCSIEjector{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() != StateAttached { + t.Errorf("expected state %d, got %d", StateAttached, d.State()) + } +} + +func TestMountPartitionToGuest_LCOW_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + guestPath, err := d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMountPartitionToGuest_LCOW_MountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{err: errors.New("mount fail")}) + if err != nil { + // This is expected - the mount error propagates. + return + } +} + +func TestUnmountPartitionFromGuest_LCOW_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmountPartitionFromGuest_LCOW_UnmountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + unmountErr := errors.New("unmount fail") + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{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_LCOW_RemovesMountWhenFullyUnmounted(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + 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{}, newDefaultEjector()) + if err != nil { + t.Fatalf("DetachFromVM after unmount: %v", err) + } + if d.State() != StateDetached { + t.Errorf("expected state %d, got %d", StateDetached, d.State()) + } +} + +func TestUnmountPartitionFromGuest_LCOW_RetryAfterDetachFailure(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + // Unmount succeeds and removes the partition from the mounts map. + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("UnmountPartitionFromGuest: %v", err) + } + // Detach fails (e.g. transient error). + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{err: errors.New("transient")}, newDefaultEjector()) + if err == nil { + t.Fatal("expected detach error") + } + // Retry: unmount is a no-op since partition is gone. + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("retry UnmountPartitionFromGuest should be no-op, got: %v", err) + } + // Retry detach now succeeds. + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) + if err != nil { + t.Fatalf("retry DetachFromVM: %v", err) + } + if d.State() != StateDetached { + t.Errorf("expected state %d, got %d", StateDetached, d.State()) + } +} diff --git a/internal/controller/device/scsi/disk/disk_test.go b/internal/controller/device/scsi/disk/disk_test.go index 0d74c7f339..34aca1efcb 100644 --- a/internal/controller/device/scsi/disk/disk_test.go +++ b/internal/controller/device/scsi/disk/disk_test.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package disk @@ -9,7 +9,6 @@ import ( "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 --- @@ -30,50 +29,6 @@ func (m *mockVMSCSIRemover) RemoveSCSIDisk(_ context.Context, _ uint, _ uint) er 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() Config { @@ -202,20 +157,9 @@ func TestAttachToVM_ErrorWhenDetached(t *testing.T) { } } -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() != StateDetached { - t.Errorf("expected state %d, got %d", StateDetached, d.State()) - } -} - -func TestDetachFromVM_FromAttached_WithLinuxGuest(t *testing.T) { +func TestDetachFromVM_FromAttached_NoMounts(t *testing.T) { d := attachedDisk(t) - err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, &mockLinuxGuestSCSIEjector{}) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -224,27 +168,10 @@ func TestDetachFromVM_FromAttached_WithLinuxGuest(t *testing.T) { } } -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() != StateAttached { - t.Errorf("expected state %d, got %d", StateAttached, 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) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{err: removeErr}, newDefaultEjector()) if err == nil { t.Fatal("expected error, got nil") } @@ -264,7 +191,7 @@ func TestDetachFromVM_SkipsWhenMountsExist(t *testing.T) { if err != nil { t.Fatalf("ReservePartition: %v", err) } - err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -276,7 +203,7 @@ func TestDetachFromVM_SkipsWhenMountsExist(t *testing.T) { func TestDetachFromVM_Idempotent_WhenReserved(t *testing.T) { d := NewReserved(0, 0, defaultConfig()) - err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) if err != nil { t.Fatalf("unexpected error detaching reserved disk: %v", err) } @@ -284,9 +211,9 @@ func TestDetachFromVM_Idempotent_WhenReserved(t *testing.T) { func TestDetachFromVM_Idempotent_WhenDetached(t *testing.T) { d := attachedDisk(t) - _ = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + _ = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) // Second detach should be idempotent. - err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, nil) + err := d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) if err != nil { t.Fatalf("unexpected error on idempotent detach: %v", err) } @@ -358,25 +285,9 @@ func TestReservePartition_ErrorWhenDetached(t *testing.T) { } } -func TestMountPartitionToGuest_Success(t *testing.T) { - d := attachedDisk(t) - cfg := mount.Config{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) + _, err := d.MountPartitionToGuest(context.Background(), 1, newDefaultMounter()) if err == nil { t.Fatal("expected error when disk is not attached") } @@ -384,47 +295,16 @@ func TestMountPartitionToGuest_ErrorWhenNotAttached(t *testing.T) { func TestMountPartitionToGuest_PartitionNotFound(t *testing.T) { d := attachedDisk(t) - _, err := d.MountPartitionToGuest(context.Background(), 99, &mockLinuxGuestSCSIMounter{}, nil) + _, err := d.MountPartitionToGuest(context.Background(), 99, newDefaultMounter()) if err == nil { t.Fatal("expected error for unreserved partition") } } -func TestMountPartitionToGuest_MountError(t *testing.T) { - d := attachedDisk(t) - cfg := mount.Config{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.Config{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) + err := d.UnmountPartitionFromGuest(context.Background(), 1, newDefaultUnmounter()) if err != nil { t.Fatalf("expected nil error for missing partition on non-attached disk, got: %v", err) } @@ -433,203 +313,8 @@ func TestUnmountPartitionFromGuest_SucceedsWhenNotAttached(t *testing.T) { 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) + err := d.UnmountPartitionFromGuest(context.Background(), 99, newDefaultUnmounter()) 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.Config{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.Config{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() != StateDetached { - t.Errorf("expected state %d, got %d", StateDetached, 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.Config{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() != StateDetached { - t.Errorf("expected state %d, got %d", StateDetached, d.State()) - } -} - -// --- Windows (WCOW) guest tests --- - -func TestMountPartitionToGuest_WCOW_Success(t *testing.T) { - d := attachedDisk(t) - cfg := mount.Config{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.Config{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.Config{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.Config{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.Config{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.Config{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() != StateDetached { - t.Errorf("expected state %d, got %d", StateDetached, d.State()) - } -} diff --git a/internal/controller/device/scsi/disk/disk_wcow.go b/internal/controller/device/scsi/disk/disk_wcow.go index a53f0fd644..9fab6cc4d5 100644 --- a/internal/controller/device/scsi/disk/disk_wcow.go +++ b/internal/controller/device/scsi/disk/disk_wcow.go @@ -1,3 +1,15 @@ -//go:build windows +//go:build windows && wcow package disk + +import "context" + +// GuestSCSIEjector is a no-op for WCOW guests, as they do not need to be +// notified of SCSI device removals. +type GuestSCSIEjector interface{} + +// ejectFromGuest is a no-op for WCOW guests, as they do not require explicit +// guest-side SCSI device removal before the host removes the disk from the VM bus. +func (d *Disk) ejectFromGuest(_ context.Context, _ GuestSCSIEjector) error { + return nil +} diff --git a/internal/controller/device/scsi/disk/disk_wcow_test.go b/internal/controller/device/scsi/disk/disk_wcow_test.go new file mode 100644 index 0000000000..bae4d2d1d5 --- /dev/null +++ b/internal/controller/device/scsi/disk/disk_wcow_test.go @@ -0,0 +1,164 @@ +//go:build windows && wcow + +package disk + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/scsi/mount" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +// mockGuestSCSIEjector is a no-op for WCOW guests (the interface is empty). +type mockGuestSCSIEjector struct{} + +type mockGuestSCSIMounter struct { + err error + scratchFn func() +} + +func (m *mockGuestSCSIMounter) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +func (m *mockGuestSCSIMounter) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + if m.scratchFn != nil { + m.scratchFn() + } + return m.err +} + +type mockGuestSCSIUnmounter struct { + err error +} + +func (m *mockGuestSCSIUnmounter) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers used by shared tests --- + +func newDefaultEjector() GuestSCSIEjector { + return &mockGuestSCSIEjector{} +} + +func newDefaultMounter() mount.GuestSCSIMounter { + return &mockGuestSCSIMounter{} +} + +func newDefaultUnmounter() mount.GuestSCSIUnmounter { + return &mockGuestSCSIUnmounter{} +} + +// --- WCOW-specific tests --- + +func TestMountPartitionToGuest_WCOW_Success(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + guestPath, err := d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + 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.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{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.Config{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, &mockGuestSCSIMounter{}) + 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.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmountPartitionFromGuest_WCOW_UnmountError(t *testing.T) { + d := attachedDisk(t) + cfg := mount.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + unmountErr := errors.New("wcow unmount fail") + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{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.Config{Partition: 1} + _, err := d.ReservePartition(context.Background(), cfg) + if err != nil { + t.Fatalf("ReservePartition: %v", err) + } + _, err = d.MountPartitionToGuest(context.Background(), 1, &mockGuestSCSIMounter{}) + if err != nil { + t.Fatalf("MountPartitionToGuest: %v", err) + } + err = d.UnmountPartitionFromGuest(context.Background(), 1, &mockGuestSCSIUnmounter{}) + if err != nil { + t.Fatalf("UnmountPartitionFromGuest: %v", err) + } + err = d.DetachFromVM(context.Background(), &mockVMSCSIRemover{}, newDefaultEjector()) + if err != nil { + t.Fatalf("DetachFromVM after unmount: %v", err) + } + if d.State() != StateDetached { + t.Errorf("expected state %d, got %d", StateDetached, d.State()) + } +} diff --git a/internal/controller/device/scsi/disk/doc.go b/internal/controller/device/scsi/disk/doc.go index 07b1fda3c6..d36e58a5b2 100644 --- a/internal/controller/device/scsi/disk/doc.go +++ b/internal/controller/device/scsi/disk/doc.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) // Package disk manages the lifecycle of a single SCSI disk attachment to a // Hyper-V VM, from host-side slot allocation through guest-side partition diff --git a/internal/controller/device/scsi/disk/state.go b/internal/controller/device/scsi/disk/state.go index bcbc2d530f..e2a012b1bc 100644 --- a/internal/controller/device/scsi/disk/state.go +++ b/internal/controller/device/scsi/disk/state.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package disk diff --git a/internal/controller/device/scsi/disk/types.go b/internal/controller/device/scsi/disk/types.go index 1976e71225..8c06ab4725 100644 --- a/internal/controller/device/scsi/disk/types.go +++ b/internal/controller/device/scsi/disk/types.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package disk @@ -6,7 +6,6 @@ import ( "context" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" - "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) // Type identifies the attachment protocol used when adding a disk to the VM's SCSI bus. @@ -55,11 +54,3 @@ type VMSCSIRemover interface { // RemoveSCSIDisk removes a SCSI disk from the Utility VM. RemoveSCSIDisk(ctx context.Context, controller uint, lun uint) error } - -// LinuxGuestSCSIEjector issues a guest-side SCSI device removal for LCOW guests. -// Pass nil to [Disk.DetachFromVM] to skip this step for WCOW guests, which handle -// SCSI hot-unplug automatically. -type LinuxGuestSCSIEjector interface { - // RemoveSCSIDevice removes a SCSI device from the LCOW guest. - RemoveSCSIDevice(ctx context.Context, settings guestresource.SCSIDevice) error -} diff --git a/internal/controller/device/scsi/doc.go b/internal/controller/device/scsi/doc.go index 9469ec5290..6756f0176f 100644 --- a/internal/controller/device/scsi/doc.go +++ b/internal/controller/device/scsi/doc.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) // Package scsi manages the full lifecycle of SCSI disk mappings on a // Hyper-V VM, from host-side slot allocation through guest-side mounting. diff --git a/internal/controller/device/scsi/mount/doc.go b/internal/controller/device/scsi/mount/doc.go index 83cc023af6..31fbed0f7b 100644 --- a/internal/controller/device/scsi/mount/doc.go +++ b/internal/controller/device/scsi/mount/doc.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) // Package mount manages the lifecycle of a single partition mount inside a // Hyper-V guest VM, from the initial reservation through mounting and diff --git a/internal/controller/device/scsi/mount/mount.go b/internal/controller/device/scsi/mount/mount.go index f2ab102c30..81b9f558ca 100644 --- a/internal/controller/device/scsi/mount/mount.go +++ b/internal/controller/device/scsi/mount/mount.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package mount @@ -76,7 +76,7 @@ func (m *Mount) Reserve(config Config) error { // MountToGuest issues the guest-side mount operation and returns the guest // path. -func (m *Mount) MountToGuest(ctx context.Context, linuxGuest LinuxGuestSCSIMounter, windowsGuest WindowsGuestSCSIMounter) (string, error) { +func (m *Mount) MountToGuest(ctx context.Context, guest GuestSCSIMounter) (string, error) { ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{ logfields.Controller: m.controller, logfields.LUN: m.lun, @@ -90,23 +90,13 @@ func (m *Mount) MountToGuest(ctx context.Context, linuxGuest LinuxGuestSCSIMount // Issue the platform-specific guest mount. On failure, advance to // StateUnmounted since no guest state was established from Reserved. - 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 = StateUnmounted - 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 = StateUnmounted - return "", err - } - } else { - return "", fmt.Errorf("both linuxGuest and windowsGuest cannot be nil") + if err := m.mountReserved(ctx, guest); err != nil { + // Move to unmounted since we know from reserved there was no + // guest state. + m.state = StateUnmounted + return "", err } + m.state = StateMounted // Note we don't increment the ref count here as the caller of // MountToGuest is responsible for calling it once per reservation, so @@ -128,7 +118,7 @@ func (m *Mount) MountToGuest(ctx context.Context, linuxGuest LinuxGuestSCSIMount // UnmountFromGuest decrements the reference count and, when it reaches zero, // issues the guest-side unmount. -func (m *Mount) UnmountFromGuest(ctx context.Context, linuxGuest LinuxGuestSCSIUnmounter, windowsGuest WindowsGuestSCSIUnmounter) error { +func (m *Mount) UnmountFromGuest(ctx context.Context, guest GuestSCSIUnmounter) error { ctx, _ = log.WithContext(ctx, logrus.WithFields(logrus.Fields{ logfields.Controller: m.controller, logfields.LUN: m.lun, @@ -147,17 +137,10 @@ func (m *Mount) UnmountFromGuest(ctx context.Context, linuxGuest LinuxGuestSCSIU log.G(ctx).Debug("unmounting partition from guest") // Last reference — issue the physical guest unmount. - 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") + if err := m.unmount(ctx, guest); err != nil { + return err } + m.state = StateUnmounted log.G(ctx).Debug("partition unmounted from guest") } diff --git a/internal/controller/device/scsi/mount/mount_lcow.go b/internal/controller/device/scsi/mount/mount_lcow.go index 2e8e06b51d..48130444f5 100644 --- a/internal/controller/device/scsi/mount/mount_lcow.go +++ b/internal/controller/device/scsi/mount/mount_lcow.go @@ -1,33 +1,87 @@ -//go:build windows +//go:build windows && lcow package mount import ( "context" "fmt" + "slices" + "strings" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// mountFmtLCOW is the guest path template for SCSI partition mounts on LCOW. +// mountFmt is the guest path template for SCSI partition mounts on LCOW. // The path encodes the controller index, LUN, and partition index so that each // combination gets a unique, stable mount point. Example: // // /run/mounts/scsi/__ const ( - mountFmtLCOW = "/run/mounts/scsi/%d_%d_%d" + mountFmt = "/run/mounts/scsi/%d_%d_%d" ) -// mountReservedLCOW issues the LCOW guest mount for a partition in the +// GuestSCSIMounter mounts a virtual disk partition inside an LCOW guest. +type GuestSCSIMounter interface { + // AddLCOWMappedVirtualDisk maps a virtual disk partition into the LCOW guest. + AddLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error +} + +// GuestSCSIUnmounter unmounts a virtual disk partition from an LCOW guest. +type GuestSCSIUnmounter interface { + // RemoveLCOWMappedVirtualDisk unmaps a virtual disk partition from the LCOW guest. + RemoveLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error +} + +// Config describes how a partition of a SCSI disk should be mounted inside +// an LCOW guest. +type Config struct { + // Partition is the target partition index (1-based) on a partitioned + // device. Zero means the whole disk. + Partition uint64 + // ReadOnly mounts the partition read-only. + ReadOnly bool + // Encrypted encrypts the device with dm-crypt. + Encrypted bool + // Options are mount flags or data passed to the guest mount call. + Options []string + // EnsureFilesystem formats the partition as Filesystem if not already + // formatted. + EnsureFilesystem bool + // Filesystem is the target filesystem type. + Filesystem string + // BlockDev mounts the device as a block device instead of a filesystem. + BlockDev bool +} + +// Equals reports whether two mount Config values describe the same mount parameters. +// Options are compared in a case-insensitive and order-insensitive manner. +func (c Config) Equals(other Config) bool { + cmpFoldCase := func(a, b string) int { + return strings.Compare(strings.ToLower(a), strings.ToLower(b)) + } + + return c.ReadOnly == other.ReadOnly && + c.Encrypted == other.Encrypted && + c.EnsureFilesystem == other.EnsureFilesystem && + c.Filesystem == other.Filesystem && + c.BlockDev == other.BlockDev && + slices.EqualFunc( + slices.SortedFunc(slices.Values(c.Options), cmpFoldCase), + slices.SortedFunc(slices.Values(other.Options), cmpFoldCase), + strings.EqualFold, + ) +} + +// mountReserved issues the LCOW guest mount for a partition in the // [StateReserved] state. It does not transition the state; that is handled // by the caller in [Mount.MountToGuest]. -func (m *Mount) mountReservedLCOW(ctx context.Context, guest LinuxGuestSCSIMounter) error { +func (m *Mount) mountReserved(ctx context.Context, guest GuestSCSIMounter) error { if m.state != StateReserved { return fmt.Errorf("unexpected mount state %s, expected reserved", m.state) } // Build the stable guest path from the controller, LUN, and partition index. - guestPath := fmt.Sprintf(mountFmtLCOW, m.controller, m.lun, m.config.Partition) + guestPath := fmt.Sprintf(mountFmt, m.controller, m.lun, m.config.Partition) settings := guestresource.LCOWMappedVirtualDisk{ MountPath: guestPath, Controller: uint8(m.controller), @@ -48,16 +102,16 @@ func (m *Mount) mountReservedLCOW(ctx context.Context, guest LinuxGuestSCSIMount return nil } -// unmountLCOW issues the LCOW guest unmount for a partition in the +// unmount issues the LCOW guest unmount for a partition in the // [StateMounted] state. It does not transition the state; that is handled // by the caller in [Mount.UnmountFromGuest]. -func (m *Mount) unmountLCOW(ctx context.Context, guest LinuxGuestSCSIUnmounter) error { +func (m *Mount) unmount(ctx context.Context, guest GuestSCSIUnmounter) error { if m.state != StateMounted { return fmt.Errorf("unexpected mount state %s, expected mounted", m.state) } // Generate the predictable guest path. - guestPath := fmt.Sprintf(mountFmtLCOW, m.controller, m.lun, m.config.Partition) + guestPath := fmt.Sprintf(mountFmt, m.controller, m.lun, m.config.Partition) settings := guestresource.LCOWMappedVirtualDisk{ MountPath: guestPath, Controller: uint8(m.controller), diff --git a/internal/controller/device/scsi/mount/mount_lcow_test.go b/internal/controller/device/scsi/mount/mount_lcow_test.go new file mode 100644 index 0000000000..2477ea3492 --- /dev/null +++ b/internal/controller/device/scsi/mount/mount_lcow_test.go @@ -0,0 +1,168 @@ +//go:build windows && lcow + +package mount + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockMounter struct { + err error +} + +func (m *mockMounter) AddLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +type mockUnmounter struct { + err error +} + +func (m *mockUnmounter) RemoveLCOWMappedVirtualDisk(_ context.Context, _ guestresource.LCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers used by shared tests --- + +func newDefaultMounter() GuestSCSIMounter { + return &mockMounter{} +} + +func newDefaultUnmounter() GuestSCSIUnmounter { + return &mockUnmounter{} +} + +func newFailingMounter(err error) GuestSCSIMounter { + return &mockMounter{err: err} +} + +func mountedMount(t *testing.T) *Mount { + t.Helper() + m := NewReserved(0, 0, defaultConfig()) + if _, err := m.MountToGuest(context.Background(), newDefaultMounter()); err != nil { + t.Fatalf("setup MountToGuest: %v", err) + } + return m +} + +// --- LCOW-specific tests --- + +func TestConfigEquals_LCOW(t *testing.T) { + base := Config{ + ReadOnly: true, + Encrypted: true, + EnsureFilesystem: true, + Filesystem: "ext4", + BlockDev: false, + Options: []string{"rw", "noatime"}, + } + same := Config{ + ReadOnly: true, + Encrypted: true, + EnsureFilesystem: true, + Filesystem: "ext4", + BlockDev: false, + Options: []string{"rw", "noatime"}, + } + if !base.Equals(same) { + t.Error("expected equal configs to be equal") + } + + // Options comparison should be order-insensitive. + reordered := same + reordered.Options = []string{"noatime", "rw"} + if !base.Equals(reordered) { + t.Error("expected configs with reordered options to be equal") + } + + // Options comparison should be case-insensitive. + uppercased := same + uppercased.Options = []string{"RW", "Noatime"} + if !base.Equals(uppercased) { + t.Error("expected configs with differently-cased options to be equal") + } + + tests := []struct { + name string + modify func(c *Config) + }{ + {"ReadOnly", func(c *Config) { c.ReadOnly = false }}, + {"Encrypted", func(c *Config) { c.Encrypted = false }}, + {"EnsureFilesystem", func(c *Config) { c.EnsureFilesystem = false }}, + {"Filesystem", func(c *Config) { c.Filesystem = "xfs" }}, + {"BlockDev", func(c *Config) { c.BlockDev = true }}, + {"Options", func(c *Config) { c.Options = []string{"ro"} }}, + {"OptionsNil", func(c *Config) { 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 TestMountToGuest_LCOW_Success(t *testing.T) { + m := NewReserved(0, 0, defaultConfig()) + guestPath, err := m.MountToGuest(context.Background(), &mockMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } + if m.State() != StateMounted { + t.Errorf("expected state %d, got %d", StateMounted, m.State()) + } +} + +func TestMountToGuest_LCOW_Error(t *testing.T) { + mountErr := errors.New("lcow mount failed") + m := NewReserved(0, 0, defaultConfig()) + _, err := m.MountToGuest(context.Background(), &mockMounter{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() != StateUnmounted { + t.Errorf("expected state %d after failure, got %d", StateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_LCOW_Success(t *testing.T) { + m := mountedMount(t) + err := m.UnmountFromGuest(context.Background(), &mockUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.State() != StateUnmounted { + t.Errorf("expected state %d, got %d", StateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_LCOW_Error(t *testing.T) { + m := mountedMount(t) + unmountErr := errors.New("lcow unmount failed") + err := m.UnmountFromGuest(context.Background(), &mockUnmounter{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) + } + // State should remain mounted since unmount failed. + if m.State() != StateMounted { + t.Errorf("expected state %d, got %d", StateMounted, m.State()) + } +} diff --git a/internal/controller/device/scsi/mount/mount_test.go b/internal/controller/device/scsi/mount/mount_test.go index 44ea88dd0e..23859d6b52 100644 --- a/internal/controller/device/scsi/mount/mount_test.go +++ b/internal/controller/device/scsi/mount/mount_test.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package mount @@ -6,52 +6,8 @@ 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 defaultConfig() Config { @@ -61,32 +17,12 @@ func defaultConfig() Config { } } -func mountedLCOW(t *testing.T) *Mount { - t.Helper() - m := NewReserved(0, 0, defaultConfig()) - 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, defaultConfig()) - 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 := Config{ - Partition: 2, - ReadOnly: true, - Encrypted: true, - Filesystem: "ext4", + Partition: 2, + ReadOnly: true, } m := NewReserved(1, 3, cfg) if m.State() != StateReserved { @@ -94,53 +30,6 @@ func TestNewReserved(t *testing.T) { } } -func TestConfigEquals(t *testing.T) { - base := Config{ - ReadOnly: true, - Encrypted: true, - EnsureFilesystem: true, - Filesystem: "ext4", - BlockDev: false, - FormatWithRefs: false, - Options: []string{"rw", "noatime"}, - } - same := Config{ - 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 *Config) - }{ - {"ReadOnly", func(c *Config) { c.ReadOnly = false }}, - {"Encrypted", func(c *Config) { c.Encrypted = false }}, - {"EnsureFilesystem", func(c *Config) { c.EnsureFilesystem = false }}, - {"Filesystem", func(c *Config) { c.Filesystem = "xfs" }}, - {"BlockDev", func(c *Config) { c.BlockDev = true }}, - {"FormatWithRefs", func(c *Config) { c.FormatWithRefs = true }}, - {"Options", func(c *Config) { c.Options = []string{"ro"} }}, - {"OptionsNil", func(c *Config) { 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 := defaultConfig() m := NewReserved(0, 0, cfg) @@ -158,7 +47,7 @@ func TestReserve_DifferentConfig(t *testing.T) { } func TestReserve_WhenMounted(t *testing.T) { - m := mountedLCOW(t) + m := mountedMount(t) cfg := defaultConfig() if err := m.Reserve(cfg); err != nil { t.Fatalf("unexpected error reserving mounted mount: %v", err) @@ -166,8 +55,8 @@ func TestReserve_WhenMounted(t *testing.T) { } func TestReserve_WhenUnmounted(t *testing.T) { - m := mountedLCOW(t) - _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), newDefaultUnmounter()) if m.State() != StateUnmounted { t.Fatalf("expected unmounted, got %d", m.State()) } @@ -177,80 +66,9 @@ func TestReserve_WhenUnmounted(t *testing.T) { } } -func TestMountToGuest_LCOW_Success(t *testing.T) { - m := NewReserved(0, 0, defaultConfig()) - 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() != StateMounted { - t.Errorf("expected state %d, got %d", StateMounted, m.State()) - } -} - -func TestMountToGuest_WCOW_Success(t *testing.T) { - m := NewReserved(0, 0, defaultConfig()) - 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() != StateMounted { - t.Errorf("expected state %d, got %d", StateMounted, m.State()) - } -} - -func TestMountToGuest_LCOW_Error(t *testing.T) { - mountErr := errors.New("lcow mount failed") - m := NewReserved(0, 0, defaultConfig()) - _, 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() != StateUnmounted { - t.Errorf("expected state %d after failure, got %d", StateUnmounted, m.State()) - } -} - -func TestMountToGuest_WCOW_Error(t *testing.T) { - mountErr := errors.New("wcow mount failed") - m := NewReserved(0, 0, defaultConfig()) - _, 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() != StateUnmounted { - t.Errorf("expected state %d after failure, got %d", StateUnmounted, m.State()) - } -} - -func TestMountToGuest_WCOW_FormatWithRefs(t *testing.T) { - scratchCalled := false - m := NewReserved(0, 0, Config{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) + m := mountedMount(t) + guestPath, err := m.MountToGuest(context.Background(), newDefaultMounter()) if err != nil { t.Fatalf("unexpected error on idempotent mount: %v", err) } @@ -263,78 +81,32 @@ func TestMountToGuest_Idempotent_WhenMounted(t *testing.T) { } func TestMountToGuest_ErrorWhenUnmounted(t *testing.T) { - m := mountedLCOW(t) - _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) - _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil) + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), newDefaultUnmounter()) + _, err := m.MountToGuest(context.Background(), newDefaultMounter()) if err == nil { t.Fatal("expected error mounting an unmounted mount") } } -func TestMountToGuest_ErrorWhenBothGuestsNil(t *testing.T) { +func TestMountToGuest_Error(t *testing.T) { + mountErr := errors.New("mount failed") m := NewReserved(0, 0, defaultConfig()) - _, err := m.MountToGuest(context.Background(), nil, nil) - if err == nil { - t.Fatal("expected error when both guests are 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() != StateUnmounted { - t.Errorf("expected state %d, got %d", StateUnmounted, 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() != StateUnmounted { - t.Errorf("expected state %d, got %d", StateUnmounted, 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) + _, err := m.MountToGuest(context.Background(), newFailingMounter(mountErr)) 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() != StateMounted { - t.Errorf("expected state %d, got %d", StateMounted, 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 !errors.Is(err, mountErr) { + t.Errorf("expected wrapped error %v, got %v", mountErr, err) } - if m.State() != StateMounted { - t.Errorf("expected state %d, got %d", StateMounted, m.State()) + if m.State() != StateUnmounted { + t.Errorf("expected state %d after failure, got %d", StateUnmounted, m.State()) } } func TestUnmountFromGuest_FromReserved_DecrementsRefCount(t *testing.T) { m := NewReserved(0, 0, defaultConfig()) - err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + err := m.UnmountFromGuest(context.Background(), newDefaultUnmounter()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -345,22 +117,14 @@ func TestUnmountFromGuest_FromReserved_DecrementsRefCount(t *testing.T) { } func TestUnmountFromGuest_ErrorWhenUnmounted(t *testing.T) { - m := mountedLCOW(t) - _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) - err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil) + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), newDefaultUnmounter()) + err := m.UnmountFromGuest(context.Background(), newDefaultUnmounter()) 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 := defaultConfig() m := NewReserved(0, 0, cfg) @@ -369,18 +133,18 @@ func TestUnmountFromGuest_MultipleRefs_DoesNotUnmountUntilLastRef(t *testing.T) t.Fatalf("Reserve: %v", err) } // Mount the guest. - if _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}, nil); err != nil { + if _, err := m.MountToGuest(context.Background(), newDefaultMounter()); 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 { + if err := m.UnmountFromGuest(context.Background(), newDefaultUnmounter()); err != nil { t.Fatalf("first UnmountFromGuest: %v", err) } if m.State() != StateMounted { t.Errorf("expected state %d after first unmount, got %d", StateMounted, m.State()) } // Second unmount should actually unmount. - if err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}, nil); err != nil { + if err := m.UnmountFromGuest(context.Background(), newDefaultUnmounter()); err != nil { t.Fatalf("second UnmountFromGuest: %v", err) } if m.State() != StateUnmounted { diff --git a/internal/controller/device/scsi/mount/mount_wcow.go b/internal/controller/device/scsi/mount/mount_wcow.go index f3cb505ae1..ee0ba70ecc 100644 --- a/internal/controller/device/scsi/mount/mount_wcow.go +++ b/internal/controller/device/scsi/mount/mount_wcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package mount @@ -9,25 +9,58 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// mountFmtWCOW is the guest path template for SCSI partition mounts on WCOW. +// mountFmt is the guest path template for SCSI partition mounts on WCOW. // The path encodes the controller index, LUN, and partition index so that each // combination gets a unique, stable mount point. Example: // // C:/mounts/scsi/__ const ( - mountFmtWCOW = "C:\\mounts\\scsi\\%d_%d_%d" + mountFmt = "C:\\mounts\\scsi\\%d_%d_%d" ) -// mountReservedWCOW issues the WCOW guest mount for a partition in the +// GuestSCSIMounter mounts a virtual disk partition inside an WCOW guest. +type GuestSCSIMounter interface { + // AddWCOWMappedVirtualDisk maps a virtual disk into the WCOW guest. + AddWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error + // AddWCOWMappedVirtualDiskForContainerScratch maps a virtual disk as a + // container scratch disk into the WCOW guest. + AddWCOWMappedVirtualDiskForContainerScratch(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error +} + +// GuestSCSIUnmounter unmounts a virtual disk partition from an LCOW guest. +type GuestSCSIUnmounter interface { + // RemoveWCOWMappedVirtualDisk unmaps a virtual disk from the WCOW guest. + RemoveWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error +} + +// Config describes how a partition of a SCSI disk should be mounted inside +// a WCOW guest. +type Config struct { + // Partition is the target partition index (1-based) on a partitioned + // device. Zero means the whole disk. + Partition uint64 + // ReadOnly mounts the partition read-only. + ReadOnly bool + // FormatWithRefs formats the disk using ReFS. + FormatWithRefs bool +} + +// Equals reports whether two mount Config values describe the same mount parameters. +func (c Config) Equals(other Config) bool { + return c.ReadOnly == other.ReadOnly && + c.FormatWithRefs == other.FormatWithRefs +} + +// mountReserved issues the WCOW guest mount for a partition in the // [StateReserved] state. It does not transition the state which is handled // by the caller in [Mount.MountToGuest]. -func (m *Mount) mountReservedWCOW(ctx context.Context, guest WindowsGuestSCSIMounter) error { +func (m *Mount) mountReserved(ctx context.Context, guest GuestSCSIMounter) error { if m.state != StateReserved { return fmt.Errorf("unexpected mount state %s, expected reserved", m.state) } // Generate a predictable guest path. - guestPath := fmt.Sprintf(mountFmtWCOW, m.controller, m.lun, m.config.Partition) + guestPath := fmt.Sprintf(mountFmt, m.controller, m.lun, m.config.Partition) settings := guestresource.WCOWMappedVirtualDisk{ ContainerPath: guestPath, Lun: int32(m.lun), @@ -47,16 +80,16 @@ func (m *Mount) mountReservedWCOW(ctx context.Context, guest WindowsGuestSCSIMou return nil } -// unmountWCOW issues the WCOW guest unmount for a partition in the +// unmount issues the WCOW guest unmount for a partition in the // [StateMounted] state. It does not transition the state; that is handled // by the caller in [Mount.UnmountFromGuest]. -func (m *Mount) unmountWCOW(ctx context.Context, guest WindowsGuestSCSIUnmounter) error { +func (m *Mount) unmount(ctx context.Context, guest GuestSCSIUnmounter) error { if m.state != StateMounted { return fmt.Errorf("unexpected mount state %s, expected mounted", m.state) } // Build the predictable guest path. - guestPath := fmt.Sprintf(mountFmtWCOW, m.controller, m.lun, m.config.Partition) + guestPath := fmt.Sprintf(mountFmt, m.controller, m.lun, m.config.Partition) settings := guestresource.WCOWMappedVirtualDisk{ ContainerPath: guestPath, Lun: int32(m.lun), diff --git a/internal/controller/device/scsi/mount/mount_wcow_test.go b/internal/controller/device/scsi/mount/mount_wcow_test.go new file mode 100644 index 0000000000..31a691d7a4 --- /dev/null +++ b/internal/controller/device/scsi/mount/mount_wcow_test.go @@ -0,0 +1,161 @@ +//go:build windows && wcow + +package mount + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockMounter struct { + err error + scratchFn func() +} + +func (m *mockMounter) AddWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +func (m *mockMounter) AddWCOWMappedVirtualDiskForContainerScratch(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + if m.scratchFn != nil { + m.scratchFn() + } + return m.err +} + +type mockUnmounter struct { + err error +} + +func (m *mockUnmounter) RemoveWCOWMappedVirtualDisk(_ context.Context, _ guestresource.WCOWMappedVirtualDisk) error { + return m.err +} + +// --- Helpers used by shared tests --- + +func newDefaultMounter() GuestSCSIMounter { + return &mockMounter{} +} + +func newDefaultUnmounter() GuestSCSIUnmounter { + return &mockUnmounter{} +} + +func newFailingMounter(err error) GuestSCSIMounter { + return &mockMounter{err: err} +} + +func mountedMount(t *testing.T) *Mount { + t.Helper() + m := NewReserved(0, 0, defaultConfig()) + if _, err := m.MountToGuest(context.Background(), newDefaultMounter()); err != nil { + t.Fatalf("setup MountToGuest: %v", err) + } + return m +} + +// --- WCOW-specific tests --- + +func TestConfigEquals_WCOW(t *testing.T) { + base := Config{ + ReadOnly: true, + FormatWithRefs: false, + } + same := Config{ + ReadOnly: true, + FormatWithRefs: false, + } + if !base.Equals(same) { + t.Error("expected equal configs to be equal") + } + + tests := []struct { + name string + modify func(c *Config) + }{ + {"ReadOnly", func(c *Config) { c.ReadOnly = false }}, + {"FormatWithRefs", func(c *Config) { c.FormatWithRefs = true }}, + } + 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 TestMountToGuest_WCOW_Success(t *testing.T) { + m := NewReserved(0, 0, defaultConfig()) + guestPath, err := m.MountToGuest(context.Background(), &mockMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } + if m.State() != StateMounted { + t.Errorf("expected state %d, got %d", StateMounted, m.State()) + } +} + +func TestMountToGuest_WCOW_Error(t *testing.T) { + mountErr := errors.New("wcow mount failed") + m := NewReserved(0, 0, defaultConfig()) + _, err := m.MountToGuest(context.Background(), &mockMounter{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() != StateUnmounted { + t.Errorf("expected state %d after failure, got %d", StateUnmounted, m.State()) + } +} + +func TestMountToGuest_WCOW_FormatWithRefs(t *testing.T) { + scratchCalled := false + m := NewReserved(0, 0, Config{Partition: 1, FormatWithRefs: true}) + wm := &mockMounter{scratchFn: func() { scratchCalled = true }} + _, err := m.MountToGuest(context.Background(), wm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !scratchCalled { + t.Error("expected AddWCOWMappedVirtualDiskForContainerScratch to be called") + } +} + +func TestUnmountFromGuest_WCOW_Success(t *testing.T) { + m := mountedMount(t) + err := m.UnmountFromGuest(context.Background(), &mockUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.State() != StateUnmounted { + t.Errorf("expected state %d, got %d", StateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_WCOW_Error(t *testing.T) { + m := mountedMount(t) + unmountErr := errors.New("wcow unmount failed") + err := m.UnmountFromGuest(context.Background(), &mockUnmounter{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() != StateMounted { + t.Errorf("expected state %d, got %d", StateMounted, m.State()) + } +} diff --git a/internal/controller/device/scsi/mount/state.go b/internal/controller/device/scsi/mount/state.go index 64ed80ef02..f9b1869f60 100644 --- a/internal/controller/device/scsi/mount/state.go +++ b/internal/controller/device/scsi/mount/state.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package mount diff --git a/internal/controller/device/scsi/mount/types.go b/internal/controller/device/scsi/mount/types.go deleted file mode 100644 index 388d68435e..0000000000 --- a/internal/controller/device/scsi/mount/types.go +++ /dev/null @@ -1,86 +0,0 @@ -//go:build windows - -package mount - -import ( - "context" - "slices" - - "github.com/Microsoft/hcsshim/internal/protocol/guestresource" -) - -// Config describes how a partition of a SCSI disk should be mounted inside -// the guest. -type Config 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 partition 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 partition 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 instead of a filesystem. - // - // Only supported for LCOW. - BlockDev bool - // FormatWithRefs formats the disk using ReFS. - // - // Only supported for WCOW scratch disks. - FormatWithRefs bool -} - -// Equals reports whether two mount Config values describe the same mount parameters. -func (c Config) Equals(other Config) bool { - return c.ReadOnly == other.ReadOnly && - c.Encrypted == other.Encrypted && - c.EnsureFilesystem == other.EnsureFilesystem && - c.Filesystem == other.Filesystem && - c.BlockDev == other.BlockDev && - c.FormatWithRefs == other.FormatWithRefs && - slices.Equal(c.Options, other.Options) -} - -// LinuxGuestSCSIMounter mounts a virtual disk partition inside an LCOW guest. -type LinuxGuestSCSIMounter interface { - // AddLCOWMappedVirtualDisk maps a virtual disk partition into the LCOW guest. - AddLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error -} - -// LinuxGuestSCSIUnmounter unmounts a virtual disk partition from an LCOW guest. -type LinuxGuestSCSIUnmounter interface { - // RemoveLCOWMappedVirtualDisk unmaps a virtual disk partition from the LCOW guest. - RemoveLCOWMappedVirtualDisk(ctx context.Context, settings guestresource.LCOWMappedVirtualDisk) error -} - -// WindowsGuestSCSIMounter mounts a virtual disk partition inside a WCOW guest. -type WindowsGuestSCSIMounter interface { - // AddWCOWMappedVirtualDisk maps a virtual disk into the WCOW guest. - AddWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error - // AddWCOWMappedVirtualDiskForContainerScratch maps a virtual disk as a - // container scratch disk into the WCOW guest. - AddWCOWMappedVirtualDiskForContainerScratch(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error -} - -// WindowsGuestSCSIUnmounter unmounts a virtual disk partition from a WCOW guest. -type WindowsGuestSCSIUnmounter interface { - // RemoveWCOWMappedVirtualDisk unmaps a virtual disk from the WCOW guest. - RemoveWCOWMappedVirtualDisk(ctx context.Context, settings guestresource.WCOWMappedVirtualDisk) error -} diff --git a/internal/controller/device/scsi/types.go b/internal/controller/device/scsi/types.go index fc94fefb06..6320fa2988 100644 --- a/internal/controller/device/scsi/types.go +++ b/internal/controller/device/scsi/types.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && (lcow || wcow) package scsi @@ -25,15 +25,9 @@ type VMSCSIOps interface { disk.VMSCSIRemover } -// LinuxGuestSCSIOps combines all guest-side SCSI operations for LCOW guests. -type LinuxGuestSCSIOps interface { - mount.LinuxGuestSCSIMounter - mount.LinuxGuestSCSIUnmounter - disk.LinuxGuestSCSIEjector -} - -// WindowsGuestSCSIOps combines all guest-side SCSI operations for WCOW guests. -type WindowsGuestSCSIOps interface { - mount.WindowsGuestSCSIMounter - mount.WindowsGuestSCSIUnmounter +// GuestSCSIOps combines all guest-side SCSI operations. +type GuestSCSIOps interface { + mount.GuestSCSIMounter + mount.GuestSCSIUnmounter + disk.GuestSCSIEjector } diff --git a/internal/vm/guestmanager/block_cims.go b/internal/vm/guestmanager/block_cims.go index 022829df9c..c84cd0b094 100644 --- a/internal/vm/guestmanager/block_cims.go +++ b/internal/vm/guestmanager/block_cims.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package guestmanager diff --git a/internal/vm/guestmanager/combinedlayers_lcow.go b/internal/vm/guestmanager/combinedlayers_lcow.go index 2a5b56e5a9..f62f8a64cd 100644 --- a/internal/vm/guestmanager/combinedlayers_lcow.go +++ b/internal/vm/guestmanager/combinedlayers_lcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && lcow package guestmanager diff --git a/internal/vm/guestmanager/combinedlayers_wcow.go b/internal/vm/guestmanager/combinedlayers_wcow.go index 77b63d18f6..4397f96fc1 100644 --- a/internal/vm/guestmanager/combinedlayers_wcow.go +++ b/internal/vm/guestmanager/combinedlayers_wcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package guestmanager diff --git a/internal/vm/guestmanager/device_lcow.go b/internal/vm/guestmanager/device_lcow.go index 61524a0ccc..a5650be72b 100644 --- a/internal/vm/guestmanager/device_lcow.go +++ b/internal/vm/guestmanager/device_lcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && lcow package guestmanager diff --git a/internal/vm/guestmanager/doc.go b/internal/vm/guestmanager/doc.go index 31820fdffc..efe0df91b2 100644 --- a/internal/vm/guestmanager/doc.go +++ b/internal/vm/guestmanager/doc.go @@ -4,26 +4,6 @@ Package guestmanager manages guest-side operations for utility VMs (UVMs) via the GCS (Guest Compute Service) connection. -It provides a concrete [Guest] struct, a top-level [Manager] interface that -aggregates connection lifecycle and container/process operations, and a set of -granular resource-scoped manager interfaces: - - - [Manager] – connection lifecycle, container and process creation, stack dumps, - and container state deletion. - - [LCOWNetworkManager] – add and remove network interfaces in an LCOW guest. - - [WCOWNetworkManager] – add and remove network interfaces and namespaces in a WCOW guest. - - [LCOWDirectoryManager] – map and unmap directories in an LCOW guest. - - [WCOWDirectoryManager] – map directories in a WCOW guest. - - [LCOWScsiManager] – add and remove mapped virtual disks and SCSI devices in an LCOW guest. - - [WCOWScsiManager] – add and remove mapped virtual disks and SCSI devices in a WCOW guest. - - [LCOWLayersManager] – add and remove combined layers in an LCOW guest. - - [WCOWLayersManager] – add and remove combined layers in a WCOW guest. - - [CIMsManager] – add and remove WCOW block CIM mounts. - - [LCOWDeviceManager] – add and remove VPCI and VPMem devices in an LCOW guest. - - [SecurityPolicyManager] – add security policies and inject policy fragments. - -All interfaces are implemented by [Guest]. - This package is strictly guest-side. It does not own or modify host-side UVM state; that is the responsibility of the sibling vmmanager package. It also does not store UVM host or guest state — state management belongs to the orchestration diff --git a/internal/vm/guestmanager/mapped_directory_lcow.go b/internal/vm/guestmanager/mapped_directory_lcow.go index 6cd9409b47..8b6a1abe55 100644 --- a/internal/vm/guestmanager/mapped_directory_lcow.go +++ b/internal/vm/guestmanager/mapped_directory_lcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && lcow package guestmanager diff --git a/internal/vm/guestmanager/mapped_directory_wcow.go b/internal/vm/guestmanager/mapped_directory_wcow.go index 8c5ee22700..19ea2e0c4a 100644 --- a/internal/vm/guestmanager/mapped_directory_wcow.go +++ b/internal/vm/guestmanager/mapped_directory_wcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package guestmanager diff --git a/internal/vm/guestmanager/network_lcow.go b/internal/vm/guestmanager/network_lcow.go index a6296dcfa6..a993d3c431 100644 --- a/internal/vm/guestmanager/network_lcow.go +++ b/internal/vm/guestmanager/network_lcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && lcow package guestmanager diff --git a/internal/vm/guestmanager/network_wcow.go b/internal/vm/guestmanager/network_wcow.go index 77fdc4f629..26885372c3 100644 --- a/internal/vm/guestmanager/network_wcow.go +++ b/internal/vm/guestmanager/network_wcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package guestmanager diff --git a/internal/vm/guestmanager/scsi_lcow.go b/internal/vm/guestmanager/scsi_lcow.go index 528912275d..c62c01d013 100644 --- a/internal/vm/guestmanager/scsi_lcow.go +++ b/internal/vm/guestmanager/scsi_lcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && lcow package guestmanager diff --git a/internal/vm/guestmanager/scsi_wcow.go b/internal/vm/guestmanager/scsi_wcow.go index d8ea38f952..d2dbf58ca0 100644 --- a/internal/vm/guestmanager/scsi_wcow.go +++ b/internal/vm/guestmanager/scsi_wcow.go @@ -1,4 +1,4 @@ -//go:build windows +//go:build windows && wcow package guestmanager