The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6559

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) ===
Includes https://github.com/lxc/lxd/pull/6558

- Links publish feature to use new storage package for mounting.
- Removes volume ID Map unshifting (as we shouldn't modify the rootfs or snapshots and new storage package ensures snapshots are readonly).
- Fixes tar writer to unshift as part of tar creation.
From cf4a59f1097729524c200732bd066e211b0885b6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 5 Dec 2019 11:35:52 +0000
Subject: [PATCH 1/5] lxd/backup: Comment consistency

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/backup.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/backup.go b/lxd/backup.go
index 15189bffd7..a01a9eb23f 100644
--- a/lxd/backup.go
+++ b/lxd/backup.go
@@ -63,6 +63,7 @@ func backupCreate(s *state.State, args db.InstanceBackupArgs, 
sourceInst instanc
        }
        defer os.RemoveAll(tmpPath)
 
+       // Check if we can load new storage layer for pool driver type.
        pool, err := storagePools.GetPoolByInstance(s, sourceInst)
        if err != storageDrivers.ErrUnknownDriver && err != 
storageDrivers.ErrNotImplemented {
                if err != nil {

From 3e7ea65dbd92539bc05cf95fed3dab330d18131e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 5 Dec 2019 11:36:49 +0000
Subject: [PATCH 2/5] lxd/daemon: Adds LXD_SHIFTFS_DISABLE env var to disable
 shiftfs

Useful when testing traditional UID shifting.

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/daemon.go | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index c5416d7dfc..5c1b92cf61 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -620,11 +620,16 @@ func (d *Daemon) init() error {
                logger.Infof(" - unprivileged file capabilities: no")
        }
 
-       if util.HasFilesystem("shiftfs") || util.LoadModule("shiftfs") == nil {
-               d.os.Shiftfs = true
-               logger.Infof(" - shiftfs support: yes")
+       // Detect shiftfs support.
+       if shared.IsTrue(os.Getenv("LXD_SHIFTFS_DISABLE")) {
+               logger.Infof(" - shiftfs support: disabled")
        } else {
-               logger.Infof(" - shiftfs support: no")
+               if util.HasFilesystem("shiftfs") || util.LoadModule("shiftfs") 
== nil {
+                       d.os.Shiftfs = true
+                       logger.Infof(" - shiftfs support: yes")
+               } else {
+                       logger.Infof(" - shiftfs support: no")
+               }
        }
 
        // Detect LXC features

From 735e1bf4f5df8cd75557c7afdf365fc4500385bb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 5 Dec 2019 11:38:48 +0000
Subject: [PATCH 3/5] doc/environment: Documents LXD_SHIFTFS_DISABLE env var

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 doc/environment.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/environment.md b/doc/environment.md
index 2925711467..88246c1890 100644
--- a/doc/environment.md
+++ b/doc/environment.md
@@ -28,3 +28,4 @@ Name                            | Description
 `LXD_SECURITY_APPARMOR`         | If set to `false`, forces AppArmor off
 `LXD_UNPRIVILEGED_ONLY`         | If set to `true`, enforces that only 
unprivileged containers can be created. Note that any privileged containers 
that have been created before setting LXD_UNPRIVILEGED_ONLY will continue to be 
privileged. To use this option effectively it should be set when the LXD daemon 
is first setup.
 `LXD_OVMF_PATH`                 | Path to an OVMF build including 
`OVMF_CODE.fd` and `OVMF_VARS.ms.fd`
+`LXD_SHIFTFS_DISABLE`           | Disable shiftfs support (useful when testing 
traditional UID shifting)

From fc8c958423017f8569c55df28a019cdc7a04bca4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 5 Dec 2019 14:48:33 +0000
Subject: [PATCH 4/5] lxd/container/lxc: Updates Export to use new storage pkg
 for mounting

Also removes ID map unshifting functionality, as new storage package will mount 
snapshots as readonly and so we cannot rely on modifying them.

Instead we will unshift UID/GIDs as part of the tar writer.

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/container_lxc.go | 126 ++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 69 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 09120cfca4..d1451a7207 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4879,63 +4879,51 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                "used":      c.lastUsedDate}
 
        if c.IsRunning() {
-               return fmt.Errorf("Cannot export a running container as an 
image")
+               return fmt.Errorf("Cannot export a running instance as an 
image")
        }
 
-       logger.Info("Exporting container", ctxMap)
+       logger.Info("Exporting instance", ctxMap)
 
-       // Start the storage
-       ourStart, err := c.StorageStart()
-       if err != nil {
-               logger.Error("Failed exporting container", ctxMap)
-               return err
-       }
-       if ourStart {
-               defer c.StorageStop()
-       }
-
-       // Unshift the container
-       idmap, err := c.DiskIdmap()
-       if err != nil {
-               logger.Error("Failed exporting container", ctxMap)
-               return err
-       }
-
-       if idmap != nil {
-               if !c.IsSnapshot() && 
shared.IsTrue(c.expandedConfig["security.protection.shift"]) {
-                       return fmt.Errorf("Container is protected against 
filesystem shifting")
+       // Check if we can load new storage layer for pool driver type.
+       pool, err := storagePools.GetPoolByInstance(c.state, c)
+       if err != storageDrivers.ErrUnknownDriver && err != 
storageDrivers.ErrNotImplemented {
+               if err != nil {
+                       return errors.Wrap(err, "Load instance storage pool")
                }
 
-               var err error
+               ourStart, err := pool.MountInstance(c, nil)
+               if err != nil {
 
-               if c.Storage().GetStorageType() == storageTypeZfs {
-                       err = idmap.UnshiftRootfs(c.RootfsPath(), 
zfsIdmapSetSkipper)
-               } else if c.Storage().GetStorageType() == storageTypeBtrfs {
-                       err = UnshiftBtrfsRootfs(c.RootfsPath(), idmap)
-               } else {
-                       err = idmap.UnshiftRootfs(c.RootfsPath(), nil)
                }
+               if ourStart {
+                       defer pool.UnmountInstance(c, nil)
+               }
+       } else {
+               // Start the storage.
+               ourStart, err := c.StorageStart()
                if err != nil {
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
-
-               if c.Storage().GetStorageType() == storageTypeZfs {
-                       defer idmap.ShiftRootfs(c.RootfsPath(), 
zfsIdmapSetSkipper)
-               } else if c.Storage().GetStorageType() == storageTypeBtrfs {
-                       defer ShiftBtrfsRootfs(c.RootfsPath(), idmap)
-               } else {
-                       defer idmap.ShiftRootfs(c.RootfsPath(), nil)
+               if ourStart {
+                       defer c.StorageStop()
                }
        }
 
-       // Create the tarball
+       // Unshift the instance.
+       idmap, err := c.DiskIdmap()
+       if err != nil {
+               logger.Error("Failed exporting instance", ctxMap)
+               return err
+       }
+
+       // Create the tarball.
        ctw := containerwriter.NewContainerTarWriter(w, idmap)
 
-       // Keep track of the first path we saw for each path with nlink>1
+       // Keep track of the first path we saw for each path with nlink>1.
        cDir := c.Path()
 
-       // Path inside the tar image is the pathname starting after cDir
+       // Path inside the tar image is the pathname starting after cDir.
        offset := len(cDir) + 1
 
        writeToTar := func(path string, fi os.FileInfo, err error) error {
@@ -4951,26 +4939,26 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                return nil
        }
 
-       // Look for metadata.yaml
+       // Look for metadata.yaml.
        fnam := filepath.Join(cDir, "metadata.yaml")
        if !shared.PathExists(fnam) {
-               // Generate a new metadata.yaml
+               // Generate a new metadata.yaml.
                tempDir, err := ioutil.TempDir("", "lxd_lxd_metadata_")
                if err != nil {
                        ctw.Close()
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
                defer os.RemoveAll(tempDir)
 
-               // Get the container's architecture
+               // Get the instance's architecture.
                var arch string
                if c.IsSnapshot() {
                        parentName, _, _ := 
shared.InstanceGetParentAndSnapshotName(c.name)
                        parent, err := instanceLoadByProjectAndName(c.state, 
c.project, parentName)
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
 
@@ -4982,12 +4970,12 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                if arch == "" {
                        arch, err = 
osarch.ArchitectureName(c.state.OS.Architectures[0])
                        if err != nil {
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
                }
 
-               // Fill in the metadata
+               // Fill in the metadata.
                meta := api.ImageMetadata{}
                meta.Architecture = arch
                meta.CreationDate = time.Now().UTC().Unix()
@@ -4996,23 +4984,23 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                data, err := yaml.Marshal(&meta)
                if err != nil {
                        ctw.Close()
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
 
-               // Write the actual file
+               // Write the actual file.
                fnam = filepath.Join(tempDir, "metadata.yaml")
                err = ioutil.WriteFile(fnam, data, 0644)
                if err != nil {
                        ctw.Close()
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
 
                fi, err := os.Lstat(fnam)
                if err != nil {
                        ctw.Close()
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
 
@@ -5020,16 +5008,16 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                if err := ctw.WriteFile(tmpOffset, fnam, fi); err != nil {
                        ctw.Close()
                        logger.Debugf("Error writing to tarfile: %s", err)
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
        } else {
                if properties != nil {
-                       // Parse the metadata
+                       // Parse the metadata.
                        content, err := ioutil.ReadFile(fnam)
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
 
@@ -5037,16 +5025,16 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                        err = yaml.Unmarshal(content, &metadata)
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
                        metadata.Properties = properties
 
-                       // Generate a new metadata.yaml
+                       // Generate a new metadata.yaml.
                        tempDir, err := ioutil.TempDir("", "lxd_lxd_metadata_")
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
                        defer os.RemoveAll(tempDir)
@@ -5054,26 +5042,26 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                        data, err := yaml.Marshal(&metadata)
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
 
-                       // Write the actual file
+                       // Write the actual file.
                        fnam = filepath.Join(tempDir, "metadata.yaml")
                        err = ioutil.WriteFile(fnam, data, 0644)
                        if err != nil {
                                ctw.Close()
-                               logger.Error("Failed exporting container", 
ctxMap)
+                               logger.Error("Failed exporting instance", 
ctxMap)
                                return err
                        }
                }
 
-               // Include metadata.yaml in the tarball
+               // Include metadata.yaml in the tarball.
                fi, err := os.Lstat(fnam)
                if err != nil {
                        ctw.Close()
                        logger.Debugf("Error statting %s during export", fnam)
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
 
@@ -5086,36 +5074,36 @@ func (c *containerLXC) Export(w io.Writer, properties 
map[string]string) error {
                if err != nil {
                        ctw.Close()
                        logger.Debugf("Error writing to tarfile: %s", err)
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
        }
 
-       // Include all the rootfs files
+       // Include all the rootfs files.
        fnam = c.RootfsPath()
        err = filepath.Walk(fnam, writeToTar)
        if err != nil {
-               logger.Error("Failed exporting container", ctxMap)
+               logger.Error("Failed exporting instance", ctxMap)
                return err
        }
 
-       // Include all the templates
+       // Include all the templates.
        fnam = c.TemplatesPath()
        if shared.PathExists(fnam) {
                err = filepath.Walk(fnam, writeToTar)
                if err != nil {
-                       logger.Error("Failed exporting container", ctxMap)
+                       logger.Error("Failed exporting instance", ctxMap)
                        return err
                }
        }
 
        err = ctw.Close()
        if err != nil {
-               logger.Error("Failed exporting container", ctxMap)
+               logger.Error("Failed exporting instance", ctxMap)
                return err
        }
 
-       logger.Info("Exported container", ctxMap)
+       logger.Info("Exported instance", ctxMap)
        return nil
 }
 

From b75477a006eae50e33812786a494b0cdb28b0250 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 5 Dec 2019 14:49:57 +0000
Subject: [PATCH 5/5] shared/containerwriter/container/tar/writer: Fixes bug
 with rootfs dir not being unshifted

The file path doesn't include the / at the start.

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 shared/containerwriter/container_tar_writer.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/shared/containerwriter/container_tar_writer.go 
b/shared/containerwriter/container_tar_writer.go
index e0a911c53b..23cecc5b2f 100644
--- a/shared/containerwriter/container_tar_writer.go
+++ b/shared/containerwriter/container_tar_writer.go
@@ -61,12 +61,12 @@ func (ctw *ContainerTarWriter) WriteFile(offset int, path 
string, fi os.FileInfo
                return fmt.Errorf("failed to get file stat: %s", err)
        }
 
-       // Unshift the id under /rootfs/ for unpriv containers
-       if strings.HasPrefix(hdr.Name, "/rootfs") {
+       // Unshift the id under rootfs/ for unpriv containers
+       if strings.HasPrefix(hdr.Name, "rootfs") {
                if ctw.idmapSet != nil {
-                       hUid, hGid := ctw.idmapSet.ShiftFromNs(int64(hdr.Uid), 
int64(hdr.Gid))
-                       hdr.Uid = int(hUid)
-                       hdr.Gid = int(hGid)
+                       hUID, hGID := ctw.idmapSet.ShiftFromNs(int64(hdr.Uid), 
int64(hdr.Gid))
+                       hdr.Uid = int(hUID)
+                       hdr.Gid = int(hGID)
                        if hdr.Uid == -1 || hdr.Gid == -1 {
                                return nil
                        }
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to