[lxc-devel] [lxd/master] Storage validation
tomponline on Github
lxc-bot at linuxcontainers.org
Wed Jan 15 16:00:07 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200115/0503757e/attachment-0001.bin>
-------------- next part --------------
From ac0efe038e5665a915889fbd0bcb0b768df94a2a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:14:22 +0000
Subject: [PATCH 01/20] lxd/storage/backend/lxd: Validate config on pool create
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/backend_lxd.go | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 0f89357600..43d5a6b273 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -85,6 +85,12 @@ func (b *lxdBackend) create(localOnly bool, op *operations.Operation) error {
return nil
}
+ // Validate config.
+ err = b.driver.Validate(b.db.Config)
+ if err != nil {
+ return err
+ }
+
// Create the storage pool on the storage device.
err = b.driver.Create()
if err != nil {
From 5dbe94631e9660c6d0df5a0b03417d1d5ee2aa10 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:14:41 +0000
Subject: [PATCH 02/20] shared/instance: Adds IsSize to validate size strings
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
shared/instance.go | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/shared/instance.go b/shared/instance.go
index 985ebd4fce..1264e991c7 100644
--- a/shared/instance.go
+++ b/shared/instance.go
@@ -115,6 +115,20 @@ func IsNotEmpty(value string) error {
return nil
}
+// IsSize checks if string is valid size according to units.ParseByteSizeString.
+func IsSize(value string) error {
+ if value == "" {
+ return nil
+ }
+
+ _, err := units.ParseByteSizeString(value)
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
// IsDeviceID validates string is four lowercase hex characters suitable as Vendor or Device ID.
func IsDeviceID(value string) error {
if value == "" {
From 4d6bba245d7f4f3903f89c231e7595271fc0dc0e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:15:57 +0000
Subject: [PATCH 03/20] lxd/storage/pools/config: Removes old LVM validation
from storagePoolValidateConfig
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools_config.go | 7 -------
1 file changed, 7 deletions(-)
diff --git a/lxd/storage_pools_config.go b/lxd/storage_pools_config.go
index de12a2c089..f64641b268 100644
--- a/lxd/storage_pools_config.go
+++ b/lxd/storage_pools_config.go
@@ -135,13 +135,6 @@ func storagePoolValidateConfig(name string, driver string, config map[string]str
return err
}
- if driver == "lvm" {
- v, ok := config["lvm.use_thinpool"]
- if ok && !shared.IsTrue(v) && config["lvm.thinpool_name"] != "" {
- return fmt.Errorf("the key \"lvm.use_thinpool\" cannot be set to a false value when \"lvm.thinpool_name\" is set for LVM storage pools")
- }
- }
-
v, ok := config["rsync.bwlimit"]
if ok && v != "" {
_, err := units.ParseByteSizeString(v)
From 2b4c810b355f4204cf0ec47dd146b2b6eab2a92a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:16:32 +0000
Subject: [PATCH 04/20] lxd/storage/utils: shared.IsSize usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/utils.go | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index ed367af146..1d06c89d08 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -390,7 +390,7 @@ func VolumeValidateConfig(s *state.State, name string, config map[string]string,
logger := logging.AddContext(logger.Log, log.Ctx{"driver": parentPool.Driver, "pool": parentPool.Name})
// Validate volume config using the new driver interface if supported.
- driver, err := drivers.Load(s, parentPool.Driver, parentPool.Name, parentPool.Config, logger, nil, validateVolumeCommonRules)
+ driver, err := drivers.Load(s, parentPool.Driver, parentPool.Name, parentPool.Config, logger, nil, commonValidators())
if err != drivers.ErrUnknownDriver {
// Note: This legacy validation function doesn't have the concept of validating
// different volumes types, so the types are hard coded as Custom and FS.
@@ -509,6 +509,17 @@ func VolumePropertiesTranslate(targetConfig map[string]string, targetParentPoolD
return newConfig, nil
}
+// validatePoolCommonRules returns a map of pool config rules common to all drivers.
+func validatePoolCommonRules() map[string]func(string) error {
+ return map[string]func(string) error{
+ "source": shared.IsAny,
+ "volatile.initial_source": shared.IsAny,
+ "volume.size": shared.IsSize,
+ "size": shared.IsSize,
+ "rsync.bwlimit": shared.IsAny,
+ }
+}
+
// validateVolumeCommonRules returns a map of volume config rules common to all drivers.
func validateVolumeCommonRules(vol drivers.Volume) map[string]func(string) error {
rules := map[string]func(string) error{
@@ -517,18 +528,7 @@ func validateVolumeCommonRules(vol drivers.Volume) map[string]func(string) error
// Note: size should not be modifiable for non-custom volumes and should be checked
// in the relevant volume update functions.
- "size": func(value string) error {
- if value == "" {
- return nil
- }
-
- _, err := units.ParseByteSizeString(value)
- if err != nil {
- return err
- }
-
- return nil
- },
+ "size": shared.IsSize,
}
// block.mount_options is only relevant for drivers that are block backed and when there
From e36bb7b90fa5fcbd2f0ec23c8ab570b7ae917205 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:16:52 +0000
Subject: [PATCH 05/20] lxd/storage/load: commonRules usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/load.go | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lxd/storage/load.go b/lxd/storage/load.go
index 6a0986c03d..ae92803903 100644
--- a/lxd/storage/load.go
+++ b/lxd/storage/load.go
@@ -60,6 +60,14 @@ func volIDFuncMake(state *state.State, poolID int64) func(volType drivers.Volume
}
}
+// commonRules returns a set of common validators.
+func commonRules() *drivers.Validators {
+ return &drivers.Validators{
+ PoolRules: validatePoolCommonRules,
+ VolumeRules: validateVolumeCommonRules,
+ }
+}
+
// CreatePool creates a new storage pool on disk and returns a Pool interface.
// If the pool's driver is not recognised then drivers.ErrUnknownDriver is returned.
func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePoolsPost, localOnly bool, op *operations.Operation) (Pool, error) {
@@ -85,7 +93,7 @@ func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePoolsPost,
logger := logging.AddContext(logger.Log, log.Ctx{"driver": dbPool.Driver, "pool": dbPool.Name})
// Load the storage driver.
- driver, err := drivers.Load(state, dbPool.Driver, dbPool.Name, dbPool.Config, logger, volIDFuncMake(state, poolID), validateVolumeCommonRules)
+ driver, err := drivers.Load(state, dbPool.Driver, dbPool.Name, dbPool.Config, logger, volIDFuncMake(state, poolID), commonRules())
if err != nil {
return nil, err
}
@@ -138,7 +146,7 @@ func GetPoolByName(state *state.State, name string) (Pool, error) {
logger := logging.AddContext(logger.Log, log.Ctx{"driver": dbPool.Driver, "pool": dbPool.Name})
// Load the storage driver.
- driver, err := drivers.Load(state, dbPool.Driver, dbPool.Name, dbPool.Config, logger, volIDFuncMake(state, poolID), validateVolumeCommonRules)
+ driver, err := drivers.Load(state, dbPool.Driver, dbPool.Name, dbPool.Config, logger, volIDFuncMake(state, poolID), commonRules())
if err != nil {
return nil, err
}
From 9dbc2d7053676c37ae7c6aec1c0b5b50ab0f6d0a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:19:43 +0000
Subject: [PATCH 06/20] lxd/storage/utils: commonRules usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/utils.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 1d06c89d08..a36580b522 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -390,7 +390,7 @@ func VolumeValidateConfig(s *state.State, name string, config map[string]string,
logger := logging.AddContext(logger.Log, log.Ctx{"driver": parentPool.Driver, "pool": parentPool.Name})
// Validate volume config using the new driver interface if supported.
- driver, err := drivers.Load(s, parentPool.Driver, parentPool.Name, parentPool.Config, logger, nil, commonValidators())
+ driver, err := drivers.Load(s, parentPool.Driver, parentPool.Name, parentPool.Config, logger, nil, commonRules())
if err != drivers.ErrUnknownDriver {
// Note: This legacy validation function doesn't have the concept of validating
// different volumes types, so the types are hard coded as Custom and FS.
From d1c2846ec33434ab38b58edff4d17cb9ee1f5a77 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:20:07 +0000
Subject: [PATCH 07/20] lxd/storage/drivers/load: Adds Validators type for
common rules
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/load.go | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lxd/storage/drivers/load.go b/lxd/storage/drivers/load.go
index eeca94a1be..2f7a114558 100644
--- a/lxd/storage/drivers/load.go
+++ b/lxd/storage/drivers/load.go
@@ -13,8 +13,14 @@ var drivers = map[string]func() driver{
"zfs": func() driver { return &zfs{} },
}
+// Validators contains functions used for validating a drivers's config.
+type Validators struct {
+ PoolRules func() map[string]func(string) error
+ VolumeRules func(vol Volume) map[string]func(string) error
+}
+
// Load returns a Driver for an existing low-level storage pool.
-func Load(state *state.State, driverName string, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonVolRulesFunc func(vol Volume) map[string]func(string) error) (Driver, error) {
+func Load(state *state.State, driverName string, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonRules *Validators) (Driver, error) {
// Locate the driver loader.
driverFunc, ok := drivers[driverName]
if !ok {
@@ -22,7 +28,7 @@ func Load(state *state.State, driverName string, name string, config map[string]
}
d := driverFunc()
- d.init(state, name, config, logger, volIDFunc, commonVolRulesFunc)
+ d.init(state, name, config, logger, volIDFunc, commonRules)
err := d.load()
if err != nil {
From 7359496bd6dd80337e835c1606d1823f8618f795 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:20:41 +0000
Subject: [PATCH 08/20] lxd/storage/drivers/interface: commonRules usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/interface.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 0e4a58a558..5d57b5fcae 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -14,7 +14,7 @@ import (
type driver interface {
Driver
- init(state *state.State, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonRulesFunc func(vol Volume) map[string]func(string) error)
+ init(state *state.State, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonRules *Validators)
load() error
}
From 13b99639a9a0644b91202a93717146d4a5829f18 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:21:39 +0000
Subject: [PATCH 09/20] lxd/storage/drivers: Call d.validatePool in Validate
function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_btrfs.go | 2 +-
lxd/storage/drivers/driver_cephfs.go | 2 +-
lxd/storage/drivers/driver_dir.go | 2 +-
lxd/storage/drivers/driver_zfs.go | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lxd/storage/drivers/driver_btrfs.go b/lxd/storage/drivers/driver_btrfs.go
index d508709fc8..065f856dca 100644
--- a/lxd/storage/drivers/driver_btrfs.go
+++ b/lxd/storage/drivers/driver_btrfs.go
@@ -233,7 +233,7 @@ func (d *btrfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *btrfs) Validate(config map[string]string) error {
- return nil
+ return d.validatePool(config, nil)
}
// Update applies any driver changes required from a configuration change.
diff --git a/lxd/storage/drivers/driver_cephfs.go b/lxd/storage/drivers/driver_cephfs.go
index 53fafa9530..437dee9800 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -226,7 +226,7 @@ func (d *cephfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *cephfs) Validate(config map[string]string) error {
- return nil
+ return d.validatePool(config, nil)
}
// Update applies any driver changes required from a configuration change.
diff --git a/lxd/storage/drivers/driver_dir.go b/lxd/storage/drivers/driver_dir.go
index 27707a379d..901724a4c1 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -91,7 +91,7 @@ func (d *dir) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *dir) Validate(config map[string]string) error {
- return nil
+ return d.validatePool(config, nil)
}
// Update applies any driver changes required from a configuration change.
diff --git a/lxd/storage/drivers/driver_zfs.go b/lxd/storage/drivers/driver_zfs.go
index e77df847f8..9d5968ab6f 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -283,7 +283,7 @@ func (d *zfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *zfs) Validate(config map[string]string) error {
- return nil
+ return d.validatePool(config, nil)
}
// Update applies any driver changes required from a configuration change.
From b70c75ee3f4cd22d33ebb2b9420d4054ce51cd0e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:22:45 +0000
Subject: [PATCH 10/20] lxd/storage/drivers/drivers/common: Updates for
commonRules
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_common.go | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index 9bac72e386..a84fd69ff3 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -21,20 +21,20 @@ import (
)
type common struct {
- name string
- config map[string]string
- getVolID func(volType VolumeType, volName string) (int64, error)
- getCommonVolumeRules func(vol Volume) map[string]func(string) error
- state *state.State
- logger logger.Logger
- patches map[string]func() error
+ name string
+ config map[string]string
+ getVolID func(volType VolumeType, volName string) (int64, error)
+ commonRules *Validators
+ state *state.State
+ logger logger.Logger
+ patches map[string]func() error
}
-func (d *common) init(state *state.State, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonVolRulesFunc func(vol Volume) map[string]func(string) error) {
+func (d *common) init(state *state.State, name string, config map[string]string, logger logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64, error), commonRules *Validators) {
d.name = name
d.config = config
d.getVolID = volIDFunc
- d.getCommonVolumeRules = commonVolRulesFunc
+ d.commonRules = commonRules
d.state = state
d.logger = logger
}
@@ -51,7 +51,7 @@ func (d *common) validateVolume(vol Volume, driverRules map[string]func(value st
checkedFields := map[string]struct{}{}
// Get rules common for all drivers.
- rules := d.getCommonVolumeRules(vol)
+ rules := d.commonRules.VolumeRules(vol)
// Merge driver specific rules into common rules.
for field, validator := range driverRules {
From a382ac3c6ccf5cb24e4831fcb5b19944b9388d4d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:23:11 +0000
Subject: [PATCH 11/20] lxd/storage/drivera/driver/common: Adds validatePool
function
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_common.go | 39 ++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index a84fd69ff3..642341b32e 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -43,6 +43,45 @@ func (d *common) load() error {
return nil
}
+// validatePool validates a pool config against common rules and optional driver specific rules.
+func (d *common) validatePool(config map[string]string, driverRules map[string]func(value string) error) error {
+ checkedFields := map[string]struct{}{}
+
+ // Get rules common for all drivers.
+ rules := d.commonRules.PoolRules()
+
+ // Merge driver specific rules into common rules.
+ for field, validator := range driverRules {
+ rules[field] = validator
+ }
+
+ // Run the validator against each field.
+ for k, validator := range rules {
+ checkedFields[k] = struct{}{} //Mark field as checked.
+ err := validator(config[k])
+ if err != nil {
+ return errors.Wrapf(err, "Invalid value for pool option %s", k)
+ }
+ }
+
+ // Look for any unchecked fields, as these are unknown fields and validation should fail.
+ for k := range config {
+ _, checked := checkedFields[k]
+ if checked {
+ continue
+ }
+
+ // User keys are not validated.
+ if strings.HasPrefix(k, "user.") {
+ continue
+ }
+
+ return fmt.Errorf("Invalid pool option: %s", k)
+ }
+
+ return nil
+}
+
// validateVolume validates a volume config against common rules and optional driver specific rules.
// This functions has a removeUnknownKeys option that if set to true will remove any unknown fields
// (excluding those starting with "user.") which can be used when translating a volume config to a
From d97a424fb9286ef53212ac504928bf21fe0b61c1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:23:39 +0000
Subject: [PATCH 12/20] lxd/storage/drivers/driver/dir/utils: commonRules usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_dir_utils.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_dir_utils.go b/lxd/storage/drivers/driver_dir_utils.go
index 83157ac9ef..187d06250c 100644
--- a/lxd/storage/drivers/driver_dir_utils.go
+++ b/lxd/storage/drivers/driver_dir_utils.go
@@ -13,7 +13,7 @@ import (
func (d *dir) withoutGetVolID() Driver {
newDriver := &dir{}
getVolID := func(volType VolumeType, volName string) (int64, error) { return volIDQuotaSkip, nil }
- newDriver.init(d.state, d.name, d.config, d.logger, getVolID, d.getCommonVolumeRules)
+ newDriver.init(d.state, d.name, d.config, d.logger, getVolID, d.commonRules)
newDriver.load()
return newDriver
From 8f9414a36cea86c9207f234608eac7f424f1b561 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:50:53 +0000
Subject: [PATCH 13/20] lxd/storage/drivers/driver/btrfs: pool validation
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_btrfs.go | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_btrfs.go b/lxd/storage/drivers/driver_btrfs.go
index 065f856dca..920d4bb4fe 100644
--- a/lxd/storage/drivers/driver_btrfs.go
+++ b/lxd/storage/drivers/driver_btrfs.go
@@ -233,7 +233,11 @@ func (d *btrfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *btrfs) Validate(config map[string]string) error {
- return d.validatePool(config, nil)
+ rules := map[string]func(value string) error{
+ "btrfs.mount_options": shared.IsAny,
+ }
+
+ return d.validatePool(config, rules)
}
// Update applies any driver changes required from a configuration change.
From ee85d83f737809c7ea8fd012408eb4521bf7f44c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:51:57 +0000
Subject: [PATCH 14/20] lxd/storage/drivers/driver/zfs: pool validation
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_zfs.go | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_zfs.go b/lxd/storage/drivers/driver_zfs.go
index 9d5968ab6f..bcd7ebcd3f 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -283,7 +283,14 @@ func (d *zfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *zfs) Validate(config map[string]string) error {
- return d.validatePool(config, nil)
+ rules := map[string]func(value string) error{
+ "zfs.pool_name": shared.IsAny,
+ "zfs.clone_copy": shared.IsBool,
+ "volume.zfs.remove_snapshots": shared.IsBool,
+ "volume.zfs.use_refquota": shared.IsBool,
+ }
+
+ return d.validatePool(config, rules)
}
// Update applies any driver changes required from a configuration change.
From 2a64f2a4a903015954930d6875454a50f38e8fd1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:52:19 +0000
Subject: [PATCH 15/20] lxd/storage/drivers/driver/cephfs: pool validation
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_cephfs.go | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_cephfs.go b/lxd/storage/drivers/driver_cephfs.go
index 437dee9800..fc14f98c8b 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -226,7 +226,14 @@ func (d *cephfs) Delete(op *operations.Operation) error {
// Validate checks that all provide keys are supported and that no conflicting or missing configuration is present.
func (d *cephfs) Validate(config map[string]string) error {
- return d.validatePool(config, nil)
+ rules := map[string]func(value string) error{
+ "cephfs.cluster_name": shared.IsAny,
+ "cephfs.path": shared.IsAny,
+ "cephfs.user.name": shared.IsAny,
+ "volatile.pool.pristine": shared.IsAny,
+ }
+
+ return d.validatePool(config, rules)
}
// Update applies any driver changes required from a configuration change.
From 4aac11b361e390604668ab18df2b56f8dbeed686 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:52:44 +0000
Subject: [PATCH 16/20] lxd/storage/drivers/driver/common: Improved error
messages in validatePool and validateVolume
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_common.go | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index 642341b32e..baa353abee 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -60,7 +60,7 @@ func (d *common) validatePool(config map[string]string, driverRules map[string]f
checkedFields[k] = struct{}{} //Mark field as checked.
err := validator(config[k])
if err != nil {
- return errors.Wrapf(err, "Invalid value for pool option %s", k)
+ return errors.Wrapf(err, "Invalid value for pool %q option %q", d.name, k)
}
}
@@ -76,7 +76,7 @@ func (d *common) validatePool(config map[string]string, driverRules map[string]f
continue
}
- return fmt.Errorf("Invalid pool option: %s", k)
+ return fmt.Errorf("Invalid option for pool %q option %q", d.name, k)
}
return nil
@@ -102,7 +102,7 @@ func (d *common) validateVolume(vol Volume, driverRules map[string]func(value st
checkedFields[k] = struct{}{} //Mark field as checked.
err := validator(vol.config[k])
if err != nil {
- return errors.Wrapf(err, "Invalid value for volume option %s", k)
+ return errors.Wrapf(err, "Invalid value for volume %q option %q", vol.name, k)
}
}
@@ -121,13 +121,13 @@ func (d *common) validateVolume(vol Volume, driverRules map[string]func(value st
if removeUnknownKeys {
delete(vol.config, k)
} else {
- return fmt.Errorf("Invalid volume option: %s", k)
+ return fmt.Errorf("Invalid option for volume %q option %q", vol.name, k)
}
}
// If volume type is not custom, don't allow "size" property.
if vol.volType != VolumeTypeCustom && vol.config["size"] != "" {
- return fmt.Errorf("Volume 'size' property is only valid for custom volume types")
+ return fmt.Errorf("Volume %q property is only valid for custom volume types", "size")
}
return nil
From 5c1ceecaf365dfc0d54885aa1ff373a8d0ab8a0d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:53:11 +0000
Subject: [PATCH 17/20] lxd/storage/pools: Fixes empty values for non-compat
pools in storagePoolClusterConfigForEtag
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools.go | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index 62e4b41a26..2bc243885a 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -521,7 +521,9 @@ func storagePoolClusterConfigForEtag(dbConfig map[string]string) map[string]stri
func storagePoolClusterFillWithNodeConfig(dbConfig, reqConfig map[string]string) map[string]string {
config := util.CopyConfig(reqConfig)
for _, key := range db.StoragePoolNodeConfigKeys {
- config[key] = dbConfig[key]
+ if _, found := dbConfig[key]; found {
+ config[key] = dbConfig[key]
+ }
}
return config
}
From a397a05fad45c3bf219e9e8ab591b935754700b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:53:47 +0000
Subject: [PATCH 18/20] lxd/storage/pools/config: shared.IsSize usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools_config.go | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/lxd/storage_pools_config.go b/lxd/storage_pools_config.go
index f64641b268..3eaef0ac58 100644
--- a/lxd/storage_pools_config.go
+++ b/lxd/storage_pools_config.go
@@ -81,14 +81,7 @@ var storagePoolConfigKeys = map[string]func(value string) error{
"lvm.vg_name": shared.IsAny,
// valid drivers: btrfs, lvm, zfs
- "size": func(value string) error {
- if value == "" {
- return nil
- }
-
- _, err := units.ParseByteSizeString(value)
- return err
- },
+ "size": shared.IsSize,
// valid drivers: btrfs, dir, lvm, zfs
"source": shared.IsAny,
@@ -108,14 +101,7 @@ var storagePoolConfigKeys = map[string]func(value string) error{
"volume.block.mount_options": shared.IsAny,
// valid drivers: ceph, lvm
- "volume.size": func(value string) error {
- if value == "" {
- return nil
- }
-
- _, err := units.ParseByteSizeString(value)
- return err
- },
+ "volume.size": shared.IsSize,
// valid drivers: zfs
"volume.zfs.remove_snapshots": shared.IsBool,
From dcde0da609cee10d5d01dcc6968e653727a64953 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 15:53:58 +0000
Subject: [PATCH 19/20] lxd/storage/pools/config: comment
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage_pools_config.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage_pools_config.go b/lxd/storage_pools_config.go
index 3eaef0ac58..e5f8d56514 100644
--- a/lxd/storage_pools_config.go
+++ b/lxd/storage_pools_config.go
@@ -214,7 +214,7 @@ func storagePoolFillDefault(name string, driver string, config map[string]string
}
if driver == "lvm" {
- // We use thin pools per default.
+ // We use thin pools by default.
useThinpool := true
if config["lvm.use_thinpool"] != "" {
useThinpool = shared.IsTrue(config["lvm.use_thinpool"])
From 5e5598290be37ca10281a3ae3706f9e1e82e0986 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 15 Jan 2020 14:25:10 +0000
Subject: [PATCH 20/20] lxd/storage/drivers/driver/lvm: Adds validation
Includes new stripes config settings.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/storage/drivers/driver_lvm.go | 19 +++++++++++++++
lxd/storage/drivers/driver_lvm_volumes.go | 28 ++++++++++++++++++++++-
2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_lvm.go b/lxd/storage/drivers/driver_lvm.go
index 9eb629d503..2ad8f0c238 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -411,11 +411,30 @@ func (d *lvm) Delete(op *operations.Operation) error {
}
func (d *lvm) Validate(config map[string]string) error {
+ rules := map[string]func(value string) error{
+ "lvm.vg_name": shared.IsAny,
+ "lvm.thinpool_name": shared.IsAny,
+ "lvm.use_thinpool": shared.IsBool,
+ }
+
+ err := d.validatePool(config, rules)
+ if err != nil {
+ return err
+ }
+
+ if v := config["lvm.use_thinpool"]; !shared.IsTrue(v) && config["lvm.thinpool_name"] != "" {
+ return fmt.Errorf("The key lvm.use_thinpool cannot be set to false when lvm.thinpool_name is set")
+ }
+
return nil
}
// Update updates the storage pool settings.
func (d *lvm) Update(changedConfig map[string]string) error {
+ if _, changed := changedConfig["lvm.use_thinpool"]; changed {
+ return fmt.Errorf("lvm.use_thinpool cannot be changed")
+ }
+
if changedConfig["lvm.vg_name"] != "" {
_, err := shared.TryRunCommand("vgrename", d.config["lvm.vg_name"], changedConfig["lvm.vg_name"])
if err != nil {
diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go
index f33aeb6387..708dcd33f3 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -223,7 +223,25 @@ func (d *lvm) HasVolume(vol Volume) bool {
// ValidateVolume validates the supplied volume config.
func (d *lvm) ValidateVolume(vol Volume, removeUnknownKeys bool) error {
- return d.validateVolume(vol, nil, removeUnknownKeys)
+ rules := map[string]func(value string) error{
+ "lvm.stripes": shared.IsUint32,
+ "lvm.stripes.size": shared.IsSize,
+ }
+
+ err := d.validateVolume(vol, rules, removeUnknownKeys)
+ if err != nil {
+ return err
+ }
+
+ if d.usesThinpool() && vol.config["lvm.stripes"] != "" {
+ return fmt.Errorf("lvm.stripes cannot be used with thin pool volumes")
+ }
+
+ if d.usesThinpool() && vol.config["lvm.stripes.size"] != "" {
+ return fmt.Errorf("lvm.stripes.size cannot be used with thin pool volumes")
+ }
+
+ return nil
}
// UpdateVolume applies config changes to the volume.
@@ -239,6 +257,14 @@ func (d *lvm) UpdateVolume(vol Volume, changedConfig map[string]string) error {
}
}
+ if _, changed := changedConfig["lvm.stripes"]; changed {
+ return fmt.Errorf("lvm.stripes cannot be changed")
+ }
+
+ if _, changed := changedConfig["lvm.stripes.size"]; changed {
+ return fmt.Errorf("lvm.stripes.size cannot be changed")
+ }
+
return nil
}
More information about the lxc-devel
mailing list