The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/2933

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===

From 726a589fbba4bfbd2246ad4c06d8f8898df0cc66 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.brau...@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.")
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to