From b9ecf91ef7b229b06e2e9f5ff8fecfb3acb67a42 Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Thu, 26 Sep 2024 22:21:41 -0700 Subject: [PATCH] wip --- internal/devices/drivers.go | 1 + internal/hcsoci/resources_lcow.go | 2 + internal/hcsoci/resources_wcow.go | 3 + internal/layers/lcow.go | 5 +- internal/layers/wcow_mount.go | 2 +- internal/uvm/scsi/manager.go | 18 ++-- internal/uvm/scsi/mount.go | 132 +++++++++--------------------- 7 files changed, 56 insertions(+), 107 deletions(-) diff --git a/internal/devices/drivers.go b/internal/devices/drivers.go index f5dc588e02..eb908583b1 100644 --- a/internal/devices/drivers.go +++ b/internal/devices/drivers.go @@ -60,6 +60,7 @@ func InstallDrivers(ctx context.Context, vm *uvm.UtilityVM, share string) (close share, true, vm.ID(), + "", &scsi.MountConfig{}, ) if err != nil { diff --git a/internal/hcsoci/resources_lcow.go b/internal/hcsoci/resources_lcow.go index b98493ce4c..633bde02eb 100644 --- a/internal/hcsoci/resources_lcow.go +++ b/internal/hcsoci/resources_lcow.go @@ -92,6 +92,7 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r * hostPath, readOnly, coi.HostingSystem.ID(), + "", &scsi.MountConfig{Options: mount.Options, BlockDev: isBlockDev}, ) if err != nil { @@ -114,6 +115,7 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r * hostPath, readOnly, coi.HostingSystem.ID(), + "", &scsi.MountConfig{Options: mount.Options, BlockDev: isBlockDev}, ) if err != nil { diff --git a/internal/hcsoci/resources_wcow.go b/internal/hcsoci/resources_wcow.go index 0503b371f6..604ba8b550 100644 --- a/internal/hcsoci/resources_wcow.go +++ b/internal/hcsoci/resources_wcow.go @@ -144,6 +144,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R mount.Source, readOnly, coi.HostingSystem.ID(), + "", &scsi.MountConfig{}, ) case MountTypeVirtualDisk: @@ -153,6 +154,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R mount.Source, readOnly, coi.HostingSystem.ID(), + "", &scsi.MountConfig{}, ) case MountTypeExtensibleVirtualDisk: @@ -161,6 +163,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R ctx, mount.Source, readOnly, + "", &scsi.MountConfig{}, ) } diff --git a/internal/layers/lcow.go b/internal/layers/lcow.go index cc934121b4..131b876659 100644 --- a/internal/layers/lcow.go +++ b/internal/layers/lcow.go @@ -14,7 +14,6 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" - "github.com/Microsoft/hcsshim/internal/guestpath" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/ospath" "github.com/Microsoft/hcsshim/internal/resources" @@ -136,6 +135,7 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers hostPath, false, vm.ID(), + guestRoot, mConfig, ) if err != nil { @@ -155,7 +155,7 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers } }() - rootfs := ospath.Join(vm.OS(), guestRoot, guestpath.RootfsPath) + rootfs := ospath.Join(vm.OS(), guestRoot, "rootfs") err = vm.CombineLayersLCOW(ctx, containerID, lcowUvmLayerPaths, containerScratchPathInUVM, rootfs) if err != nil { return "", "", nil, err @@ -194,6 +194,7 @@ func addLCOWLayer(ctx context.Context, vm *uvm.UtilityVM, layer *LCOWLayer) (uvm layer.VHDPath, true, "", + "", &scsi.MountConfig{ Partition: layer.Partition, Options: []string{"ro"}, diff --git a/internal/layers/wcow_mount.go b/internal/layers/wcow_mount.go index d12593d179..a5f706f940 100644 --- a/internal/layers/wcow_mount.go +++ b/internal/layers/wcow_mount.go @@ -331,7 +331,7 @@ func mountHypervIsolatedWCIFSLayers(ctx context.Context, l *wcowWCIFSLayers, vm hostPath := filepath.Join(l.scratchLayerPath, "sandbox.vhdx") log.G(ctx).WithField("hostPath", hostPath).Debug("mounting scratch VHD") - scsiMount, err := vm.SCSIManager.AddVirtualDisk(ctx, hostPath, false, vm.ID(), &scsi.MountConfig{}) + scsiMount, err := vm.SCSIManager.AddVirtualDisk(ctx, hostPath, false, vm.ID(), "", &scsi.MountConfig{}) if err != nil { return nil, nil, fmt.Errorf("failed to add SCSI scratch VHD: %w", err) } diff --git a/internal/uvm/scsi/manager.go b/internal/uvm/scsi/manager.go index fd7173b5b1..ff8374038e 100644 --- a/internal/uvm/scsi/manager.go +++ b/internal/uvm/scsi/manager.go @@ -141,6 +141,7 @@ func (m *Manager) AddVirtualDisk( hostPath string, readOnly bool, vmID string, + guestPath string, mc *MountConfig, ) (*Mount, error) { if m == nil { @@ -169,6 +170,7 @@ func (m *Manager) AddVirtualDisk( readOnly: readOnly, typ: "VirtualDisk", }, + guestPath, mcInternal) } @@ -187,6 +189,7 @@ func (m *Manager) AddPhysicalDisk( hostPath string, readOnly bool, vmID string, + guestPath string, mc *MountConfig, ) (*Mount, error) { if m == nil { @@ -215,6 +218,7 @@ func (m *Manager) AddPhysicalDisk( readOnly: readOnly, typ: "PassThru", }, + guestPath, mcInternal) } @@ -233,6 +237,7 @@ func (m *Manager) AddExtensibleVirtualDisk( ctx context.Context, hostPath string, readOnly bool, + guestPath string, mc *MountConfig, ) (*Mount, error) { if m == nil { @@ -260,10 +265,11 @@ func (m *Manager) AddExtensibleVirtualDisk( typ: "ExtensibleVirtualDisk", evdType: evdType, }, + guestPath, mcInternal) } -func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, mountConfig *mountConfig) (_ *Mount, err error) { +func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, guestPath string, mountConfig *mountConfig) (_ *Mount, err error) { controller, lun, err := m.attachManager.attach(ctx, attachConfig) if err != nil { return nil, err @@ -274,9 +280,8 @@ func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, mountConf } }() - var guestPath string if mountConfig != nil { - guestPath, err = m.mountManager.mount(ctx, controller, lun, mountConfig) + guestPath, err = m.mountManager.mount(ctx, controller, lun, guestPath, mountConfig) if err != nil { return nil, err } @@ -287,14 +292,9 @@ func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, mountConf func (m *Manager) remove(ctx context.Context, controller, lun uint, guestPath string) error { if guestPath != "" { - removed, err := m.mountManager.unmount(ctx, guestPath) - if err != nil { + if err := m.mountManager.unmount(ctx, guestPath); err != nil { return err } - - if !removed { - return nil - } } if _, err := m.attachManager.detach(ctx, controller, lun); err != nil { diff --git a/internal/uvm/scsi/mount.go b/internal/uvm/scsi/mount.go index ed77fa9991..4e28a58569 100644 --- a/internal/uvm/scsi/mount.go +++ b/internal/uvm/scsi/mount.go @@ -5,36 +5,31 @@ package scsi import ( "context" "fmt" - "reflect" - "sort" "sync" + "sync/atomic" ) type mountManager struct { - m sync.Mutex - mounter mounter - // Tracks current mounts. Entries will be nil if the mount was unmounted, meaning the index is - // available for use. - mounts []*mount - mountFmt string + mounter mounter + mounts map[string]*mount // Guest path -> mount struct. Tracks current mounts. + m sync.Mutex // Guards access to the mounts map. + guestMountFmt string + mountIndex atomic.Uint64 } -func newMountManager(mounter mounter, mountFmt string) *mountManager { +func newMountManager(mounter mounter, guestMountFmt string) *mountManager { return &mountManager{ - mounter: mounter, - mountFmt: mountFmt, + mounter: mounter, + mounts: make(map[string]*mount), + guestMountFmt: guestMountFmt, } } type mount struct { path string - index int controller uint lun uint config *mountConfig - waitErr error - waitCh chan struct{} - refCount uint } type mountConfig struct { @@ -47,107 +42,54 @@ type mountConfig struct { filesystem string } -func (mm *mountManager) mount(ctx context.Context, controller, lun uint, c *mountConfig) (_ string, err error) { - // Normalize the mount config for comparison. - // Config equality relies on the options slices being compared element-wise. Sort the options - // slice first so that two slices with different ordering compare as equal. We assume that - // order will never matter for mount options. - sort.Strings(c.options) +func (mm *mountManager) mount(ctx context.Context, controller, lun uint, path string, c *mountConfig) (_ string, err error) { + if path == "" { + path = fmt.Sprintf(mm.guestMountFmt, mm.mountIndex.Add(1)) + } + mount := &mount{ + path: path, + controller: controller, + lun: lun, + config: c, + } + if err := func() error { + mm.m.Lock() + defer mm.m.Unlock() - mount, existed := mm.trackMount(controller, lun, c) - if existed { - select { - case <-ctx.Done(): - return "", ctx.Err() - case <-mount.waitCh: - if mount.waitErr != nil { - return "", mount.waitErr - } + if _, ok := mm.mounts[path]; ok { + return fmt.Errorf("another mount is already present at %s", path) } - return mount.path, nil + + mm.mounts[path] = mount + return nil + }(); err != nil { + return "", err } defer func() { if err != nil { mm.m.Lock() - mm.untrackMount(mount) + delete(mm.mounts, path) mm.m.Unlock() } - - mount.waitErr = err - close(mount.waitCh) }() if err := mm.mounter.mount(ctx, controller, lun, mount.path, c); err != nil { return "", fmt.Errorf("mount scsi controller %d lun %d at %s: %w", controller, lun, mount.path, err) } - return mount.path, nil + return path, nil } -func (mm *mountManager) unmount(ctx context.Context, path string) (bool, error) { +func (mm *mountManager) unmount(ctx context.Context, path string) error { mm.m.Lock() defer mm.m.Unlock() - var mount *mount - for _, mount = range mm.mounts { - if mount != nil && mount.path == path { - break - } - } - - mount.refCount-- - if mount.refCount > 0 { - return false, nil - } - + mount := mm.mounts[path] if err := mm.mounter.unmount(ctx, mount.controller, mount.lun, mount.path, mount.config); err != nil { - return false, fmt.Errorf("unmount scsi controller %d lun %d at path %s: %w", mount.controller, mount.lun, mount.path, err) + return fmt.Errorf("unmount scsi controller %d lun %d at path %s: %w", mount.controller, mount.lun, mount.path, err) } - mm.untrackMount(mount) - - return true, nil -} -func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount, bool) { - mm.m.Lock() - defer mm.m.Unlock() - - var freeIndex int = -1 - for i, mount := range mm.mounts { - if mount == nil { - if freeIndex == -1 { - freeIndex = i - } - } else if controller == mount.controller && - lun == mount.lun && - reflect.DeepEqual(c, mount.config) { - - mount.refCount++ - return mount, true - } - } - - // New mount. - mount := &mount{ - controller: controller, - lun: lun, - config: c, - refCount: 1, - waitCh: make(chan struct{}), - } - if freeIndex == -1 { - mount.index = len(mm.mounts) - mm.mounts = append(mm.mounts, mount) - } else { - mount.index = freeIndex - mm.mounts[freeIndex] = mount - } - // Use the mount index to produce a unique guest path. - mount.path = fmt.Sprintf(mm.mountFmt, mount.index) - return mount, false -} + delete(mm.mounts, path) -// Caller must be holding mm.m. -func (mm *mountManager) untrackMount(mount *mount) { - mm.mounts[mount.index] = nil + return nil }