Merge pull request #100894 from clickyotomy/sk/loop-dev-sysfs

Handle invalid `losetup' options

pkg/volume/storageos/storageos_util.go

@@ -19,6 +19,7 @@ package storageos
 import (
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -346,7 +347,7 @@ func pathDeviceType(path string) (deviceType, error) {
 // attachFileDevice takes a path to a regular file and makes it available as an
 // attached block device.
 func attachFileDevice(path string, exec utilexec.Interface) (string, error) {
-	blockDevicePath, err := getLoopDevice(path, exec)
+	blockDevicePath, err := getLoopDevice(path)
 	if err != nil && err.Error() != ErrDeviceNotFound {
 		return "", err
 	}
@@ -363,7 +364,7 @@ func attachFileDevice(path string, exec utilexec.Interface) (string, error) {
 }
 
 // Returns the full path to the loop device associated with the given path.
-func getLoopDevice(path string, exec utilexec.Interface) (string, error) {
+func getLoopDevice(path string) (string, error) {
 	_, err := os.Stat(path)
 	if os.IsNotExist(err) {
 		return "", errors.New(ErrNotAvailable)
@@ -372,23 +373,18 @@ func getLoopDevice(path string, exec utilexec.Interface) (string, error) {
 		return "", fmt.Errorf("not attachable: %v", err)
 	}
 
-	args := []string{"-j", path}
+	return getLoopDeviceFromSysfs(path)
-	out, err := exec.Command(losetupPath, args...).CombinedOutput()
-	if err != nil {
-		klog.V(2).Infof("Failed device discover command for path %s: %v", path, err)
-		return "", err
-	}
-	return parseLosetupOutputForDevice(out)
 }
 
 func makeLoopDevice(path string, exec utilexec.Interface) (string, error) {
-	args := []string{"-f", "-P", "--show", path}
+	args := []string{"-f", "-P", path}
 	out, err := exec.Command(losetupPath, args...).CombinedOutput()
 	if err != nil {
-		klog.V(2).Infof("Failed device create command for path %s: %v", path, err)
+		klog.V(2).Infof("Failed device create command for path %s: %v %s", path, err, out)
 		return "", err
 	}
-	return parseLosetupOutputForDevice(out)
+
+	return getLoopDeviceFromSysfs(path)
 }
 
 func removeLoopDevice(device string, exec utilexec.Interface) error {
@@ -406,16 +402,35 @@ func isLoopDevice(device string) bool {
 	return strings.HasPrefix(device, "/dev/loop")
 }
 
-func parseLosetupOutputForDevice(output []byte) (string, error) {
+// getLoopDeviceFromSysfs finds the backing file for a loop
-	if len(output) == 0 {
+// device from sysfs via "/sys/block/loop*/loop/backing_file".
+func getLoopDeviceFromSysfs(path string) (string, error) {
+	// If the file is a symlink.
+	realPath, err := filepath.EvalSymlinks(path)
+	if err != nil {
 		return "", errors.New(ErrDeviceNotFound)
 	}
 
-	// losetup returns device in the format:
+	devices, err := filepath.Glob("/sys/block/loop*")
-	// /dev/loop1: [0073]:148662 (/var/lib/storageos/volumes/308f14af-cf0a-08ff-c9c3-b48104318e05)
+	if err != nil {
-	device := strings.TrimSpace(strings.SplitN(string(output), ":", 2)[0])
-	if len(device) == 0 {
 		return "", errors.New(ErrDeviceNotFound)
 	}
-	return device, nil
+
+	for _, device := range devices {
+		backingFile := fmt.Sprintf("%s/loop/backing_file", device)
+
+		// The contents of this file is the absolute path of "path".
+		data, err := ioutil.ReadFile(backingFile)
+		if err != nil {
+			continue
+		}
+
+		// Return the first match.
+		backingFilePath := strings.TrimSpace(string(data))
+		if backingFilePath == path || backingFilePath == realPath {
+			return fmt.Sprintf("/dev/%s", filepath.Base(device)), nil
+		}
+	}
+
+	return "", errors.New(ErrDeviceNotFound)
 }

pkg/volume/util/volumepathhandler/volume_path_handler_linux.go

@@ -19,9 +19,9 @@ limitations under the License.
 package volumepathhandler
 
 import (
-	"bufio"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -83,32 +83,20 @@ func (v VolumePathHandler) GetLoopDevice(path string) (string, error) {
 		return "", fmt.Errorf("not attachable: %v", err)
 	}
 
-	args := []string{"-j", path}
+	return getLoopDeviceFromSysfs(path)
-	cmd := exec.Command(losetupPath, args...)
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		klog.V(2).Infof("Failed device discover command for path %s: %v %s", path, err, out)
-		return "", fmt.Errorf("losetup -j %s failed: %v", path, err)
-	}
-	return parseLosetupOutputForDevice(out, path)
 }
 
 func makeLoopDevice(path string) (string, error) {
-	args := []string{"-f", "--show", path}
+	args := []string{"-f", path}
 	cmd := exec.Command(losetupPath, args...)
+
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		klog.V(2).Infof("Failed device create command for path: %s %v %s ", path, err, out)
+		klog.V(2).Infof("Failed device create command for path: %s %v %s", path, err, out)
-		return "", fmt.Errorf("losetup -f --show %s failed: %v", path, err)
+		return "", fmt.Errorf("losetup %s failed: %v", strings.Join(args, " "), err)
 	}
 
-	// losetup -f --show {path} returns device in the format:
+	return getLoopDeviceFromSysfs(path)
-	// /dev/loop1
-	if len(out) == 0 {
-		return "", errors.New(ErrDeviceNotFound)
-	}
-
-	return strings.TrimSpace(string(out)), nil
 }
 
 // removeLoopDevice removes specified loopback device
@@ -126,51 +114,37 @@ func removeLoopDevice(device string) error {
 	return nil
 }
 
-func parseLosetupOutputForDevice(output []byte, path string) (string, error) {
+// getLoopDeviceFromSysfs finds the backing file for a loop
-	if len(output) == 0 {
+// device from sysfs via "/sys/block/loop*/loop/backing_file".
-		return "", errors.New(ErrDeviceNotFound)
+func getLoopDeviceFromSysfs(path string) (string, error) {
-	}
+	// If the file is a symlink.
-
 	realPath, err := filepath.EvalSymlinks(path)
 	if err != nil {
 		return "", fmt.Errorf("failed to evaluate path %s: %s", path, err)
 	}
 
-	// losetup -j {path} returns device in the format:
+	devices, err := filepath.Glob("/sys/block/loop*")
-	// /dev/loop1: [0073]:148662 ({path})
+	if err != nil {
-	// /dev/loop2: [0073]:148662 (/dev/sdX)
+		return "", fmt.Errorf("failed to list loop devices in sysfs: %s", err)
-	//
-	// losetup -j shows all the loop device for the same device that has the same
-	// major/minor number, by resolving symlink and matching major/minor number.
-	// Therefore, there will be other path than {path} in output, as shown in above output.
-	s := string(output)
-	// Find the line that exact matches to the path, or "({path})"
-	var matched string
-	scanner := bufio.NewScanner(strings.NewReader(s))
-	for scanner.Scan() {
-		// losetup output has symlinks expanded
-		if strings.HasSuffix(scanner.Text(), "("+realPath+")") {
-			matched = scanner.Text()
-			break
-		}
-		// Just in case losetup changes, check for the original path too
-		if strings.HasSuffix(scanner.Text(), "("+path+")") {
-			matched = scanner.Text()
-			break
-		}
 	}
-	if len(matched) == 0 {
-		return "", errors.New(ErrDeviceNotFound)
-	}
-	s = matched
 
-	// Get device name, or the 0th field of the output separated with ":".
+	for _, device := range devices {
-	// We don't need 1st field or later to be splitted, so passing 2 to SplitN.
+		backingFile := fmt.Sprintf("%s/loop/backing_file", device)
-	device := strings.TrimSpace(strings.SplitN(s, ":", 2)[0])
+
-	if len(device) == 0 {
+		// The contents of this file is the absolute path of "path".
-		return "", errors.New(ErrDeviceNotFound)
+		data, err := ioutil.ReadFile(backingFile)
+		if err != nil {
+			continue
+		}
+
+		// Return the first match.
+		backingFilePath := strings.TrimSpace(string(data))
+		if backingFilePath == path || backingFilePath == realPath {
+			return fmt.Sprintf("/dev/%s", filepath.Base(device)), nil
+		}
 	}
-	return device, nil
+
+	return "", errors.New(ErrDeviceNotFound)
 }
 
 // FindGlobalMapPathUUIDFromPod finds {pod uuid} bind mount under globalMapPath