[lxc-devel] [lxd/master] Storage: Updates volume size treatment to be consistent across drivers

tomponline on Github lxc-bot at linuxcontainers.org
Fri May 1 10:28:23 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1036 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200501/f0cb1a43/attachment-0001.bin>
-------------- next part --------------
From 970117bf3834e3a1eb1e5a5ad5636a85496d5c2b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:09:14 +0100
Subject: [PATCH 01/17] lxd/storage/drivers/utils: Updates
 roundVolumeBlockFileSizeBytes to take byte size

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

diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go
index 7945ca3cde..3ff08a0de8 100644
--- a/lxd/storage/drivers/utils.go
+++ b/lxd/storage/drivers/utils.go
@@ -16,7 +16,6 @@ import (
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/idmap"
-	"github.com/lxc/lxd/shared/units"
 )
 
 const minBlockBoundary = 8192
@@ -314,46 +313,41 @@ func createSparseFile(filePath string, sizeBytes int64) error {
 }
 
 // roundVolumeBlockFileSizeBytes parses the supplied size string and then rounds it to the nearest 8k bytes.
-func roundVolumeBlockFileSizeBytes(blockSize string) (int64, error) {
-	blockSizeBytes, err := units.ParseByteSizeString(blockSize)
-	if err != nil {
-		return -1, err
-	}
-
+func roundVolumeBlockFileSizeBytes(sizeBytes int64) (int64, error) {
 	// Qemu requires image files to be in traditional storage block boundaries.
 	// We use 8k here to ensure our images are compatible with all of our backend drivers.
-	if blockSizeBytes < minBlockBoundary {
-		blockSizeBytes = minBlockBoundary
+	if sizeBytes < minBlockBoundary {
+		sizeBytes = minBlockBoundary
 	}
 
 	// Round the size to closest minBlockBoundary bytes to avoid qemu boundary issues.
-	return int64(blockSizeBytes/minBlockBoundary) * minBlockBoundary, nil
+	return int64(sizeBytes/minBlockBoundary) * minBlockBoundary, nil
 }
 
 // ensureVolumeBlockFile creates or resizes the raw block file for a volume to the specified size.
-func ensureVolumeBlockFile(path, blockSize string) error {
-	if blockSize == "" {
-		blockSize = defaultBlockSize
+func ensureVolumeBlockFile(path string, sizeBytes int64) error {
+	if sizeBytes <= 0 {
+		return fmt.Errorf("Size cannot be zero")
 	}
 
 	// Get rounded block size to avoid qemu boundary issues.
-	blockSizeBytes, err := roundVolumeBlockFileSizeBytes(blockSize)
+	sizeBytes, err := roundVolumeBlockFileSizeBytes(sizeBytes)
 	if err != nil {
 		return err
 	}
 
 	if shared.PathExists(path) {
-		_, err = shared.RunCommand("qemu-img", "resize", "-f", "raw", path, fmt.Sprintf("%d", blockSizeBytes))
+		_, err = shared.RunCommand("qemu-img", "resize", "-f", "raw", path, fmt.Sprintf("%d", sizeBytes))
 		if err != nil {
-			return errors.Wrapf(err, "Failed resizing disk image %s to size %s", path, blockSize)
+			return errors.Wrapf(err, "Failed resizing disk image %s to size %s", path, sizeBytes)
 		}
 	} else {
 		// If path doesn't exist, then there has been no filler function
 		// supplied to create it from another source. So instead create an empty
 		// volume (use for PXE booting a VM).
-		_, err = shared.RunCommand("qemu-img", "create", "-f", "raw", path, fmt.Sprintf("%d", blockSizeBytes))
+		_, err = shared.RunCommand("qemu-img", "create", "-f", "raw", path, fmt.Sprintf("%d", sizeBytes))
 		if err != nil {
-			return errors.Wrapf(err, "Failed creating disk image %s as size %s", path, blockSize)
+			return errors.Wrapf(err, "Failed creating disk image %s as size %s", path, sizeBytes)
 		}
 	}
 

From 0b1388432dcc310bcbe47f1e811d05de9f460edb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:09:56 +0100
Subject: [PATCH 02/17] lxd/storage/drivers/generic/vfs: Updates
 genericVFSResizeBlockFile to accept size as bytes

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

diff --git a/lxd/storage/drivers/generic_vfs.go b/lxd/storage/drivers/generic_vfs.go
index 57ad06a62a..eaee1fb6df 100644
--- a/lxd/storage/drivers/generic_vfs.go
+++ b/lxd/storage/drivers/generic_vfs.go
@@ -779,8 +779,8 @@ func genericVFSBackupUnpack(d Driver, vol Volume, snapshots []string, srcData io
 // genericVFSResizeBlockFile resizes an existing block file to the specified size. Returns true if resize took
 // place, false if not. Both requested size and existing file size are rounded to nearest block size using
 // roundVolumeBlockFileSizeBytes() before decision whether to resize is taken.
-func genericVFSResizeBlockFile(filePath, size string) (bool, error) {
-	if size == "" || size == "0" {
+func genericVFSResizeBlockFile(filePath string, sizeBytes int64) (bool, error) {
+	if sizeBytes <= 0 {
 		return false, fmt.Errorf("Size cannot be zero")
 	}
 
@@ -792,7 +792,7 @@ func genericVFSResizeBlockFile(filePath, size string) (bool, error) {
 	oldSizeBytes := fi.Size()
 
 	// Round the supplied size the same way the block files created are so its accurate comparison.
-	newSizeBytes, err := roundVolumeBlockFileSizeBytes(size)
+	newSizeBytes, err := roundVolumeBlockFileSizeBytes(sizeBytes)
 	if err != nil {
 		return false, err
 	}
@@ -806,7 +806,7 @@ func genericVFSResizeBlockFile(filePath, size string) (bool, error) {
 	}
 
 	// Resize block file.
-	err = ensureVolumeBlockFile(filePath, size)
+	err = ensureVolumeBlockFile(filePath, sizeBytes)
 	if err != nil {
 		return false, err
 	}

From 4d010d2f17c9e252da8a8ece24f175d1b76a3013 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:11:07 +0100
Subject: [PATCH 03/17] lxd/storage/drivers/driver/btrfs/utils: Adds volumeSize
 function

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

diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go
index e097cfbd60..5b30f043c4 100644
--- a/lxd/storage/drivers/driver_btrfs_utils.go
+++ b/lxd/storage/drivers/driver_btrfs_utils.go
@@ -378,3 +378,15 @@ func (d *btrfs) receiveSubvolume(path string, targetPath string, conn io.ReadWri
 
 	return nil
 }
+
+// volumeSize returns the size to use when creating new a volume.
+func (d *btrfs) volumeSize(vol Volume) string {
+	size := vol.ExpandedConfig("size")
+
+	// Block images always need a size.
+	if vol.contentType == ContentTypeBlock && (size == "" || size == "0") {
+		return defaultBlockSize
+	}
+
+	return size
+}

From cab52b86791eefc06cd91af211eb184d9278e0e3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:12:14 +0100
Subject: [PATCH 04/17] lxd/storage/drivers/driver/btrfs/volumes: Updates
 CreateVolume to use volumeSize()

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

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index a8ca986e9e..410e3d8ed5 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -60,7 +60,13 @@ func (d *btrfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Op
 	// If we are creating a block volume, resize it to the requested size or the default.
 	// We expect the filler function to have converted the qcow2 image to raw into the rootBlockPath.
 	if vol.contentType == ContentTypeBlock {
-		err := ensureVolumeBlockFile(rootBlockPath, vol.ExpandedConfig("size"))
+		// Convert to bytes.
+		sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol))
+		if err != nil {
+			return err
+		}
+
+		err = ensureVolumeBlockFile(rootBlockPath, sizeBytes)
 		if err != nil {
 			return err
 		}
@@ -74,7 +80,7 @@ func (d *btrfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Op
 		}
 	} else if vol.contentType == ContentTypeFS {
 		// Set initial quota for filesystem volumes.
-		err := d.SetVolumeQuota(vol, vol.ExpandedConfig("size"), op)
+		err := d.SetVolumeQuota(vol, d.volumeSize(vol), op)
 		if err != nil {
 			return err
 		}
@@ -249,9 +255,9 @@ func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bo
 	// This is so the pool default volume size isn't take into account for volume copies.
 	volSize := vol.config["size"]
 
-	// If source is an image then use expanded config so that we take into account pool default volume size.
+	// If source is an image then take into account default volume sizes if not specified.
 	if srcVol.volType == VolumeTypeImage {
-		volSize = vol.ExpandedConfig("size")
+		volSize = d.volumeSize(vol)
 	}
 
 	err = d.SetVolumeQuota(vol, volSize, op)

From 364fdfc9d317a36473e2ed45dd80aae9897c93f5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:13:01 +0100
Subject: [PATCH 05/17] lxd/storage/drivers/driver/btrfs/volumes: Updates
 SetVolumeQuota to be byte oriented internally

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

diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go
index 410e3d8ed5..9048a01f4c 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -449,20 +449,27 @@ func (d *btrfs) GetVolumeUsage(vol Volume) (int64, error) {
 }
 
 // SetVolumeQuota sets the quota on the volume.
+// Does nothing if supplied with an empty/zero size for block volumes, and for filesystem volumes removes quota.
 func (d *btrfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
+	// Convert to bytes.
+	sizeBytes, err := units.ParseByteSizeString(size)
+	if err != nil {
+		return err
+	}
+
 	// For VM block files, resize the file if needed.
 	if vol.contentType == ContentTypeBlock {
+		// Do nothing if size isn't specified.
+		if sizeBytes <= 0 {
+			return nil
+		}
+
 		rootBlockPath, err := d.GetVolumeDiskPath(vol)
 		if err != nil {
 			return err
 		}
 
-		// If size not specified in volume config, then use pool's default block size.
-		if size == "" || size == "0" {
-			size = defaultBlockSize
-		}
-
-		resized, err := genericVFSResizeBlockFile(rootBlockPath, size)
+		resized, err := genericVFSResizeBlockFile(rootBlockPath, sizeBytes)
 		if err != nil {
 			return err
 		}
@@ -483,12 +490,6 @@ func (d *btrfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation
 	// For non-VM block volumes, set filesystem quota.
 	volPath := vol.MountPath()
 
-	// Convert to bytes.
-	sizeBytes, err := units.ParseByteSizeString(size)
-	if err != nil {
-		return err
-	}
-
 	// Try to locate an existing quota group.
 	qgroup, _, err := d.getQGroup(volPath)
 	if err != nil && !d.state.OS.RunningInUserNS {

From 84c7ad8eb7ff0fd54fcafcfeb4969d6ac1fdca5d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:13:32 +0100
Subject: [PATCH 06/17] lxd/storage/drivers/driver/ceph/utils: Updates
 volumeSize comment for consistency

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

diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go
index e9abe379c6..47ebdb1be2 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -529,7 +529,7 @@ func (d *ceph) rbdListVolumeSnapshots(vol Volume) ([]string, error) {
 	return snapshots, nil
 }
 
-// volumeSize returns the size to use when creating new RBD volumes.
+// volumeSize returns the size to use when creating new a volume.
 func (d *ceph) volumeSize(vol Volume) string {
 	size := vol.ExpandedConfig("size")
 	if size == "" || size == "0" {

From 41a4b25c88cbc83b4229994573a08dfeff120fad Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:14:07 +0100
Subject: [PATCH 07/17] lxd/storage/drivers/driver/ceph/volumes: Updates
 CreateVolumeFromCopy to use volumeSize()

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

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index ece231b52b..11bf747c5e 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -313,9 +313,9 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo
 		// This is so the pool default volume size isn't take into account for volume copies.
 		volSize := vol.config["size"]
 
-		// If source is an image then use expanded config so that we take into account pool volume size.
+		// If source is an image then take into account default volume sizes if not specified.
 		if srcVol.volType == VolumeTypeImage {
-			volSize = vol.ExpandedConfig("size")
+			volSize = d.volumeSize(vol)
 		}
 
 		err = d.SetVolumeQuota(vol, volSize, op)

From 2bcc5a053f0e9cfbcddfda6d3d27eb107f57489c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:14:39 +0100
Subject: [PATCH 08/17] lxd/storage/drivers/driver/ceph/volumes: Updates
 SetVolumeQuota to be byte oriented internally

Also renames newSizeBytes to sizeBytes for consistency with other driver's SetVolumeQuota function.

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

diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go
index 11bf747c5e..53521927ff 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -802,7 +802,19 @@ func (d *ceph) GetVolumeUsage(vol Volume) (int64, error) {
 }
 
 // SetVolumeQuota applies a size limit on volume.
+// Does nothing if supplied with an empty/zero size.
 func (d *ceph) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
+	// Convert to bytes.
+	sizeBytes, err := units.ParseByteSizeString(size)
+	if err != nil {
+		return err
+	}
+
+	// Do nothing if size isn't specified.
+	if sizeBytes <= 0 {
+		return nil
+	}
+
 	fsType := d.getRBDFilesystem(vol)
 
 	RBDDevPath, err := d.getRBDMappedDevPath(vol)
@@ -822,26 +834,20 @@ func (d *ceph) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 		return errors.Wrapf(err, "Error getting current size")
 	}
 
-	newSizeBytes, err := units.ParseByteSizeString(size)
-	if err != nil {
-		return err
-	}
-
-	// The right disjunct just means that someone unset the size property in the instance's config.
-	// We obviously cannot resize to 0.
-	if oldSizeBytes == newSizeBytes || newSizeBytes == 0 {
+	// Do nothing is volume is already specified size.
+	if oldSizeBytes == sizeBytes {
 		return nil
 	}
 
 	// Resize filesystem if needed.
-	if newSizeBytes < oldSizeBytes {
+	if sizeBytes < oldSizeBytes {
 		if vol.contentType == ContentTypeBlock && !vol.allowUnsafeResize {
 			return errors.Wrap(ErrCannotBeShrunk, "You cannot shrink block volumes")
 		}
 
 		// Shrink the filesystem.
 		if vol.contentType == ContentTypeFS {
-			err = shrinkFileSystem(fsType, RBDDevPath, vol, newSizeBytes)
+			err = shrinkFileSystem(fsType, RBDDevPath, vol, sizeBytes)
 			if err != nil {
 				return err
 			}
@@ -855,7 +861,7 @@ func (d *ceph) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 			"--id", d.config["ceph.user.name"],
 			"--cluster", d.config["ceph.cluster_name"],
 			"--pool", d.config["ceph.osd.pool_name"],
-			"--size", fmt.Sprintf("%dB", newSizeBytes),
+			"--size", fmt.Sprintf("%dB", sizeBytes),
 			d.getRBDVolumeName(vol, "", false, false))
 		if err != nil {
 			return err
@@ -868,7 +874,7 @@ func (d *ceph) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 			"--id", d.config["ceph.user.name"],
 			"--cluster", d.config["ceph.cluster_name"],
 			"--pool", d.config["ceph.osd.pool_name"],
-			"--size", fmt.Sprintf("%dB", newSizeBytes),
+			"--size", fmt.Sprintf("%dB", sizeBytes),
 			d.getRBDVolumeName(vol, "", false, false))
 		if err != nil {
 			return err

From efb41dd08fd751841498d02607ecb7f72a761ac0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:15:44 +0100
Subject: [PATCH 09/17] lxd/storage/drivers/driver/dir/utils: Adds volumeSize
 function

And updates setupInitialQuota to use it.

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

diff --git a/lxd/storage/drivers/driver_dir_utils.go b/lxd/storage/drivers/driver_dir_utils.go
index 4ca5e3035e..ba9bd00d2f 100644
--- a/lxd/storage/drivers/driver_dir_utils.go
+++ b/lxd/storage/drivers/driver_dir_utils.go
@@ -48,7 +48,7 @@ func (d *dir) setupInitialQuota(vol Volume) (func(), error) {
 	revert.Add(revertFunc)
 
 	// Set the quota.
-	err = d.setQuota(volPath, volID, vol.ExpandedConfig("size"))
+	err = d.setQuota(volPath, volID, d.volumeSize(vol))
 	if err != nil {
 		return nil, err
 	}
@@ -149,3 +149,15 @@ func (d *dir) setQuota(path string, volID int64, size string) error {
 
 	return quota.SetProjectQuota(path, d.quotaProjectID(volID), sizeBytes)
 }
+
+// volumeSize returns the size to use when creating new a volume.
+func (d *dir) volumeSize(vol Volume) string {
+	size := vol.ExpandedConfig("size")
+
+	// Block images always need a size.
+	if vol.contentType == ContentTypeBlock && (size == "" || size == "0") {
+		return defaultBlockSize
+	}
+
+	return size
+}

From 6e8fb93fa9fd6ca316ea37808bad8070b2302773 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:16:48 +0100
Subject: [PATCH 10/17] lxd/storage/drivers/driver/dir/volumes: Updates
 CreateVolume to use volumeSize

And updates usage of genericVFSResizeBlockFile.

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

diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index 58b1d0b157..e850c8b344 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -15,6 +15,7 @@ import (
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/instancewriter"
 	log "github.com/lxc/lxd/shared/log15"
+	"github.com/lxc/lxd/shared/units"
 )
 
 // CreateVolume creates an empty volume and can optionally fill it by executing the supplied
@@ -64,7 +65,13 @@ func (d *dir) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 	// If we are creating a block volume, resize it to the requested size or the default.
 	// We expect the filler function to have converted the qcow2 image to raw into the rootBlockPath.
 	if vol.contentType == ContentTypeBlock {
-		err := ensureVolumeBlockFile(rootBlockPath, vol.ExpandedConfig("size"))
+		// Convert to bytes.
+		sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol))
+		if err != nil {
+			return err
+		}
+
+		err = ensureVolumeBlockFile(rootBlockPath, sizeBytes)
 		if err != nil {
 			return err
 		}

From 6c1b4c6ba7230df553f861833061374848211d7c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:17:32 +0100
Subject: [PATCH 11/17] lxd/storage/drivers/driver/dir/volumes: Updates
 SetVolumeQuota to be byte oriented internally

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

diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go
index e850c8b344..61a8516080 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -249,20 +249,27 @@ func (d *dir) GetVolumeUsage(vol Volume) (int64, error) {
 }
 
 // SetVolumeQuota sets the quota on the volume.
+// Does nothing if supplied with an empty/zero size for block volumes, and for filesystem volumes removes quota.
 func (d *dir) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
+	// Convert to bytes.
+	sizeBytes, err := units.ParseByteSizeString(size)
+	if err != nil {
+		return err
+	}
+
 	// For VM block files, resize the file if needed.
 	if vol.contentType == ContentTypeBlock {
+		// Do nothing if size isn't specified.
+		if sizeBytes <= 0 {
+			return nil
+		}
+
 		rootBlockPath, err := d.GetVolumeDiskPath(vol)
 		if err != nil {
 			return err
 		}
 
-		// If size not specified in volume config, then use pool's default block size.
-		if size == "" || size == "0" {
-			size = defaultBlockSize
-		}
-
-		resized, err := genericVFSResizeBlockFile(rootBlockPath, size)
+		resized, err := genericVFSResizeBlockFile(rootBlockPath, sizeBytes)
 		if err != nil {
 			return err
 		}

From fd989192783ec09ec78b6575d850f93dea1b6bac Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:18:59 +0100
Subject: [PATCH 12/17] lxd/storage/drivers/driver/lvm/utils: Updates
 copyThinpoolVolume to use volumeSize()

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go b/lxd/storage/drivers/driver_lvm_utils.go
index 37f914be05..4c6f543528 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -54,7 +54,7 @@ func (d *lvm) volumeFilesystem(vol Volume) string {
 	return DefaultFilesystem
 }
 
-// volumeSize returns the size to use when creating new logical volumes.
+// volumeSize returns the size to use when creating new a volume.
 func (d *lvm) volumeSize(vol Volume) string {
 	size := vol.ExpandedConfig("size")
 	if size == "" || size == "0" {
@@ -654,9 +654,9 @@ func (d *lvm) copyThinpoolVolume(vol, srcVol Volume, srcSnapshots []Volume, refr
 	// This is so the pool default volume size isn't take into account for volume copies.
 	volSize := vol.config["size"]
 
-	// If source is an image then use expanded config so that we take into account pool default volume size.
+	// If source is an image then take into account default volume sizes if not specified.
 	if srcVol.volType == VolumeTypeImage {
-		volSize = vol.ExpandedConfig("size")
+		volSize = d.volumeSize(vol)
 	}
 
 	err = d.SetVolumeQuota(vol, volSize, nil)

From 458aa8005a652f0419ca25bd4b4578e305ab2724 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:19:54 +0100
Subject: [PATCH 13/17] lxd/storage/drivers/driver/lvm/volumes: Updates
 SetVolumeQuota variables and comments

For consistency with other drivers.

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go
index 62ef87fc49..a001ba1561 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -316,13 +316,14 @@ func (d *lvm) GetVolumeUsage(vol Volume) (int64, error) {
 }
 
 // SetVolumeQuota sets the quota on the volume.
+// Does nothing if supplied with an empty/zero size.
 func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
-	// Can't do anything if the size property has been removed from volume config.
+	// Do nothing if size isn't specified.
 	if size == "" || size == "0" {
 		return nil
 	}
 
-	newSizeBytes, err := d.roundedSizeBytesString(size)
+	sizeBytes, err := d.roundedSizeBytesString(size)
 	if err != nil {
 		return err
 	}
@@ -342,7 +343,7 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 	}
 
 	// Round up the number of extents required for new quota size, as this is what the lvresize tool will do.
-	newNumExtents := math.Ceil(float64(newSizeBytes) / float64(vgExtentSize))
+	newNumExtents := math.Ceil(float64(sizeBytes) / float64(vgExtentSize))
 	oldNumExtents := math.Ceil(float64(oldSizeBytes) / float64(vgExtentSize))
 	extentDiff := int(newNumExtents - oldNumExtents)
 
@@ -351,25 +352,25 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 		return nil
 	}
 
-	logCtx := log.Ctx{"dev": volDevPath, "size": fmt.Sprintf("%db", newSizeBytes)}
+	logCtx := log.Ctx{"dev": volDevPath, "size": fmt.Sprintf("%db", sizeBytes)}
 
 	// Resize filesystem if needed.
 	if vol.contentType == ContentTypeFS {
-		if newSizeBytes < oldSizeBytes {
+		if sizeBytes < oldSizeBytes {
 			// Shrink filesystem to new size first, then shrink logical volume.
-			err = shrinkFileSystem(d.volumeFilesystem(vol), volDevPath, vol, newSizeBytes)
+			err = shrinkFileSystem(d.volumeFilesystem(vol), volDevPath, vol, sizeBytes)
 			if err != nil {
 				return err
 			}
 			d.logger.Debug("Logical volume filesystem shrunk", logCtx)
 
-			err = d.resizeLogicalVolume(volDevPath, newSizeBytes)
+			err = d.resizeLogicalVolume(volDevPath, sizeBytes)
 			if err != nil {
 				return err
 			}
-		} else if newSizeBytes > oldSizeBytes {
+		} else if sizeBytes > oldSizeBytes {
 			// Grow logical volume to new size first, then grow filesystem to fill it.
-			err = d.resizeLogicalVolume(volDevPath, newSizeBytes)
+			err = d.resizeLogicalVolume(volDevPath, sizeBytes)
 			if err != nil {
 				return err
 			}
@@ -381,11 +382,11 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 			d.logger.Debug("Logical volume filesystem grown", logCtx)
 		}
 	} else {
-		if newSizeBytes < oldSizeBytes && !vol.allowUnsafeResize {
+		if sizeBytes < oldSizeBytes && !vol.allowUnsafeResize {
 			return errors.Wrap(ErrCannotBeShrunk, "You cannot shrink block volumes")
 		}
 
-		err = d.resizeLogicalVolume(volDevPath, newSizeBytes)
+		err = d.resizeLogicalVolume(volDevPath, sizeBytes)
 		if err != nil {
 			return err
 

From e9b0e7d7f8982c5194c3003d7ebd9cfd9a751035 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:20:24 +0100
Subject: [PATCH 14/17] lxd/storage/drivers/driver/zfs/utils: Adds volumeSize
 function

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

diff --git a/lxd/storage/drivers/driver_zfs_utils.go b/lxd/storage/drivers/driver_zfs_utils.go
index 9cdd6c127e..caa92148ab 100644
--- a/lxd/storage/drivers/driver_zfs_utils.go
+++ b/lxd/storage/drivers/driver_zfs_utils.go
@@ -316,3 +316,15 @@ func (d *zfs) receiveDataset(dataset string, conn io.ReadWriteCloser, writeWrapp
 
 	return nil
 }
+
+// volumeSize returns the size to use when creating new a volume.
+func (d *zfs) volumeSize(vol Volume) string {
+	size := vol.ExpandedConfig("size")
+
+	// Block images always need a size.
+	if vol.contentType == ContentTypeBlock && (size == "" || size == "0") {
+		return defaultBlockSize
+	}
+
+	return size
+}

From 154734d47e9756224300e27ae179a2b6c4f8e6d2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:20:57 +0100
Subject: [PATCH 15/17] lxd/storage/drivers/driver/zfs/volumes: Updates
 CreateVolume to use volumeSize()

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

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index bf75224da0..52cd8d54ed 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -133,21 +133,12 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 		}
 
 		// Apply the size limit.
-		size := vol.ExpandedConfig("size")
-		if size != "" {
-			err := d.SetVolumeQuota(vol, size, op)
-			if err != nil {
-				return err
-			}
+		err = d.SetVolumeQuota(vol, d.volumeSize(vol), op)
+		if err != nil {
+			return err
 		}
 	} else {
-		// Convert the size.
-		size := vol.ExpandedConfig("size")
-		if size == "" {
-			size = defaultBlockSize
-		}
-
-		sizeBytes, err := units.ParseByteSizeString(size)
+		sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol))
 		if err != nil {
 			return err
 		}

From abf50f877f89522519b08d0fe2513463dd67db4b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:21:23 +0100
Subject: [PATCH 16/17] lxd/storage/drivers/driver/zfs/volumes: Updates
 CreateVolumeFromCopy to use volumeSize()

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

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index 52cd8d54ed..ea14fa6db8 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -590,9 +590,9 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 	// This is so the pool default volume size isn't take into account for volume copies.
 	volSize := vol.config["size"]
 
-	// If source is an image then use expanded config so that we take into account pool default volume size.
+	// If source is an image then take into account default volume sizes if not specified.
 	if srcVol.volType == VolumeTypeImage {
-		volSize = vol.ExpandedConfig("size")
+		volSize = d.volumeSize(vol)
 	}
 
 	err := d.SetVolumeQuota(vol, volSize, op)

From 12988d6143409d8095748f5b9a08e8eedc0a5982 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 1 May 2020 11:21:43 +0100
Subject: [PATCH 17/17] lxd/storage/drivers/driver/zfs/volumes: Updates
 SetVolumeQuota to be byte oriented internally

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

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go
index ea14fa6db8..b9f88d13d8 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -879,15 +879,9 @@ func (d *zfs) GetVolumeUsage(vol Volume) (int64, error) {
 	return valueInt, nil
 }
 
+// SetVolumeQuota sets the quota on the volume.
+// Does nothing if supplied with an empty/zero size for block volumes, and for filesystem volumes removes quota.
 func (d *zfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation) error {
-	if size == "" || size == "0" {
-		if vol.contentType == ContentTypeBlock {
-			size = defaultBlockSize
-		} else {
-			size = "0"
-		}
-	}
-
 	// Convert to bytes.
 	sizeBytes, err := units.ParseByteSizeString(size)
 	if err != nil {
@@ -896,6 +890,11 @@ func (d *zfs) SetVolumeQuota(vol Volume, size string, op *operations.Operation)
 
 	// Handle volume datasets.
 	if vol.contentType == ContentTypeBlock {
+		// Do nothing if size isn't specified.
+		if sizeBytes <= 0 {
+			return nil
+		}
+
 		sizeBytes = (sizeBytes / minBlockBoundary) * minBlockBoundary
 
 		oldSizeBytesStr, err := d.getDatasetProperty(d.dataset(vol, false), "volsize")


More information about the lxc-devel mailing list