The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7437
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) === Also adds: - Volume.ConfigBlockFilesystem() - Volume.SetQuota()
From 00bfc41a03cb95f356349b8a12308774b07868e4 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Mon, 18 May 2020 13:42:27 +0100 Subject: [PATCH 1/6] lxd/storage/drivers/volume: Adds SetQuota function Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/volume.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index 07ff0d433c..ab13e100e7 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -326,3 +326,8 @@ func (v Volume) NewVMBlockFilesystemVolume() Volume { return NewVolume(v.driver, v.pool, v.volType, ContentTypeFS, v.name, newConf, v.poolConfig) } + +// SetQuota calls SetVolumeQuota on the Volume's driver. +func (v Volume) SetQuota(size string, op *operations.Operation) error { + return v.driver.SetVolumeQuota(v, size, op) +} From ee933b7cbd1127ad3ac4e5fe96898da032313ef3 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 26 May 2020 11:12:24 +0100 Subject: [PATCH 2/6] lxd/storage/drivers/volume: Adds config functions - ConfigBlockFilesystem - ConfigSize Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/volume.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index ab13e100e7..f2446b45b2 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -331,3 +331,30 @@ func (v Volume) NewVMBlockFilesystemVolume() Volume { func (v Volume) SetQuota(size string, op *operations.Operation) error { return v.driver.SetVolumeQuota(v, size, op) } + +// ConfigBlockFilesystem returns the filesystem to use for block volumes. Returns config value "block.filesystem" +// if defined in volume or pool's volume config, otherwise the DefaultFilesystem. +func (v Volume) ConfigBlockFilesystem() string { + fs := v.ExpandedConfig("block.filesystem") + if fs != "" { + return fs + } + + return DefaultFilesystem +} + +// ConfigSize returns the size to use when creating new a volume. Returns config value "size" if defined in volume +// or pool's volume config, otherwise for block volumes and block-backed volumes the defaultBlockSize. For other +// volumes an empty string is returned if no size is defined. +func (v Volume) ConfigSize() string { + size := v.ExpandedConfig("size") + + // If volume size isn't defined in either volume or pool config, then for block volumes or block-backed + // volumes return the defaultBlockSize. + if (size == "" || size == "0") && (v.contentType == ContentTypeBlock || v.driver.Info().BlockBacking) { + return defaultBlockSize + } + + // Return defined size or empty string if not defined. + return size +} From f943f047e881435d711feab1aa12c9c931bad4b3 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 26 May 2020 11:20:36 +0100 Subject: [PATCH 3/6] lxd/storage/drivers/driver/lvm/utils: Removes functions moved into Volume struct Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_lvm_utils.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/lxd/storage/drivers/driver_lvm_utils.go b/lxd/storage/drivers/driver_lvm_utils.go index fed31681bc..ea219a6602 100644 --- a/lxd/storage/drivers/driver_lvm_utils.go +++ b/lxd/storage/drivers/driver_lvm_utils.go @@ -50,26 +50,6 @@ func (d *lvm) thinpoolName() string { return "LXDThinPool" } -// volumeFilesystem returns the filesystem to use for logical volumes. -func (d *lvm) volumeFilesystem(vol Volume) string { - fs := vol.ExpandedConfig("block.filesystem") - if fs != "" { - return fs - } - - return DefaultFilesystem -} - -// 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" { - return defaultBlockSize - } - - return size -} - // mountOptions returns the mount options for volumes. func (d *lvm) volumeMountOptions(vol Volume) string { if d.config["block.mount_options"] != "" { From b900e88b4b2328077a5aa199071e42d8efe7f79f Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 26 May 2020 11:16:42 +0100 Subject: [PATCH 4/6] lxd/storage/drivers/driver/lvm/utils: Usage of volume config functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_lvm_utils.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lxd/storage/drivers/driver_lvm_utils.go b/lxd/storage/drivers/driver_lvm_utils.go index ea219a6602..94aba49468 100644 --- a/lxd/storage/drivers/driver_lvm_utils.go +++ b/lxd/storage/drivers/driver_lvm_utils.go @@ -57,7 +57,7 @@ func (d *lvm) volumeMountOptions(vol Volume) string { } // Use some special options if the filesystem for the volume is BTRFS. - if d.volumeFilesystem(vol) == "btrfs" { + if vol.ConfigBlockFilesystem() == "btrfs" { return "user_subvol_rm_allowed,discard" } @@ -319,7 +319,7 @@ func (d *lvm) roundedSizeBytesString(size string) (int64, error) { func (d *lvm) createLogicalVolume(vgName, thinPoolName string, vol Volume, makeThinLv bool) error { var err error - lvSizeBytes, err := d.roundedSizeBytesString(d.volumeSize(vol)) + lvSizeBytes, err := d.roundedSizeBytesString(vol.ConfigSize()) if err != nil { return err } @@ -370,7 +370,7 @@ func (d *lvm) createLogicalVolume(vgName, thinPoolName string, vol Volume, makeT volDevPath := d.lvmDevPath(vgName, vol.volType, vol.contentType, vol.name) if vol.contentType == ContentTypeFS { - _, err = makeFSType(volDevPath, d.volumeFilesystem(vol), nil) + _, err = makeFSType(volDevPath, vol.ConfigBlockFilesystem(), nil) if err != nil { return errors.Wrapf(err, "Error making filesystem on LVM logical volume") } @@ -390,7 +390,7 @@ func (d *lvm) createLogicalVolume(vgName, thinPoolName string, vol Volume, makeT } } - d.logger.Debug("Logical volume created", log.Ctx{"vg_name": vgName, "lv_name": lvFullName, "size": fmt.Sprintf("%db", lvSizeBytes), "fs": d.volumeFilesystem(vol)}) + d.logger.Debug("Logical volume created", log.Ctx{"vg_name": vgName, "lv_name": lvFullName, "size": fmt.Sprintf("%db", lvSizeBytes), "fs": vol.ConfigBlockFilesystem()}) return nil } @@ -414,7 +414,7 @@ func (d *lvm) createLogicalVolumeSnapshot(vgName string, srcVol, snapVol Volume, // According to LVM tools 15-20% of the original volume should be sufficient. // However, let's not be stingy at first otherwise we might force users to fiddle around with lvextend. if !makeThinLv { - lvSizeBytes, err := d.roundedSizeBytesString(d.volumeSize(snapVol)) + lvSizeBytes, err := d.roundedSizeBytesString(snapVol.ConfigSize()) if err != nil { return "", err } @@ -624,14 +624,14 @@ func (d *lvm) copyThinpoolVolume(vol, srcVol Volume, srcSnapshots []Volume, refr // Generate a new filesystem UUID if needed (this is required because some filesystems won't allow // volumes with the same UUID to be mounted at the same time). This should be done before volume // resize as some filesystems will need to mount the filesystem to resize. - if renegerateFilesystemUUIDNeeded(d.volumeFilesystem(vol)) { + if renegerateFilesystemUUIDNeeded(vol.ConfigBlockFilesystem()) { _, err = d.activateVolume(volDevPath) if err != nil { return err } - d.logger.Debug("Regenerating filesystem UUID", log.Ctx{"dev": volDevPath, "fs": d.volumeFilesystem(vol)}) - err = regenerateFilesystemUUID(d.volumeFilesystem(vol), volDevPath) + d.logger.Debug("Regenerating filesystem UUID", log.Ctx{"dev": volDevPath, "fs": vol.ConfigBlockFilesystem()}) + err = regenerateFilesystemUUID(vol.ConfigBlockFilesystem(), volDevPath) if err != nil { return err } @@ -652,7 +652,7 @@ func (d *lvm) copyThinpoolVolume(vol, srcVol Volume, srcSnapshots []Volume, refr // If source is an image then take into account default volume sizes if not specified. if srcVol.volType == VolumeTypeImage { - volSize = d.volumeSize(vol) + volSize = vol.ConfigSize() } err = d.SetVolumeQuota(vol, volSize, nil) From b5cd5af6fa02a0c117cf26cf74d4f68dc4c456a2 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 26 May 2020 11:17:36 +0100 Subject: [PATCH 5/6] lxd/storage/drivers/driver/lvm/volumes: Volume config function usage Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_lvm_volumes.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go index bead2cf8c7..7c40ba6093 100644 --- a/lxd/storage/drivers/driver_lvm_volumes.go +++ b/lxd/storage/drivers/driver_lvm_volumes.go @@ -380,7 +380,7 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation) if vol.contentType == ContentTypeFS { if sizeBytes < oldSizeBytes { // Shrink filesystem to new size first, then shrink logical volume. - err = shrinkFileSystem(d.volumeFilesystem(vol), volDevPath, vol, sizeBytes) + err = shrinkFileSystem(vol.ConfigBlockFilesystem(), volDevPath, vol, sizeBytes) if err != nil { return err } @@ -397,7 +397,7 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op *operations.Operation) return err } - err = growFileSystem(d.volumeFilesystem(vol), volDevPath, vol) + err = growFileSystem(vol.ConfigBlockFilesystem(), volDevPath, vol) if err != nil { return err } @@ -458,7 +458,7 @@ func (d *lvm) MountVolume(vol Volume, op *operations.Operation) (bool, error) { } mountFlags, mountOptions := resolveMountOptions(d.volumeMountOptions(vol)) - err = TryMount(volDevPath, mountPath, d.volumeFilesystem(vol), mountFlags, mountOptions) + err = TryMount(volDevPath, mountPath, vol.ConfigBlockFilesystem(), mountFlags, mountOptions) if err != nil { return false, errors.Wrapf(err, "Failed to mount LVM logical volume") } @@ -735,7 +735,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo // we do not want to modify a snapshot in case it is corrupted for some reason, so at mount time // we take another snapshot of the snapshot, regenerate the temporary snapshot's UUID and then // mount that. - regenerateFSUUID := renegerateFilesystemUUIDNeeded(d.volumeFilesystem(snapVol)) + regenerateFSUUID := renegerateFilesystemUUIDNeeded(snapVol.ConfigBlockFilesystem()) if regenerateFSUUID { // Instantiate a new volume to be the temporary writable snapshot. tmpVolName := fmt.Sprintf("%s%s", snapVol.name, tmpVolSuffix) @@ -764,7 +764,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo } if regenerateFSUUID { - tmpVolFsType := d.volumeFilesystem(mountVol) + tmpVolFsType := mountVol.ConfigBlockFilesystem() // When mounting XFS filesystems temporarily we can use the nouuid option rather than fully // regenerating the filesystem UUID. @@ -775,7 +775,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo } } else { d.logger.Debug("Regenerating filesystem UUID", log.Ctx{"dev": volDevPath, "fs": tmpVolFsType}) - err = regenerateFilesystemUUID(d.volumeFilesystem(mountVol), volDevPath) + err = regenerateFilesystemUUID(mountVol.ConfigBlockFilesystem(), volDevPath) if err != nil { return false, err } @@ -783,7 +783,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo } // Finally attempt to mount the volume that needs mounting. - err = TryMount(volDevPath, mountPath, d.volumeFilesystem(mountVol), mountFlags|unix.MS_RDONLY, mountOptions) + err = TryMount(volDevPath, mountPath, mountVol.ConfigBlockFilesystem(), mountFlags|unix.MS_RDONLY, mountOptions) if err != nil { return false, errors.Wrapf(err, "Failed to mount LVM snapshot volume") } @@ -965,14 +965,14 @@ func (d *lvm) RestoreVolume(vol Volume, snapshotName string, op *operations.Oper }) // If the volume's filesystem needs to have its UUID regenerated to allow mount then do so now. - if vol.contentType == ContentTypeFS && renegerateFilesystemUUIDNeeded(d.volumeFilesystem(vol)) { + if vol.contentType == ContentTypeFS && renegerateFilesystemUUIDNeeded(vol.ConfigBlockFilesystem()) { _, err = d.activateVolume(volDevPath) if err != nil { return err } - d.logger.Debug("Regenerating filesystem UUID", log.Ctx{"dev": volDevPath, "fs": d.volumeFilesystem(vol)}) - err = regenerateFilesystemUUID(d.volumeFilesystem(vol), volDevPath) + d.logger.Debug("Regenerating filesystem UUID", log.Ctx{"dev": volDevPath, "fs": vol.ConfigBlockFilesystem()}) + err = regenerateFilesystemUUID(vol.ConfigBlockFilesystem(), volDevPath) if err != nil { return err } From ee5b8b4871e885d232764850734ea771d670dcc3 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 26 May 2020 12:10:06 +0100 Subject: [PATCH 6/6] lxd/storage/drivers: Replace volumeSize() with vol.ConfigSize() Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_btrfs_utils.go | 12 ------------ lxd/storage/drivers/driver_btrfs_volumes.go | 6 +++--- lxd/storage/drivers/driver_ceph_utils.go | 10 ---------- lxd/storage/drivers/driver_ceph_volumes.go | 4 ++-- lxd/storage/drivers/driver_dir_utils.go | 14 +------------- lxd/storage/drivers/driver_dir_volumes.go | 2 +- lxd/storage/drivers/driver_zfs_utils.go | 12 ------------ lxd/storage/drivers/driver_zfs_volumes.go | 6 +++--- 8 files changed, 10 insertions(+), 56 deletions(-) diff --git a/lxd/storage/drivers/driver_btrfs_utils.go b/lxd/storage/drivers/driver_btrfs_utils.go index a4abe51f86..0f66774042 100644 --- a/lxd/storage/drivers/driver_btrfs_utils.go +++ b/lxd/storage/drivers/driver_btrfs_utils.go @@ -300,18 +300,6 @@ func (d *btrfs) sendSubvolume(path string, parent string, conn io.ReadWriteClose 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 -} - // setSubvolumeReadonlyProperty sets the readonly property on the subvolume to true or false. func (d *btrfs) setSubvolumeReadonlyProperty(path string, readonly bool) error { // Silently ignore requests to set subvolume readonly property if running in a user namespace as we won't diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go index 8367af8775..c8dc862b20 100644 --- a/lxd/storage/drivers/driver_btrfs_volumes.go +++ b/lxd/storage/drivers/driver_btrfs_volumes.go @@ -66,7 +66,7 @@ func (d *btrfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Op // We expect the filler function to have converted the qcow2 image to raw into the rootBlockPath. if vol.contentType == ContentTypeBlock { // Convert to bytes. - sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol)) + sizeBytes, err := units.ParseByteSizeString(vol.ConfigSize()) if err != nil { return err } @@ -85,7 +85,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, d.volumeSize(vol), op) + err := d.SetVolumeQuota(vol, vol.ConfigSize(), op) if err != nil { return err } @@ -363,7 +363,7 @@ func (d *btrfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bo // If source is an image then take into account default volume sizes if not specified. if srcVol.volType == VolumeTypeImage { - volSize = d.volumeSize(vol) + volSize = vol.ConfigSize() } err = d.SetVolumeQuota(vol, volSize, op) diff --git a/lxd/storage/drivers/driver_ceph_utils.go b/lxd/storage/drivers/driver_ceph_utils.go index 47ebdb1be2..9bb3d72d33 100644 --- a/lxd/storage/drivers/driver_ceph_utils.go +++ b/lxd/storage/drivers/driver_ceph_utils.go @@ -529,16 +529,6 @@ func (d *ceph) rbdListVolumeSnapshots(vol Volume) ([]string, error) { return snapshots, nil } -// 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" { - return defaultBlockSize - } - - return size -} - // getRBDFilesystem returns the filesystem the RBD storage volume is supposed to be created with. func (d *ceph) getRBDFilesystem(vol Volume) string { if vol.config["block.filesystem"] != "" { diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go index 3884998869..b55d26308c 100644 --- a/lxd/storage/drivers/driver_ceph_volumes.go +++ b/lxd/storage/drivers/driver_ceph_volumes.go @@ -124,7 +124,7 @@ func (d *ceph) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Ope } // Create volume. - err := d.rbdCreateVolume(vol, d.volumeSize(vol)) + err := d.rbdCreateVolume(vol, vol.ConfigSize()) if err != nil { return err } @@ -327,7 +327,7 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots boo // If source is an image then take into account default volume sizes if not specified. if srcVol.volType == VolumeTypeImage { - volSize = d.volumeSize(vol) + volSize = vol.ConfigSize() } err = d.SetVolumeQuota(vol, volSize, op) diff --git a/lxd/storage/drivers/driver_dir_utils.go b/lxd/storage/drivers/driver_dir_utils.go index 055456a308..5badee6c3f 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, d.volumeSize(vol)) + err = d.setQuota(volPath, volID, vol.ConfigSize()) if err != nil { return nil, err } @@ -144,15 +144,3 @@ 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 -} diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go index a980940e6e..2001ce382c 100644 --- a/lxd/storage/drivers/driver_dir_volumes.go +++ b/lxd/storage/drivers/driver_dir_volumes.go @@ -67,7 +67,7 @@ func (d *dir) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper // We expect the filler function to have converted the qcow2 image to raw into the rootBlockPath. if vol.contentType == ContentTypeBlock { // Convert to bytes. - sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol)) + sizeBytes, err := units.ParseByteSizeString(vol.ConfigSize()) if err != nil { return err } diff --git a/lxd/storage/drivers/driver_zfs_utils.go b/lxd/storage/drivers/driver_zfs_utils.go index caa92148ab..9cdd6c127e 100644 --- a/lxd/storage/drivers/driver_zfs_utils.go +++ b/lxd/storage/drivers/driver_zfs_utils.go @@ -316,15 +316,3 @@ 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 -} diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go index 77bd91634c..f12e01badf 100644 --- a/lxd/storage/drivers/driver_zfs_volumes.go +++ b/lxd/storage/drivers/driver_zfs_volumes.go @@ -134,12 +134,12 @@ func (d *zfs) CreateVolume(vol Volume, filler *VolumeFiller, op *operations.Oper } // Apply the size limit. - err = d.SetVolumeQuota(vol, d.volumeSize(vol), op) + err = d.SetVolumeQuota(vol, vol.ConfigSize(), op) if err != nil { return err } } else { - sizeBytes, err := units.ParseByteSizeString(d.volumeSize(vol)) + sizeBytes, err := units.ParseByteSizeString(vol.ConfigSize()) if err != nil { return err } @@ -593,7 +593,7 @@ func (d *zfs) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool // If source is an image then take into account default volume sizes if not specified. if srcVol.volType == VolumeTypeImage { - volSize = d.volumeSize(vol) + volSize = vol.ConfigSize() } err := d.SetVolumeQuota(vol, volSize, op)
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel