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

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) ===
Initial clean up of createFromMigration.
From c9e8c6667dd9a6e9f703dc1fcaf04489d14c965c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 28 Nov 2019 12:20:36 +0000
Subject: [PATCH 1/2] lxd/storage/utils: Adds InstanceContentType

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 25e8e273fd..db4eb0f1ff 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -10,6 +10,7 @@ import (
        "golang.org/x/sys/unix"
 
        "github.com/lxc/lxd/lxd/db"
+       "github.com/lxc/lxd/lxd/instance"
        "github.com/lxc/lxd/lxd/instance/instancetype"
        "github.com/lxc/lxd/lxd/state"
        "github.com/lxc/lxd/lxd/storage/drivers"
@@ -756,3 +757,13 @@ func ImageUnpack(imageFile, destPath, destBlockFile 
string, blockBackend, runnin
 
        return nil
 }
+
+// InstanceContentType returns the instance's content type.
+func InstanceContentType(inst instance.Instance) drivers.ContentType {
+       contentType := drivers.ContentTypeFS
+       if inst.Type() == instancetype.VM {
+               contentType = drivers.ContentTypeBlock
+       }
+
+       return contentType
+}

From 41914ea10b9e52957cf5b1309fc2bc3302d50c2e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 28 Nov 2019 16:11:09 +0000
Subject: [PATCH 2/2] lxd/container/post: Cleans up createFromMigration

- Links migration type check to new storage pkg.
- Changes references to containers to instance.
- Adds defer style clean up.

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

diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 774e12891b..3a74b04de2 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -26,6 +26,8 @@ import (
        "github.com/lxc/lxd/lxd/migration"
        "github.com/lxc/lxd/lxd/operations"
        "github.com/lxc/lxd/lxd/response"
+       storagePools "github.com/lxc/lxd/lxd/storage"
+       storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
        "github.com/lxc/lxd/shared"
        "github.com/lxc/lxd/shared/api"
        log "github.com/lxc/lxd/shared/log15"
@@ -203,20 +205,18 @@ func createFromNone(d *Daemon, project string, req 
*api.InstancesPost) response.
 }
 
 func createFromMigration(d *Daemon, project string, req *api.InstancesPost) 
response.Response {
-       // Validate migration mode
+       // Validate migration mode.
        if req.Source.Mode != "pull" && req.Source.Mode != "push" {
                return response.NotImplemented(fmt.Errorf("Mode '%s' not 
implemented", req.Source.Mode))
        }
 
-       var c instance.Instance
-
        // Parse the architecture name
        architecture, err := osarch.ArchitectureId(req.Architecture)
        if err != nil {
                return response.BadRequest(err)
        }
 
-       // Pre-fill default profile
+       // Pre-fill default profile.
        if req.Profiles == nil {
                req.Profiles = []string{"default"}
        }
@@ -226,7 +226,11 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                return response.BadRequest(err)
        }
 
-       // Prepare the container creation request
+       if dbType != instancetype.Container {
+               return response.BadRequest(fmt.Errorf("Instance type not 
container"))
+       }
+
+       // Prepare the instance creation request.
        args := db.InstanceArgs{
                Project:      project,
                Architecture: architecture,
@@ -241,7 +245,7 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                Stateful:     req.Stateful,
        }
 
-       // Early profile validation
+       // Early profile validation.
        profiles, err := d.cluster.Profiles(project)
        if err != nil {
                return response.InternalError(err)
@@ -259,12 +263,11 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
        }
 
        if storagePool == "" {
-               return response.BadRequest(fmt.Errorf("Can't find a storage 
pool for the container to use"))
+               return response.BadRequest(fmt.Errorf("Can't find a storage 
pool for the instance to use"))
        }
 
        if localRootDiskDeviceKey == "" && storagePoolProfile == "" {
-               // Give the container it's own local root disk device with a
-               // pool property.
+               // Give the container it's own local root disk device with a 
pool property.
                rootDev := map[string]string{}
                rootDev["type"] = "disk"
                rootDev["path"] = "/"
@@ -273,8 +276,8 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                        args.Devices = deviceConfig.Devices{}
                }
 
-               // Make sure that we do not overwrite a device the user
-               // is currently using under the name "root".
+               // Make sure that we do not overwrite a device the user is 
currently using under the
+               // name "root".
                rootDevName := "root"
                for i := 0; i < 100; i++ {
                        if args.Devices[rootDevName] == nil {
@@ -289,22 +292,25 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                args.Devices[localRootDiskDeviceKey]["pool"] = storagePool
        }
 
-       // Early check for refresh
+       var inst instance.Instance
+
+       // Early check for refresh.
        if req.Source.Refresh {
-               // Check if the container exists
-               inst, err := instanceLoadByProjectAndName(d.State(), project, 
req.Name)
+               // Check if the instance exists.
+               inst, err = instanceLoadByProjectAndName(d.State(), project, 
req.Name)
                if err != nil {
                        req.Source.Refresh = false
                } else if inst.IsRunning() {
                        return response.BadRequest(fmt.Errorf("Cannot refresh a 
running container"))
                }
+       }
 
-               if inst.Type() != instancetype.Container {
-                       return response.BadRequest(fmt.Errorf("Instance type 
not container"))
+       revert := true
+       defer func() {
+               if revert && !req.Source.Refresh && inst != nil {
+                       inst.Delete()
                }
-
-               c = inst
-       }
+       }()
 
        if !req.Source.Refresh {
                /* Only create a container from an image if we're going to
@@ -322,18 +328,18 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                 */
                _, _, err = d.cluster.ImageGet(args.Project, 
req.Source.BaseImage, false, true)
                if err != nil {
-                       c, err = instanceCreateAsEmpty(d, args)
+                       inst, err = instanceCreateAsEmpty(d, args)
                        if err != nil {
                                return response.InternalError(err)
                        }
                } else {
-                       // Retrieve the future storage pool
-                       inst, err := instanceLoad(d.State(), args, nil)
+                       // Retrieve the future storage pool.
+                       tmpInst, err := instanceLoad(d.State(), args, nil)
                        if err != nil {
                                return response.InternalError(err)
                        }
 
-                       _, rootDiskDevice, err := 
shared.GetRootDiskDevice(inst.ExpandedDevices().CloneNative())
+                       _, rootDiskDevice, err := 
shared.GetRootDiskDevice(tmpInst.ExpandedDevices().CloneNative())
                        if err != nil {
                                return response.InternalError(err)
                        }
@@ -344,18 +350,36 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
 
                        storagePool = rootDiskDevice["pool"]
 
-                       ps, err := storagePoolInit(d.State(), storagePool)
-                       if err != nil {
-                               return response.InternalError(err)
+                       var migrationType migration.MigrationFSType
+
+                       // Check if we can load new storage layer for pool 
driver type.
+                       pool, err := storagePools.GetPoolByName(d.State(), 
storagePool)
+                       if err != storageDrivers.ErrUnknownDriver {
+                               if err != nil {
+                                       return response.InternalError(err)
+                               }
+
+                               // Get preferred migration type from storage 
backend.
+                               migrationTypes := 
pool.MigrationTypes(storagePools.InstanceContentType(tmpInst))
+                               if len(migrationTypes) > 0 {
+                                       migrationType = migrationTypes[0].FSType
+                               }
+                       } else {
+                               ps, err := storagePoolInit(d.State(), 
storagePool)
+                               if err != nil {
+                                       return response.InternalError(err)
+                               }
+
+                               migrationType = ps.MigrationType()
                        }
 
-                       if ps.MigrationType() == 
migration.MigrationFSType_RSYNC {
-                               c, err = instanceCreateFromImage(d, args, 
req.Source.BaseImage, nil)
+                       if migrationType == migration.MigrationFSType_RSYNC {
+                               inst, err = instanceCreateFromImage(d, args, 
req.Source.BaseImage, nil)
                                if err != nil {
                                        return response.InternalError(err)
                                }
                        } else {
-                               c, err = instanceCreateAsEmpty(d, args)
+                               inst, err = instanceCreateAsEmpty(d, args)
                                if err != nil {
                                        return response.InternalError(err)
                                }
@@ -367,26 +391,17 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
        if req.Source.Certificate != "" {
                certBlock, _ := pem.Decode([]byte(req.Source.Certificate))
                if certBlock == nil {
-                       if !req.Source.Refresh {
-                               c.Delete()
-                       }
                        return response.InternalError(fmt.Errorf("Invalid 
certificate"))
                }
 
                cert, err = x509.ParseCertificate(certBlock.Bytes)
                if err != nil {
-                       if !req.Source.Refresh {
-                               c.Delete()
-                       }
                        return response.InternalError(err)
                }
        }
 
        config, err := shared.GetTLSConfig("", "", "", cert)
        if err != nil {
-               if !req.Source.Refresh {
-                       c.Delete()
-               }
                return response.InternalError(err)
        }
 
@@ -401,7 +416,7 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                Dialer: websocket.Dialer{
                        TLSClientConfig: config,
                        NetDial:         shared.RFC3493Dialer},
-               Instance:     c,
+               Instance:     inst,
                Secrets:      req.Source.Websockets,
                Push:         push,
                Live:         req.Source.Live,
@@ -411,34 +426,35 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
 
        sink, err := NewMigrationSink(&migrationArgs)
        if err != nil {
-               c.Delete()
                return response.InternalError(err)
        }
 
        run := func(op *operations.Operation) error {
+               opRevert := true
+               defer func() {
+                       if opRevert && !req.Source.Refresh && inst != nil {
+                               inst.Delete()
+                       }
+               }()
+
                // And finally run the migration.
                err = sink.Do(op)
                if err != nil {
-                       logger.Error("Error during migration sink", 
log.Ctx{"err": err})
-                       if !req.Source.Refresh {
-                               c.Delete()
-                       }
                        return fmt.Errorf("Error transferring container data: 
%s", err)
                }
 
-               err = c.DeferTemplateApply("copy")
+               err = inst.DeferTemplateApply("copy")
                if err != nil {
-                       if !req.Source.Refresh {
-                               c.Delete()
-                       }
                        return err
                }
 
+               opRevert = false
                return nil
        }
 
        resources := map[string][]string{}
-       resources["containers"] = []string{req.Name}
+       resources["instances"] = []string{req.Name}
+       resources["containers"] = resources["instances"]
 
        var op *operations.Operation
        if push {
@@ -453,6 +469,7 @@ func createFromMigration(d *Daemon, project string, req 
*api.InstancesPost) resp
                }
        }
 
+       revert = false
        return operations.OperationResponse(op)
 }
 
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to