[lxc-devel] [lxd/master] Storage: Adds per-drive volume default settings

tomponline on Github lxc-bot at linuxcontainers.org
Thu Nov 19 12:55:45 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 476 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201119/66f5e164/attachment-0001.bin>
-------------- next part --------------
From 432633b96f40a2efce03dfb36bdf02228fdf386c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:42:23 +0000
Subject: [PATCH 01/13] lxd/patches/utils: Adds legacy volumeFillDefault
 function for patches

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/patches_utils.go | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/lxd/patches_utils.go b/lxd/patches_utils.go
index 256422f32e..fa00348024 100644
--- a/lxd/patches_utils.go
+++ b/lxd/patches_utils.go
@@ -14,6 +14,7 @@ import (
 	"github.com/lxc/lxd/lxd/state"
 	storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
 	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/api"
 	"github.com/lxc/lxd/shared/units"
 )
 
@@ -223,3 +224,27 @@ func lvmGetLVSize(lvPath string) (string, error) {
 
 	return detectedSize, nil
 }
+
+// volumeFillDefault fills default settings into a volume config.
+// Deprecated. Please use FillInstanceConfig() on the storage pool.
+func volumeFillDefault(config map[string]string, parentPool *api.StoragePool) error {
+	if parentPool.Driver == "lvm" || parentPool.Driver == "ceph" {
+		if config["block.filesystem"] == "" {
+			config["block.filesystem"] = parentPool.Config["volume.block.filesystem"]
+		}
+		if config["block.filesystem"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			config["block.filesystem"] = storageDrivers.DefaultFilesystem
+		}
+
+		if config["block.mount_options"] == "" {
+			config["block.mount_options"] = parentPool.Config["volume.block.mount_options"]
+		}
+		if config["block.mount_options"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			config["block.mount_options"] = "discard"
+		}
+	}
+
+	return nil
+}

From f0daf911c1f8ca9cf322a1184bb506eb6e1c529a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:44:13 +0000
Subject: [PATCH 02/13] lxd/patches: Updates patches to switch from
 driver.VolumeFillDefault to volumeFillDefault

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/patches.go | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/lxd/patches.go b/lxd/patches.go
index ef0bbee33c..194a5695c9 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -877,7 +877,7 @@ func upgradeFromStorageTypeBtrfs(name string, d *Daemon, defaultPoolName string,
 		// Initialize empty storage volume configuration for the
 		// container.
 		containerPoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(containerPoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(containerPoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -965,7 +965,7 @@ func upgradeFromStorageTypeBtrfs(name string, d *Daemon, defaultPoolName string,
 			// Initialize empty storage volume configuration for the
 			// container.
 			snapshotPoolVolumeConfig := map[string]string{}
-			err = driver.VolumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
+			err = volumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
 			if err != nil {
 				return err
 			}
@@ -1046,7 +1046,7 @@ func upgradeFromStorageTypeBtrfs(name string, d *Daemon, defaultPoolName string,
 	images := append(imgPublic, imgPrivate...)
 	for _, img := range images {
 		imagePoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(imagePoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(imagePoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -1167,7 +1167,7 @@ func upgradeFromStorageTypeDir(name string, d *Daemon, defaultPoolName string, d
 		// Initialize empty storage volume configuration for the
 		// container.
 		containerPoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(containerPoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(containerPoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -1284,7 +1284,7 @@ func upgradeFromStorageTypeDir(name string, d *Daemon, defaultPoolName string, d
 		// Initialize empty storage volume configuration for the
 		// container.
 		snapshotPoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -1314,7 +1314,7 @@ func upgradeFromStorageTypeDir(name string, d *Daemon, defaultPoolName string, d
 	images := append(imgPublic, imgPrivate...)
 	for _, img := range images {
 		imagePoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(imagePoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(imagePoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -1476,7 +1476,7 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 		// Initialize empty storage volume configuration for the
 		// container.
 		containerPoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(containerPoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(containerPoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -1634,7 +1634,7 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 			// Initialize empty storage volume configuration for the
 			// container.
 			snapshotPoolVolumeConfig := map[string]string{}
-			err = driver.VolumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
+			err = volumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
 			if err != nil {
 				return err
 			}
@@ -1814,7 +1814,7 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d
 
 	for _, img := range images {
 		imagePoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(imagePoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(imagePoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -2006,7 +2006,7 @@ func upgradeFromStorageTypeZfs(name string, d *Daemon, defaultPoolName string, d
 		// Initialize empty storage volume configuration for the
 		// container.
 		containerPoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(containerPoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(containerPoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -2092,7 +2092,7 @@ func upgradeFromStorageTypeZfs(name string, d *Daemon, defaultPoolName string, d
 			// Initialize empty storage volume configuration for the
 			// container.
 			snapshotPoolVolumeConfig := map[string]string{}
-			err = driver.VolumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
+			err = volumeFillDefault(snapshotPoolVolumeConfig, defaultPool)
 			if err != nil {
 				return err
 			}
@@ -2148,7 +2148,7 @@ func upgradeFromStorageTypeZfs(name string, d *Daemon, defaultPoolName string, d
 	images := append(imgPublic, imgPrivate...)
 	for _, img := range images {
 		imagePoolVolumeConfig := map[string]string{}
-		err = driver.VolumeFillDefault(imagePoolVolumeConfig, defaultPool)
+		err = volumeFillDefault(imagePoolVolumeConfig, defaultPool)
 		if err != nil {
 			return err
 		}
@@ -2640,7 +2640,7 @@ func patchStorageApiUpdateStorageConfigs(name string, d *Daemon) error {
 			}
 
 			// Insert default values.
-			err := driver.VolumeFillDefault(volume.Config, pool)
+			err := volumeFillDefault(volume.Config, pool)
 			if err != nil {
 				return err
 			}
@@ -2816,7 +2816,7 @@ func patchStorageApiDetectLVSize(name string, d *Daemon) error {
 				volume.Config = map[string]string{}
 
 				// Insert default values.
-				err := driver.VolumeFillDefault(volume.Config, pool)
+				err := volumeFillDefault(volume.Config, pool)
 				if err != nil {
 					return err
 				}

From c54218714cd930771a1a9f33a544048fe8e448b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:45:29 +0000
Subject: [PATCH 03/13] lxd/storage/drivers/interface: Adds FillVolumeConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/interface.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 82450ad5e5..7e04b92e1a 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -46,6 +46,7 @@ type Driver interface {
 	ApplyPatch(name string) error
 
 	// Volumes.
+	FillVolumeConfig(vol Volume) error
 	ValidateVolume(vol Volume, removeUnknownKeys bool) error
 	CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Operation) error
 	CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool, op *operations.Operation) error

From 940cdb499856182c99087c0f5f9597188b31220f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:45:43 +0000
Subject: [PATCH 04/13] lxd/storage/drivers/driver/common: Adds
 FillVolumeConfig no-op for common drivers

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_common.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lxd/storage/drivers/driver_common.go b/lxd/storage/drivers/driver_common.go
index e15950acad..1aaa5561fe 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -77,6 +77,11 @@ func (d *common) validatePool(config map[string]string, driverRules map[string]f
 	return nil
 }
 
+// FillVolumeConfig populate volume with default config.
+func (d *common) FillVolumeConfig(vol Volume) error {
+	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 bcd4898f938f6fdc0e6e843aa33b2b90fe12a737 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:47:31 +0000
Subject: [PATCH 05/13] lxd/storage/drivers/driver/{ceph,lvm}: Adds
 FillVolumeConfig function to populate default filesystem settings

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_ceph_volumes.go | 31 ++++++++++++++++++++++
 lxd/storage/drivers/driver_lvm_volumes.go  | 31 ++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index be2143c02c..8d1b302236 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -708,6 +708,37 @@ func (d *ceph) HasVolume(vol Volume) bool {
 	return err == nil
 }
 
+// FillVolumeConfig populate volume with default config.
+func (d *ceph) FillVolumeConfig(vol Volume) error {
+	// Only validate filesystem config keys for filesystem volumes or VM block volumes (which have an
+	// associated filesystem volume).
+	if vol.ContentType() == ContentTypeFS || vol.IsVMBlock() {
+		// Inherit filesystem from pool if not set.
+		if vol.config["block.filesystem"] == "" {
+			vol.config["block.filesystem"] = d.config["volume.block.filesystem"]
+		}
+
+		// Default filesystem if neither volume nor pool specify an override.
+		if vol.config["block.filesystem"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			vol.config["block.filesystem"] = DefaultFilesystem
+		}
+
+		// Inherit filesystem mount options from pool if not set.
+		if vol.config["block.mount_options"] == "" {
+			vol.config["block.mount_options"] = d.config["volume.block.mount_options"]
+		}
+
+		// Default filesystem mount options if neither volume nor pool specify an override.
+		if vol.config["block.mount_options"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			vol.config["block.mount_options"] = "discard"
+		}
+	}
+
+	return nil
+}
+
 // ValidateVolume validates the supplied volume config.
 func (d *ceph) ValidateVolume(vol Volume, removeUnknownKeys bool) error {
 	rules := map[string]func(value string) error{
diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go
index 2e0561f1b1..6580fc206b 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -239,6 +239,37 @@ func (d *lvm) HasVolume(vol Volume) bool {
 	return volExists
 }
 
+// FillVolumeConfig populate volume with default config.
+func (d *lvm) FillVolumeConfig(vol Volume) error {
+	// Only validate filesystem config keys for filesystem volumes or VM block volumes (which have an
+	// associated filesystem volume).
+	if vol.ContentType() == ContentTypeFS || vol.IsVMBlock() {
+		// Inherit filesystem from pool if not set.
+		if vol.config["block.filesystem"] == "" {
+			vol.config["block.filesystem"] = d.config["volume.block.filesystem"]
+		}
+
+		// Default filesystem if neither volume nor pool specify an override.
+		if vol.config["block.filesystem"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			vol.config["block.filesystem"] = DefaultFilesystem
+		}
+
+		// Inherit filesystem mount options from pool if not set.
+		if vol.config["block.mount_options"] == "" {
+			vol.config["block.mount_options"] = d.config["volume.block.mount_options"]
+		}
+
+		// Default filesystem mount options if neither volume nor pool specify an override.
+		if vol.config["block.mount_options"] == "" {
+			// Unchangeable volume property: Set unconditionally.
+			vol.config["block.mount_options"] = "discard"
+		}
+	}
+
+	return nil
+}
+
 // ValidateVolume validates the supplied volume config.
 func (d *lvm) ValidateVolume(vol Volume, removeUnknownKeys bool) error {
 	rules := map[string]func(value string) error{

From aeee0a8d5b2a057f70166952afcc6964102be4cf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:49:29 +0000
Subject: [PATCH 06/13] lxd/storage/utils: Updates VolumeDBCreate to accept a
 Pool and call driver.FillVolumeConfig

 - Avoids extra pool lookup.
 - Allows per-drive volume defaults to be populated.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/utils.go | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 393d8dbad6..8cabd43f9c 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -192,7 +192,7 @@ func VolumeContentTypeNameToContentType(contentTypeName string) (int, error) {
 }
 
 // VolumeDBCreate creates a volume in the database.
-func VolumeDBCreate(s *state.State, project, poolName, volumeName, volumeDescription, volumeTypeName string, snapshot bool, volumeConfig map[string]string, expiryDate time.Time, contentTypeName string) error {
+func VolumeDBCreate(s *state.State, pool Pool, projectName string, volumeName string, volumeDescription string, volumeTypeName string, snapshot bool, volumeConfig map[string]string, expiryDate time.Time, contentTypeName string) error {
 	// Convert the volume type name to our internal integer representation.
 	volDBType, err := VolumeTypeNameToDBType(volumeTypeName)
 	if err != nil {
@@ -204,14 +204,8 @@ func VolumeDBCreate(s *state.State, project, poolName, volumeName, volumeDescrip
 		return err
 	}
 
-	// Load storage pool the volume will be attached to.
-	poolID, poolStruct, err := s.Cluster.GetStoragePool(poolName)
-	if err != nil {
-		return err
-	}
-
 	// Check that a storage volume of the same storage volume type does not already exist.
-	volumeID, _ := s.Cluster.GetStoragePoolNodeVolumeID(project, volumeName, volDBType, poolID)
+	volumeID, _ := s.Cluster.GetStoragePoolNodeVolumeID(projectName, volumeName, volDBType, pool.ID())
 	if volumeID > 0 {
 		return fmt.Errorf("A storage volume of type %s already exists", volumeTypeName)
 	}
@@ -226,25 +220,28 @@ func VolumeDBCreate(s *state.State, project, poolName, volumeName, volumeDescrip
 		return err
 	}
 
-	// Validate the requested storage volume configuration.
-	err = VolumeValidateConfig(s, volumeName, volType, volumeConfig, poolStruct)
+	vol := drivers.NewVolume(pool.Driver(), pool.Name(), volType, drivers.ContentType(contentTypeName), volumeName, volumeConfig, pool.Driver().Config())
+
+	// Fill default config.
+	err = pool.Driver().FillVolumeConfig(vol)
 	if err != nil {
 		return err
 	}
 
-	err = VolumeFillDefault(volumeConfig, poolStruct)
+	// Validate config.
+	err = pool.Driver().ValidateVolume(vol, false)
 	if err != nil {
 		return err
 	}
 
 	// Create the database entry for the storage volume.
 	if snapshot {
-		_, err = s.Cluster.CreateStorageVolumeSnapshot(project, volumeName, volumeDescription, volDBType, poolID, volumeConfig, expiryDate)
+		_, err = s.Cluster.CreateStorageVolumeSnapshot(projectName, volumeName, volumeDescription, volDBType, pool.ID(), vol.Config(), expiryDate)
 	} else {
-		_, err = s.Cluster.CreateStoragePoolVolume(project, volumeName, volumeDescription, volDBType, poolID, volumeConfig, volDBContentType)
+		_, err = s.Cluster.CreateStoragePoolVolume(projectName, volumeName, volumeDescription, volDBType, pool.ID(), vol.Config(), volDBContentType)
 	}
 	if err != nil {
-		return fmt.Errorf("Error inserting %q of type %q into database %q", poolName, volumeTypeName, err)
+		return fmt.Errorf("Error inserting %q of type %q into database %q", pool.Name(), volumeTypeName, err)
 	}
 
 	return nil

From b78d3f2c19a171dd32ae746e3e397ead685d04ff Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:50:32 +0000
Subject: [PATCH 07/13] lxd/storage/backend/lxd: VolumeDBCreate usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/backend_lxd.go | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 74cd942d07..a411e36df1 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2222,7 +2222,7 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 		}
 	}
 
-	err = VolumeDBCreate(b.state, project.Default, b.name, fingerprint, "", db.StoragePoolVolumeTypeNameImage, false, volConfig, time.Time{}, dbContentType)
+	err = VolumeDBCreate(b.state, b, project.Default, fingerprint, "", db.StoragePoolVolumeTypeNameImage, false, volConfig, time.Time{}, dbContentType)
 	if err != nil {
 		return err
 	}
@@ -2335,7 +2335,7 @@ func (b *lxdBackend) CreateCustomVolume(projectName string, volName string, desc
 	}
 
 	// Create database entry for new storage volume.
-	err = VolumeDBCreate(b.state, projectName, b.name, volName, desc, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(contentType))
+	err = VolumeDBCreate(b.state, b, projectName, volName, desc, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(contentType))
 	if err != nil {
 		return err
 	}
@@ -2459,7 +2459,7 @@ func (b *lxdBackend) CreateCustomVolumeFromCopy(projectName string, volName stri
 		}
 
 		// Create database entry for new storage volume.
-		err = VolumeDBCreate(b.state, projectName, b.name, volName, desc, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(contentType))
+		err = VolumeDBCreate(b.state, b, projectName, volName, desc, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(contentType))
 		if err != nil {
 			return err
 		}
@@ -2471,7 +2471,7 @@ func (b *lxdBackend) CreateCustomVolumeFromCopy(projectName string, volName stri
 				newSnapshotName := drivers.GetSnapshotVolumeName(volName, snapName)
 
 				// Create database entry for new storage volume snapshot.
-				err = VolumeDBCreate(b.state, projectName, b.name, newSnapshotName, desc, db.StoragePoolVolumeTypeNameCustom, true, vol.Config(), time.Time{}, string(contentType))
+				err = VolumeDBCreate(b.state, b, projectName, newSnapshotName, desc, db.StoragePoolVolumeTypeNameCustom, true, vol.Config(), time.Time{}, string(contentType))
 				if err != nil {
 					return err
 				}
@@ -2653,7 +2653,7 @@ func (b *lxdBackend) CreateCustomVolumeFromMigration(projectName string, conn io
 	}
 
 	// Create database entry for new storage volume.
-	err = VolumeDBCreate(b.state, projectName, b.name, args.Name, args.Description, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, args.ContentType)
+	err = VolumeDBCreate(b.state, b, projectName, args.Name, args.Description, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, args.ContentType)
 	if err != nil {
 		return err
 	}
@@ -2665,7 +2665,7 @@ func (b *lxdBackend) CreateCustomVolumeFromMigration(projectName string, conn io
 			newSnapshotName := drivers.GetSnapshotVolumeName(args.Name, snapName)
 
 			// Create database entry for new storage volume snapshot.
-			err = VolumeDBCreate(b.state, projectName, b.name, newSnapshotName, args.Description, db.StoragePoolVolumeTypeNameCustom, true, vol.Config(), time.Time{}, args.ContentType)
+			err = VolumeDBCreate(b.state, b, projectName, newSnapshotName, args.Description, db.StoragePoolVolumeTypeNameCustom, true, vol.Config(), time.Time{}, args.ContentType)
 			if err != nil {
 				return err
 			}
@@ -3124,7 +3124,7 @@ func (b *lxdBackend) CreateCustomVolumeSnapshot(projectName, volName string, new
 	}
 
 	// Create database entry for new storage volume snapshot.
-	err = VolumeDBCreate(b.state, projectName, b.name, fullSnapshotName, parentVol.Description, db.StoragePoolVolumeTypeNameCustom, true, parentVol.Config, newExpiryDate, parentVol.ContentType)
+	err = VolumeDBCreate(b.state, b, projectName, fullSnapshotName, parentVol.Description, db.StoragePoolVolumeTypeNameCustom, true, parentVol.Config, newExpiryDate, parentVol.ContentType)
 	if err != nil {
 		return err
 	}
@@ -3629,7 +3629,7 @@ func (b *lxdBackend) CreateCustomVolumeFromBackup(srcBackup backup.Info, srcData
 	}
 
 	// Create database entry for new storage volume using the validated config.
-	err = VolumeDBCreate(b.state, srcBackup.Project, b.name, srcBackup.Name, srcBackup.Config.Volume.Description, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(vol.ContentType()))
+	err = VolumeDBCreate(b.state, b, srcBackup.Project, srcBackup.Name, srcBackup.Config.Volume.Description, db.StoragePoolVolumeTypeNameCustom, false, vol.Config(), time.Time{}, string(vol.ContentType()))
 	if err != nil {
 		return err
 	}
@@ -3652,7 +3652,7 @@ func (b *lxdBackend) CreateCustomVolumeFromBackup(srcBackup backup.Info, srcData
 			return err
 		}
 
-		err = VolumeDBCreate(b.state, srcBackup.Project, b.name, fullSnapName, snapshot.Description, db.StoragePoolVolumeTypeNameCustom, true, snapVol.Config(), *snapshot.ExpiresAt, string(snapVol.ContentType()))
+		err = VolumeDBCreate(b.state, b, srcBackup.Project, fullSnapName, snapshot.Description, db.StoragePoolVolumeTypeNameCustom, true, snapVol.Config(), *snapshot.ExpiresAt, string(snapVol.ContentType()))
 		if err != nil {
 			return err
 		}

From 478f74f076e3d5537e6dce7161b6f39da339c4e8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:50:08 +0000
Subject: [PATCH 08/13] lxd/storage/utils: Removes VolumeFillDefault and
 VolumeValidateConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/utils.go | 77 --------------------------------------------
 1 file changed, 77 deletions(-)

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 8cabd43f9c..c070c3375f 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -307,83 +307,6 @@ var StorageVolumeConfigKeys = map[string]func(value string) ([]string, error){
 	},
 }
 
-// VolumeValidateConfig validations volume config. Deprecated.
-func VolumeValidateConfig(s *state.State, volName string, volType drivers.VolumeType, config map[string]string, parentPool *api.StoragePool) error {
-	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, commonRules())
-	if err != drivers.ErrUnknownDriver {
-		// Note: This legacy validation function doesn't have the concept of validating different content
-		// types, so it is hardcoded as ContentTypeFS.
-		return driver.ValidateVolume(drivers.NewVolume(driver, parentPool.Name, volType, drivers.ContentTypeFS, volName, config, parentPool.Config), false)
-	}
-
-	// Otherwise fallback to doing legacy validation.
-	for key, val := range config {
-		// User keys are not validated.
-		if strings.HasPrefix(key, "user.") {
-			continue
-		}
-
-		// Validate storage volume config keys.
-		validator, ok := StorageVolumeConfigKeys[key]
-		if !ok {
-			return fmt.Errorf("Invalid storage volume configuration key: %s", key)
-		}
-
-		_, err := validator(val)
-		if err != nil {
-			return err
-		}
-
-		if parentPool.Driver != "zfs" || parentPool.Driver == "dir" {
-			if config["zfs.use_refquota"] != "" {
-				return fmt.Errorf("the key volume.zfs.use_refquota cannot be used with non zfs storage volumes")
-			}
-
-			if config["zfs.remove_snapshots"] != "" {
-				return fmt.Errorf("the key volume.zfs.remove_snapshots cannot be used with non zfs storage volumes")
-			}
-		}
-
-		if parentPool.Driver == "dir" {
-			if config["block.mount_options"] != "" {
-				return fmt.Errorf("the key block.mount_options cannot be used with dir storage volumes")
-			}
-
-			if config["block.filesystem"] != "" {
-				return fmt.Errorf("the key block.filesystem cannot be used with dir storage volumes")
-			}
-		}
-	}
-
-	return nil
-}
-
-// VolumeFillDefault fills default settings into a volume config.
-func VolumeFillDefault(config map[string]string, parentPool *api.StoragePool) error {
-	if parentPool.Driver == "lvm" || parentPool.Driver == "ceph" {
-		if config["block.filesystem"] == "" {
-			config["block.filesystem"] = parentPool.Config["volume.block.filesystem"]
-		}
-		if config["block.filesystem"] == "" {
-			// Unchangeable volume property: Set unconditionally.
-			config["block.filesystem"] = drivers.DefaultFilesystem
-		}
-
-		if config["block.mount_options"] == "" {
-			config["block.mount_options"] = parentPool.Config["volume.block.mount_options"]
-		}
-		if config["block.mount_options"] == "" {
-			// Unchangeable volume property: Set unconditionally.
-			config["block.mount_options"] = "discard"
-		}
-	}
-
-	return nil
-}
-
 // VolumeSnapshotsGet returns a list of snapshots of the form <volume>/<snapshot-name>.
 func VolumeSnapshotsGet(s *state.State, projectName string, pool string, volume string, volType int) ([]db.StorageVolumeArgs, error) {
 	poolID, err := s.Cluster.GetStoragePoolID(pool)

From 89765ec19228101408d5116036b2cefda947c734 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:44:40 +0000
Subject: [PATCH 09/13] lxd/storage/pool/interface: Adds FillInstanceConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/pool_interface.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/storage/pool_interface.go b/lxd/storage/pool_interface.go
index 5423c89829..ec04657621 100644
--- a/lxd/storage/pool_interface.go
+++ b/lxd/storage/pool_interface.go
@@ -35,6 +35,7 @@ type Pool interface {
 	ApplyPatch(name string) error
 
 	// Instances.
+	FillInstanceConfig(inst instance.Instance, config map[string]string) error
 	CreateInstance(inst instance.Instance, op *operations.Operation) error
 	CreateInstanceFromBackup(srcBackup backup.Info, srcData io.ReadSeeker, op *operations.Operation) (func(instance.Instance) error, func(), error)
 	CreateInstanceFromCopy(inst instance.Instance, src instance.Instance, snapshots bool, op *operations.Operation) error

From 97194874c4089ef337d9b23d184562f35e6ea31e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:48:23 +0000
Subject: [PATCH 10/13] lxd/storage/backend/lxd: Implements FillInstanceConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/backend_lxd.go | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index a411e36df1..a2c86b94e9 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -430,6 +430,38 @@ func (b *lxdBackend) instanceRootVolumeConfig(inst instance.Instance) (map[strin
 	return vol.Config, nil
 }
 
+// FillInstanceConfig populates the supplied instance volume config map with any defaults based on the storage
+// pool and instance type being used.
+func (b *lxdBackend) FillInstanceConfig(inst instance.Instance, config map[string]string) error {
+	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name()})
+	logger.Debug("FillInstanceConfig started")
+	defer logger.Debug("FillInstanceConfig finished")
+
+	volType, err := InstanceTypeToVolumeType(inst.Type())
+	if err != nil {
+		return err
+	}
+
+	contentType := InstanceContentType(inst)
+
+	// Get the volume name on storage.
+	volStorageName := project.Instance(inst.Project(), inst.Name())
+
+	// Fill default config in volume (creates internal copy of supplied config and modifies that).
+	vol := b.newVolume(volType, contentType, volStorageName, config)
+	err = b.driver.FillVolumeConfig(vol)
+	if err != nil {
+		return err
+	}
+
+	// Copy filled volume config back into supplied config map.
+	for k, v := range vol.Config() {
+		config[k] = v
+	}
+
+	return nil
+}
+
 // CreateInstance creates an empty instance.
 func (b *lxdBackend) CreateInstance(inst instance.Instance, op *operations.Operation) error {
 	logger := logging.AddContext(b.logger, log.Ctx{"project": inst.Project(), "instance": inst.Name()})

From c21b90025e5a66949e0ab6de818354a19c9dec21 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:50:49 +0000
Subject: [PATCH 11/13] lxd/storage/backend/mock: Adds FillInstanceConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/backend_mock.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lxd/storage/backend_mock.go b/lxd/storage/backend_mock.go
index bfc443fa29..ec0f9c198f 100644
--- a/lxd/storage/backend_mock.go
+++ b/lxd/storage/backend_mock.go
@@ -67,6 +67,10 @@ func (b *mockBackend) ApplyPatch(name string) error {
 	return nil
 }
 
+func (b *mockBackend) FillInstanceConfig(inst instance.Instance, config map[string]string) error {
+	return nil
+}
+
 func (b *mockBackend) CreateInstance(inst instance.Instance, op *operations.Operation) error {
 	return nil
 }

From 0ba1df4f34d46a26f157ed453394cb0f11ccc53d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:51:26 +0000
Subject: [PATCH 12/13] lxd/instance/drivers/driver/lxc: Updates lxcCreate to
 use storagePool.FillInstanceConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_lxc.go | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go
index 054c698372..f21fcb315d 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -233,39 +233,29 @@ func lxcCreate(s *state.State, args db.InstanceArgs) (instance.Instance, error)
 		return nil, fmt.Errorf("The container's root device is missing the pool property")
 	}
 
-	storagePool := rootDiskDevice["pool"]
-
-	// Get the storage pool ID for the container.
-	poolID, dbPool, err := s.Cluster.GetStoragePool(storagePool)
-	if err != nil {
-		return nil, err
-	}
-
 	// Initialize the storage pool.
-	pool, err := storagePools.GetPoolByName(c.state, storagePool)
+	c.storagePool, err = storagePools.GetPoolByName(c.state, rootDiskDevice["pool"])
 	if err != nil {
 		return nil, err
 	}
 
-	// Fill in any default volume config.
+	// Fill default config.
 	volumeConfig := map[string]string{}
-	err = storagePools.VolumeFillDefault(volumeConfig, dbPool)
+	err = c.storagePool.FillInstanceConfig(c, volumeConfig)
 	if err != nil {
 		return nil, err
 	}
 
 	// Create a new storage volume database entry for the container's storage volume.
 	if c.IsSnapshot() {
-		_, err = s.Cluster.CreateStorageVolumeSnapshot(args.Project, args.Name, "", db.StoragePoolVolumeTypeContainer, poolID, volumeConfig, time.Time{})
+		_, err = s.Cluster.CreateStorageVolumeSnapshot(args.Project, args.Name, "", db.StoragePoolVolumeTypeContainer, c.storagePool.ID(), volumeConfig, time.Time{})
 	} else {
-		_, err = s.Cluster.CreateStoragePoolVolume(args.Project, args.Name, "", db.StoragePoolVolumeTypeContainer, poolID, volumeConfig, db.StoragePoolVolumeContentTypeFS)
+		_, err = s.Cluster.CreateStoragePoolVolume(args.Project, args.Name, "", db.StoragePoolVolumeTypeContainer, c.storagePool.ID(), volumeConfig, db.StoragePoolVolumeContentTypeFS)
 	}
 	if err != nil {
 		return nil, err
 	}
 
-	c.storagePool = pool
-
 	// Setup initial idmap config
 	var idmap *idmap.IdmapSet
 	base := int64(0)

From 28bf8ac84bcb40351ab832c90c8dc75bd49fb03e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 19 Nov 2020 12:51:52 +0000
Subject: [PATCH 13/13] lxd/instance/drivers/driver/qemu: Updates qemuCreate to
 use storagePool.FillInstanceConfig

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index abd05b2e85..ce3d08adc8 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -250,27 +250,25 @@ func qemuCreate(s *state.State, args db.InstanceArgs) (instance.Instance, error)
 		return nil, fmt.Errorf("The instances's root device is missing the pool property")
 	}
 
-	storagePool := rootDiskDevice["pool"]
-
-	// Get the storage pool ID for the instance.
-	poolID, pool, err := s.Cluster.GetStoragePool(storagePool)
+	// Initialize the storage pool.
+	vm.storagePool, err = storagePools.GetPoolByName(vm.state, rootDiskDevice["pool"])
 	if err != nil {
 		return nil, err
 	}
 
-	// Fill in any default volume config.
+	// Fill default config.
 	volumeConfig := map[string]string{}
-	err = storagePools.VolumeFillDefault(volumeConfig, pool)
+	err = vm.storagePool.FillInstanceConfig(vm, volumeConfig)
 	if err != nil {
 		return nil, err
 	}
 
 	// Create a new database entry for the instance's storage volume.
 	if vm.IsSnapshot() {
-		_, err = s.Cluster.CreateStorageVolumeSnapshot(args.Project, args.Name, "", db.StoragePoolVolumeTypeVM, poolID, volumeConfig, time.Time{})
+		_, err = s.Cluster.CreateStorageVolumeSnapshot(args.Project, args.Name, "", db.StoragePoolVolumeTypeVM, vm.storagePool.ID(), volumeConfig, time.Time{})
 
 	} else {
-		_, err = s.Cluster.CreateStoragePoolVolume(args.Project, args.Name, "", db.StoragePoolVolumeTypeVM, poolID, volumeConfig, db.StoragePoolVolumeContentTypeBlock)
+		_, err = s.Cluster.CreateStoragePoolVolume(args.Project, args.Name, "", db.StoragePoolVolumeTypeVM, vm.storagePool.ID(), volumeConfig, db.StoragePoolVolumeContentTypeBlock)
 	}
 	if err != nil {
 		return nil, err


More information about the lxc-devel mailing list