From c1d649299c57828e494dc743c99e32a53d22a0b1 Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Fri, 27 Sep 2024 11:23:53 -0700 Subject: [PATCH 1/2] scsi: Take optional guest path for mount Currently the SCSI mount manager will generate a new path for each new mount, based on a format string that it is instantiated with. However, it turns out some code in the GCS (e.g. sandbox mounts) assumes that the container scratch is mounted at a certain path. The long-term best solution here is probably to pass what paths to use explicitly to the GCS, but that would be more impactful. We need a more contained fix. This commit addresses the issue by allowing an optional guest path to be given for a SCSI mount. The mount manager has been changed as follows: - If a guest path is not supplied: The mount can re-use (refcount++) any existing mount with the same controller/lun/options. If a new mount is created, the mount manager will generate a path for it. - If a guest path is supplied: The mount can re-use (refcount++) any existing mount with the same controller/lun/guestpath/options. If a new mount is created, the mount manager will use the supplied path for it. Accordingly, code calling into the mount manager has been updated to pass an empty string for the guest path. The exception to this is the LCOW layer mounting code, which will pass an explicit guest path for the scratch disk. As far as I know, WCOW does not depend on a specific path for its scratch disk. Signed-off-by: Kevin Parsons --- internal/devices/drivers.go | 1 + internal/hcsoci/resources_lcow.go | 2 ++ internal/hcsoci/resources_wcow.go | 3 +++ internal/layers/lcow.go | 2 ++ internal/layers/wcow_mount.go | 2 +- internal/lcow/disk.go | 2 +- internal/lcow/scratch.go | 1 + internal/tools/uvmboot/mounts.go | 1 + internal/uvm/scsi/manager.go | 11 ++++++++--- internal/uvm/scsi/mount.go | 23 ++++++++++++++++------- 10 files changed, 36 insertions(+), 12 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..2a6da2a622 100644 --- a/internal/layers/lcow.go +++ b/internal/layers/lcow.go @@ -136,6 +136,7 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers hostPath, false, vm.ID(), + guestRoot, mConfig, ) if err != nil { @@ -194,6 +195,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/lcow/disk.go b/internal/lcow/disk.go index 4e68f26258..08fad8d2ea 100644 --- a/internal/lcow/disk.go +++ b/internal/lcow/disk.go @@ -31,7 +31,7 @@ func FormatDisk(ctx context.Context, lcowUVM *uvm.UtilityVM, destPath string) er }).Debug("lcow::FormatDisk opts") // Attach without mounting. - scsi, err := lcowUVM.SCSIManager.AddPhysicalDisk(ctx, destPath, false, lcowUVM.ID(), nil) + scsi, err := lcowUVM.SCSIManager.AddPhysicalDisk(ctx, destPath, false, lcowUVM.ID(), "", nil) if err != nil { return err } diff --git a/internal/lcow/scratch.go b/internal/lcow/scratch.go index 3c424dcbad..e44a6b388a 100644 --- a/internal/lcow/scratch.go +++ b/internal/lcow/scratch.go @@ -75,6 +75,7 @@ func CreateScratch(ctx context.Context, lcowUVM *uvm.UtilityVM, destFile string, destFile, false, lcowUVM.ID(), + "", &scsi.MountConfig{ BlockDev: true, }, diff --git a/internal/tools/uvmboot/mounts.go b/internal/tools/uvmboot/mounts.go index 2f160c01ca..0f546b9f86 100644 --- a/internal/tools/uvmboot/mounts.go +++ b/internal/tools/uvmboot/mounts.go @@ -25,6 +25,7 @@ func mountSCSI(ctx context.Context, c *cli.Context, vm *uvm.UtilityVM) error { m.host, !m.writable, vm.ID(), + "", &scsi.MountConfig{}, ) if err != nil { diff --git a/internal/uvm/scsi/manager.go b/internal/uvm/scsi/manager.go index fd7173b5b1..50aad34699 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 } diff --git a/internal/uvm/scsi/mount.go b/internal/uvm/scsi/mount.go index ed77fa9991..df5651e008 100644 --- a/internal/uvm/scsi/mount.go +++ b/internal/uvm/scsi/mount.go @@ -47,14 +47,17 @@ type mountConfig struct { filesystem string } -func (mm *mountManager) mount(ctx context.Context, controller, lun uint, c *mountConfig) (_ string, err error) { +func (mm *mountManager) mount(ctx context.Context, controller, lun uint, path string, 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) - mount, existed := mm.trackMount(controller, lun, c) + mount, existed, err := mm.trackMount(controller, lun, path, c) + if err != nil { + return "", err + } if existed { select { case <-ctx.Done(): @@ -108,7 +111,7 @@ func (mm *mountManager) unmount(ctx context.Context, path string) (bool, error) return true, nil } -func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount, bool) { +func (mm *mountManager) trackMount(controller, lun uint, path string, c *mountConfig) (*mount, bool, error) { mm.m.Lock() defer mm.m.Unlock() @@ -120,15 +123,19 @@ func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount } } else if controller == mount.controller && lun == mount.lun && + (path == "" || path == mount.path) && reflect.DeepEqual(c, mount.config) { mount.refCount++ - return mount, true + return mount, true, nil + } else if path != "" && path == mount.path { + return nil, false, fmt.Errorf("cannot mount over an existing mountpoint: %s", path) } } // New mount. mount := &mount{ + path: path, // If path is empty, this will be replaced with a generated path below. controller: controller, lun: lun, config: c, @@ -142,9 +149,11 @@ func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount 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 + if mount.path == "" { + // Use the mount index to produce a unique guest path. + mount.path = fmt.Sprintf(mm.mountFmt, mount.index) + } + return mount, false, nil } // Caller must be holding mm.m. From 3b7c087db13be3514524d6a736fc6f84ffc1259c Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Fri, 27 Sep 2024 11:50:24 -0700 Subject: [PATCH 2/2] scsi: Add tests and fix refcount bug Adds various tests to the SCSI manager code. As part of this testing, a bug tracking attachment refcounts was also found. The attachment refcount is increased every time a new top-level request comes in, even if an existing mount is re-used. This means that the attachment refcount should also be decremented every time one is released, even if the mount refcount is not at 0. Signed-off-by: Kevin Parsons --- internal/uvm/scsi/manager.go | 7 +- internal/uvm/scsi/manager_test.go | 239 ++++++++++++++++++++++++++++ internal/uvm/scsi/mount.go | 8 +- test/functional/uvm_scratch_test.go | 2 +- test/functional/uvm_scsi_test.go | 6 +- 5 files changed, 248 insertions(+), 14 deletions(-) create mode 100644 internal/uvm/scsi/manager_test.go diff --git a/internal/uvm/scsi/manager.go b/internal/uvm/scsi/manager.go index 50aad34699..ff8374038e 100644 --- a/internal/uvm/scsi/manager.go +++ b/internal/uvm/scsi/manager.go @@ -292,14 +292,9 @@ func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, guestPath 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/manager_test.go b/internal/uvm/scsi/manager_test.go new file mode 100644 index 0000000000..8c52d51b24 --- /dev/null +++ b/internal/uvm/scsi/manager_test.go @@ -0,0 +1,239 @@ +//go:build windows + +package scsi + +import ( + "context" + "reflect" + "testing" +) + +func removeIndex[T any](a []T, i int) []T { + return append(a[:i], a[i+1:]...) +} + +func TestRemoveIndex(t *testing.T) { + a := []int{1, 2, 3, 4, 5} + a = removeIndex(a, 2) + if !reflect.DeepEqual(a, []int{1, 2, 4, 5}) { + t.Errorf("wrong values: %v", a) + } + a = removeIndex(a, 1) + if !reflect.DeepEqual(a, []int{1, 4, 5}) { + t.Errorf("wrong values: %v", a) + } + a = removeIndex(a, 0) + if !reflect.DeepEqual(a, []int{4, 5}) { + t.Errorf("wrong values: %v", a) + } + a = removeIndex(a, 1) + if !reflect.DeepEqual(a, []int{4}) { + t.Errorf("wrong values: %v", a) + } + a = removeIndex(a, 0) + if !reflect.DeepEqual(a, []int{}) { + t.Errorf("wrong values: %v", a) + } +} + +type testAttachment struct { + controller uint + lun uint + config *attachConfig +} + +type testMount struct { + controller uint + lun uint + path string + config *mountConfig +} + +type hostBackend struct { + attachments []*testAttachment +} + +func (hb *hostBackend) attach(ctx context.Context, controller uint, lun uint, config *attachConfig) error { + hb.attachments = append(hb.attachments, &testAttachment{ + controller: controller, + lun: lun, + config: config, + }) + return nil +} + +func (hb *hostBackend) detach(ctx context.Context, controller uint, lun uint) error { + for i, a := range hb.attachments { + if a.controller == controller && a.lun == lun { + hb.attachments = removeIndex(hb.attachments, i) + break + } + } + return nil +} + +func (hb *hostBackend) attachmentPaths() []string { + ret := []string{} + for _, a := range hb.attachments { + ret = append(ret, a.config.path) + } + return ret +} + +type guestBackend struct { + mounts []*testMount +} + +func (gb *guestBackend) mount(ctx context.Context, controller uint, lun uint, path string, config *mountConfig) error { + gb.mounts = append(gb.mounts, &testMount{ + controller: controller, + lun: lun, + path: path, + config: config, + }) + return nil +} + +func (gb *guestBackend) unmount(ctx context.Context, controller uint, lun uint, path string, config *mountConfig) error { + for i, m := range gb.mounts { + if m.path == path { + gb.mounts = removeIndex(gb.mounts, i) + break + } + } + return nil +} + +func (gb *guestBackend) unplug(ctx context.Context, controller uint, lun uint) error { + return nil +} + +func (gb *guestBackend) mountPaths() []string { + ret := []string{} + for _, m := range gb.mounts { + ret = append(ret, m.path) + } + return ret +} + +func TestAddAddRemoveRemove(t *testing.T) { + ctx := context.Background() + + hb := &hostBackend{} + gb := &guestBackend{} + mgr, err := NewManager(hb, gb, 4, 64, "/var/run/scsi/%d", nil) + if err != nil { + t.Fatal(err) + } + m1, err := mgr.AddVirtualDisk(ctx, "path", true, "", "", &MountConfig{}) + if err != nil { + t.Fatal(err) + } + m2, err := mgr.AddVirtualDisk(ctx, "path", true, "", "", &MountConfig{}) + if err != nil { + t.Fatal(err) + } + if m1.GuestPath() == "" { + t.Error("guest path for m1 should not be empty") + } + if m1.GuestPath() != m2.GuestPath() { + t.Errorf("expected same guest paths for both mounts, but got %q and %q", m1.GuestPath(), m2.GuestPath()) + } + if !reflect.DeepEqual(hb.attachmentPaths(), []string{"path"}) { + t.Errorf("wrong attachment paths after add: %v", hb.attachmentPaths()) + } + if err := m1.Release(ctx); err != nil { + t.Fatal(err) + } + if err := m2.Release(ctx); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(hb.attachmentPaths(), []string{}) { + t.Errorf("wrong attachment paths after remove: %v", hb.attachmentPaths()) + } +} + +func TestGuestPath(t *testing.T) { + ctx := context.Background() + + hb := &hostBackend{} + gb := &guestBackend{} + mgr, err := NewManager(hb, gb, 4, 64, "/var/run/scsi/%d", nil) + if err != nil { + t.Fatal(err) + } + + m1, err := mgr.AddVirtualDisk(ctx, "path", true, "", "/mnt1", &MountConfig{}) + if err != nil { + t.Fatal(err) + } + // m1 should get the guest path it asked for. + if m1.GuestPath() != "/mnt1" { + t.Errorf("wrong guest path for m1: %s", m1.GuestPath()) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{"/mnt1"}) { + t.Errorf("wrong mount paths after adding m1: %v", gb.mountPaths()) + } + + m2, err := mgr.AddVirtualDisk(ctx, "path", true, "", "", &MountConfig{}) + if err != nil { + t.Fatal(err) + } + // m2 didn't ask for a guest path, so it should re-use m1. + if m2.GuestPath() != "/mnt1" { + t.Errorf("wrong guest path for m2: %s", m2.GuestPath()) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{"/mnt1"}) { + t.Errorf("wrong mount paths after adding m2: %v", gb.mountPaths()) + } + + m3, err := mgr.AddVirtualDisk(ctx, "path", true, "", "/mnt2", &MountConfig{}) + if err != nil { + t.Fatal(err) + } + // m3 should get the guest path it asked for. + if m3.GuestPath() != "/mnt2" { + t.Errorf("wrong guest path for m3: %s", m2.GuestPath()) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{"/mnt1", "/mnt2"}) { + t.Errorf("wrong mount paths after adding m3: %v", gb.mountPaths()) + } + + if err := m3.Release(ctx); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{"/mnt1"}) { + t.Errorf("wrong mount paths after removing m3: %v", gb.mountPaths()) + } + if err := m2.Release(ctx); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{"/mnt1"}) { + t.Errorf("wrong mount paths after removing m2: %v", gb.mountPaths()) + } + if err := m1.Release(ctx); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(gb.mountPaths(), []string{}) { + t.Errorf("wrong mount paths after removing m1: %v", gb.mountPaths()) + } +} + +func TestConflictingGuestPath(t *testing.T) { + ctx := context.Background() + + hb := &hostBackend{} + gb := &guestBackend{} + mgr, err := NewManager(hb, gb, 4, 64, "/var/run/scsi/%d", nil) + if err != nil { + t.Fatal(err) + } + + if _, err := mgr.AddVirtualDisk(ctx, "path", true, "", "/mnt1", &MountConfig{}); err != nil { + t.Fatal(err) + } + // Different host path but same guest path as m1, should conflict. + if _, err := mgr.AddVirtualDisk(ctx, "path2", true, "", "/mnt1", &MountConfig{}); err == nil { + t.Fatalf("expected error but got none") + } +} diff --git a/internal/uvm/scsi/mount.go b/internal/uvm/scsi/mount.go index df5651e008..1f2fb831cc 100644 --- a/internal/uvm/scsi/mount.go +++ b/internal/uvm/scsi/mount.go @@ -87,7 +87,7 @@ func (mm *mountManager) mount(ctx context.Context, controller, lun uint, path st return mount.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() @@ -100,15 +100,15 @@ func (mm *mountManager) unmount(ctx context.Context, path string) (bool, error) mount.refCount-- if mount.refCount > 0 { - return false, nil + return nil } 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 + return nil } func (mm *mountManager) trackMount(controller, lun uint, path string, c *mountConfig) (*mount, bool, error) { diff --git a/test/functional/uvm_scratch_test.go b/test/functional/uvm_scratch_test.go index ada16b2fea..76213b2475 100644 --- a/test/functional/uvm_scratch_test.go +++ b/test/functional/uvm_scratch_test.go @@ -49,7 +49,7 @@ func TestScratchCreateLCOW(t *testing.T) { } // Make sure it can be added (verifies it has access correctly) - scsiMount, err := targetUVM.SCSIManager.AddVirtualDisk(context.Background(), destTwo, false, targetUVM.ID(), nil) + scsiMount, err := targetUVM.SCSIManager.AddVirtualDisk(context.Background(), destTwo, false, targetUVM.ID(), "", nil) if err != nil { t.Fatal(err) } diff --git a/test/functional/uvm_scsi_test.go b/test/functional/uvm_scsi_test.go index 1bf06a64b2..7ffc5ed550 100644 --- a/test/functional/uvm_scsi_test.go +++ b/test/functional/uvm_scsi_test.go @@ -66,7 +66,7 @@ func testAddSCSI(u *uvm.UtilityVM, disks []string, attachOnly bool) ([]*scsi.Mou if !attachOnly { mc = &scsi.MountConfig{} } - scsiMount, err := u.SCSIManager.AddVirtualDisk(context.Background(), disks[i], false, u.ID(), mc) + scsiMount, err := u.SCSIManager.AddVirtualDisk(context.Background(), disks[i], false, u.ID(), "", mc) if err != nil { return nil, err } @@ -280,7 +280,7 @@ func TestParallelScsiOps(t *testing.T) { continue } - mount, err := u.SCSIManager.AddVirtualDisk(context.Background(), path, false, u.ID(), nil) + mount, err := u.SCSIManager.AddVirtualDisk(context.Background(), path, false, u.ID(), "", nil) if err != nil { os.Remove(path) t.Errorf("failed to add SCSI disk for worker: %d, iteration: %d with err: %v", scsiIndex, iteration, err) @@ -293,7 +293,7 @@ func TestParallelScsiOps(t *testing.T) { break } - mount, err = u.SCSIManager.AddVirtualDisk(context.Background(), path, false, u.ID(), &scsi.MountConfig{}) + mount, err = u.SCSIManager.AddVirtualDisk(context.Background(), path, false, u.ID(), "", &scsi.MountConfig{}) if err != nil { os.Remove(path) t.Errorf("failed to add SCSI disk for worker: %d, iteration: %d with err: %v", scsiIndex, iteration, err)