[lxc-devel] [lxd/master] Move container taring logic into standalone class

JulianAustralia on Github lxc-bot at linuxcontainers.org
Mon Jun 24 03:13:31 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 704 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190623/bceeb1a6/attachment.bin>
-------------- next part --------------
From a19fd0058f2a650773b3db95897aebf4bf0d888b Mon Sep 17 00:00:00 2001
From: Julian Watson <juwa at google.com>
Date: Mon, 17 Jun 2019 11:39:00 +1000
Subject: [PATCH] Move container taring logic into standalone class

This has been done because the existing logic was difficult to understand,
as implementation details of the tarring, such as requiring a linkMap, were
exposed to the call site; it was also slower than necessary as it would decode
the container's idmap set, even if the call site had already decoded it (it had
in the single place it was being used).

Signed-off-by: Julian Watson <juwa at google.com>
---
 lxd/container_lxc.go                          | 136 +++---------------
 .../containerwriter/container_tar_writer.go   | 124 ++++++++++++++++
 2 files changed, 144 insertions(+), 116 deletions(-)
 create mode 100644 shared/containerwriter/container_tar_writer.go

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 285db3e460..a49d7d9bfb 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1,7 +1,6 @@
 package main
 
 import (
-	"archive/tar"
 	"bufio"
 	"encoding/json"
 	"fmt"
@@ -37,6 +36,7 @@ import (
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/api"
+	"github.com/lxc/lxd/shared/containerwriter"
 	"github.com/lxc/lxd/shared/idmap"
 	"github.com/lxc/lxd/shared/logger"
 	"github.com/lxc/lxd/shared/netutils"
@@ -5951,10 +5951,9 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 	}
 
 	// Create the tarball
-	tw := tar.NewWriter(w)
+	ctw := containerwriter.NewContainerTarWriter(w, idmap)
 
 	// Keep track of the first path we saw for each path with nlink>1
-	linkmap := map[uint64]string{}
 	cDir := c.Path()
 
 	// Path inside the tar image is the pathname starting after cDir
@@ -5965,7 +5964,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			return err
 		}
 
-		err = c.tarStoreFile(linkmap, offset, tw, path, fi)
+		err = ctw.WriteFile(offset, path, fi)
 		if err != nil {
 			logger.Debugf("Error tarring up %s: %s", path, err)
 			return err
@@ -5979,7 +5978,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 		// Generate a new metadata.yaml
 		tempDir, err := ioutil.TempDir("", "lxd_lxd_metadata_")
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Error("Failed exporting container", ctxMap)
 			return err
 		}
@@ -5991,7 +5990,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			parentName, _, _ := containerGetParentAndSnapshotName(c.name)
 			parent, err := containerLoadByProjectAndName(c.state, c.project, parentName)
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6017,7 +6016,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 
 		data, err := yaml.Marshal(&meta)
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Error("Failed exporting container", ctxMap)
 			return err
 		}
@@ -6026,21 +6025,21 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 		fnam = filepath.Join(tempDir, "metadata.yaml")
 		err = ioutil.WriteFile(fnam, data, 0644)
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Error("Failed exporting container", ctxMap)
 			return err
 		}
 
 		fi, err := os.Lstat(fnam)
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Error("Failed exporting container", ctxMap)
 			return err
 		}
 
 		tmpOffset := len(path.Dir(fnam)) + 1
-		if err := c.tarStoreFile(linkmap, tmpOffset, tw, fnam, fi); err != nil {
-			tw.Close()
+		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)
 			return err
@@ -6050,7 +6049,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			// Parse the metadata
 			content, err := ioutil.ReadFile(fnam)
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6058,7 +6057,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			metadata := new(api.ImageMetadata)
 			err = yaml.Unmarshal(content, &metadata)
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6067,7 +6066,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			// Generate a new metadata.yaml
 			tempDir, err := ioutil.TempDir("", "lxd_lxd_metadata_")
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6075,7 +6074,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 
 			data, err := yaml.Marshal(&metadata)
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6084,7 +6083,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 			fnam = filepath.Join(tempDir, "metadata.yaml")
 			err = ioutil.WriteFile(fnam, data, 0644)
 			if err != nil {
-				tw.Close()
+				ctw.Close()
 				logger.Error("Failed exporting container", ctxMap)
 				return err
 			}
@@ -6093,7 +6092,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 		// Include metadata.yaml in the tarball
 		fi, err := os.Lstat(fnam)
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Debugf("Error statting %s during export", fnam)
 			logger.Error("Failed exporting container", ctxMap)
 			return err
@@ -6101,12 +6100,12 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 
 		if properties != nil {
 			tmpOffset := len(path.Dir(fnam)) + 1
-			err = c.tarStoreFile(linkmap, tmpOffset, tw, fnam, fi)
+			err = ctw.WriteFile(tmpOffset, fnam, fi)
 		} else {
-			err = c.tarStoreFile(linkmap, offset, tw, fnam, fi)
+			err = ctw.WriteFile(offset, fnam, fi)
 		}
 		if err != nil {
-			tw.Close()
+			ctw.Close()
 			logger.Debugf("Error writing to tarfile: %s", err)
 			logger.Error("Failed exporting container", ctxMap)
 			return err
@@ -6131,7 +6130,7 @@ func (c *containerLXC) Export(w io.Writer, properties map[string]string) error {
 		}
 	}
 
-	err = tw.Close()
+	err = ctw.Close()
 	if err != nil {
 		logger.Error("Failed exporting container", ctxMap)
 		return err
@@ -7180,101 +7179,6 @@ func (c *containerLXC) processesState() int64 {
 	return int64(len(pids))
 }
 
-func (c *containerLXC) tarStoreFile(linkmap map[uint64]string, offset int, tw *tar.Writer, path string, fi os.FileInfo) error {
-	var err error
-	var major, minor, nlink int
-	var ino uint64
-
-	link := ""
-	if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
-		link, err = os.Readlink(path)
-		if err != nil {
-			return fmt.Errorf("failed to resolve symlink: %s", err)
-		}
-	}
-
-	// Sockets cannot be stored in tarballs, just skip them (consistent with tar)
-	if fi.Mode()&os.ModeSocket == os.ModeSocket {
-		return nil
-	}
-
-	hdr, err := tar.FileInfoHeader(fi, link)
-	if err != nil {
-		return fmt.Errorf("failed to create tar info header: %s", err)
-	}
-
-	hdr.Name = path[offset:]
-	if fi.IsDir() || fi.Mode()&os.ModeSymlink == os.ModeSymlink {
-		hdr.Size = 0
-	} else {
-		hdr.Size = fi.Size()
-	}
-
-	hdr.Uid, hdr.Gid, major, minor, ino, nlink, err = shared.GetFileStat(path)
-	if err != nil {
-		return fmt.Errorf("failed to get file stat: %s", err)
-	}
-
-	// Unshift the id under /rootfs/ for unpriv containers
-	if strings.HasPrefix(hdr.Name, "/rootfs") {
-		idmapset, err := c.DiskIdmap()
-		if err != nil {
-			return err
-		}
-
-		if idmapset != nil {
-			huid, hgid := 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
-			}
-		}
-	}
-
-	if major != -1 {
-		hdr.Devmajor = int64(major)
-		hdr.Devminor = int64(minor)
-	}
-
-	// If it's a hardlink we've already seen use the old name
-	if fi.Mode().IsRegular() && nlink > 1 {
-		if firstpath, found := linkmap[ino]; found {
-			hdr.Typeflag = tar.TypeLink
-			hdr.Linkname = firstpath
-			hdr.Size = 0
-		} else {
-			linkmap[ino] = hdr.Name
-		}
-	}
-
-	// Handle xattrs (for real files only)
-	if link == "" {
-		hdr.Xattrs, err = shared.GetAllXattr(path)
-		if err != nil {
-			return fmt.Errorf("Failed to read xattr for '%s': %s", path, err)
-		}
-	}
-
-	if err := tw.WriteHeader(hdr); err != nil {
-		return fmt.Errorf("Failed to write tar header: %s", err)
-	}
-
-	if hdr.Typeflag == tar.TypeReg {
-		f, err := os.Open(path)
-		if err != nil {
-			return fmt.Errorf("Failed to open the file: %s", err)
-		}
-		defer f.Close()
-
-		if _, err := io.Copy(tw, f); err != nil {
-			return fmt.Errorf("Failed to copy file content: %s", err)
-		}
-	}
-
-	return nil
-}
-
 // Storage functions
 func (c *containerLXC) Storage() storage {
 	if c.storage == nil {
diff --git a/shared/containerwriter/container_tar_writer.go b/shared/containerwriter/container_tar_writer.go
new file mode 100644
index 0000000000..56f5937f2a
--- /dev/null
+++ b/shared/containerwriter/container_tar_writer.go
@@ -0,0 +1,124 @@
+package containerwriter
+
+import (
+	"archive/tar"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/idmap"
+)
+
+type ContainerTarWriter struct {
+	tarWriter *tar.Writer
+	idmapSet  *idmap.IdmapSet
+	linkMap   map[uint64]string
+}
+
+func NewContainerTarWriter(writer io.Writer, idmapSet *idmap.IdmapSet) *ContainerTarWriter {
+	ctw := new(ContainerTarWriter)
+	ctw.tarWriter = tar.NewWriter(writer)
+	ctw.idmapSet = idmapSet
+	ctw.linkMap = map[uint64]string{}
+	return ctw
+}
+
+func (ctw *ContainerTarWriter) WriteFile(offset int, path string, fi os.FileInfo) error {
+	var err error
+	var major, minor, nlink int
+	var ino uint64
+
+	link := ""
+	if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
+		link, err = os.Readlink(path)
+		if err != nil {
+			return fmt.Errorf("failed to resolve symlink: %s", err)
+		}
+	}
+
+	// Sockets cannot be stored in tarballs, just skip them (consistent with tar)
+	if fi.Mode()&os.ModeSocket == os.ModeSocket {
+		return nil
+	}
+
+	hdr, err := tar.FileInfoHeader(fi, link)
+	if err != nil {
+		return fmt.Errorf("failed to create tar info header: %s", err)
+	}
+
+	hdr.Name = path[offset:]
+	if fi.IsDir() || fi.Mode()&os.ModeSymlink == os.ModeSymlink {
+		hdr.Size = 0
+	} else {
+		hdr.Size = fi.Size()
+	}
+
+	hdr.Uid, hdr.Gid, major, minor, ino, nlink, err = shared.GetFileStat(path)
+	if err != nil {
+		return fmt.Errorf("failed to get file stat: %s", err)
+	}
+
+	// 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)
+			if hdr.Uid == -1 || hdr.Gid == -1 {
+				return nil
+			}
+		}
+	}
+
+	if major != -1 {
+		hdr.Devmajor = int64(major)
+		hdr.Devminor = int64(minor)
+	}
+
+	// If it's a hardlink we've already seen use the old name
+	if fi.Mode().IsRegular() && nlink > 1 {
+		if firstPath, found := ctw.linkMap[ino]; found {
+			hdr.Typeflag = tar.TypeLink
+			hdr.Linkname = firstPath
+			hdr.Size = 0
+		} else {
+			ctw.linkMap[ino] = hdr.Name
+		}
+	}
+
+	// Handle xattrs (for real files only)
+	if link == "" {
+		hdr.Xattrs, err = shared.GetAllXattr(path)
+		if err != nil {
+			return fmt.Errorf("failed to read xattr for '%s': %s", path, err)
+		}
+	}
+
+	if err := ctw.tarWriter.WriteHeader(hdr); err != nil {
+		return fmt.Errorf("failed to write tar header: %s", err)
+	}
+
+	if hdr.Typeflag == tar.TypeReg {
+		f, err := os.Open(path)
+		if err != nil {
+			return fmt.Errorf("failed to open the file: %s", err)
+		}
+		defer f.Close()
+
+		if _, err := io.Copy(ctw.tarWriter, f); err != nil {
+			return fmt.Errorf("failed to copy file content: %s", err)
+		}
+	}
+
+	return nil
+}
+
+func (ctw *ContainerTarWriter) Close() error {
+	err := ctw.tarWriter.Close()
+	if err != nil {
+		return fmt.Errorf("failed to close tar writer: %s", err)
+	}
+	return nil
+}


More information about the lxc-devel mailing list