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