[lxc-devel] [lxd/master] Cleanup storage shifting logic
stgraber on Github
lxc-bot at linuxcontainers.org
Wed Mar 27 22:27:02 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190327/d3b1d05a/attachment.bin>
-------------- next part --------------
From a9abf8a0e8aa79cd189e0f03097e64f94fa80e7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 27 Mar 2019 18:15:16 -0400
Subject: [PATCH 1/2] lxd/storage: Rename ShiftIfNecessary to
resetContainerDiskIdmap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/migrate_container.go | 2 +-
lxd/storage.go | 4 +---
lxd/storage_migration.go | 6 +++---
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/lxd/migrate_container.go b/lxd/migrate_container.go
index 6c9b5ee908..69d3ec2eeb 100644
--- a/lxd/migrate_container.go
+++ b/lxd/migrate_container.go
@@ -983,7 +983,7 @@ func (c *migrationSink) Do(migrateOp *operation) error {
return
}
- err = ShiftIfNecessary(c.src.container, srcIdmap)
+ err = resetContainerDiskIdmap(c.src.container, srcIdmap)
if err != nil {
fsTransfer <- err
return
diff --git a/lxd/storage.go b/lxd/storage.go
index 3ed516ca2e..1f9438a05d 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -744,9 +744,7 @@ func deleteSnapshotMountpoint(snapshotMountpoint string, snapshotsSymlinkTarget
return nil
}
-// ShiftIfNecessary sets the volatile.last_state.idmap key to the idmap last
-// used by the container.
-func ShiftIfNecessary(container container, srcIdmap *idmap.IdmapSet) error {
+func resetContainerDiskIdmap(container container, srcIdmap *idmap.IdmapSet) error {
dstIdmap, err := container.IdmapSet()
if err != nil {
return err
diff --git a/lxd/storage_migration.go b/lxd/storage_migration.go
index fcf87d6014..81e6c6f014 100644
--- a/lxd/storage_migration.go
+++ b/lxd/storage_migration.go
@@ -300,7 +300,7 @@ func rsyncMigrationSink(conn *websocket.Conn, op *operation, args MigrationSinkA
return err
}
- err = ShiftIfNecessary(args.Container, args.Idmap)
+ err = resetContainerDiskIdmap(args.Container, args.Idmap)
if err != nil {
return err
}
@@ -351,7 +351,7 @@ func rsyncMigrationSink(conn *websocket.Conn, op *operation, args MigrationSinkA
return err
}
- err = ShiftIfNecessary(args.Container, args.Idmap)
+ err = resetContainerDiskIdmap(args.Container, args.Idmap)
if err != nil {
return err
}
@@ -383,7 +383,7 @@ func rsyncMigrationSink(conn *websocket.Conn, op *operation, args MigrationSinkA
}
}
- err = ShiftIfNecessary(args.Container, args.Idmap)
+ err = resetContainerDiskIdmap(args.Container, args.Idmap)
if err != nil {
return err
}
From a383b8aaa8ba773eee73d3c775fd785919a69894 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 27 Mar 2019 18:23:40 -0400
Subject: [PATCH 2/2] lxd/storage: Remove setUnprivUserACL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Containers fix their permissions on startup based on whether they run
privileged or not, this is much more reliable than relying on the
individual storage drivers doing the right thing.
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/storage_btrfs.go | 12 ------------
lxd/storage_ceph.go | 5 -----
lxd/storage_dir.go | 10 ----------
lxd/storage_lvm_utils.go | 5 -----
lxd/storage_shared.go | 42 ----------------------------------------
5 files changed, 74 deletions(-)
diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index 2d1d3bfffd..2c2fee436a 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -1037,12 +1037,6 @@ func (s *storageBtrfs) copyContainer(target container, source container) error {
return err
}
- err = s.setUnprivUserACL(source, targetContainerSubvolumeName)
- if err != nil {
- s.ContainerDelete(target)
- return err
- }
-
err = target.TemplateApply("copy")
if err != nil {
return err
@@ -1372,12 +1366,6 @@ func (s *storageBtrfs) ContainerRestore(container container, sourceContainer con
}
}
- // Now allow unprivileged users to access its data.
- err = s.setUnprivUserACL(sourceContainer, targetContainerSubvolumeName)
- if err != nil {
- failure = err
- }
-
if failure == nil {
undo = false
_, sourcePool, _ := srcContainerStorage.GetContainerPoolInfo()
diff --git a/lxd/storage_ceph.go b/lxd/storage_ceph.go
index 21e513966d..a626b4367d 100644
--- a/lxd/storage_ceph.go
+++ b/lxd/storage_ceph.go
@@ -1396,11 +1396,6 @@ func (s *storageCeph) ContainerCopy(target container, source container,
defer s.ContainerUmount(target, targetContainerMountPoint)
}
- err = s.setUnprivUserACL(source, targetContainerMountPoint)
- if err != nil {
- return err
- }
-
err = target.TemplateApply("copy")
if err != nil {
logger.Errorf(`Failed to apply copy template for container "%s": %s`, target.Name(), err)
diff --git a/lxd/storage_dir.go b/lxd/storage_dir.go
index 046fd8d600..f82e821b26 100644
--- a/lxd/storage_dir.go
+++ b/lxd/storage_dir.go
@@ -646,11 +646,6 @@ func (s *storageDir) copyContainer(target container, source container) error {
return fmt.Errorf("failed to rsync container: %s: %s", string(output), err)
}
- err = s.setUnprivUserACL(source, targetContainerMntPoint)
- if err != nil {
- return err
- }
-
err = target.TemplateApply("copy")
if err != nil {
return err
@@ -875,11 +870,6 @@ func (s *storageDir) ContainerRestore(container container, sourceContainer conta
return fmt.Errorf("failed to rsync container: %s: %s", string(output), err)
}
- // Now allow unprivileged users to access its data.
- if err := s.setUnprivUserACL(sourceContainer, targetPath); err != nil {
- return err
- }
-
logger.Debugf("Restored DIR storage volume for container \"%s\" from %s to %s", s.volume.Name, sourceContainer.Name(), container.Name())
return nil
}
diff --git a/lxd/storage_lvm_utils.go b/lxd/storage_lvm_utils.go
index f2a902d670..c5a883d096 100644
--- a/lxd/storage_lvm_utils.go
+++ b/lxd/storage_lvm_utils.go
@@ -469,11 +469,6 @@ func (s *storageLvm) copyContainer(target container, source container, refresh b
return err
}
- err = s.setUnprivUserACL(source, targetContainerMntPoint)
- if err != nil {
- return err
- }
-
err = target.TemplateApply("copy")
if err != nil {
return err
diff --git a/lxd/storage_shared.go b/lxd/storage_shared.go
index acea19e4a4..ae7d7ee771 100644
--- a/lxd/storage_shared.go
+++ b/lxd/storage_shared.go
@@ -5,7 +5,6 @@ import (
"os"
"github.com/lxc/lxd/lxd/state"
- "github.com/lxc/lxd/shared"
"github.com/lxc/lxd/shared/api"
"github.com/lxc/lxd/shared/logger"
"github.com/pkg/errors"
@@ -37,7 +36,6 @@ func (s *storageShared) GetStorageTypeVersion() string {
}
func (s *storageShared) initialShiftRootfs(c container, skipper func(dir string, absPath string, fi os.FileInfo) bool) error {
- dpath := c.Path()
rpath := c.RootfsPath()
logger.Debugf("Shifting root filesystem \"%s\" for \"%s\"", rpath, c.Name())
@@ -57,46 +55,6 @@ func (s *storageShared) initialShiftRootfs(c container, skipper func(dir string,
return errors.Wrap(err, "Shift rootfs")
}
- /* Set an acl so the container root can descend the container dir */
- // TODO: i changed this so it calls s.setUnprivUserAcl, which does
- // the acl change only if the container is not privileged, think thats right.
- return s.setUnprivUserACL(c, dpath)
-}
-
-func (s *storageShared) setUnprivUserACL(c container, destPath string) error {
- idmapset, err := c.IdmapSet()
- if err != nil {
- return err
- }
-
- // Skip for privileged containers
- if idmapset == nil {
- return nil
- }
-
- // Make sure the map is valid. Skip if container uid 0 == host uid 0
- uid, _ := idmapset.ShiftIntoNs(0, 0)
- switch uid {
- case -1:
- return fmt.Errorf("Container doesn't have a uid 0 in its map")
- case 0:
- return nil
- }
-
- // Attempt to set a POSIX ACL first.
- acl := fmt.Sprintf("%d:rx", uid)
- _, err = shared.RunCommand("setfacl", "-m", acl, destPath)
- if err == nil {
- return nil
- }
-
- // Fallback to chmod if the fs doesn't support it.
- _, err = shared.RunCommand("chmod", "+x", destPath)
- if err != nil {
- logger.Debugf("Failed to set executable bit on the container path: %s", err)
- return err
- }
-
return nil
}
More information about the lxc-devel
mailing list