[lxc-devel] [lxd/master] storage: property inheritance

brauner on Github lxc-bot at linuxcontainers.org
Thu Feb 23 00:34:26 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170223/9fc0f836/attachment.bin>
-------------- next part --------------
From 726a589fbba4bfbd2246ad4c06d8f8898df0cc66 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 23 Feb 2017 01:20:30 +0100
Subject: [PATCH 1/5] storage-pools: implement correct inheritance

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_pools_config.go | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/lxd/storage_pools_config.go b/lxd/storage_pools_config.go
index 6a8ca31..2751dad 100644
--- a/lxd/storage_pools_config.go
+++ b/lxd/storage_pools_config.go
@@ -103,6 +103,10 @@ func storagePoolFillDefault(name string, driver string, config map[string]string
 	if driver != "dir" {
 		var size uint64
 		if config["size"] == "" {
+			if driver == "lvm" {
+				return nil
+			}
+
 			st := syscall.Statfs_t{}
 			err := syscall.Statfs(shared.VarPath(), &st)
 			if err != nil {
@@ -128,30 +132,14 @@ func storagePoolFillDefault(name string, driver string, config map[string]string
 		config["size"] = strconv.FormatUint(uint64(size), 10)
 	}
 
-	if driver == "lvm" {
-		if config["lvm.thinpool_name"] == "" {
-			config["lvm.thinpool_name"] = "LXDThinpool"
-		}
-
-		if config["volume.block.filesystem"] == "" {
-			config["volume.block.filesystem"] = "ext4"
-		}
-
-		if config["volume.block.mount_options"] == "" {
-			config["volume.block.mount_options"] = "discard"
-		}
-	}
-
-	if config["volume.size"] == "" {
-		if driver == "lvm" {
-			sz, err := shared.ParseByteSizeString("10GB")
+	if driver == "lvm" || driver == "btrfs" || driver == "zfs" {
+		if config["volume.size"] != "" {
+			sz, err := shared.ParseByteSizeString(config["size"])
 			if err != nil {
 				return err
 			}
 			size := uint64(sz)
 			config["volume.size"] = strconv.FormatUint(uint64(size), 10)
-		} else {
-			config["volume.size"] = "0"
 		}
 	}
 

From e6f40d5fea586244a3708fba48c07a3bd0b7268c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 23 Feb 2017 01:21:14 +0100
Subject: [PATCH 2/5] storage-volumes: implement correct inheritance

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_volumes_config.go | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/lxd/storage_volumes_config.go b/lxd/storage_volumes_config.go
index e20e6cf..914346c 100644
--- a/lxd/storage_volumes_config.go
+++ b/lxd/storage_volumes_config.go
@@ -72,10 +72,6 @@ func storageVolumeValidateConfig(name string, config map[string]string, parentPo
 			if config["block.filesystem"] == "" {
 				config["block.filesystem"] = parentPool.Config["volume.block.filesystem"]
 			}
-
-			if config["block.mount_options"] == "" {
-				config["block.mount_options"] = parentPool.Config["volume.block.mount_options"]
-			}
 		}
 	}
 
@@ -89,8 +85,15 @@ func storageVolumeValidateConfig(name string, config map[string]string, parentPo
 
 func storageVolumeFillDefault(name string, config map[string]string, parentPool *api.StoragePool) error {
 	if parentPool.Driver == "dir" {
-		config["size"] = "0"
+		config["size"] = ""
 	} else if parentPool.Driver == "lvm" {
+		if config["block.filesystem"] == "" {
+			config["block.filesystem"] = parentPool.Config["volume.block.filesystem"]
+		}
+		if config["block.filesystem"] == "" {
+			config["block.filesystem"] = "ext4"
+		}
+
 		if config["size"] == "0" || config["size"] == "" {
 			config["size"] = parentPool.Config["volume.size"]
 		}
@@ -103,23 +106,18 @@ func storageVolumeFillDefault(name string, config map[string]string, parentPool
 			size := uint64(sz)
 			config["size"] = strconv.FormatUint(uint64(size), 10)
 		}
-	} else {
-		if config["size"] == "" {
-			config["size"] = parentPool.Config["volume.size"]
+	} else if parentPool.Driver == "zfs" || parentPool.Driver == "btrfs" {
+		if config["size"] != "" {
+			sz, err := shared.ParseByteSizeString("10GB")
+			if err != nil {
+				return err
+			}
+			size := uint64(sz)
+			config["size"] = strconv.FormatUint(uint64(size), 10)
 		}
 
 		if config["size"] == "" {
-			config["size"] = "0"
-		}
-	}
-
-	if parentPool.Driver == "lvm" {
-		if config["block.filesystem"] == "" {
-			config["block.filesystem"] = "ext4"
-		}
-
-		if config["block.mount_options"] == "" && config["block.filesystem"] == "ext4" {
-			config["block.mount_options"] = "discard"
+			config["size"] = parentPool.Config["volume.size"]
 		}
 	}
 

From f4ab2fa77c2d07af29c001fb01e7e0c82e5345e6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 23 Feb 2017 01:21:36 +0100
Subject: [PATCH 3/5] btrfs: only use size in the loop case

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_btrfs.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index 15ae003..c8ba153 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -131,6 +131,9 @@ func (s *storageBtrfs) StoragePoolCreate() error {
 			return fmt.Errorf("Failed to create the BTRFS pool: %s", output)
 		}
 	} else {
+		// Unset size property since it doesn't make sense.
+		s.pool.Config["size"] = ""
+
 		if filepath.IsAbs(source) {
 			isBlockDev = shared.IsBlockdevPath(source)
 			if isBlockDev {

From 434cc8343ebdabde512e14984adf991660e2382a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 23 Feb 2017 01:22:09 +0100
Subject: [PATCH 4/5] lvm: implement correct property inheritance

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_lvm.go | 83 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 23 deletions(-)

diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index d4484e1..3bed5b0 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -173,6 +173,47 @@ type storageLvm struct {
 	storageShared
 }
 
+func (s *storageLvm) getLvmBlockMountOptions() string {
+	if s.volume.Config["block.mount_options"] != "" {
+		return s.volume.Config["block.mount_options"]
+	}
+
+	if s.pool.Config["volume.block.mount_options"] != "" {
+		return s.pool.Config["volume.block.mount_options"]
+	}
+
+	if s.getLvmFilesystem() == "ext4" {
+		return "discard"
+	}
+
+	return ""
+}
+
+func (s *storageLvm) getLvmFilesystem() string {
+	if s.volume.Config["block.filesystem"] != "" {
+		return s.volume.Config["block.filesystem"]
+	}
+
+	if s.pool.Config["volume.block.filesystem"] != "" {
+		return s.pool.Config["volume.block.filesystem"]
+	}
+
+	return "ext4"
+}
+
+func (s *storageLvm) getLvmVolumeSize() string {
+	// Will never be empty.
+	return s.volume.Config["size"]
+}
+
+func (s *storageLvm) getLvmThinpoolName() string {
+	if s.pool.Config["lvm.thinpool_name"] != "" {
+		return s.pool.Config["lvm.thinpool_name"]
+	}
+
+	return "LXDThinpool"
+}
+
 func (s *storageLvm) getOnDiskPoolName() string {
 	if s.vgName != "" {
 		return s.vgName
@@ -238,11 +279,7 @@ func (s *storageLvm) StoragePoolInit(config map[string]interface{}) (storage, er
 	}
 
 	source := s.pool.Config["source"]
-	s.thinPoolName = s.pool.Config["lvm.thinpool_name"]
-	if s.thinPoolName == "" {
-		// Should actually always be set. But let's be sure.
-		s.thinPoolName = "LXDThinpool"
-	}
+	s.thinPoolName = s.getLvmThinpoolName()
 
 	if s.pool.Config["lvm.vg_name"] != "" {
 		s.vgName = s.pool.Config["lvm.vg_name"]
@@ -432,9 +469,9 @@ func (s *storageLvm) StoragePoolVolumeCreate() error {
 	tryUndo := true
 
 	poolName := s.getOnDiskPoolName()
-	thinPoolName := s.pool.Config["lvm.thinpool_name"]
-	lvFsType := s.volume.Config["block.filesystem"]
-	lvSize := s.volume.Config["size"]
+	thinPoolName := s.getLvmThinpoolName()
+	lvFsType := s.getLvmFilesystem()
+	lvSize := s.getLvmVolumeSize()
 
 	volumeType, err := storagePoolVolumeTypeNameToApiEndpoint(s.volume.Type)
 	if err != nil {
@@ -502,7 +539,7 @@ func (s *storageLvm) StoragePoolVolumeMount() (bool, error) {
 		return false, nil
 	}
 
-	lvFsType := s.volume.Config["block.filesystem"]
+	lvFsType := s.getLvmFilesystem()
 	volumeType, err := storagePoolVolumeTypeNameToApiEndpoint(s.volume.Type)
 	if err != nil {
 		return false, err
@@ -510,7 +547,7 @@ func (s *storageLvm) StoragePoolVolumeMount() (bool, error) {
 
 	poolName := s.getOnDiskPoolName()
 	lvmVolumePath := getLvmDevPath(poolName, volumeType, s.volume.Name)
-	mountOptions := s.volume.Config["block.mount_options"]
+	mountOptions := s.getLvmBlockMountOptions()
 	err = tryMount(lvmVolumePath, customPoolVolumeMntPoint, lvFsType, 0, mountOptions)
 	if err != nil {
 		return false, err
@@ -624,9 +661,9 @@ func (s *storageLvm) ContainerCreate(container container) error {
 
 	containerName := container.Name()
 	containerLvmName := containerNameToLVName(containerName)
-	thinPoolName := s.pool.Config["lvm.thinpool_name"]
-	lvFsType := s.volume.Config["block.filesystem"]
-	lvSize := s.volume.Config["size"]
+	thinPoolName := s.getLvmThinpoolName()
+	lvFsType := s.getLvmFilesystem()
+	lvSize := s.getLvmVolumeSize()
 
 	poolName := s.getOnDiskPoolName()
 	err := s.createThinLV(poolName, thinPoolName, containerLvmName, lvFsType, lvSize, storagePoolVolumeApiEndpointContainers)
@@ -732,7 +769,7 @@ func (s *storageLvm) ContainerCreateFromImage(container container, fingerprint s
 	}
 
 	// Generate a new xfs's UUID
-	lvFsType := s.volume.Config["block.filesystem"]
+	lvFsType := s.getLvmFilesystem()
 	if lvFsType == "xfs" {
 		err := xfsGenerateNewUUID(containerLvSnapshotPath)
 		if err != nil {
@@ -896,10 +933,10 @@ func (s *storageLvm) ContainerCopy(container container, sourceContainer containe
 
 func (s *storageLvm) ContainerMount(name string, path string) (bool, error) {
 	containerLvmName := containerNameToLVName(name)
-	lvFsType := s.volume.Config["block.filesystem"]
+	lvFsType := s.getLvmFilesystem()
 	poolName := s.getOnDiskPoolName()
 	containerLvmPath := getLvmDevPath(poolName, storagePoolVolumeApiEndpointContainers, containerLvmName)
-	mountOptions := s.volume.Config["block.mount_options"]
+	mountOptions := s.getLvmBlockMountOptions()
 	containerMntPoint := getContainerMountPoint(s.pool.Name, name)
 
 	containerMountLockID := getContainerMountLockID(s.pool.Name, name)
@@ -1213,9 +1250,9 @@ func (s *storageLvm) ContainerSnapshotStart(container container) error {
 		}
 	}()
 
-	lvFsType := s.volume.Config["block.filesystem"]
+	lvFsType := s.getLvmFilesystem()
 	containerLvmPath := getLvmDevPath(poolName, storagePoolVolumeApiEndpointContainers, tmpTargetLvmName)
-	mountOptions := s.volume.Config["block.mount_options"]
+	mountOptions := s.getLvmBlockMountOptions()
 	containerMntPoint := getSnapshotMountPoint(s.pool.Name, sourceName)
 
 	// Generate a new xfs's UUID
@@ -1268,9 +1305,9 @@ func (s *storageLvm) ImageCreate(fingerprint string) error {
 	tryUndo := true
 
 	poolName := s.getOnDiskPoolName()
-	thinPoolName := s.pool.Config["lvm.thinpool_name"]
-	lvFsType := s.volume.Config["block.filesystem"]
-	lvSize := s.volume.Config["size"]
+	thinPoolName := s.getLvmThinpoolName()
+	lvFsType := s.getLvmFilesystem()
+	lvSize := s.getLvmVolumeSize()
 
 	err := s.createImageDbPoolVolume(fingerprint)
 	if err != nil {
@@ -1350,14 +1387,14 @@ func (s *storageLvm) ImageMount(fingerprint string) (bool, error) {
 	}
 
 	// Shouldn't happen.
-	lvmFstype := s.volume.Config["block.filesystem"]
+	lvmFstype := s.getLvmFilesystem()
 	if lvmFstype == "" {
 		return false, fmt.Errorf("No filesystem type specified.")
 	}
 
 	poolName := s.getOnDiskPoolName()
 	lvmVolumePath := getLvmDevPath(poolName, storagePoolVolumeApiEndpointImages, fingerprint)
-	lvmMountOptions := s.volume.Config["block.mount_options"]
+	lvmMountOptions := s.getLvmBlockMountOptions()
 	// Shouldn't be necessary since it should be validated in the config
 	// checks.
 	if lvmFstype == "ext4" && lvmMountOptions == "" {

From 06b7c13301b3c3cf1f4e19374a1419b81582f45c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 23 Feb 2017 01:22:29 +0100
Subject: [PATCH 5/5] zfs: only use size property in the loop case

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/storage_zfs.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 56b2316..f37ee5f 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -1355,6 +1355,9 @@ func (s *storageZfs) zfsPoolCreate() error {
 			return fmt.Errorf("Failed to create the ZFS pool: %s", output)
 		}
 	} else {
+		// Unset size property since it doesn't make sense.
+		s.pool.Config["size"] = ""
+
 		if filepath.IsAbs(vdev) {
 			if !shared.IsBlockdevPath(vdev) {
 				return fmt.Errorf("Custom loop file locations are not supported.")


More information about the lxc-devel mailing list