The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8118
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) ===
- Updates `EnsureImage` to take into account existing image volume's `volatile.rootfs.size` property when calculating the current size the volume should be. Avoids trying to shrink an existing volume that is larger than the default size when the pool doesn't have volume size limit.
- Improves comments in `CreateInstanceFromImage`.
From eb99aae132343179d4909cd59d3541fbbff5b4bd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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 {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
