The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7310
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) ===
This PR fixes an issue where for DIR and BTRFS drivers, if the custom volume root permission was modified inside the instance, it was then modified on restart back to the default value of `0711`.
This issue was reported here https://discuss.linuxcontainers.org/t/secondary-disk-rights-issue/7679/
From 29dc3614a3367f5e229f43f09476dd32ba6035f4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 6 May 2020 09:18:32 +0100
Subject: [PATCH 1/2] test/suites/storage/volume/attach: Adds test for custom
volume root perm persistence
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
test/suites/storage_volume_attach.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/test/suites/storage_volume_attach.sh
b/test/suites/storage_volume_attach.sh
index 80b732a54a..bdbc2f6d28 100644
--- a/test/suites/storage_volume_attach.sh
+++ b/test/suites/storage_volume_attach.sh
@@ -90,6 +90,13 @@ test_storage_volume_attach() {
# attach second container
lxc storage volume attach "lxdtest-$(basename "${LXD_DIR}")" testvolume c2
testvolume
+ # check that setting perms on the root of the custom volume persists after a
reboot.
+ lxc exec c2 -- stat -c '%a' /testvolume | grep 711
+ lxc exec c2 -- chmod 0700 /testvolume
+ lxc exec c2 -- stat -c '%a' /testvolume | grep 700
+ lxc restart --force c2
+ lxc exec c2 -- stat -c '%a' /testvolume | grep 700
+
# delete containers
lxc delete -f c1
lxc delete -f c2
From e913337a2196797090fa4e4e1a9ca757c4d3b5de Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 6 May 2020 09:26:32 +0100
Subject: [PATCH 2/2] lxd/storage/drivers: Fixes custom volume root mount perm
issue for BTRFS and DIR
The mount-time EnsureMountPath() call was resetting the permission of the
custom volume root path to 0711.
However for BTRFS and DIR drivers, because there is no actual mount occurring,
this was resetting any permission set inside the instance.
For these drivers we now do not call EnsureMountPath if the path exists and the
volumne type is custom.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_btrfs_volumes.go | 28 ++++++++++++++-------
lxd/storage/drivers/driver_dir_volumes.go | 25 ++++++++++++------
2 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go
b/lxd/storage/drivers/driver_btrfs_volumes.go
index 9048a01f4c..275fc5d97d 100644
--- a/lxd/storage/drivers/driver_btrfs_volumes.go
+++ b/lxd/storage/drivers/driver_btrfs_volumes.go
@@ -571,14 +571,19 @@ func (d *btrfs) GetVolumeDiskPath(vol Volume) (string,
error) {
return genericVFSGetVolumeDiskPath(vol)
}
-// MountVolume simulates mounting a volume.
+// MountVolume simulates mounting a volume. As the driver doesn't have volumes
to mount it returns
+// false indicating that there is no need to issue an unmount.
func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool,
error) {
- err := vol.EnsureMountPath()
- if err != nil {
- return false, err
+ // Don't attempt to modify the permission of an existing custom volume
root.
+ // A user inside the instance may have modified this and we don't want
to reset it on restart.
+ if !shared.PathExists(vol.MountPath()) || vol.volType !=
VolumeTypeCustom {
+ err := vol.EnsureMountPath()
+ if err != nil {
+ return false, err
+ }
}
- return true, nil
+ return false, nil
}
// UnmountVolume simulates unmounting a volume.
@@ -871,12 +876,17 @@ func (d *btrfs) DeleteVolumeSnapshot(snapVol Volume, op
*operations.Operation) e
// MountVolumeSnapshot sets up a read-only mount on top of the snapshot to
avoid accidental modifications.
func (d *btrfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation)
(bool, error) {
- err := snapVol.EnsureMountPath()
- if err != nil {
- return false, err
+ snapPath := snapVol.MountPath()
+
+ // Don't attempt to modify the permission of an existing custom volume
root.
+ // A user inside the instance may have modified this and we don't want
to reset it on restart.
+ if !shared.PathExists(snapPath) || snapVol.volType != VolumeTypeCustom {
+ err := snapVol.EnsureMountPath()
+ if err != nil {
+ return false, err
+ }
}
- snapPath := snapVol.MountPath()
return mountReadOnly(snapPath, snapPath)
}
diff --git a/lxd/storage/drivers/driver_dir_volumes.go
b/lxd/storage/drivers/driver_dir_volumes.go
index 61a8516080..5055dd7ea4 100644
--- a/lxd/storage/drivers/driver_dir_volumes.go
+++ b/lxd/storage/drivers/driver_dir_volumes.go
@@ -301,12 +301,16 @@ func (d *dir) GetVolumeDiskPath(vol Volume) (string,
error) {
return genericVFSGetVolumeDiskPath(vol)
}
-// MountVolume simulates mounting a volume. As dir driver doesn't have volumes
to mount it returns
+// MountVolume simulates mounting a volume. As the driver doesn't have volumes
to mount it returns
// false indicating that there is no need to issue an unmount.
func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
- err := vol.EnsureMountPath()
- if err != nil {
- return false, err
+ // Don't attempt to modify the permission of an existing custom volume
root.
+ // A user inside the instance may have modified this and we don't want
to reset it on restart.
+ if !shared.PathExists(vol.MountPath()) || vol.volType !=
VolumeTypeCustom {
+ err := vol.EnsureMountPath()
+ if err != nil {
+ return false, err
+ }
}
return false, nil
@@ -395,12 +399,17 @@ func (d *dir) DeleteVolumeSnapshot(snapVol Volume, op
*operations.Operation) err
// MountVolumeSnapshot sets up a read-only mount on top of the snapshot to
avoid accidental modifications.
func (d *dir) MountVolumeSnapshot(snapVol Volume, op *operations.Operation)
(bool, error) {
- err := snapVol.EnsureMountPath()
- if err != nil {
- return false, err
+ snapPath := snapVol.MountPath()
+
+ // Don't attempt to modify the permission of an existing custom volume
root.
+ // A user inside the instance may have modified this and we don't want
to reset it on restart.
+ if !shared.PathExists(snapPath) || snapVol.volType != VolumeTypeCustom {
+ err := snapVol.EnsureMountPath()
+ if err != nil {
+ return false, err
+ }
}
- snapPath := snapVol.MountPath()
return mountReadOnly(snapPath, snapPath)
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
