Skip to content

Commit

Permalink
process review feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
preslavgerchev committed May 28, 2024
1 parent c2c7e5b commit 370b91f
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 24 deletions.
2 changes: 1 addition & 1 deletion providers/os/connection/device/linux/device_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (c *LinuxDeviceManager) identifyViaDeviceName(deviceName string) (*snapshot
// if we don't have a device name we can just return the first non-boot, non-mounted partition.
// this is a best-guess approach
if deviceName == "" {
// TOOD: we should rename/simplify this method
// TODO: we should rename/simplify this method
return blockDevices.GetUnnamedBlockEntry()
}

Expand Down
11 changes: 3 additions & 8 deletions providers/os/connection/device/linux/lun.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,19 @@ func findMatchingDeviceByBlock(scsiDevices scsiDevices, blockDevices *snapshot.B
return snapshot.BlockDevice{}, errors.New("no matching blocks found")
}

var targetDevice snapshot.BlockDevice

for _, b := range matchingBlocks {
log.Debug().Str("name", b.Name).Msg("device connection> checking block")
for _, ch := range b.Children {
if len(ch.MountPoint) > 0 && ch.MountPoint != "" {
log.Debug().Str("name", ch.Name).Msg("device connection> has mounted partitons, skipping")
} else {
targetDevice = b
// we found a block that has no mounted partitions
return b, nil
}
}
}

if len(targetDevice.Name) == 0 {
return snapshot.BlockDevice{}, errors.New("no matching block found")
}

return targetDevice, nil
return snapshot.BlockDevice{}, errors.New("no matching block found")
}

// parses the output from running 'lsscsi --brief' and gets the device info, the output looks like this:
Expand Down
26 changes: 26 additions & 0 deletions providers/os/connection/device/linux/lun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,30 @@ func TestFindDeviceByBlock(t *testing.T) {
_, err := findMatchingDeviceByBlock(devices, blockDevices)
assert.Error(t, err)
})
t.Run("empty target as all blocks are mounted", func(t *testing.T) {
blockDevices := &snapshot.BlockDevices{
BlockDevices: []snapshot.BlockDevice{
{
Name: "sda",
Children: []snapshot.BlockDevice{
{
Name: "sda1",
MountPoint: "/",
},
},
},
{
Name: "sdb",
Children: []snapshot.BlockDevice{
{
Name: "sdb1",
MountPoint: "/tmp",
},
},
},
},
}
_, err := findMatchingDeviceByBlock(devices, blockDevices)
assert.Error(t, err)
})
}
15 changes: 6 additions & 9 deletions providers/os/connection/snapshot/blockdevices.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,20 @@ func (blockEntries BlockDevices) GetMountablePartitionByDevice(device string) (*
}

// Searches for a device by name
func (blockEntries BlockDevices) FindDevice(device string) (BlockDevice, error) {
log.Debug().Str("device", device).Msg("searching for device")
func (blockEntries BlockDevices) FindDevice(name string) (BlockDevice, error) {
log.Debug().Str("device", name).Msg("searching for device")
var secondName string
if strings.HasPrefix(device, "/dev/sd") {
if strings.HasPrefix(name, "/dev/sd") {
// sdh and xvdh are interchangeable
end := strings.TrimPrefix(device, "/dev/sd")
end := strings.TrimPrefix(name, "/dev/sd")
secondName = "/dev/xvd" + end
}
for i := range blockEntries.BlockDevices {
d := blockEntries.BlockDevices[i]
log.Debug().Str("name", d.Name).Interface("children", d.Children).Interface("mountpoint", d.MountPoint).Msg("found block device")
fullDeviceName := "/dev/" + d.Name
if device != fullDeviceName { // check if the device name matches
if secondName == "" {
continue
}
if secondName != fullDeviceName { // check if the device name matches the second name option (sdh and xvdh are interchangeable)
if name != fullDeviceName { // check if the device name matches
if secondName == "" || secondName != fullDeviceName {
continue
}
}
Expand Down
12 changes: 6 additions & 6 deletions providers/os/connection/snapshot/volumemounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,13 @@ func (m *VolumeMounter) GetMountablePartition(device string) (*PartitionInfo, er
// TODO: i dont know what the difference between GetUnnamedBlockEntry and GetUnmountedBlockEntry is
// we need to simplify those
return blockDevices.GetUnnamedBlockEntry()
} else {
d, err := blockDevices.FindDevice(device)
if err != nil {
return nil, err
}
return d.GetMountablePartition()
}

d, err := blockDevices.FindDevice(device)
if err != nil {
return nil, err
}
return d.GetMountablePartition()
}

func (m *VolumeMounter) mountVolume(fsInfo *PartitionInfo) error {
Expand Down

0 comments on commit 370b91f

Please sign in to comment.