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

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 lays the groundwork needed to link backup restoration to new storage layer.

- Renames containerCreateFromBackup to instanceCreateFromBackup.
- Modifies instanceCreateFromBackup to return a revert function rather than the old storage pool (used to delete the unpacked backup files on error).
- Restructures instanceCreateFromBackup to only use the storage pool in a single place (which will then be replaced with the new storage pkg where possible).
 
From 329c4e2ad3059c17c6c0cdf859d94acf58138786 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 26 Nov 2019 16:53:25 +0000
Subject: [PATCH 1/4] shared/archive/linux: Adds some explanation to
 DetectCompressionFile

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 shared/archive_linux.go | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/shared/archive_linux.go b/shared/archive_linux.go
index a77a09b70a..9a07136577 100644
--- a/shared/archive_linux.go
+++ b/shared/archive_linux.go
@@ -23,6 +23,9 @@ func DetectCompression(fname string) ([]string, string, 
[]string, error) {
        return DetectCompressionFile(f)
 }
 
+// DetectCompressionFile detects the compression type of a file and returns 
the tar arguments needed
+// to unpack the file, compression type (in the form of a file extension), and 
the command needed
+// to decompress the file to an uncompressed tarball.
 func DetectCompressionFile(f io.ReadSeeker) ([]string, string, []string, 
error) {
        // read header parts to detect compression method
        // bz2 - 2 bytes, 'BZ' signature/magic number
@@ -50,8 +53,7 @@ func DetectCompressionFile(f io.ReadSeeker) ([]string, 
string, []string, error)
        case bytes.Equal(header[257:262], []byte{'u', 's', 't', 'a', 'r'}):
                return []string{"-xf"}, ".tar", []string{}, nil
        case bytes.Equal(header[0:4], []byte{'h', 's', 'q', 's'}):
-               return []string{"-xf"}, ".squashfs",
-                       []string{"sqfs2tar", "--no-skip"}, nil
+               return []string{"-xf"}, ".squashfs", []string{"sqfs2tar", 
"--no-skip"}, nil
        default:
                return nil, "", nil, fmt.Errorf("Unsupported compression")
        }

From 097bd67c51200665b6214425bbee03fb24df5825 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 26 Nov 2019 16:53:50 +0000
Subject: [PATCH 2/4] lxd/images: Adds more output detail when tar2sqfs fails
 in compressFile

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

diff --git a/lxd/images.go b/lxd/images.go
index cf989c4011..c8f04132af 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -122,14 +122,13 @@ func compressFile(compress string, infile io.Reader, 
outfile io.Writer) error {
                defer os.Remove(tempfile.Name())
 
                // Prepare 'tar2sqfs' arguments
-               args := []string{"tar2sqfs", "--no-skip", "--force",
-                       "--compressor", "xz", tempfile.Name()}
+               args := []string{"tar2sqfs", "--no-skip", "--force", 
"--compressor", "xz", tempfile.Name()}
                cmd = exec.Command(args[0], args[1:]...)
                cmd.Stdin = infile
 
-               err = cmd.Run()
+               output, err := cmd.CombinedOutput()
                if err != nil {
-                       return err
+                       return fmt.Errorf("tar2sqfs: %v (%v)", err, 
strings.TrimSpace(string(output)))
                }
                // Replay the result to outfile
                tempfile.Seek(0, 0)

From 010b400f716a5b7d662095c912dd3b02c407ae1c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 26 Nov 2019 16:54:48 +0000
Subject: [PATCH 3/4] lxd/container: instanceCreateFromBackup restructure so as
 not to return storage

- Lays the groundwork to support both old and new storage layers.

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

diff --git a/lxd/container.go b/lxd/container.go
index 1406e91ebf..125e5c4b8e 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -305,26 +305,22 @@ func instanceCreateAsEmpty(d *Daemon, args 
db.InstanceArgs) (instance.Instance,
        return inst, nil
 }
 
-func containerCreateFromBackup(s *state.State, info backup.Info, data 
io.ReadSeeker,
-       customPool bool) (storage, error) {
-       var pool storage
+// instanceCreateFromBackup imports a backup file to restore an instance. 
Returns a revert function
+// that can be called to delete the files created during import if subsequent 
steps fail.
+func instanceCreateFromBackup(s *state.State, info backup.Info, srcData 
io.ReadSeeker, customPool bool) (func(), error) {
        var fixBackupFile = false
+       poolName := info.Pool
 
-       // Get storage pool from index.yaml
-       pool, storageErr := storagePoolInit(s, info.Pool)
-       if storageErr != nil && errors.Cause(storageErr) != db.ErrNoSuchObject {
-               // Unexpected error
-               return nil, storageErr
-       }
-
-       if errors.Cause(storageErr) == db.ErrNoSuchObject {
-               // The pool doesn't exist, and the backup is in binary format 
so we
-               // cannot alter the backup.yaml.
+       // Check storage pool from index.yaml exists. If the storage pool in 
the index.yaml doesn't
+       // exist, try and use the default profile's storage pool.
+       _, _, err := s.Cluster.StoragePoolGet(poolName)
+       if errors.Cause(err) == db.ErrNoSuchObject {
+               // The backup is in binary format so we cannot alter the 
backup.yaml.
                if info.HasBinaryFormat {
-                       return nil, storageErr
+                       return nil, err
                }
 
-               // Get the default profile
+               // Get the default profile.
                _, profile, err := s.Cluster.ProfileGet(info.Project, "default")
                if err != nil {
                        return nil, errors.Wrap(err, "Failed to get default 
profile")
@@ -335,43 +331,42 @@ func containerCreateFromBackup(s *state.State, info 
backup.Info, data io.ReadSee
                        return nil, errors.Wrap(err, "Failed to get root disk 
device")
                }
 
-               // Use the default-profile's root pool
-               pool, err = storagePoolInit(s, v["pool"])
-               if err != nil {
-                       return nil, errors.Wrap(err, "Failed to initialize 
storage pool")
-               }
+               // Use the default-profile's root pool.
+               poolName = v["pool"]
 
+               // Mark requirement to change the pool information in the 
backup.yaml file.
                fixBackupFile = true
+       } else if err != nil {
+               return nil, err
        }
 
-       // Find the compression algorithm
-       tarArgs, algo, decomArgs, err := shared.DetectCompressionFile(data)
+       // Find the compression algorithm.
+       tarArgs, algo, decomArgs, err := shared.DetectCompressionFile(srcData)
        if err != nil {
                return nil, err
        }
-       data.Seek(0, 0)
+       srcData.Seek(0, 0)
 
        if algo == ".squashfs" {
-               // Create a temporary file. 'sqfs2tar' tool do not support 
reading
-               // from stdin. So write the compressed data to the temporary 
file
-               // and pass it as program argument
-               tempfile, err := ioutil.TempFile("", "lxd_decompress_")
+               // The 'sqfs2tar' tool does not support reading from stdin. So 
we need to create a
+               // temporary file, write the compressed data to it and pass it 
as program argument.
+               tempFile, err := ioutil.TempFile("", "lxd_decompress_")
                if err != nil {
                        return nil, err
                }
-               defer tempfile.Close()
-               defer os.Remove(tempfile.Name())
+               defer tempFile.Close()
+               defer os.Remove(tempFile.Name())
 
-               // Write the compressed data
-               _, err = io.Copy(tempfile, data)
+               // Copy the compressed data stream to the temporart file.
+               _, err = io.Copy(tempFile, srcData)
                if err != nil {
                        return nil, err
                }
 
-               // Prepare to pass the temporary file as program argument
-               decomArgs := append(decomArgs, tempfile.Name())
+               // Pass the temporary file as program argument to the 
decompression command.
+               decomArgs := append(decomArgs, tempFile.Name())
 
-               // Create another temporary file to write the decompressed data
+               // Create another temporary file to write the decompressed data.
                tarData, err := ioutil.TempFile("", "lxd_decompress_")
                if err != nil {
                        return nil, err
@@ -379,37 +374,42 @@ func containerCreateFromBackup(s *state.State, info 
backup.Info, data io.ReadSee
                defer tarData.Close()
                defer os.Remove(tarData.Name())
 
-               // Decompress to tarData temporary file
-               err = shared.RunCommandWithFds(nil, tarData,
-                       decomArgs[0], decomArgs[1:]...)
+               // Decompress to tarData temporary file.
+               err = shared.RunCommandWithFds(nil, tarData, decomArgs[0], 
decomArgs[1:]...)
                if err != nil {
                        return nil, err
                }
                tarData.Seek(0, 0)
 
-               // Unpack tarball from decompressed temporary file
-               err = pool.ContainerBackupLoad(info, tarData, tarArgs)
-               if err != nil {
-                       return nil, err
-               }
+               // Set source data stream to newly created tar file handle.
+               srcData = tarData
+       }
 
-       } else {
-               // Unpack tarball
-               err = pool.ContainerBackupLoad(info, data, tarArgs)
-               if err != nil {
-                       return nil, err
-               }
+       pool, err := storagePoolInit(s, poolName)
+       if err != nil {
+               return nil, err
+       }
+
+       // Unpack tarball from the source tar stream.
+       err = pool.ContainerBackupLoad(info, srcData, tarArgs)
+       if err != nil {
+               return nil, err
        }
 
        if fixBackupFile || customPool {
-               // Update the pool
+               // Update pool information in the backup.yaml file.
                err = backupFixStoragePool(s.Cluster, info, !customPool)
                if err != nil {
                        return nil, err
                }
        }
 
-       return pool, nil
+       // Create revert function to remove the files created so far.
+       revertFunc := func() {
+               pool.ContainerDelete(&containerLXC{name: info.Name, project: 
info.Project})
+       }
+
+       return revertFunc, nil
 }
 
 func containerCreateEmptySnapshot(s *state.State, args db.InstanceArgs) 
(instance.Instance, error) {

From 1be89b525fc12b9fd4199f80cd6c4eb4f3ea1015 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 26 Nov 2019 16:55:42 +0000
Subject: [PATCH 4/4] lxd/containers/post: Updates createFromBackup to not need
 storage returned from instanceCreateFromBackup

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/containers_post.go | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 8de9aabbe1..eae96c5597 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -625,7 +625,7 @@ func createFromCopy(d *Daemon, project string, req 
*api.InstancesPost) response.
 }
 
 func createFromBackup(d *Daemon, project string, data io.Reader, pool string) 
response.Response {
-       // Write the data to a temp file
+       // Write the data to a temp file.
        f, err := ioutil.TempFile("", "lxd_backup_")
        if err != nil {
                return response.InternalError(err)
@@ -638,7 +638,7 @@ func createFromBackup(d *Daemon, project string, data 
io.Reader, pool string) re
                return response.InternalError(err)
        }
 
-       // Parse the backup information
+       // Parse the backup information.
        f.Seek(0, 0)
        bInfo, err := backup.GetInfo(f)
        if err != nil {
@@ -647,7 +647,7 @@ func createFromBackup(d *Daemon, project string, data 
io.Reader, pool string) re
        }
        bInfo.Project = project
 
-       // Override pool
+       // Override pool.
        if pool != "" {
                bInfo.Pool = pool
        }
@@ -655,19 +655,27 @@ func createFromBackup(d *Daemon, project string, data 
io.Reader, pool string) re
        run := func(op *operations.Operation) error {
                defer f.Close()
 
-               // Dump tarball to storage
+               // Dump tarball to storage.
                f.Seek(0, 0)
-               cPool, err := containerCreateFromBackup(d.State(), *bInfo, f, 
pool != "")
+               revertFunc, err := instanceCreateFromBackup(d.State(), *bInfo, 
f, pool != "")
                if err != nil {
-                       return errors.Wrap(err, "Create container from backup")
+                       return errors.Wrap(err, "Create instance from backup")
                }
 
+               revert := true
+               defer func() {
+                       if !revert {
+                               return
+                       }
+
+                       revertFunc()
+               }()
+
                body, err := json.Marshal(&internalImportPost{
                        Name:  bInfo.Name,
                        Force: true,
                })
                if err != nil {
-                       cPool.ContainerDelete(&containerLXC{name: bInfo.Name, 
project: project})
                        return errors.Wrap(err, "Marshal internal import 
request")
                }
 
@@ -680,13 +688,12 @@ func createFromBackup(d *Daemon, project string, data 
io.Reader, pool string) re
                resp := internalImport(d, req)
 
                if resp.String() != "success" {
-                       cPool.ContainerDelete(&containerLXC{name: bInfo.Name, 
project: project})
                        return fmt.Errorf("Internal import request: %v", 
resp.String())
                }
 
                c, err := instanceLoadByProjectAndName(d.State(), project, 
bInfo.Name)
                if err != nil {
-                       return errors.Wrap(err, "Load container")
+                       return errors.Wrap(err, "Load instance")
                }
 
                _, err = c.StorageStop()
@@ -694,11 +701,13 @@ func createFromBackup(d *Daemon, project string, data 
io.Reader, pool string) re
                        return errors.Wrap(err, "Stop storage pool")
                }
 
+               revert = false
                return nil
        }
 
        resources := map[string][]string{}
-       resources["containers"] = []string{bInfo.Name}
+       resources["instances"] = []string{bInfo.Name}
+       resources["containers"] = resources["instances"]
 
        op, err := operations.OperationCreate(d.State(), project, 
operations.OperationClassTask, db.OperationBackupRestore,
                resources, nil, run, nil, nil)
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to