[lxc-devel] [lxd/master] storage: improve storage volume attachment

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 524 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170224/35a0f35c/attachment.bin>
-------------- next part --------------
From c03228448b0b58dbce452bb9a115e20964153902 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 24 Feb 2017 11:49:40 +0100
Subject: [PATCH] storage: improve storage volume attachment

We currently only allow attaching storage volumes of type custom. So remove any
code that gives a different impression. Also, handle things a little nicer.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/container_lxc.go | 70 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 22 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 336d3db..90140ae 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -5891,40 +5891,66 @@ func (c *containerLXC) createDiskDevice(name string, m types.Device) (string, er
 	isFile := false
 	if m["pool"] == "" {
 		isFile = !shared.IsDir(srcPath) && !deviceIsBlockdev(srcPath)
-	}
+	} else {
+		// Deal with mounting storage volumes created via the storage
+		// api. Extract the name of the storage volume that we are
+		// supposed to attach. We assume that the only syntactically
+		// valid ways of specifying a storage volume are:
+		// - <volume_name>
+		// - <type>/<volume_name>
+		// Currently, <type> must either be empty or "custom". We do not
+		// yet support container mounts.
 
-	// Deal with mounting storage volumes created via the storage api.
-	if m["pool"] != "" {
-		// Extract the name of the storage volume that we are supposed
-		// to attach. We assume that the only syntactically valid ways
-		// of specifying a storage volume are:
-		// - vol1
-		// - storage_type/vol1
+		if filepath.IsAbs(m["source"]) {
+			return "", fmt.Errorf("When the \"pool\" property is set \"source\" must specify the name of a volume, not a path.")
+		}
+
+		volumeTypeName := ""
 		volumeName := filepath.Clean(m["source"])
 		slash := strings.Index(volumeName, "/")
 		if (slash > 0) && (len(volumeName) > slash) {
-			volumeName = volumeName[(slash + 1):]
-		}
-
-		// Check if it is the rootfs of another container that we're
-		// supposed to mount. If not it must be a custom volume.
-		volumeType := storagePoolVolumeTypeCustom
-		if strings.HasSuffix(m["source"], storagePoolVolumeApiEndpointContainers+"/") {
-			volumeType = storagePoolVolumeTypeContainer
-			srcPath = shared.VarPath("storage-pools", m["pool"], m["source"])
-		} else {
-			srcPath = shared.VarPath("storage-pools", m["pool"], storagePoolVolumeApiEndpointCustom, m["source"])
+			// Extract volume name.
+			volumeName = m["source"][(slash + 1):]
+			// Extract volume type.
+			volumeTypeName = m["source"][:slash]
+		}
+
+		switch volumeTypeName {
+		case storagePoolVolumeTypeNameContainer:
+			return "", fmt.Errorf("Using container storage volumes is not supported.")
+		case "":
+			// We simply received the name of a storage volume.
+			volumeTypeName = storagePoolVolumeTypeNameCustom
+			fallthrough
+		case storagePoolVolumeTypeNameCustom:
+			srcPath = shared.VarPath("storage-pools", m["pool"], volumeTypeName, volumeName)
+		case storagePoolVolumeTypeNameImage:
+			return "", fmt.Errorf("Using image storage volumes is not supported.")
+		default:
+			return "", fmt.Errorf("Unknown storage type prefix \"%s\" found.", volumeTypeName)
 		}
 
 		// Initialize a new storage interface and check if the
 		// pool/volume is mounted. If it is not, mount it.
+		volumeType, _ := storagePoolVolumeTypeNameToType(volumeTypeName)
 		s, err := storagePoolVolumeInit(c.daemon, m["pool"], volumeName, volumeType)
 		if err != nil && !isOptional {
-			return "", fmt.Errorf("Failed to initialize storage volume \"%s\" on storage pool \"%s\": %s.", volumeName, m["pool"], err)
+			return "", fmt.Errorf("Failed to initialize storage volume \"%s\" of type \"%s\" on storage pool \"%s\": %s.",
+				volumeName,
+				volumeTypeName,
+				m["pool"], err)
 		} else if err == nil {
-			_, err := s.StoragePoolVolumeMount()
+			_, err = s.StoragePoolVolumeMount()
 			if err != nil {
-				shared.LogWarnf("Could not mount storage volume \"%s\" on storage pool \"%s\": %s.", volumeName, m["pool"], err)
+				msg := fmt.Sprintf("Could not mount storage volume \"%s\" of type \"%s\" on storage pool \"%s\": %s.",
+					volumeName,
+					volumeTypeName,
+					m["pool"])
+				if !isOptional {
+					shared.LogErrorf(msg)
+					return "", err
+				}
+				shared.LogWarnf(msg)
 			}
 		}
 	}