[lxc-devel] [lxd/master] Storage: EnsureImage and CreateInstanceFromImage improvements on size management

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


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 634 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201105/3d8e998b/attachment.bin>
-------------- next part --------------
From eb99aae132343179d4909cd59d3541fbbff5b4bd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Nov 2020 12:09:43 +0000
Subject: [PATCH 1/3] lxd/storage/utils: Improves logging and uses size value
 from vol.ConfigSizeFromSource in ImageUnpack

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 7675c7371f..f802933416 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -515,6 +515,8 @@ func validateVolumeCommonRules(vol drivers.Volume) map[string]func(string) error
 // 	- Unpack metadata tarball into mountPath.
 //	- Check rootBlockPath is a file and convert qcow2 file into raw format in rootBlockPath.
 func ImageUnpack(imageFile string, vol drivers.Volume, destBlockFile string, blockBackend, runningInUserns bool, tracker *ioprogress.ProgressTracker) (int64, error) {
+	logger := logging.AddContext(logger.Log, log.Ctx{"imageFile": imageFile, "vol": vol.Name()})
+
 	// For all formats, first unpack the metadata (or combined) tarball into destPath.
 	imageRootfsFile := imageFile + ".rootfs"
 	destPath := vol.MountPath()
@@ -594,20 +596,23 @@ func ImageUnpack(imageFile string, vol drivers.Volume, destBlockFile string, blo
 			}
 
 			if volSizeBytes < imgInfo.VirtualSize {
-				// Create a partial image volume struct and then use it to check that target
-				// volume size can be increased as needed.
+				// If the target volume's size is smaller than the image unpack size, then we need
+				// to check whether it is inline with the pool's settings to allow us to increase
+				// the target volume's size. Create a partial image volume struct and then use it
+				// to check that target volume size can be set as needed.
 				imgVolConfig := map[string]string{
 					"volatile.rootfs.size": fmt.Sprintf("%d", imgInfo.VirtualSize),
 				}
 				imgVol := drivers.NewVolume(nil, "", drivers.VolumeTypeImage, drivers.ContentTypeBlock, "", imgVolConfig, nil)
 
-				_, err = vol.ConfigSizeFromSource(imgVol)
+				logger.Debug("Checking image unpack size")
+				newVolSize, err := vol.ConfigSizeFromSource(imgVol)
 				if err != nil {
 					return -1, err
 				}
 
-				logger.Debugf("Increasing %q volume size from %d to %d to accomomdate image %q unpack", dstPath, volSizeBytes, imgInfo.VirtualSize, imgPath)
-				err = vol.SetQuota(fmt.Sprintf("%d", imgInfo.VirtualSize), nil)
+				logger.Debug("Increasing volume size", log.Ctx{"imgPath": imgPath, "dstPath": dstPath, "oldSize": volSizeBytes, "newSize": newVolSize})
+				err = vol.SetQuota(newVolSize, nil)
 				if err != nil {
 					return -1, errors.Wrapf(err, "Error increasing volume size")
 				}
@@ -616,7 +621,7 @@ func ImageUnpack(imageFile string, vol drivers.Volume, destBlockFile string, blo
 
 		// Convert the qcow2 format to a raw block device using qemu's dd mode to avoid issues with
 		// loop backed storage pools. Use the MinBlockBoundary block size to speed up conversion.
-		logger.Debugf("Converting qcow2 image %q to raw disk %q", imgPath, dstPath)
+		logger.Debug("Converting qcow2 image to raw disk", log.Ctx{"imgPath": imgPath, "dstPath": dstPath})
 		_, err = shared.RunCommand("qemu-img", "dd", "-f", "qcow2", "-O", "raw", fmt.Sprintf("bs=%d", drivers.MinBlockBoundary), fmt.Sprintf("if=%s", imgPath), fmt.Sprintf("of=%s", dstPath))
 		if err != nil {
 			return -1, errors.Wrapf(err, "Failed converting image to raw at %q", dstPath)

From 3268680868c19a47d5dc00dac3845a8b96cd7c35 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Nov 2020 12:10:43 +0000
Subject: [PATCH 2/3] lxd/storage/backend/lxd: Improves logging in
 CreateInstanceFromImage

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index c74270ec76..e3f202ef3c 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1007,9 +1007,9 @@ func (b *lxdBackend) CreateInstanceFromImage(inst instance.Instance, fingerprint
 			return err
 		}
 	} else {
-		// If the driver does support optimized images then ensure the optimized image
-		// volume has been created for the archive's fingerprint and then proceed to create
-		// a new volume by copying the optimized image volume.
+		// If the driver supports optimized images then ensure the optimized image volume has been created
+		// for the images's fingerprint and that it matches the pool's current volume settings, and if not
+		// recreating using the pool's current volume settings.
 		err = b.EnsureImage(fingerprint, op)
 		if err != nil {
 			return err
@@ -1023,18 +1023,26 @@ func (b *lxdBackend) CreateInstanceFromImage(inst instance.Instance, fingerprint
 
 		imgVol := b.newVolume(drivers.VolumeTypeImage, contentType, fingerprint, imgDBVol.Config)
 
-		// Derive size to use for new volume from source (and check it doesn't exceed volume size limits).
-		volSize, err := vol.ConfigSizeFromSource(imgVol)
+		// Derive the volume size to use for a new volume when copying from a source volume.
+		// Where possible (if the source volume has a volatile.rootfs.size property), it checks that the
+		// source volume isn't larger than the volume's "size" and the pool's "volume.size" setting.
+		logger.Debug("Checking volume size")
+		newVolSize, err := vol.ConfigSizeFromSource(imgVol)
 		if err != nil {
 			return err
 		}
 
-		vol.SetConfigSize(volSize)
+		// Set the derived size directly as the "size" property on the new volume so that it is applied.
+		vol.SetConfigSize(newVolSize)
+
+		// Proceed to create a new volume by copying the optimized image volume.
 		err = b.driver.CreateVolumeFromCopy(vol, imgVol, false, op)
 
-		// If the driver returns ErrCannotBeShrunk, this means that the cached volume is larger than the
-		// requested new volume size and the cached image volume, once snapshotted, cannot be shrunk.
+		// If the driver returns ErrCannotBeShrunk, this means that the cached volume that the new volume
+		// is to be created from is larger than the requested new volume size, and cannot be shrunk.
 		// So we unpack the image directly into a new volume rather than use the optimized snapsot.
+		// This is slower but allows for individual volumes to be created from an image that are smaller
+		// than the pool's volume settings.
 		if errors.Cause(err) == drivers.ErrCannotBeShrunk {
 			logger.Debug("Cached image volume is larger than new volume and cannot be shrunk, creating non-optimized volume")
 

From 7c8a570b20b41c8a9cfad871d02c36ae773f1ac1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 5 Nov 2020 12:11:20 +0000
Subject: [PATCH 3/3] lxd/storage/backend/lxd: Improves logging and uses
 imgVol.ConfigSizeFromSource in EnsureImage

By using imgVol.ConfigSizeFromSource from an existing image record, we ensure that we don't try to shrink an existing optimized volume that is larger than the default volume size (when the pool doesn't have a volume size restriction).

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index e3f202ef3c..28a7b11975 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2075,8 +2075,10 @@ func (b *lxdBackend) poolBlockFilesystem() string {
 	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.
+// EnsureImage creates an optimized volume of the image if supported by the storage pool driver and the volume
+// doesn't already exist. If the volume already exists then it is checked to ensure it matches the pools current
+// volume settings ("volume.size" and "block.filesystem" if applicable). If not the optimized volume is removed
+// and regenerated to apply the pool's current volume settings.
 func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) error {
 	logger := logging.AddContext(b.logger, log.Ctx{"fingerprint": fingerprint})
 	logger.Debug("EnsureImage started")
@@ -2116,11 +2118,18 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 		}
 	}
 
+	// 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)
+
 	// 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.
 	// We need to do this for VM block images too, as they create a filesystem based config volume too.
 	if imgDBVol != nil {
-		if b.Driver().Info().BlockBacking && imgDBVol.Config["block.filesystem"] != b.poolBlockFilesystem() {
+		// Add existing image volume's config to imgVol.
+		imgVol = b.newVolume(drivers.VolumeTypeImage, contentType, fingerprint, imgDBVol.Config)
+
+		if b.Driver().Info().BlockBacking && imgVol.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 {
@@ -2129,19 +2138,28 @@ func (b *lxdBackend) EnsureImage(fingerprint string, op *operations.Operation) e
 		}
 	}
 
-	// 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) {
 		if imgDBVol != nil {
-			// If there is an existing volume, then make sure it has the same size as the pool's
-			// current volume.size option (with using ConfigSize()) and if not then resize/recreate
-			// depending on what the store driver supports.
+			// Work out what size the image volume should be as if we were creating from scratch.
+			// This takes into account the existing volume's "volatile.rootfs.size" setting if set so
+			// as to avoid trying to shrink a larger image volume back to the default size when it is
+			// allowed to be larger than the default as the pool doesn't specify a volume.size.
+			logger.Debug("Checking image volume size")
+			newVolSize, err := imgVol.ConfigSizeFromSource(imgVol)
+			if err != nil {
+				return err
+			}
+
+			imgVol.SetConfigSize(newVolSize)
+
+			// Try applying the current size policy to the existin volume. If it is the same the driver
+			// should make no changes, and if not then attempt to resize it to the new policy.
 			logger.Debug("Setting image volume size", "size", imgVol.ConfigSize())
 			err = b.driver.SetVolumeQuota(imgVol, imgVol.ConfigSize(), op)
 			if errors.Cause(err) == drivers.ErrCannotBeShrunk || errors.Cause(err) == drivers.ErrNotSupported {
+				// If the driver cannot resize the existing image volume to the new policy size
+				// then delete the image volume and try to recreate using the new policy settings.
 				logger.Debug("Volume size of pool has changed since cached image volume created and cached volume cannot be resized, regenerating image volume")
 				err = b.DeleteImage(fingerprint, op)
 				if err != nil {


More information about the lxc-devel mailing list