Move loop device creation to CreateVolume

This creates the loop device at CreateVolume and uses the loop device
name as the volume ID.
Also, replaces dd with fallocate to create block files.

Author: darkowlzz

pkg/hostpath/controllerserver.go

@@ -20,8 +20,10 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"path/filepath"
 	"sort"
 	"strconv"
+	"strings"
 
 	"github.com/golang/protobuf/ptypes"
 
@@ -133,21 +135,38 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	if capacity >= maxStorageCapacity {
 		return nil, status.Errorf(codes.OutOfRange, "Requested capacity %d exceeds maximum allowed %d", capacity, maxStorageCapacity)
 	}
-	volumeID := uuid.NewUUID().String()
-	path := provisionRoot + volumeID
+
+	var volumeID, path string
 
 	switch requestedAccessType {
 	case blockAccess:
+		// Get a free loop device.
 		executor := utilexec.New()
-		of := fmt.Sprintf("%s=%s", "of", path)
-		count := fmt.Sprintf("%s=%d", "count", capacity/mib)
+		out, err := executor.Command("losetup", "-f").CombinedOutput()
+		if err != nil {
+			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get a free loop device: %v: %s", err, out))
+		}
+		loopDevice := strings.TrimSpace(string(out))
+
+		// Use the loop device name as the volume ID.
+		volumeID = filepath.Base(loopDevice)
+		path = provisionRoot + volumeID
+		size := fmt.Sprintf("%dM", capacity/mib)
 		// Create a block file.
-		out, err := executor.Command("dd", "if=/dev/zero", of, "bs=1M", count).CombinedOutput()
+		out, err = executor.Command("fallocate", "-l", size, path).CombinedOutput()
 		if err != nil {
 			glog.V(3).Infof("failed to create block device: %v", string(out))
 			return nil, err
 		}
+
+		// Associate block file with the loop device.
+		out, err = executor.Command("losetup", loopDevice, path).CombinedOutput()
+		if err != nil {
+			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to associate loop device with block file: %v: %s", err, out))
+		}
 	case mountAccess:
+		volumeID = uuid.NewUUID().String()
+		path = provisionRoot + volumeID
 		err := os.MkdirAll(path, 0777)
 		if err != nil {
 			glog.V(3).Infof("failed to create volume: %v", err)
@@ -205,6 +224,21 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 	}
 	volumeID := req.VolumeId
 	glog.V(4).Infof("deleting volume %s", volumeID)
+
+	// Create the loop device path using volume ID.
+	loopDevicePath := filepath.Join("/dev", volumeID)
+	// Disassociate the loop device.
+	executor := utilexec.New()
+	out, err := executor.Command("losetup", "-d", loopDevicePath).CombinedOutput()
+	if err != nil {
+		if strings.Contains(string(out), "No such device or address") {
+			// The loop device is no longer associated. Do nothing.
+		} else {
+			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to disassociate loop device: %v: %s", err, out))
+		}
+	}
+
+	// Delete the block file.
 	path := provisionRoot + volumeID
 	os.RemoveAll(path)
 	delete(hostPathVolumes, volumeID)

pkg/hostpath/hostpath.go

@@ -50,7 +50,6 @@ type hostPathVolume struct {
 	VolSize       int64      `json:"volSize"`
 	VolPath       string     `json:"volPath"`
 	VolAccessType accessType `json:"volAccessType"`
-	VolLoopDevice string     `json:"volLoopDevice"`
 }
 
 type hostPathSnapshot struct {

pkg/hostpath/nodeserver.go

@@ -19,7 +19,7 @@ package hostpath
 import (
 	"fmt"
 	"os"
-	"strings"
+	"path/filepath"
 
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
@@ -71,36 +71,45 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			return nil, status.Error(codes.InvalidArgument, "cannot publish a non-block volume as block volume")
 		}
 
+		// Create the loop device path using volume ID.
+		loopDevice := filepath.Join("/dev", vol.VolID)
+
 		executor := utilexec.New()
-		// Get a free loop device.
-		out, err := executor.Command("losetup", "-f").CombinedOutput()
-		if err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get a free loop device: %v: %s", err, out))
-		}
-		loopDevice := strings.TrimSpace(string(out))
 
-		losetupArgs := []string{}
-		if req.GetReadonly() {
-			losetupArgs = append(losetupArgs, "-r")
+		// Get FileInfo to check if the file already exists.
+		fi, err := os.Lstat(targetPath)
+		if os.IsNotExist(err) {
+			// The target path does not exists. Create a symlink.
+			out, err := executor.Command("ln", "-s", loopDevice, targetPath).CombinedOutput()
+			if err != nil {
+				return nil, status.Error(codes.Internal, fmt.Sprintf("failed to create symlink to target path: %v: %s", err, out))
+			}
+			return &csi.NodePublishVolumeResponse{}, nil
 		}
-		// Append loop device and file path.
-		losetupArgs = append(losetupArgs, loopDevice, vol.VolPath)
-
-		// Associate block file with the loop device.
-		out, err = executor.Command("losetup", losetupArgs...).CombinedOutput()
 		if err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to associate loop device with block file: %v: %s", err, out))
+			return nil, status.Errorf(codes.Internal, "failed to check if the target path exists: %v", err)
 		}
 
-		// Create symlink to the target path.
-		out, err = executor.Command("ln", "-s", loopDevice, targetPath).CombinedOutput()
-		if err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to create symlink to target path: %v: %s", err, out))
-		}
+		// If it's a symlink, check if it resolves to the same loop device.
+		if fi.Mode()&os.ModeSymlink != 0 {
+			link, err := os.Readlink(fi.Name())
+			if err != nil {
+				return nil, status.Errorf(codes.Internal, "failed to resolve the target path symbolic link: %v", err)
+			}
 
-		// Update the volume info.
-		vol.VolLoopDevice = loopDevice
-		hostPathVolumes[vol.VolID] = vol
+			if link != loopDevice {
+				return nil, status.Errorf(codes.AlreadyExists, "target path linked to another device(%s)", link)
+			}
+			// Do nothing. Symlink already exists with the right loop device.
+		} else {
+			// A regular file exists. This could be a left over file that failed
+			// to delete while unpublishing. Reuse it by forcefully creating
+			// symlink to it.
+			out, err := executor.Command("ln", "-s", "-f", loopDevice, targetPath).CombinedOutput()
+			if err != nil {
+				return nil, status.Error(codes.Internal, fmt.Sprintf("failed to forcefully create symlink to an existing target file: %v: %s", err, out))
+			}
+		}
 	} else if req.GetVolumeCapability().GetMount() != nil {
 		if vol.VolAccessType != mountAccess {
 			return nil, status.Error(codes.InvalidArgument, "cannot publish a non-mount volume as mount volume")
@@ -171,18 +180,10 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu
 	switch vol.VolAccessType {
 	case blockAccess:
 		// Remove the symlink.
-		os.RemoveAll(targetPath)
-
-		// Disassociate the loop device.
-		executor := utilexec.New()
-		out, err := executor.Command("losetup", "-d", vol.VolLoopDevice).CombinedOutput()
+		err = os.RemoveAll(targetPath)
 		if err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to disassociate loop device: %v: %s", err, out))
+			return nil, status.Error(codes.Internal, err.Error())
 		}
-
-		// Update the volume info.
-		vol.VolLoopDevice = ""
-		hostPathVolumes[vol.VolID] = vol
 		glog.V(4).Infof("hostpath: volume %s has been unpublished.", targetPath)
 	case mountAccess:
 		// Unmounting the image