Skip to content

Commit

Permalink
volume/fc: fix FibreChannel volume plugin matching wrong disks
Browse files Browse the repository at this point in the history
Before:
  findDisk()
    fcPathExp := "^(pci-.*-fc|fc)-0x" + wwn + "-lun-" + lun
After:
  findDisk()
    fcPathExp := "^(pci-.*-fc|fc)-0x" + wwn + "-lun-" + lun + "$"

fc path may have the same wwns but different luns.for example:
pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-1
pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-12

Function findDisk() may mismatch the fc path, return the wrong device and wrong associated devicemapper parent.
This may cause a disater that pods attach wrong disks. Accutally it happended in my testing environment before.
  • Loading branch information
xakdwch committed Jun 23, 2022
1 parent 2314413 commit ddcc448
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 5 deletions.
4 changes: 2 additions & 2 deletions pkg/volume/fc/fc_util.go
Expand Up @@ -62,7 +62,7 @@ func (handler *osIOHandler) WriteFile(filename string, data []byte, perm os.File

// given a wwn and lun, find the device and associated devicemapper parent
func findDisk(wwn, lun string, io ioHandler, deviceUtil volumeutil.DeviceUtil) (string, string) {
fcPathExp := "^(pci-.*-fc|fc)-0x" + wwn + "-lun-" + lun
fcPathExp := "^(pci-.*-fc|fc)-0x" + wwn + "-lun-" + lun + "$"
r := regexp.MustCompile(fcPathExp)
devPath := byPath
if dirs, err := io.ReadDir(devPath); err == nil {
Expand All @@ -71,7 +71,7 @@ func findDisk(wwn, lun string, io ioHandler, deviceUtil volumeutil.DeviceUtil) (
if r.MatchString(name) {
if disk, err1 := io.EvalSymlinks(devPath + name); err1 == nil {
dm := deviceUtil.FindMultipathDeviceForDevice(disk)
klog.Infof("fc: find disk: %v, dm: %v", disk, dm)
klog.Infof("fc: find disk: %v, dm: %v, fc path: %v", disk, dm, name)
return disk, dm
}
}
Expand Down
41 changes: 38 additions & 3 deletions pkg/volume/fc/fc_util_test.go
Expand Up @@ -66,7 +66,16 @@ func (handler *fakeIOHandler) ReadDir(dirname string) ([]os.FileInfo, error) {
f3 := &fakeFileInfo{
name: "abc-0000:41:00.0-fc-0x5005076810213404-lun-0",
}
return []os.FileInfo{f1, f2, f3}, nil
f4 := &fakeFileInfo{
name: "pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-12",
}
f5 := &fakeFileInfo{
name: "pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-1",
}
f6 := &fakeFileInfo{
name: "fc-0x5005076810213b32-lun-25",
}
return []os.FileInfo{f4, f5, f6, f1, f2, f3}, nil
case "/sys/block/":
f := &fakeFileInfo{
name: "dm-1",
Expand All @@ -86,7 +95,21 @@ func (handler *fakeIOHandler) Lstat(name string) (os.FileInfo, error) {
}

func (handler *fakeIOHandler) EvalSymlinks(path string) (string, error) {
return "/dev/sda", nil
switch path {
case "/dev/disk/by-path/pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-0":
return "/dev/sda", nil
case "/dev/disk/by-path/pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-1":
return "/dev/sdb", nil
case "/dev/disk/by-path/fc-0x5005076810213b32-lun-2":
return "/dev/sdc", nil
case "/dev/disk/by-path/pci-0000:41:00.0-fc-0x500a0981891b8dc5-lun-12":
return "/dev/sdl", nil
case "/dev/disk/by-path/fc-0x5005076810213b32-lun-25":
return "/dev/sdx", nil
case "/dev/disk/by-id/scsi-3600508b400105e210000900000490000":
return "/dev/sdd", nil
}
return "", nil
}

func (handler *fakeIOHandler) WriteFile(filename string, data []byte, perm os.FileMode) error {
Expand All @@ -98,17 +121,26 @@ func TestSearchDisk(t *testing.T) {
name string
wwns []string
lun string
disk string
expectError bool
}{
{
name: "PCI disk",
name: "PCI disk 0",
wwns: []string{"500a0981891b8dc5"},
lun: "0",
disk: "/dev/sda",
},
{
name: "PCI disk 1",
wwns: []string{"500a0981891b8dc5"},
lun: "1",
disk: "/dev/sdb",
},
{
name: "Non PCI disk",
wwns: []string{"5005076810213b32"},
lun: "2",
disk: "/dev/sdc",
},
{
name: "Invalid Storage Controller",
Expand Down Expand Up @@ -145,6 +177,9 @@ func TestSearchDisk(t *testing.T) {
if devicePath == "" && !test.expectError {
t.Errorf("no fc disk found")
}
if devicePath != test.disk {
t.Errorf("matching wrong disk, expected: %s, actual: %s", test.disk, devicePath)
}
})
}
}
Expand Down

0 comments on commit ddcc448

Please sign in to comment.