Storage: Round ZFS volume sizes to nearest 16KiB

lxd/storage/drivers/driver_common.go

@@ -549,23 +549,12 @@ func (d *common) DeleteBucketKey(bucket Volume, keyName string, op *operations.O
 	return nil
 }
 
-// roundVolumeBlockSizeBytes returns size rounded to the nearest multiple of MinBlockBoundary bytes that is equal
-// to or larger than sizeBytes.
-func (d *common) roundVolumeBlockSizeBytes(sizeBytes int64) int64 {
+// roundVolumeBlockSizeBytes returns sizeBytes rounded up to the next multiple
+// of MinBlockBoundary.
+func (d *common) roundVolumeBlockSizeBytes(vol Volume, sizeBytes int64) int64 {
 	// 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 sizeBytes < MinBlockBoundary {
-		sizeBytes = MinBlockBoundary
-	}
-
-	roundedSizeBytes := int64(sizeBytes/MinBlockBoundary) * MinBlockBoundary
-
-	// Ensure the rounded size is at least the size specified in sizeBytes.
-	if roundedSizeBytes < sizeBytes {
-		roundedSizeBytes += MinBlockBoundary
-	}
-
-	return roundedSizeBytes
+	return roundAbove(MinBlockBoundary, sizeBytes)
 }
 
 func (d *common) isBlockBacked(vol Volume) bool {

lxd/storage/drivers/driver_lvm.go

@@ -503,6 +503,8 @@ func (d *lvm) Delete(op *operations.Operation) error {
 	return nil
 }
 
+// Validate checks that all provide keys are supported and that no conflicting
+// or missing configuration is present.
 func (d *lvm) Validate(config map[string]string) error {
 	rules := map[string]func(value string) error{
 		"size": validate.Optional(validate.IsSize),
@@ -782,21 +784,10 @@ func (d *lvm) GetResources() (*api.ResourcesStoragePool, error) {
 	return &res, nil
 }
 
-// roundVolumeBlockSizeBytes returns size rounded to the nearest multiple of the volume group extent size that is
-// equal to or larger than sizeBytes.
-func (d *lvm) roundVolumeBlockSizeBytes(sizeBytes int64) int64 {
+// roundVolumeBlockSizeBytes returns sizeBytes rounded up to the next multiple
+// of the volume group extent size.
+func (d *lvm) roundVolumeBlockSizeBytes(vol Volume, sizeBytes int64) int64 {
 	// Get the volume group's physical extent size, and use that as minimum size.
 	vgExtentSize, _ := d.volumeGroupExtentSize(d.config["lvm.vg_name"])
-	if sizeBytes < vgExtentSize {
-		sizeBytes = vgExtentSize
-	}
-
-	roundedSizeBytes := int64(sizeBytes/vgExtentSize) * vgExtentSize
-
-	// Ensure the rounded size is at least the size specified in sizeBytes.
-	if roundedSizeBytes < sizeBytes {
-		roundedSizeBytes += vgExtentSize
-	}
-
-	return roundedSizeBytes
+	return roundAbove(vgExtentSize, sizeBytes)
 }

lxd/storage/drivers/driver_zfs.go

@@ -623,6 +623,7 @@ func (d *zfs) Unmount() (bool, error) {
 	return true, nil
 }
 
+// GetResources returns utilization statistics for the storage pool.
 func (d *zfs) GetResources() (*api.ResourcesStoragePool, error) {
 	// Get the total amount of space.
 	availableStr, err := d.getDatasetProperty(d.config["zfs.pool_name"], "available")
@@ -655,7 +656,8 @@ func (d *zfs) GetResources() (*api.ResourcesStoragePool, error) {
 	return &res, nil
 }
 
-// MigrationType returns the type of transfer methods to be used when doing migrations between pools in preference order.
+// MigrationTypes returns the type of transfer methods to be used when doing
+// migrations between pools in preference order.
 func (d *zfs) MigrationTypes(contentType ContentType, refresh bool, copySnapshots bool) []migration.Type {
 	var rsyncFeatures []string
 
@@ -743,3 +745,17 @@ func (d *zfs) patchDropBlockVolumeFilesystemExtension() error {
 
 	return nil
 }
+
+// roundVolumeBlockSizeBytes returns sizeBytes rounded up to the next multiple
+// of `vol`'s "zfs.blocksize".
+func (d *zfs) roundVolumeBlockSizeBytes(vol Volume, sizeBytes int64) int64 {
+	minBlockSize, err := units.ParseByteSizeString(vol.ExpandedConfig("zfs.blocksize"))
+
+	// minBlockSize will be 0 if zfs.blocksize=""
+	if minBlockSize <= 0 || err != nil {
+		// 16KiB is the default volblocksize
+		minBlockSize = 16 * 1024
+	}
+
+	return roundAbove(minBlockSize, sizeBytes)
+}

lxd/storage/drivers/driver_zfs_utils.go

@@ -82,8 +82,6 @@ func (d *zfs) createDataset(dataset string, options ...string) error {
 }
 
 func (d *zfs) createVolume(dataset string, size int64, options ...string) error {
-	size = d.roundVolumeBlockSizeBytes(size)
-
 	args := []string{"create", "-s", "-V", fmt.Sprintf("%d", size)}
 	for _, option := range options {
 		args = append(args, "-o")

lxd/storage/drivers/driver_zfs_volumes.go

@@ -84,7 +84,7 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 				}
 
 				// Round to block boundary.
-				poolVolSizeBytes = d.roundVolumeBlockSizeBytes(poolVolSizeBytes)
+				poolVolSizeBytes = d.roundVolumeBlockSizeBytes(vol, poolVolSizeBytes)
 
 				// If the cached volume size is different than the pool volume size, then we can't use the
 				// deleted cached image volume and instead we will rename it to a random UUID so it can't
@@ -159,11 +159,6 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 			return err
 		}
 	} else {
-		sizeBytes, err := units.ParseByteSizeString(vol.ConfigSize())
-		if err != nil {
-			return err
-		}
-
 		var opts []string
 
 		if vol.contentType == ContentTypeFS {
@@ -207,6 +202,13 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper
 			opts = append(opts, fmt.Sprintf("volblocksize=%d", sizeBytes))
 		}
 
+		sizeBytes, err := units.ParseByteSizeString(vol.ConfigSize())
+		if err != nil {
+			return err
+		}
+
+		sizeBytes = d.roundVolumeBlockSizeBytes(vol, sizeBytes)
+
 		// Create the volume dataset.
 		err = d.createVolume(d.dataset(vol, false), sizeBytes, opts...)
 		if err != nil {
@@ -1022,7 +1024,7 @@ func (d *zfs) CreateVolumeFromMigration(vol VolumeCopy, conn io.ReadWriteCloser,
 			return fmt.Errorf("Failed sending ZFS migration header: %w", err)
 		}
 
-		err = conn.Close() //End the frame.
+		err = conn.Close() // End the frame.
 		if err != nil {
 			return fmt.Errorf("Failed closing ZFS migration header frame: %w", err)
 		}
@@ -1744,7 +1746,7 @@ func (d *zfs) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op
 			return nil
 		}
 
-		sizeBytes = d.roundVolumeBlockSizeBytes(sizeBytes)
+		sizeBytes = d.roundVolumeBlockSizeBytes(vol, sizeBytes)
 
 		oldSizeBytesStr, err := d.getDatasetProperty(d.dataset(vol, false), "volsize")
 		if err != nil {
@@ -2515,7 +2517,7 @@ func (d *zfs) MigrateVolume(vol VolumeCopy, conn io.ReadWriteCloser, volSrcArgs
 			return fmt.Errorf("Failed sending ZFS migration header: %w", err)
 		}
 
-		err = conn.Close() //End the frame.
+		err = conn.Close() // End the frame.
 		if err != nil {
 			return fmt.Errorf("Failed closing ZFS migration header frame: %w", err)
 		}
@@ -3185,7 +3187,7 @@ func (d *zfs) mountVolumeSnapshot(snapVol Volume, snapshotDataset string, mountP
 	return cleanup, nil
 }
 
-// UnmountVolume simulates unmounting a volume snapshot.
+// UnmountVolumeSnapshot simulates unmounting a volume snapshot.
 func (d *zfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (bool, error) {
 	unlock, err := snapVol.MountLock()
 	if err != nil {

lxd/storage/drivers/interface.go

@@ -28,7 +28,7 @@ type Driver interface {
 	// Internal.
 	Info() Info
 	HasVolume(vol Volume) (bool, error)
-	roundVolumeBlockSizeBytes(sizeBytes int64) int64
+	roundVolumeBlockSizeBytes(vol Volume, sizeBytes int64) int64
 	isBlockBacked(vol Volume) bool
 
 	// Export struct details.

lxd/storage/drivers/utils.go

@@ -327,7 +327,7 @@ func ensureVolumeBlockFile(vol Volume, path string, sizeBytes int64, allowUnsafe
 	}
 
 	// Get rounded block size to avoid QEMU boundary issues.
-	sizeBytes = vol.driver.roundVolumeBlockSizeBytes(sizeBytes)
+	sizeBytes = vol.driver.roundVolumeBlockSizeBytes(vol, sizeBytes)
 
 	if shared.PathExists(path) {
 		fi, err := os.Stat(path)
@@ -819,18 +819,20 @@ func loopFileSizeDefault() (uint64, error) {
 // It tries to enable direct I/O if supported.
 func loopDeviceSetup(sourcePath string) (string, error) {
 	out, err := shared.RunCommand("losetup", "--find", "--nooverlap", "--direct-io=on", "--show", sourcePath)
-	if err != nil {
-		if strings.Contains(err.Error(), "direct io") || strings.Contains(err.Error(), "Invalid argument") {
-			out, err = shared.RunCommand("losetup", "--find", "--nooverlap", "--show", sourcePath)
-			if err != nil {
-				return "", err
-			}
-		} else {
-			return "", err
-		}
+	if err == nil {
+		return strings.TrimSpace(out), nil
+	}
+
+	if !(strings.Contains(err.Error(), "direct io") || strings.Contains(err.Error(), "Invalid argument")) {
+		return "", err
+	}
+
+	out, err = shared.RunCommand("losetup", "--find", "--nooverlap", "--show", sourcePath)
+	if err == nil {
+		return strings.TrimSpace(out), nil
 	}
 
-	return strings.TrimSpace(out), nil
+	return "", err
 }
 
 // loopFileAutoDetach enables auto detach mode for a loop device.
@@ -876,3 +878,19 @@ func wipeBlockHeaders(path string) error {
 func IsContentBlock(contentType ContentType) bool {
 	return contentType == ContentTypeBlock || contentType == ContentTypeISO
 }
+
+// roundAbove returns the next multiple of `above` greater than `val`.
+func roundAbove(above, val int64) int64 {
+	if val < above {
+		val = above
+	}
+
+	rounded := int64(val/above) * above
+
+	// Ensure the rounded size is at least x.
+	if rounded < val {
+		rounded += above
+	}
+
+	return rounded
+}

lxd/storage/drivers/volume.go

@@ -520,7 +520,7 @@ func (v Volume) ConfigSizeFromSource(srcVol Volume) (string, error) {
 		// directly usable with the same size setting without also rounding for this check.
 		// Because we are not altering the actual size returned to use for the new volume, this will not
 		// affect storage drivers that do not use rounding.
-		volSizeBytes = v.driver.roundVolumeBlockSizeBytes(volSizeBytes)
+		volSizeBytes = v.driver.roundVolumeBlockSizeBytes(v, volSizeBytes)
 
 		// The volume/pool specified size is smaller than image minimum size. We must not continue as
 		// these specified sizes provide protection against unpacking a massive image and filling the pool.

test/suites/storage.sh

@@ -45,6 +45,10 @@ test_storage() {
   [ "$(lxc storage volume get "$storage_pool" "$storage_volume" snapshots.expiry)" = "3d" ]
   lxc storage volume delete "$storage_pool" "$storage_volume"
 
+  # Ensure non-power-of-two sizes are rounded appropriately (most relevant for zfs)
+  lxc storage volume create "$storage_pool" "$storage_volume" --type=block size=13GB
+  lxc storage volume delete "$storage_pool" "$storage_volume"
+
   lxc storage delete "$storage_pool"
 
   # Test btrfs resize