[lxc-devel] [lxd/master] Storage EnsureImage filesystem regeneration

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 451 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200109/6ddc4730/attachment.bin>
-------------- next part --------------
From 01d9c0a3bc0aeb2f34b519476738d247efc279e7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 9 Jan 2020 19:35:32 +0000
Subject: [PATCH 1/3] lxd/storage/drivers/volume: Adds DefaultFilesystem
 constant of ext4

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

diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index fcba388445..ce968b7645 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -11,6 +11,9 @@ import (
 
 var defaultBlockSize = "10GB"
 
+// DefaultFilesystem filesytem to use for block devices by default.
+var DefaultFilesystem = "ext4"
+
 var volIDQuotaSkip = int64(-1)
 
 // VolumeType represents a storage volume type.

From c6dff25631f33f0898f7258cdb6e3a1948c21bda Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 9 Jan 2020 19:35:08 +0000
Subject: [PATCH 2/3] lxd/storage/utils: Uses DefaultFilesystem in
 VolumeFillDefault

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 0790164ca9..a2f2969542 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -447,7 +447,7 @@ func VolumeFillDefault(name string, config map[string]string, parentPool *api.St
 		}
 		if config["block.filesystem"] == "" {
 			// Unchangeable volume property: Set unconditionally.
-			config["block.filesystem"] = "ext4"
+			config["block.filesystem"] = drivers.DefaultFilesystem
 		}
 
 		if config["block.mount_options"] == "" {

From 314aa97421e27d24a57d43ae85a801a874302dbf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 9 Jan 2020 19:34:36 +0000
Subject: [PATCH 3/3] lxd/storage/backend/lxd: Updates EnsureImage to detech
 filesystem changes and regenerate

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index d9bc9439ea..41409f25eb 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1807,6 +1807,15 @@ func (b *lxdBackend) UnmountInstanceSnapshot(inst instance.Instance, op *operati
 	return b.driver.UnmountVolumeSnapshot(vol, op)
 }
 
+// poolBlockFilesystem returns the filesystem used for new block device filesystems.
+func (b *lxdBackend) poolBlockFilesystem() string {
+	if b.db.Config["volume.block.filesystem"] != "" {
+		return b.db.Config["volume.block.filesystem"]
+	}
+
+	return drivers.DefaultFilesystem
+}
+
 // EnsureImage creates an optimized volume of the image if supported by the storage pool driver and
 // the volume doesn't already exist.
 func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) error {
@@ -1824,31 +1833,48 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 	unlock := locking.Lock(b.name, string(drivers.VolumeTypeImage), fmt.Sprintf("EnsureImage_%v", fingerprint))
 	defer unlock()
 
-	// There's no need to pass the content type or config. Both are not needed
-	// when checking the existence of volumes.
-	vol := b.newVolume(drivers.VolumeTypeImage, "", fingerprint, nil)
-
-	// Check if we already have a suitable volume.
-	if b.driver.HasVolume(vol) {
-		return nil
-	}
-
 	// Load image info from database.
 	_, image, err := b.state.Cluster.ImageGetFromAnyProject(fingerprint)
 	if err != nil {
 		return err
 	}
 
+	// Derive content type from image type. Image types are not the same as instance types, so don't use
+	// instance type constants for comparison.
 	contentType := drivers.ContentTypeFS
-
-	// Image types are not the same as instance types, so don't use instance type constants.
 	if image.Type == "virtual-machine" {
 		contentType = drivers.ContentTypeBlock
 	}
 
+	// Try and load any existing volume config on this storage pool so we can compare filesystems if needed.
+	_, imgDBVol, err := b.state.Cluster.StoragePoolNodeVolumeGetTypeByProject("default", fingerprint, db.StoragePoolVolumeTypeImage, b.ID())
+	if err != nil {
+		if err != db.ErrNoSuchObject {
+			return err
+		}
+	}
+
+	// If an existing DB row was found, check if filesystem is the same as the current pool's filesystem.
+	// If not we need to delete the existing cached image volume and re-create using new filesystem.
+	if imgDBVol != nil && contentType == drivers.ContentTypeFS {
+		if imgDBVol.Config["block.filesystem"] != b.poolBlockFilesystem() {
+			logger.Debug("Filesystem of pool has changed since cached image volume created, regenerating image volume")
+			err = b.DeleteImage(fingerprint, op)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
 	// Create the new image volume. No config for an image volume so set to nil.
+	// Pool config values will be read by the underlying driver if needed.
 	imgVol := b.newVolume(drivers.VolumeTypeImage, contentType, fingerprint, nil)
 
+	// Check if we already have a suitable volume on storage device.
+	if b.driver.HasVolume(imgVol) {
+		return nil
+	}
+
 	volFiller := drivers.VolumeFiller{
 		Fingerprint: fingerprint,
 		Fill:        b.imageFiller(fingerprint, op),