From 687523f2c3e72c840ac69f6e77a92d10041a84c7 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Thu, 26 Mar 2026 22:22:00 -0700 Subject: [PATCH] Implements the VM VPMem Controller This change implements the VPMem controller operations for the new V2 shim. Signed-off-by: Justin Terry --- go.mod | 2 +- .../controller/device/vpmem/controller.go | 172 +++++++ .../device/vpmem/controller_test.go | 414 ++++++++++++++++ .../controller/device/vpmem/device/device.go | 166 +++++++ .../device/vpmem/device/device_test.go | 440 ++++++++++++++++++ .../controller/device/vpmem/mount/mount.go | 136 ++++++ .../device/vpmem/mount/mount_test.go | 217 +++++++++ internal/controller/device/vpmem/vpmem.go | 7 + internal/vm/guestmanager/device_lcow.go | 12 - internal/vm/vmmanager/vpmem.go | 11 - 10 files changed, 1553 insertions(+), 24 deletions(-) create mode 100644 internal/controller/device/vpmem/controller.go create mode 100644 internal/controller/device/vpmem/controller_test.go create mode 100644 internal/controller/device/vpmem/device/device.go create mode 100644 internal/controller/device/vpmem/device/device_test.go create mode 100644 internal/controller/device/vpmem/mount/mount.go create mode 100644 internal/controller/device/vpmem/mount/mount_test.go create mode 100644 internal/controller/device/vpmem/vpmem.go diff --git a/go.mod b/go.mod index c19e49c4e5..ce0de078ac 100644 --- a/go.mod +++ b/go.mod @@ -43,6 +43,7 @@ require ( github.com/containerd/typeurl/v2 v2.2.3 github.com/google/go-cmp v0.7.0 github.com/google/go-containerregistry v0.20.1 + github.com/google/uuid v1.6.0 github.com/linuxkit/virtsock v0.0.0-20241009230534-cb6a20cc0422 github.com/mattn/go-shellwords v1.0.12 github.com/moby/sys/user v0.4.0 @@ -98,7 +99,6 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/gorilla/mux v1.8.1 // indirect github.com/josephspurrier/goversioninfo v1.5.0 // indirect github.com/klauspost/compress v1.18.0 // indirect diff --git a/internal/controller/device/vpmem/controller.go b/internal/controller/device/vpmem/controller.go new file mode 100644 index 0000000000..e7b73e419c --- /dev/null +++ b/internal/controller/device/vpmem/controller.go @@ -0,0 +1,172 @@ +//go:build windows + +package vpmem + +import ( + "context" + "fmt" + "sync" + + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/device" + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/mount" + + "github.com/google/uuid" +) + +type VMVPMemOps interface { + device.VMVPMemAdder + device.VMVPMemRemover +} + +type LinuxGuestVPMemOps interface { + mount.LinuxGuestVPMemMounter + mount.LinuxGuestVPMemUnmounter +} + +// The controller manages all VPMem attached devices and guest mounted +// directories. +// +// It is required that all callers: +// +// 1. Obtain a reservation using Reserve(). +// +// 2. Use the reservation to Mount() to ensure resource availability. +// +// 3. Call Unmount() to release the reservation and all resources. +// +// If Mount() fails, the caller must call Unmount() to release the reservation +// and all resources. +// +// If Unmount() fails, the caller must call Unmount() again until it succeeds to +// release the reservation and all resources. +type Controller struct { + vm VMVPMemOps + guest LinuxGuestVPMemOps + + mu sync.Mutex + + // Every call to Reserve gets a unique reservation ID which holds the slot + // index for the device. + reservations map[uuid.UUID]*reservation + + // For fast lookup we keep a hostPath to slot mapping for all allocated + // devices. + devicesByPath map[string]uint32 + + // Tracks all allocated and unallocated available VPMem device slots. + slots []*device.Device +} + +func New(maxDevices uint32, vm VMVPMemOps, guest LinuxGuestVPMemOps) *Controller { + return &Controller{ + vm: vm, + guest: guest, + mu: sync.Mutex{}, + reservations: make(map[uuid.UUID]*reservation), + devicesByPath: make(map[string]uint32), + slots: make([]*device.Device, maxDevices), + } +} + +// Reserve creates a referenced counted mapping entry for a VPMem attachment +// based on the device host path. +// +// If an error is returned from this function, it is guaranteed that no +// reservation mapping was made and no Unmount() call is necessary to clean up. +func (c *Controller) Reserve(ctx context.Context, deviceConfig device.DeviceConfig, mountConfig mount.MountConfig) (uuid.UUID, error) { + c.mu.Lock() + defer c.mu.Unlock() + + // Generate a new reservation id. + id := uuid.New() + if _, ok := c.reservations[id]; ok { + return uuid.Nil, fmt.Errorf("reservation ID collision") + } + r := &reservation{} + + // Determine if this hostPath already had a device known. + if slot, ok := c.devicesByPath[deviceConfig.HostPath]; ok { + r.slot = slot + d := c.slots[slot] + + // Verify the caller config is the same. + if !d.Config().Equals(deviceConfig) { + return uuid.Nil, fmt.Errorf("cannot reserve ref on device with different config") + } + + if _, err := d.ReserveMount(ctx, mountConfig); err != nil { + return uuid.Nil, fmt.Errorf("reserve mount: %w", err) + } + } else { + // No hostPath was found. Find a slot for the device. + nextSlot := uint32(0) + found := false + for i := uint32(0); i < uint32(len(c.slots)); i++ { + if c.slots[i] == nil { + nextSlot = i + found = true + break + } + } + if !found { + return uuid.Nil, fmt.Errorf("no available slots") + } + + // Create the Device and Mount in the reserved states. + d := device.NewReserved(nextSlot, deviceConfig) + if _, err := d.ReserveMount(ctx, mountConfig); err != nil { + return uuid.Nil, fmt.Errorf("reserve mount: %w", err) + } + c.slots[nextSlot] = d + c.devicesByPath[deviceConfig.HostPath] = nextSlot + r.slot = nextSlot + } + + // Ensure our reservation is saved for all future operations. + c.reservations[id] = r + return id, nil +} + +func (c *Controller) Mount(ctx context.Context, reservation uuid.UUID) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if r, ok := c.reservations[reservation]; ok { + d := c.slots[r.slot] + if err := d.AttachToVM(ctx, c.vm); err != nil { + return "", fmt.Errorf("attach device to vm: %w", err) + } + guestPath, err := d.MountToGuest(ctx, c.guest) + if err != nil { + return "", fmt.Errorf("mount to guest: %w", err) + } + return guestPath, nil + } + return "", fmt.Errorf("reservation %s not found", reservation) +} + +func (c *Controller) Unmount(ctx context.Context, reservation uuid.UUID) error { + c.mu.Lock() + defer c.mu.Unlock() + + if r, ok := c.reservations[reservation]; ok { + d := c.slots[r.slot] + if err := d.UnmountFromGuest(ctx, c.guest); err != nil { + return fmt.Errorf("unmount from guest: %w", err) + } + if err := d.DetachFromVM(ctx, c.vm); err != nil { + return fmt.Errorf("detach device from vm: %w", err) + } + if d.State() == device.DeviceStateDetached { + delete(c.devicesByPath, d.HostPath()) + c.slots[r.slot] = nil + } + delete(c.reservations, reservation) + return nil + } + return fmt.Errorf("reservation %s not found", reservation) +} + +type reservation struct { + slot uint32 +} diff --git a/internal/controller/device/vpmem/controller_test.go b/internal/controller/device/vpmem/controller_test.go new file mode 100644 index 0000000000..01808ea627 --- /dev/null +++ b/internal/controller/device/vpmem/controller_test.go @@ -0,0 +1,414 @@ +//go:build windows + +package vpmem + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/device" + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + "github.com/google/uuid" +) + +// --- Mock types --- + +type mockVMOps struct { + addErr error + removeErr error +} + +func (m *mockVMOps) AddVPMemDevice(_ context.Context, _ hcsschema.VirtualPMemDevice, _ uint32) error { + return m.addErr +} + +func (m *mockVMOps) RemoveVPMemDevice(_ context.Context, _ uint32) error { + return m.removeErr +} + +type mockGuestOps struct { + mountErr error + unmountErr error +} + +func (m *mockGuestOps) AddLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.mountErr +} + +func (m *mockGuestOps) RemoveLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.unmountErr +} + +// --- Helpers --- + +func defaultDeviceConfig() device.DeviceConfig { + return device.DeviceConfig{ + HostPath: `C:\test\layer.vhd`, + ReadOnly: true, + ImageFormat: "Vhd1", + } +} + +func defaultMountConfig() mount.MountConfig { + return mount.MountConfig{} +} + +func newController(vm *mockVMOps, guest *mockGuestOps) *Controller { + return New(64, vm, guest) +} + +func reservedController(t *testing.T) (*Controller, uuid.UUID) { + t.Helper() + c := newController(&mockVMOps{}, &mockGuestOps{}) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("setup Reserve: %v", err) + } + return c, id +} + +func mountedController(t *testing.T) (*Controller, uuid.UUID) { + t.Helper() + c, id := reservedController(t) + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("setup Mount: %v", err) + } + return c, id +} + +// --- Tests: New --- + +func TestNew(t *testing.T) { + c := New(64, &mockVMOps{}, &mockGuestOps{}) + if c == nil { + t.Fatal("expected non-nil controller") + } +} + +// --- Tests: Reserve --- + +func TestReserve_Success(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id == uuid.Nil { + t.Fatal("expected non-nil reservation ID") + } +} + +func TestReserve_SameDeviceSameMount(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + dc := defaultDeviceConfig() + mc := defaultMountConfig() + id1, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + id2, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + if id1 == id2 { + t.Error("expected different reservation IDs") + } +} + +func TestReserve_SamePathDifferentDeviceConfig(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + dc := defaultDeviceConfig() + mc := defaultMountConfig() + _, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + dc2 := dc + dc2.ReadOnly = false + _, err = c.Reserve(context.Background(), dc2, mc) + if err == nil { + t.Fatal("expected error for same path with different device config") + } +} + +func TestReserve_DifferentDevices(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + mc := defaultMountConfig() + _, err := c.Reserve(context.Background(), device.DeviceConfig{HostPath: `C:\a.vhd`, ReadOnly: true, ImageFormat: "Vhd1"}, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + _, err = c.Reserve(context.Background(), device.DeviceConfig{HostPath: `C:\b.vhd`, ReadOnly: true, ImageFormat: "Vhd1"}, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } +} + +func TestReserve_NoAvailableSlots(t *testing.T) { + c := New(0, &mockVMOps{}, &mockGuestOps{}) + _, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err == nil { + t.Fatal("expected error when no slots available") + } +} + +func TestReserve_FillsAllSlots(t *testing.T) { + c := New(64, &mockVMOps{}, &mockGuestOps{}) + mc := defaultMountConfig() + for i := range 64 { + dc := device.DeviceConfig{ + HostPath: fmt.Sprintf(`C:\layer%d.vhd`, i), + ReadOnly: true, + ImageFormat: "Vhd1", + } + _, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("reserve slot %d: %v", i, err) + } + } + // Next reservation should fail. + _, err := c.Reserve(context.Background(), device.DeviceConfig{HostPath: `C:\overflow.vhd`, ReadOnly: true, ImageFormat: "Vhd1"}, mc) + if err == nil { + t.Fatal("expected error when all slots are full") + } +} + +// --- Tests: Mount --- + +func TestMount_Success(t *testing.T) { + c, id := reservedController(t) + guestPath, err := c.Mount(context.Background(), id) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMount_UnknownReservation(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + _, err := c.Mount(context.Background(), uuid.New()) + if err == nil { + t.Fatal("expected error for unknown reservation") + } +} + +func TestMount_AttachError(t *testing.T) { + vm := &mockVMOps{addErr: errors.New("attach failed")} + c := New(64, vm, &mockGuestOps{}) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.Mount(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestMount_MountError(t *testing.T) { + guest := &mockGuestOps{mountErr: errors.New("mount failed")} + c := New(64, &mockVMOps{}, guest) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.Mount(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestMount_Idempotent(t *testing.T) { + c, id := reservedController(t) + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("first mount: %v", err) + } + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("second mount (idempotent): %v", err) + } +} + +// --- Tests: Unmount --- + +func TestUnmount_Success(t *testing.T) { + c, id := mountedController(t) + err := c.Unmount(context.Background(), id) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmount_UnknownReservation(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + err := c.Unmount(context.Background(), uuid.New()) + if err == nil { + t.Fatal("expected error for unknown reservation") + } +} + +func TestUnmount_CleansUpSlotWhenFullyDetached(t *testing.T) { + c, id := mountedController(t) + err := c.Unmount(context.Background(), id) + if err != nil { + t.Fatalf("Unmount: %v", err) + } + // The slot should be freed, so we can reserve the same path again. + _, err = c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("re-reserve after unmount: %v", err) + } +} + +func TestUnmount_UnmountError(t *testing.T) { + guest := &mockGuestOps{} + c := New(64, &mockVMOps{}, guest) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("Mount: %v", err) + } + // Now inject an unmount error. + guest.unmountErr = errors.New("unmount failed") + err = c.Unmount(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestUnmount_DetachError(t *testing.T) { + vm := &mockVMOps{} + c := New(64, vm, &mockGuestOps{}) + id, err := c.Reserve(context.Background(), defaultDeviceConfig(), defaultMountConfig()) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("Mount: %v", err) + } + // Now inject a remove error. + vm.removeErr = errors.New("remove failed") + err = c.Unmount(context.Background(), id) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestUnmount_MultipleReservationsSameDevice(t *testing.T) { + c := newController(&mockVMOps{}, &mockGuestOps{}) + dc := defaultDeviceConfig() + mc := defaultMountConfig() + + id1, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + id2, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + + // Mount both. + if _, err := c.Mount(context.Background(), id1); err != nil { + t.Fatalf("Mount id1: %v", err) + } + if _, err := c.Mount(context.Background(), id2); err != nil { + t.Fatalf("Mount id2: %v", err) + } + + // Unmount first. Device should still be attached because id2 holds a ref. + if err := c.Unmount(context.Background(), id1); err != nil { + t.Fatalf("Unmount id1: %v", err) + } + + // Unmount second. Now device should be fully detached and slot freed. + if err := c.Unmount(context.Background(), id2); err != nil { + t.Fatalf("Unmount id2: %v", err) + } + + // Slot should be free for reuse. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after full unmount: %v", err) + } +} + +// --- Tests: Reserve + Unmount lifecycle --- + +func TestReserveAfterUnmount_ReusesSlot(t *testing.T) { + c, id := mountedController(t) + if err := c.Unmount(context.Background(), id); err != nil { + t.Fatalf("Unmount: %v", err) + } + // Reserve a different device in the now-freed slot. + dc := device.DeviceConfig{HostPath: `C:\other.vhd`, ReadOnly: true, ImageFormat: "Vhd1"} + _, err := c.Reserve(context.Background(), dc, defaultMountConfig()) + if err != nil { + t.Fatalf("reserve after unmount: %v", err) + } +} + +func TestUnmount_AfterFailedMount(t *testing.T) { + vm := &mockVMOps{addErr: errors.New("attach failed")} + c := New(64, vm, &mockGuestOps{}) + dc := defaultDeviceConfig() + mc := defaultMountConfig() + + id, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + _, err = c.Mount(context.Background(), id) + if err == nil { + t.Fatal("expected Mount to fail") + } + // Unmount should succeed and clean up. + err = c.Unmount(context.Background(), id) + if err != nil { + t.Fatalf("Unmount after failed Mount: %v", err) + } + // Slot should be freed for reuse. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after cleanup: %v", err) + } +} + +func TestUnmount_RetryAfterDetachFailure(t *testing.T) { + vm := &mockVMOps{} + c := New(64, vm, &mockGuestOps{}) + dc := defaultDeviceConfig() + mc := defaultMountConfig() + + id, err := c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("Reserve: %v", err) + } + if _, err := c.Mount(context.Background(), id); err != nil { + t.Fatalf("Mount: %v", err) + } + // Inject remove error for first attempt. + vm.removeErr = errors.New("transient remove failure") + err = c.Unmount(context.Background(), id) + if err == nil { + t.Fatal("expected detach error") + } + // Fix the error and retry. + vm.removeErr = nil + err = c.Unmount(context.Background(), id) + if err != nil { + t.Fatalf("retry Unmount: %v", err) + } + // Slot should be freed. + _, err = c.Reserve(context.Background(), dc, mc) + if err != nil { + t.Fatalf("re-reserve after retry: %v", err) + } +} diff --git a/internal/controller/device/vpmem/device/device.go b/internal/controller/device/vpmem/device/device.go new file mode 100644 index 0000000000..2aba33b929 --- /dev/null +++ b/internal/controller/device/vpmem/device/device.go @@ -0,0 +1,166 @@ +//go:build windows + +package device + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" +) + +// DeviceConfig describes the host-side VPMem device to attach to the VM. +type DeviceConfig struct { + // HostPath is the path on the host to the VHD to be attached. + HostPath string + // ReadOnly specifies whether the device should be attached with read-only access. + ReadOnly bool + // ImageFormat specifies the image format of the VHD (e.g. "Vhd1"). + ImageFormat string +} + +// Equals reports whether two DeviceConfig values describe the same attachment parameters. +func (d DeviceConfig) Equals(other DeviceConfig) bool { + return d.HostPath == other.HostPath && + d.ReadOnly == other.ReadOnly && + d.ImageFormat == other.ImageFormat +} + +type DeviceState int + +const ( + // The device has never been attached. + DeviceStateReserved DeviceState = iota + // The device is currently attached to the guest. + DeviceStateAttached + // The device was previously attached and detached, this is terminal. + DeviceStateDetached +) + +type VMVPMemAdder interface { + AddVPMemDevice(ctx context.Context, device hcsschema.VirtualPMemDevice, deviceNumber uint32) error +} + +type VMVPMemRemover interface { + RemoveVPMemDevice(ctx context.Context, deviceNumber uint32) error +} + +// Device represents a VPMem device attached to the VM. It manages the lifecycle +// of the device attachment as well as the guest mount on the device. +// +// All operations on the device are expected to be ordered by the caller. No +// locking is done at this layer. +type Device struct { + deviceNumber uint32 + config DeviceConfig + + state DeviceState + mount *mount.Mount +} + +// NewReserved creates a new Device in the reserved state with the provided +// configuration. +func NewReserved(deviceNumber uint32, config DeviceConfig) *Device { + return &Device{ + deviceNumber: deviceNumber, + config: config, + state: DeviceStateReserved, + } +} + +func (d *Device) State() DeviceState { + return d.state +} + +func (d *Device) Config() DeviceConfig { + return d.config +} + +func (d *Device) HostPath() string { + return d.config.HostPath +} + +func (d *Device) AttachToVM(ctx context.Context, vm VMVPMemAdder) error { + switch d.state { + case DeviceStateReserved: + if err := vm.AddVPMemDevice(ctx, hcsschema.VirtualPMemDevice{ + HostPath: d.config.HostPath, + ReadOnly: d.config.ReadOnly, + ImageFormat: d.config.ImageFormat, + }, d.deviceNumber); err != nil { + // Move to detached since we know from reserved there was no guest + // state. + d.state = DeviceStateDetached + return fmt.Errorf("attach VPMem device to VM: %w", err) + } + d.state = DeviceStateAttached + return nil + case DeviceStateAttached: + return nil + case DeviceStateDetached: + return fmt.Errorf("device already detached") + } + return nil +} + +func (d *Device) DetachFromVM(ctx context.Context, vm VMVPMemRemover) error { + switch d.state { + case DeviceStateReserved: + return nil + case DeviceStateAttached: + // Ensure for correctness nobody leaked a mount. + if d.mount != nil { + // This device is still active by a mount. Leave it. + return nil + } + if err := vm.RemoveVPMemDevice(ctx, d.deviceNumber); err != nil { + return fmt.Errorf("detach VPMem device from VM: %w", err) + } + d.state = DeviceStateDetached + return nil + case DeviceStateDetached: + return nil + } + return fmt.Errorf("unexpected device state %d", d.state) +} + +func (d *Device) ReserveMount(ctx context.Context, config mount.MountConfig) (*mount.Mount, error) { + if d.state != DeviceStateReserved && d.state != DeviceStateAttached { + return nil, fmt.Errorf("unexpected device state %d, expected reserved or attached", d.state) + } + + if d.mount != nil { + if err := d.mount.Reserve(config); err != nil { + return nil, fmt.Errorf("reserve mount: %w", err) + } + return d.mount, nil + } + m := mount.NewReserved(d.deviceNumber, config) + d.mount = m + return m, nil +} + +func (d *Device) MountToGuest(ctx context.Context, guest mount.LinuxGuestVPMemMounter) (string, error) { + if d.state != DeviceStateAttached { + return "", fmt.Errorf("unexpected device state %d, expected attached", d.state) + } + if d.mount == nil { + return "", fmt.Errorf("no mount reserved on device") + } + return d.mount.MountToGuest(ctx, guest) +} + +func (d *Device) UnmountFromGuest(ctx context.Context, guest mount.LinuxGuestVPMemUnmounter) error { + if d.mount == nil { + // Consider a missing mount a success for retry logic in the caller. + return nil + } + if err := d.mount.UnmountFromGuest(ctx, guest); err != nil { + return fmt.Errorf("unmount from guest: %w", err) + } + if d.mount.State() == mount.MountStateUnmounted { + d.mount = nil + } + return nil +} diff --git a/internal/controller/device/vpmem/device/device_test.go b/internal/controller/device/vpmem/device/device_test.go new file mode 100644 index 0000000000..4c7887ef8c --- /dev/null +++ b/internal/controller/device/vpmem/device/device_test.go @@ -0,0 +1,440 @@ +//go:build windows + +package device + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/controller/device/vpmem/mount" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockVMVPMemAdder struct { + err error +} + +func (m *mockVMVPMemAdder) AddVPMemDevice(_ context.Context, _ hcsschema.VirtualPMemDevice, _ uint32) error { + return m.err +} + +type mockVMVPMemRemover struct { + err error +} + +func (m *mockVMVPMemRemover) RemoveVPMemDevice(_ context.Context, _ uint32) error { + return m.err +} + +type mockLinuxGuestVPMemMounter struct { + err error +} + +func (m *mockLinuxGuestVPMemMounter) AddLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.err +} + +type mockLinuxGuestVPMemUnmounter struct { + err error +} + +func (m *mockLinuxGuestVPMemUnmounter) RemoveLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.err +} + +// --- Helpers --- + +func defaultConfig() DeviceConfig { + return DeviceConfig{ + HostPath: `C:\test\layer.vhd`, + ReadOnly: true, + ImageFormat: "Vhd1", + } +} + +func attachedDevice(t *testing.T) *Device { + t.Helper() + d := NewReserved(0, defaultConfig()) + if err := d.AttachToVM(context.Background(), &mockVMVPMemAdder{}); err != nil { + t.Fatalf("setup AttachToVM: %v", err) + } + return d +} + +// --- Tests --- + +func TestNewReserved(t *testing.T) { + cfg := DeviceConfig{ + HostPath: `C:\test\layer.vhd`, + ReadOnly: true, + ImageFormat: "Vhd1", + } + d := NewReserved(3, cfg) + + if d.State() != DeviceStateReserved { + t.Errorf("expected state %d, got %d", DeviceStateReserved, d.State()) + } + if d.Config() != cfg { + t.Errorf("expected config %+v, got %+v", cfg, d.Config()) + } + if d.HostPath() != cfg.HostPath { + t.Errorf("expected host path %q, got %q", cfg.HostPath, d.HostPath()) + } +} + +func TestDeviceConfigEquals(t *testing.T) { + base := DeviceConfig{ + HostPath: `C:\a.vhd`, + ReadOnly: true, + ImageFormat: "Vhd1", + } + same := base + if !base.Equals(same) { + t.Error("expected equal configs to be equal") + } + + diffPath := base + diffPath.HostPath = `C:\b.vhd` + if base.Equals(diffPath) { + t.Error("expected different HostPath to be not equal") + } + + diffReadOnly := base + diffReadOnly.ReadOnly = false + if base.Equals(diffReadOnly) { + t.Error("expected different ReadOnly to be not equal") + } + + diffFormat := base + diffFormat.ImageFormat = "Vhdx" + if base.Equals(diffFormat) { + t.Error("expected different ImageFormat to be not equal") + } +} + +func TestAttachToVM_FromReserved_Success(t *testing.T) { + d := NewReserved(0, defaultConfig()) + err := d.AttachToVM(context.Background(), &mockVMVPMemAdder{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != DeviceStateAttached { + t.Errorf("expected state %d, got %d", DeviceStateAttached, d.State()) + } +} + +func TestAttachToVM_FromReserved_Error(t *testing.T) { + addErr := errors.New("add failed") + d := NewReserved(0, defaultConfig()) + err := d.AttachToVM(context.Background(), &mockVMVPMemAdder{err: addErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, addErr) { + t.Errorf("expected wrapped error %v, got %v", addErr, err) + } + if d.State() != DeviceStateDetached { + t.Errorf("expected state %d after failure, got %d", DeviceStateDetached, d.State()) + } +} + +func TestAttachToVM_Idempotent_WhenAttached(t *testing.T) { + d := attachedDevice(t) + if err := d.AttachToVM(context.Background(), &mockVMVPMemAdder{}); err != nil { + t.Fatalf("unexpected error on idempotent attach: %v", err) + } + if d.State() != DeviceStateAttached { + t.Errorf("expected state %d, got %d", DeviceStateAttached, d.State()) + } +} + +func TestAttachToVM_ErrorWhenDetached(t *testing.T) { + d := NewReserved(0, defaultConfig()) + // Fail attachment to move to detached. + _ = d.AttachToVM(context.Background(), &mockVMVPMemAdder{err: errors.New("fail")}) + if d.State() != DeviceStateDetached { + t.Fatalf("expected detached state, got %d", d.State()) + } + err := d.AttachToVM(context.Background(), &mockVMVPMemAdder{}) + if err == nil { + t.Fatal("expected error when attaching detached device") + } +} + +func TestDetachFromVM_FromAttached_Success(t *testing.T) { + d := attachedDevice(t) + err := d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d.State() != DeviceStateDetached { + t.Errorf("expected state %d, got %d", DeviceStateDetached, d.State()) + } +} + +func TestDetachFromVM_RemoveError(t *testing.T) { + d := attachedDevice(t) + removeErr := errors.New("remove failed") + err := d.DetachFromVM(context.Background(), &mockVMVPMemRemover{err: removeErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, removeErr) { + t.Errorf("expected wrapped error %v, got %v", removeErr, err) + } + // State should remain attached since removal failed. + if d.State() != DeviceStateAttached { + t.Errorf("expected state %d, got %d", DeviceStateAttached, d.State()) + } +} + +func TestDetachFromVM_SkipsWhenMountExists(t *testing.T) { + d := attachedDevice(t) + _, err := d.ReserveMount(context.Background(), mount.MountConfig{}) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + err = d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Should remain attached because there is an outstanding mount. + if d.State() != DeviceStateAttached { + t.Errorf("expected state %d, got %d", DeviceStateAttached, d.State()) + } +} + +func TestDetachFromVM_Idempotent_WhenReserved(t *testing.T) { + d := NewReserved(0, defaultConfig()) + err := d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("unexpected error detaching reserved device: %v", err) + } +} + +func TestDetachFromVM_Idempotent_WhenDetached(t *testing.T) { + d := attachedDevice(t) + _ = d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + // Second detach should be idempotent. + err := d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("unexpected error on idempotent detach: %v", err) + } +} + +func TestReserveMount_Success(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + m, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m == nil { + t.Fatal("expected non-nil mount") + } + if m.State() != mount.MountStateReserved { + t.Errorf("expected mount state %d, got %d", mount.MountStateReserved, m.State()) + } +} + +func TestReserveMount_SuccessFromReservedDevice(t *testing.T) { + d := NewReserved(0, defaultConfig()) + cfg := mount.MountConfig{} + m, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m == nil { + t.Fatal("expected non-nil mount") + } +} + +func TestReserveMount_DuplicateReturnsSameMount(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + m1, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("first reserve: %v", err) + } + m2, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("second reserve: %v", err) + } + if m1 != m2 { + t.Error("expected same mount object on duplicate reservation") + } +} + +func TestReserveMount_ErrorWhenDetached(t *testing.T) { + d := NewReserved(0, defaultConfig()) + _ = d.AttachToVM(context.Background(), &mockVMVPMemAdder{err: errors.New("fail")}) + _, err := d.ReserveMount(context.Background(), mount.MountConfig{}) + if err == nil { + t.Fatal("expected error when device is detached") + } +} + +func TestMountToGuest_Success(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + guestPath, err := d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } +} + +func TestMountToGuest_ErrorWhenNotAttached(t *testing.T) { + d := NewReserved(0, defaultConfig()) + _, err := d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err == nil { + t.Fatal("expected error when device is not attached") + } +} + +func TestMountToGuest_NoMount(t *testing.T) { + d := attachedDevice(t) + _, err := d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err == nil { + t.Fatal("expected error for unreserved mount") + } +} + +func TestMountToGuest_MountError(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + _, err = d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{err: errors.New("mount fail")}) + if err != nil { + // This is expected - the mount error propagates. + return + } +} + +func TestUnmountFromGuest_Success(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + _, err = d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err != nil { + t.Fatalf("MountToGuest: %v", err) + } + err = d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestUnmountFromGuest_NoMount_IsNoOp(t *testing.T) { + d := attachedDevice(t) + err := d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("expected nil error for missing mount, got: %v", err) + } +} + +func TestUnmountFromGuest_SucceedsWhenNotAttached(t *testing.T) { + d := NewReserved(0, defaultConfig()) + err := d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("expected nil error for missing mount on non-attached device, got: %v", err) + } +} + +func TestUnmountFromGuest_UnmountError(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + _, err = d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err != nil { + t.Fatalf("MountToGuest: %v", err) + } + unmountErr := errors.New("unmount fail") + err = d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{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 TestUnmountFromGuest_RemovesMountWhenFullyUnmounted(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + _, err = d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err != nil { + t.Fatalf("MountToGuest: %v", err) + } + err = d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("UnmountFromGuest: %v", err) + } + // After full unmount the mount should be removed, so detach should succeed. + err = d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("DetachFromVM after unmount: %v", err) + } + if d.State() != DeviceStateDetached { + t.Errorf("expected state %d, got %d", DeviceStateDetached, d.State()) + } +} + +func TestUnmountFromGuest_RetryAfterDetachFailure(t *testing.T) { + d := attachedDevice(t) + cfg := mount.MountConfig{} + _, err := d.ReserveMount(context.Background(), cfg) + if err != nil { + t.Fatalf("ReserveMount: %v", err) + } + _, err = d.MountToGuest(context.Background(), &mockLinuxGuestVPMemMounter{}) + if err != nil { + t.Fatalf("MountToGuest: %v", err) + } + // Unmount succeeds and removes the mount. + err = d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("UnmountFromGuest: %v", err) + } + // Detach fails (e.g. transient error). + err = d.DetachFromVM(context.Background(), &mockVMVPMemRemover{err: errors.New("transient")}) + if err == nil { + t.Fatal("expected detach error") + } + // Retry: unmount is a no-op since mount is gone. + err = d.UnmountFromGuest(context.Background(), &mockLinuxGuestVPMemUnmounter{}) + if err != nil { + t.Fatalf("retry UnmountFromGuest should be no-op, got: %v", err) + } + // Retry detach now succeeds. + err = d.DetachFromVM(context.Background(), &mockVMVPMemRemover{}) + if err != nil { + t.Fatalf("retry DetachFromVM: %v", err) + } + if d.State() != DeviceStateDetached { + t.Errorf("expected state %d, got %d", DeviceStateDetached, d.State()) + } +} diff --git a/internal/controller/device/vpmem/mount/mount.go b/internal/controller/device/vpmem/mount/mount.go new file mode 100644 index 0000000000..0319839e66 --- /dev/null +++ b/internal/controller/device/vpmem/mount/mount.go @@ -0,0 +1,136 @@ +//go:build windows + +package mount + +import ( + "context" + "fmt" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// MountConfig describes how a VPMem device should be mounted inside the guest. +type MountConfig struct{} + +// Equals reports whether two MountConfig values describe the same mount +// parameters. +func (mc MountConfig) Equals(other MountConfig) bool { + return true +} + +type MountState int + +const ( + // The mount has never been mounted. + MountStateReserved MountState = iota + // The mount is currently mounted in the guest. + MountStateMounted + // The mount was previously mounted and unmounted. + MountStateUnmounted +) + +type LinuxGuestVPMemMounter interface { + AddLCOWMappedVPMemDevice(ctx context.Context, settings guestresource.LCOWMappedVPMemDevice) error +} + +type LinuxGuestVPMemUnmounter interface { + RemoveLCOWMappedVPMemDevice(ctx context.Context, settings guestresource.LCOWMappedVPMemDevice) error +} + +const ( + mountFmtVPMem = "/run/layers/p%d" +) + +// Mount defines a mount of a VPMem device inside the guest. It manages the +// lifecycle of the mount inside the guest independent of the lifecycle of the +// device attachment. +// +// All operations on the mount are expected to be ordered by the caller. No +// locking is done at this layer. +type Mount struct { + deviceNumber uint32 + config MountConfig + + state MountState + refCount int + guestPath string +} + +// NewReserved creates a new Mount in the reserved state with the provided +// configuration. +func NewReserved(deviceNumber uint32, config MountConfig) *Mount { + return &Mount{ + deviceNumber: deviceNumber, + config: config, + state: MountStateReserved, + refCount: 1, + } +} + +func (m *Mount) State() MountState { + return m.state +} + +func (m *Mount) GuestPath() string { + return m.guestPath +} + +func (m *Mount) Reserve(config MountConfig) error { + if !m.config.Equals(config) { + return fmt.Errorf("cannot reserve ref on mount with different config") + } + if m.state != MountStateReserved && m.state != MountStateMounted { + return fmt.Errorf("cannot reserve ref on mount in state %d", m.state) + } + m.refCount++ + return nil +} + +func (m *Mount) MountToGuest(ctx context.Context, guest LinuxGuestVPMemMounter) (string, error) { + switch m.state { + case MountStateReserved: + guestPath := fmt.Sprintf(mountFmtVPMem, m.deviceNumber) + settings := guestresource.LCOWMappedVPMemDevice{ + DeviceNumber: m.deviceNumber, + MountPath: guestPath, + } + if err := guest.AddLCOWMappedVPMemDevice(ctx, settings); err != nil { + // Move to unmounted since we know from reserved there was no + // guest state. + m.state = MountStateUnmounted + return "", fmt.Errorf("add LCOW mapped VPMem device %d: %w", m.deviceNumber, err) + } + m.guestPath = guestPath + m.state = MountStateMounted + return m.guestPath, nil + case MountStateMounted: + return m.guestPath, nil + case MountStateUnmounted: + return "", fmt.Errorf("cannot mount an unmounted mount") + } + return "", nil +} + +func (m *Mount) UnmountFromGuest(ctx context.Context, guest LinuxGuestVPMemUnmounter) error { + switch m.state { + case MountStateReserved: + m.refCount-- + return nil + case MountStateMounted: + if m.refCount == 1 { + settings := guestresource.LCOWMappedVPMemDevice{ + DeviceNumber: m.deviceNumber, + MountPath: m.guestPath, + } + if err := guest.RemoveLCOWMappedVPMemDevice(ctx, settings); err != nil { + return fmt.Errorf("remove LCOW mapped VPMem device %d: %w", m.deviceNumber, err) + } + m.state = MountStateUnmounted + } + m.refCount-- + return nil + case MountStateUnmounted: + return fmt.Errorf("cannot unmount an unmounted mount") + } + return nil +} diff --git a/internal/controller/device/vpmem/mount/mount_test.go b/internal/controller/device/vpmem/mount/mount_test.go new file mode 100644 index 0000000000..34aa23bf87 --- /dev/null +++ b/internal/controller/device/vpmem/mount/mount_test.go @@ -0,0 +1,217 @@ +//go:build windows + +package mount + +import ( + "context" + "errors" + "testing" + + "github.com/Microsoft/hcsshim/internal/protocol/guestresource" +) + +// --- Mock types --- + +type mockLinuxMounter struct { + err error +} + +func (m *mockLinuxMounter) AddLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.err +} + +type mockLinuxUnmounter struct { + err error +} + +func (m *mockLinuxUnmounter) RemoveLCOWMappedVPMemDevice(_ context.Context, _ guestresource.LCOWMappedVPMemDevice) error { + return m.err +} + +// --- Helpers --- + +func defaultMountConfig() MountConfig { + return MountConfig{} +} + +func mountedMount(t *testing.T) *Mount { + t.Helper() + m := NewReserved(0, defaultMountConfig()) + if _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}); err != nil { + t.Fatalf("setup MountToGuest: %v", err) + } + return m +} + +// --- Tests --- + +func TestNewReserved(t *testing.T) { + cfg := MountConfig{} + m := NewReserved(5, cfg) + if m.State() != MountStateReserved { + t.Errorf("expected state %d, got %d", MountStateReserved, m.State()) + } +} + +func TestMountConfigEquals(t *testing.T) { + a := MountConfig{} + b := MountConfig{} + if !a.Equals(b) { + t.Error("expected equal configs to be equal") + } +} + +func TestReserve_SameConfig(t *testing.T) { + cfg := defaultMountConfig() + m := NewReserved(0, cfg) + if err := m.Reserve(cfg); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestReserve_WhenMounted(t *testing.T) { + m := mountedMount(t) + cfg := defaultMountConfig() + if err := m.Reserve(cfg); err != nil { + t.Fatalf("unexpected error reserving mounted mount: %v", err) + } +} + +func TestReserve_WhenUnmounted(t *testing.T) { + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + if m.State() != MountStateUnmounted { + t.Fatalf("expected unmounted, got %d", m.State()) + } + err := m.Reserve(defaultMountConfig()) + if err == nil { + t.Fatal("expected error reserving unmounted mount") + } +} + +func TestMountToGuest_Success(t *testing.T) { + m := NewReserved(0, defaultMountConfig()) + guestPath, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath") + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestMountToGuest_Error(t *testing.T) { + mountErr := errors.New("mount failed") + m := NewReserved(0, defaultMountConfig()) + _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{err: mountErr}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, mountErr) { + t.Errorf("expected wrapped error %v, got %v", mountErr, err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d after failure, got %d", MountStateUnmounted, m.State()) + } +} + +func TestMountToGuest_Idempotent_WhenMounted(t *testing.T) { + m := mountedMount(t) + guestPath, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}) + if err != nil { + t.Fatalf("unexpected error on idempotent mount: %v", err) + } + if guestPath == "" { + t.Error("expected non-empty guestPath on idempotent mount") + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestMountToGuest_ErrorWhenUnmounted(t *testing.T) { + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}) + if err == nil { + t.Fatal("expected error mounting an unmounted mount") + } +} + +func TestUnmountFromGuest_Success(t *testing.T) { + m := mountedMount(t) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d, got %d", MountStateUnmounted, m.State()) + } +} + +func TestUnmountFromGuest_Error(t *testing.T) { + m := mountedMount(t) + unmountErr := errors.New("unmount failed") + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{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() != MountStateMounted { + t.Errorf("expected state %d, got %d", MountStateMounted, m.State()) + } +} + +func TestUnmountFromGuest_FromReserved_DecrementsRefCount(t *testing.T) { + m := NewReserved(0, defaultMountConfig()) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Should still be reserved since no guest mount was done. + if m.State() != MountStateReserved { + t.Errorf("expected state %d, got %d", MountStateReserved, m.State()) + } +} + +func TestUnmountFromGuest_ErrorWhenUnmounted(t *testing.T) { + m := mountedMount(t) + _ = m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}) + if err == nil { + t.Fatal("expected error unmounting already-unmounted mount") + } +} + +func TestUnmountFromGuest_MultipleRefs_DoesNotUnmountUntilLastRef(t *testing.T) { + cfg := defaultMountConfig() + m := NewReserved(0, cfg) + // Add a second reservation. + if err := m.Reserve(cfg); err != nil { + t.Fatalf("Reserve: %v", err) + } + // Mount the guest. + if _, err := m.MountToGuest(context.Background(), &mockLinuxMounter{}); err != nil { + t.Fatalf("MountToGuest: %v", err) + } + // First unmount should decrement ref but stay mounted. + if err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}); err != nil { + t.Fatalf("first UnmountFromGuest: %v", err) + } + if m.State() != MountStateMounted { + t.Errorf("expected state %d after first unmount, got %d", MountStateMounted, m.State()) + } + // Second unmount should actually unmount. + if err := m.UnmountFromGuest(context.Background(), &mockLinuxUnmounter{}); err != nil { + t.Fatalf("second UnmountFromGuest: %v", err) + } + if m.State() != MountStateUnmounted { + t.Errorf("expected state %d after final unmount, got %d", MountStateUnmounted, m.State()) + } +} diff --git a/internal/controller/device/vpmem/vpmem.go b/internal/controller/device/vpmem/vpmem.go new file mode 100644 index 0000000000..01c8c31c9b --- /dev/null +++ b/internal/controller/device/vpmem/vpmem.go @@ -0,0 +1,7 @@ +//go:build windows + +// Package vpmem implements the VPMem controller for managing VPMem attached +// devices to a Utility VM as well as the Guest Mounts surfacing those devices +// for use by the Guest containers. + +package vpmem diff --git a/internal/vm/guestmanager/device_lcow.go b/internal/vm/guestmanager/device_lcow.go index c6cece1fa0..61524a0ccc 100644 --- a/internal/vm/guestmanager/device_lcow.go +++ b/internal/vm/guestmanager/device_lcow.go @@ -11,18 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// LCOWDeviceManager exposes VPCI and VPMem device operations in the LCOW guest. -type LCOWDeviceManager interface { - // AddVPCIDevice adds a VPCI device to the guest. - AddVPCIDevice(ctx context.Context, settings guestresource.LCOWMappedVPCIDevice) error - // AddVPMemDevice adds a VPMem device to the guest. - AddVPMemDevice(ctx context.Context, settings guestresource.LCOWMappedVPMemDevice) error - // RemoveVPMemDevice removes a VPMem device from the guest. - RemoveVPMemDevice(ctx context.Context, settings guestresource.LCOWMappedVPMemDevice) error -} - -var _ LCOWDeviceManager = (*Guest)(nil) - // AddVPCIDevice adds a VPCI device in the guest. func (gm *Guest) AddVPCIDevice(ctx context.Context, settings guestresource.LCOWMappedVPCIDevice) error { request := &hcsschema.ModifySettingRequest{ diff --git a/internal/vm/vmmanager/vpmem.go b/internal/vm/vmmanager/vpmem.go index b3e7994ed9..fddaf66fe8 100644 --- a/internal/vm/vmmanager/vpmem.go +++ b/internal/vm/vmmanager/vpmem.go @@ -11,17 +11,6 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" ) -// VPMemManager manages adding and removing virtual persistent memory devices for a Utility VM. -type VPMemManager interface { - // AddVPMemDevice adds a virtual pmem device to the Utility VM. - AddVPMemDevice(ctx context.Context, id uint32, settings hcsschema.VirtualPMemDevice) error - - // RemoveVPMemDevice removes a virtual pmem device from the Utility VM. - RemoveVPMemDevice(ctx context.Context, id uint32) error -} - -var _ VPMemManager = (*UtilityVM)(nil) - func (uvm *UtilityVM) AddVPMemDevice(ctx context.Context, id uint32, settings hcsschema.VirtualPMemDevice) error { request := &hcsschema.ModifySettingRequest{ RequestType: guestrequest.RequestTypeAdd,