The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7488
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) ===
Addresses an issue discovered in https://discuss.linuxcontainers.org/t/volume-block-mount-options-not-working/8049
From 31d86a624c5476ca60c11e8b566530442a53fe1d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:41:49 +0100
Subject: [PATCH 1/6] lxd/storage/drivers/driver/ceph/utils: Removes
getRBDFilesystem
Should use vol.ConfigBlockFilesystem() instead.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph_utils.go | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/lxd/storage/drivers/driver_ceph_utils.go
b/lxd/storage/drivers/driver_ceph_utils.go
index 9e4b24d1ae..5cb653ea84 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -524,19 +524,6 @@ func (d *ceph) rbdListVolumeSnapshots(vol Volume)
([]string, error) {
return snapshots, nil
}
-// 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"] != "" {
- return vol.config["block.filesystem"]
- }
-
- if vol.poolConfig["volume.block.filesystem"] != "" {
- return vol.poolConfig["volume.block.filesystem"]
- }
-
- return "ext4"
-}
-
// getRBDMountOptions returns the mount options the storage volume is supposed
to be mounted with
// the option string that is returned needs to be passed to the approriate
// helper (currently named "LXDResolveMountoptions") which will take on the job
From 2726319ebeaabba58d696bd79ccbe20eaa9170dd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:42:39 +0100
Subject: [PATCH 2/6] lxd/storage/drivers/driver/ceph: Replaces use of
d.getRBDFilesystem with vol.ConfigBlockFilesystem
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph_utils.go | 4 ++--
lxd/storage/drivers/driver_ceph_volumes.go | 14 +++++++-------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lxd/storage/drivers/driver_ceph_utils.go
b/lxd/storage/drivers/driver_ceph_utils.go
index 5cb653ea84..7273a3db18 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -537,7 +537,7 @@ func (d *ceph) getRBDMountOptions(vol Volume) string {
return vol.poolConfig["volume.block.mount_options"]
}
- if d.getRBDFilesystem(vol) == "btrfs" {
+ if vol.ConfigBlockFilesystem() == "btrfs" {
return "user_subvol_rm_allowed,discard"
}
@@ -1032,7 +1032,7 @@ func (d *ceph) getRBDVolumeName(vol Volume, snapName
string, zombie bool, withPo
// Only use filesystem suffix on filesystem type image volumes (for all
content types).
if vol.volType == VolumeTypeImage || vol.volType ==
cephVolumeTypeZombieImage {
- parentName = fmt.Sprintf("%s_%s", parentName,
d.getRBDFilesystem(vol))
+ parentName = fmt.Sprintf("%s_%s", parentName,
vol.ConfigBlockFilesystem())
}
if vol.contentType == ContentTypeBlock {
diff --git a/lxd/storage/drivers/driver_ceph_volumes.go
b/lxd/storage/drivers/driver_ceph_volumes.go
index ab3bd6c9ac..15a347fae2 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -139,7 +139,7 @@ func (d *ceph) CreateVolume(vol Volume, filler
*VolumeFiller, op *operations.Ope
revert.Add(func() { d.rbdUnmapVolume(vol, true) })
// Get filesystem.
- RBDFilesystem := d.getRBDFilesystem(vol)
+ RBDFilesystem := vol.ConfigBlockFilesystem()
if vol.contentType == ContentTypeFS {
_, err = makeFSType(RBDDevPath, RBDFilesystem, nil)
@@ -311,7 +311,7 @@ func (d *ceph) CreateVolumeFromCopy(vol Volume, srcVol
Volume, copySnapshots boo
if vol.contentType == ContentTypeFS {
// Re-generate the UUID. Do this first as ensuring
permissions and setting quota can
// rely on being able to mount the volume.
- err = d.generateUUID(d.getRBDFilesystem(v), RBDDevPath)
+ err = d.generateUUID(v.ConfigBlockFilesystem(),
RBDDevPath)
if err != nil {
return err
}
@@ -587,7 +587,7 @@ func (d *ceph) CreateVolumeFromMigration(vol Volume, conn
io.ReadWriteCloser, vo
defer d.rbdUnmapVolume(vol, true)
// Re-generate the UUID.
- err = d.generateUUID(d.getRBDFilesystem(vol), RBDDevPath)
+ err = d.generateUUID(vol.ConfigBlockFilesystem(), RBDDevPath)
if err != nil {
return err
}
@@ -824,7 +824,7 @@ func (d *ceph) SetVolumeQuota(vol Volume, size string, op
*operations.Operation)
return nil
}
- fsType := d.getRBDFilesystem(vol)
+ fsType := vol.ConfigBlockFilesystem()
RBDDevPath, err := d.getRBDMappedDevPath(vol)
if err != nil {
@@ -928,7 +928,7 @@ func (d *ceph) MountVolume(vol Volume, op
*operations.Operation) (bool, error) {
return false, err
}
- RBDFilesystem := d.getRBDFilesystem(vol)
+ RBDFilesystem := vol.ConfigBlockFilesystem()
mountFlags, mountOptions :=
resolveMountOptions(d.getRBDMountOptions(vol))
err = TryMount(RBDDevPath, mountPath, RBDFilesystem,
mountFlags, mountOptions)
if err != nil {
@@ -1311,7 +1311,7 @@ func (d *ceph) MountVolumeSnapshot(snapVol Volume, op
*operations.Operation) (bo
return false, nil
}
- RBDFilesystem := d.getRBDFilesystem(snapVol)
+ RBDFilesystem := snapVol.ConfigBlockFilesystem()
mountFlags, mountOptions :=
resolveMountOptions(d.getRBDMountOptions(snapVol))
if renegerateFilesystemUUIDNeeded(RBDFilesystem) {
@@ -1453,7 +1453,7 @@ func (d *ceph) RestoreVolume(vol Volume, snapshotName
string, op *operations.Ope
defer d.rbdUnmapVolume(snapVol, true)
// Re-generate the UUID.
- err = d.generateUUID(d.getRBDFilesystem(snapVol), RBDDevPath)
+ err = d.generateUUID(snapVol.ConfigBlockFilesystem(), RBDDevPath)
if err != nil {
return err
}
From 38cb6ae9232f749373884e2be10e70587975e13a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:50:27 +0100
Subject: [PATCH 3/6] lxd/storage/drivers/volume: Adds ConfigBlockMountOptions
function
Removes the need for driver specific logic in LVM and Ceph.
And defines defaultFilesystemMountOptions constant as "discard".
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/volume.go | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go
index 60de988d7d..f343581549 100644
--- a/lxd/storage/drivers/volume.go
+++ b/lxd/storage/drivers/volume.go
@@ -25,6 +25,9 @@ const vmBlockFilesystemSize = "50MB"
// DefaultFilesystem filesytem to use for block devices by default.
const DefaultFilesystem = "ext4"
+// defaultFilesystemMountOpts mount options to use for filesystem block
devices by default.
+const defaultFilesystemMountOptions = "discard"
+
// volIDQuotaSkip is used to indicate to drivers that quotas should not be
setup, used during backup import.
const volIDQuotaSkip = int64(-1)
@@ -349,6 +352,17 @@ func (v Volume) ConfigBlockFilesystem() string {
return DefaultFilesystem
}
+// ConfigBlockMountOptions returns the filesystem mount options to use for
block volumes. Returns config value
+// "block.mount_options" if defined in volume or pool's volume config,
otherwise defaultFilesystemMountOptions.
+func (v Volume) ConfigBlockMountOptions() string {
+ fs := v.ExpandedConfig("block.mount_options")
+ if fs != "" {
+ return fs
+ }
+
+ return defaultFilesystemMountOptions
+}
+
// 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.
From 67869d0cdb2e557af70783cf1e78ca2a4e4d5f05 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:51:16 +0100
Subject: [PATCH 4/6] lxd/storage/drivers/driver/ceph/utils: Removes
getRBDMountOptions in place of vol.ConfigBlockMountOptions()
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph_utils.go | 20 --------------------
1 file changed, 20 deletions(-)
diff --git a/lxd/storage/drivers/driver_ceph_utils.go
b/lxd/storage/drivers/driver_ceph_utils.go
index 7273a3db18..6c399b48a2 100644
--- a/lxd/storage/drivers/driver_ceph_utils.go
+++ b/lxd/storage/drivers/driver_ceph_utils.go
@@ -524,26 +524,6 @@ func (d *ceph) rbdListVolumeSnapshots(vol Volume)
([]string, error) {
return snapshots, nil
}
-// getRBDMountOptions returns the mount options the storage volume is supposed
to be mounted with
-// the option string that is returned needs to be passed to the approriate
-// helper (currently named "LXDResolveMountoptions") which will take on the job
-// of splitting it into appropriate flags and string options.
-func (d *ceph) getRBDMountOptions(vol Volume) string {
- if vol.config["block.mount_options"] != "" {
- return vol.config["block.mount_options"]
- }
-
- if vol.poolConfig["volume.block.mount_options"] != "" {
- return vol.poolConfig["volume.block.mount_options"]
- }
-
- if vol.ConfigBlockFilesystem() == "btrfs" {
- return "user_subvol_rm_allowed,discard"
- }
-
- return "discard"
-}
-
// copyWithSnapshots creates a non-sparse copy of a container including its
snapshots.
// This does not introduce a dependency relation between the source RBD storage
// volume and the target RBD storage volume.
From 9799d829be0165dd3b877fb039aabdd9bb032541 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:52:01 +0100
Subject: [PATCH 5/6] lxd/storage/drivers/driver/lvm/utils: Removes
volumeMountOptions in place of vol.ConfigBlockMountOptions()
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_lvm_utils.go | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/lxd/storage/drivers/driver_lvm_utils.go
b/lxd/storage/drivers/driver_lvm_utils.go
index cabd70a97f..4db8cb5b6f 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -49,20 +49,6 @@ func (d *lvm) thinpoolName() string {
return "LXDThinPool"
}
-// mountOptions returns the mount options for volumes.
-func (d *lvm) volumeMountOptions(vol Volume) string {
- if d.config["block.mount_options"] != "" {
- return d.config["block.mount_options"]
- }
-
- // Use some special options if the filesystem for the volume is BTRFS.
- if vol.ConfigBlockFilesystem() == "btrfs" {
- return "user_subvol_rm_allowed,discard"
- }
-
- return "discard"
-}
-
// openLoopFile opens a loopback file and disable auto detach.
func (d *lvm) openLoopFile(source string) (*os.File, error) {
if source == "" {
From 8a9d03a6d2899f05465520e832ca33c7be22fd24 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 5 Jun 2020 09:52:40 +0100
Subject: [PATCH 6/6] lxd/storage/drivers: Replaces driver specific mount
options resolution with vol.ConfigBlockMountOptions()
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph_volumes.go | 4 ++--
lxd/storage/drivers/driver_lvm_volumes.go | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lxd/storage/drivers/driver_ceph_volumes.go
b/lxd/storage/drivers/driver_ceph_volumes.go
index 15a347fae2..b5e2af4b3f 100644
--- a/lxd/storage/drivers/driver_ceph_volumes.go
+++ b/lxd/storage/drivers/driver_ceph_volumes.go
@@ -929,7 +929,7 @@ func (d *ceph) MountVolume(vol Volume, op
*operations.Operation) (bool, error) {
}
RBDFilesystem := vol.ConfigBlockFilesystem()
- mountFlags, mountOptions :=
resolveMountOptions(d.getRBDMountOptions(vol))
+ mountFlags, mountOptions :=
resolveMountOptions(vol.ConfigBlockMountOptions())
err = TryMount(RBDDevPath, mountPath, RBDFilesystem,
mountFlags, mountOptions)
if err != nil {
return false, err
@@ -1312,7 +1312,7 @@ func (d *ceph) MountVolumeSnapshot(snapVol Volume, op
*operations.Operation) (bo
}
RBDFilesystem := snapVol.ConfigBlockFilesystem()
- mountFlags, mountOptions :=
resolveMountOptions(d.getRBDMountOptions(snapVol))
+ mountFlags, mountOptions :=
resolveMountOptions(snapVol.ConfigBlockMountOptions())
if renegerateFilesystemUUIDNeeded(RBDFilesystem) {
if RBDFilesystem == "xfs" {
diff --git a/lxd/storage/drivers/driver_lvm_volumes.go
b/lxd/storage/drivers/driver_lvm_volumes.go
index 9fab452bb3..7b27aaf798 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -454,7 +454,7 @@ func (d *lvm) MountVolume(vol Volume, op
*operations.Operation) (bool, error) {
return false, err
}
- mountFlags, mountOptions :=
resolveMountOptions(d.volumeMountOptions(vol))
+ mountFlags, mountOptions :=
resolveMountOptions(vol.ConfigBlockMountOptions())
err = TryMount(volDevPath, mountPath,
vol.ConfigBlockFilesystem(), mountFlags, mountOptions)
if err != nil {
return false, errors.Wrapf(err, "Failed to mount LVM
logical volume")
@@ -722,7 +722,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op
*operations.Operation) (boo
// Default to mounting the original snapshot directly. This may
be changed below if a temporary
// snapshot needs to be taken.
mountVol := snapVol
- mountFlags, mountOptions :=
resolveMountOptions(d.volumeMountOptions(mountVol))
+ mountFlags, mountOptions :=
resolveMountOptions(mountVol.ConfigBlockMountOptions())
// Regenerate filesystem UUID if needed. This is because some
filesystems do not allow mounting
// multiple volumes that share the same UUID. As snapshotting a
volume will copy its UUID we need
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
