Skip to content

Commit

Permalink
Refactor loop.AttachFromFile
Browse files Browse the repository at this point in the history 
The loop.AttachFromFile code, used to attach an image to a free loop
device, or to find a shared loop device has been refactored.

Previously, a complex loop handled both finding shared loop devices
and attaching to new free devices. The logic was difficult to follow,
especially with regard to tracking which errors were fatal and would
bubble up, vs result in loop continuation, or retries.

The shared and 'fresh' attachment have been separated into two
functions. Some additional logging / error detail has been added.

The test code has been modified to prevent a leak of a loop device.

Fixes sylabs#463
  • Loading branch information
@dtrudg
dtrudg committed Dec 8, 2021
1 parent f68750d commit 266e3dd
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 84 deletions.
234 changes: 151 additions & 83 deletions pkg/util/loop/loop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,38 +79,28 @@ type Info64 struct {
Init [2]uint64
}

var errTransientAttach = errors.New("failed to successfully allocate a loop device (please retry)")
// errTransientAttach is used to indicate hitting errors within loop device setup that are transient.
// These may be cleared by our automatic retries, or by the user re-running.
var errTransientAttach = errors.New("transient error, please retry")

// Error retry attempts & interval
const (
maxRetries = 5
retryInterval = 250 * time.Millisecond
)

// AttachFromFile attempts to find a suitable loop device to use for the specified image.
// It runs through /dev/loopXX, up to MaxLoopDevices to find a free loop device, or
// to share a loop device already associated to file (if shared loop devices are enabled).
// If a usable loop device is found, then loop.Fd is set and no error is returned.
// If a usable loop device is not found, and this is due to a transient EAGAIN / EBUSY error,
// then it will retry up to maxRetries times, retryInterval apart, before returning an error.
func (loop *Device) AttachFromFile(image *os.File, mode int, number *int) error {
maxRetries := 5
var err error

for i := 0; i < maxRetries; i++ {
err := loop.attachFromFile(image, mode, number)
if err != nil {
if !errors.Is(err, errTransientAttach) {
return err
}
sylog.Debugf("Transient error detected: %s", err)
time.Sleep(250 * time.Millisecond)
} else {
return nil
}
}
return fmt.Errorf("failed to set loop flags: %s", err)
}

// attachFromFile finds a free loop device, opens it, and stores file descriptor
// provided by image file pointer
func (loop *Device) attachFromFile(image *os.File, mode int, number *int) error {
var path string
var loopFd int
var transientErrorFound bool

if image == nil {
return fmt.Errorf("empty file pointer")
}

fi, err := image.Stat()
if err != nil {
return err
Expand All @@ -120,77 +110,112 @@ func (loop *Device) attachFromFile(image *os.File, mode int, number *int) error
// cast to uint64 as st.Dev is uint32 on MIPS
imageDev := uint64(st.Dev)

if loop.Shared {
ok, err := loop.shareLoop(imageIno, imageDev, mode, number)
if err != nil {
return err
}
// We found a shared loop device, and loop.Fd was set
if ok {
return nil
}
loop.Shared = false
}

for i := 0; i < maxRetries; i++ {
err = loop.attachLoop(image, mode, number)
if err == nil {
return nil
}
if !errors.Is(err, errTransientAttach) {
return err
}
// At least one error while we were working through loop devices was a transient one
// that should resolve by itself, so let's try again!
sylog.Debugf("%v", err)
time.Sleep(retryInterval)
}
return fmt.Errorf("failed to attach loop device: %s", err)
}

// shareLoop runs over /dev/loopXX devices, looking for one that already has our image attached.
// If a loop device can be shared, loop.Fd is set, and ok will be true.
// If no loop device can be shared, ok will be false.
func (loop *Device) shareLoop(imageIno, imageDev uint64, mode int, number *int) (ok bool, err error) {
// Because we hold a lock on /dev here, avoid delayed retries inside this function,
// as it could impact parallel startup of many instances of Singularity or
// other programs.
fd, err := lock.Exclusive("/dev")
if err != nil {
return err
return false, err
}
defer lock.Release(fd)

freeDevice := -1

for device := 0; device <= loop.MaxLoopDevices; device++ {
for device := 0; device < loop.MaxLoopDevices; device++ {
*number = device

if device == loop.MaxLoopDevices {
if loop.Shared {
loop.Shared = false
if freeDevice != -1 {
device = freeDevice
continue
}
}
if transientErrorFound {
return fmt.Errorf("%w", errTransientAttach)
// Try to open an existing loop device, but don't create a new one
loopFd, err := openLoopDev(device, mode, false)
if err != nil {
if !os.IsNotExist(err) {
sylog.Debugf("Couldn't open loop device %d: %v\n", device, err)
}
return fmt.Errorf("no loop devices available")
continue
}

path = fmt.Sprintf("/dev/loop%d", device)
if fi, err := os.Stat(path); err != nil {
dev := int((7 << 8) | (device & 0xff) | ((device & 0xfff00) << 12))
esys := syscall.Mknod(path, syscall.S_IFBLK|0o660, dev)
if errno, ok := esys.(syscall.Errno); ok {
if errno != syscall.EEXIST {
return esys
}
}
} else if fi.Mode()&os.ModeDevice == 0 {
return fmt.Errorf("%s is not a block device", path)
status, err := GetStatusFromFd(uintptr(loopFd))
if err != nil {
syscall.Close(loopFd)
sylog.Debugf("Couldn't get status from loop device %d: %v\n", device, err)
continue
}

if loopFd, err = syscall.Open(path, mode, 0o600); err != nil {
continue
if status.Inode == imageIno && status.Device == imageDev &&
status.Flags&FlagsReadOnly == loop.Info.Flags&FlagsReadOnly &&
status.Offset == loop.Info.Offset && status.SizeLimit == loop.Info.SizeLimit {
// keep the reference to the loop device file descriptor to
// be sure that the loop device won't be released between this
// check and the mount of the filesystem
sylog.Debugf("Sharing loop device %d", device)
loop.fd = new(int)
*loop.fd = loopFd
return true, nil
}
if loop.Shared {
status, err := GetStatusFromFd(uintptr(loopFd))
if err != nil {
syscall.Close(loopFd)
sylog.Debugf("Could not get loop device %d status: %s", device, err)
continue
}
// there is no associated image with loop device, save indice so second loop
// iteration will start from this device
if status.Inode == 0 && freeDevice == -1 {
freeDevice = device
syscall.Close(loopFd)
continue
}
if status.Inode == imageIno && status.Device == imageDev &&
status.Flags&FlagsReadOnly == loop.Info.Flags&FlagsReadOnly &&
status.Offset == loop.Info.Offset && status.SizeLimit == loop.Info.SizeLimit {
// keep the reference to the loop device file descriptor to
// be sure that the loop device won't be released between this
// check and the mount of the filesystem
sylog.Debugf("Sharing loop device %d", device)
loop.fd = new(int)
*loop.fd = loopFd
return nil
}
syscall.Close(loopFd)
continue
syscall.Close(loopFd)
}
return false, nil
}

// attachLoop will find a free /dev/loopXX device, or create a new one, and attach image to it.
// For most failures with loopN, it will try loopN+1, continuing up to loop.MaxLoopDevices.
// If there was an EAGAIN/EBUSY error on setting loop flags this is transient, and the returned
// errTransientAttach indicates it is likely worth trying again.
func (loop *Device) attachLoop(image *os.File, mode int, number *int) error {
var path string
// Keep track of the last transient error we hit (if any)
// If we fail to find a loop device, but hit at least one transient error then it's worth trying again.
var transientError error

// Because we hold a lock on /dev here, avoid delayed retries inside this function,
// as it could impact parallel startup of many instances of Singularity or
// other programs.
fd, err := lock.Exclusive("/dev")
if err != nil {
return err
}
defer lock.Release(fd)

for device := 0; device < loop.MaxLoopDevices; device++ {
*number = device

// Try to open the loop device, creating the device node if needed
loopFd, err := openLoopDev(device, mode, true)
if err != nil {
sylog.Debugf("couldn't openLoopDev loop device %d: %v", device, err)
}

_, _, esys := syscall.Syscall(syscall.SYS_IOCTL, uintptr(loopFd), CmdSetFd, image.Fd())
// On error, we'll move on to try the next loop device
if esys != 0 {
syscall.Close(loopFd)
continue
Expand All @@ -201,10 +226,12 @@ func (loop *Device) attachFromFile(image *os.File, mode int, number *int) error
}

if _, _, err := syscall.Syscall(syscall.SYS_IOCTL, uintptr(loopFd), CmdSetStatus64, uintptr(unsafe.Pointer(loop.Info))); err != 0 {
// clear associated file descriptor to release the loop device
// If we hit an error then dissociate our image from the loop device
syscall.Syscall(syscall.SYS_IOCTL, uintptr(loopFd), CmdClrFd, 0)
// EAGAIN and EBUSY will likely clear themselves... so track we hit one and keep trying
if err == syscall.EAGAIN || err == syscall.EBUSY {
transientErrorFound = true
sylog.Debugf("transient error %v for loop device %d, continuing", err, device)
transientError = err
continue
}
return fmt.Errorf("failed to set loop flags on loop device: %s", syscall.Errno(err))
Expand All @@ -215,7 +242,48 @@ func (loop *Device) attachFromFile(image *os.File, mode int, number *int) error
return nil
}

return nil
if transientError != nil {
return fmt.Errorf("%w: %v", errTransientAttach, err)
}

return fmt.Errorf("no loop devices available")
}

// openLoopDev will attempt to open the specified loop device number, with specified mode.
// If it is not present in /dev, and create is true, a mknod call will be used to create it.
// Returns the fd for the opened device, or -1 if it was not possible to openLoopDev it.
func openLoopDev(device, mode int, create bool) (loopFd int, err error) {
path := fmt.Sprintf("/dev/loop%d", device)
fi, err := os.Stat(path)

// If it doesn't exist, and create is false.. we're done..
if os.IsNotExist(err) && !create {
return -1, err
}
// If there's another stat error that's likely fatal.. we're done..
if err != nil && !os.IsNotExist(err) {
return -1, fmt.Errorf("could not stat %s: %w", path, err)
}

// Create the device node if we need to
if os.IsNotExist(err) {
dev := int((7 << 8) | (device & 0xff) | ((device & 0xfff00) << 12))
esys := syscall.Mknod(path, syscall.S_IFBLK|0o660, dev)
if errno, ok := esys.(syscall.Errno); ok {
if errno != syscall.EEXIST {
return -1, fmt.Errorf("could not mknod %s: %w", path, esys)
}
}
} else if fi.Mode()&os.ModeDevice == 0 {
return -1, fmt.Errorf("%s is not a block device", path)
}

// Now open the loop device
loopFd, err = syscall.Open(path, mode, 0o600)
if err != nil {
return -1, fmt.Errorf("could not open %s: %w", path, err)
}
return loopFd, nil
}

// AttachFromPath finds a free loop device, opens it, and stores file descriptor
Expand Down
3 changes: 2 additions & 1 deletion pkg/util/loop/loop_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ func TestLoop(t *testing.T) {
MaxLoopDevices: GetMaxLoopDevices(),
Info: info,
}
defer loopDevOne.Close()
loopDevTwo := &Device{
MaxLoopDevices: GetMaxLoopDevices(),
Info: info,
}
defer loopDevTwo.Close()

loopOne := -1
loopTwo := -1
Expand All @@ -46,7 +48,6 @@ func TestLoop(t *testing.T) {
if err := loopDevOne.AttachFromPath("/etc/passwd", os.O_RDONLY, &loopOne); err != nil {
t.Error(err)
}
defer loopDevOne.Close()

f, err := os.Open("/etc/passwd")
if err != nil {
Expand Down

0 comments on commit 266e3dd

Please sign in to comment.