[lxc-devel] [lxd/master] Storage createFromCopy instance rename

tomponline on Github lxc-bot at linuxcontainers.org
Tue Nov 19 17:25:22 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 413 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191119/f712af49/attachment-0001.bin>
-------------- next part --------------
From 74baafdb9d7cd6bdf5fc9f45889af2ad7eed5c84 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 10:46:59 +0000
Subject: [PATCH 1/5] lxd/container: container to instance renames, comment
 improvements

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 65 +++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index fde7831bac..626245a8ac 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -320,7 +320,7 @@ func instanceCreateAsEmpty(d *Daemon, args db.InstanceArgs) (Instance, error) {
 	}
 
 	// Apply any post-storage configuration.
-	err = containerConfigureInternal(d.State(), inst)
+	err = instanceConfigureInternal(d.State(), inst)
 	if err != nil {
 		return nil, err
 	}
@@ -561,7 +561,7 @@ func instanceCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *o
 	}
 
 	// Apply any post-storage configuration.
-	err = containerConfigureInternal(d.State(), inst)
+	err = instanceConfigureInternal(d.State(), inst)
 	if err != nil {
 		return nil, errors.Wrap(err, "Configure instance")
 	}
@@ -575,7 +575,7 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	var err error
 
 	if refresh {
-		// Load the target container
+		// Load the target instance.
 		ct, err = instanceLoadByProjectAndName(s, args.Project, args.Name)
 		if err != nil {
 			refresh = false
@@ -583,7 +583,7 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	}
 
 	if !refresh {
-		// Create the container.
+		// Create the instance.
 		ct, err = instanceCreateInternal(s, args)
 		if err != nil {
 			return nil, err
@@ -591,12 +591,11 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	}
 
 	if refresh && ct.IsRunning() {
-		return nil, fmt.Errorf("Cannot refresh a running container")
+		return nil, fmt.Errorf("Cannot refresh a running instance")
 	}
 
-	// At this point we have already figured out the parent
-	// container's root disk device so we can simply
-	// retrieve it from the expanded devices.
+	// At this point we have already figured out the parent container's root disk device so we
+	// can simply retrieve it from the expanded devices.
 	parentStoragePool := ""
 	parentExpandedDevices := ct.ExpandedDevices()
 	parentLocalRootDiskDeviceKey, parentLocalRootDiskDevice, _ := shared.GetRootDiskDevice(parentExpandedDevices.CloneNative())
@@ -604,18 +603,18 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 		parentStoragePool = parentLocalRootDiskDevice["pool"]
 	}
 
-	csList := []*Instance{}
+	snapList := []*Instance{}
 	var snapshots []Instance
 
 	if !containerOnly {
 		if refresh {
-			// Compare snapshots
-			syncSnapshots, deleteSnapshots, err := containerCompareSnapshots(sourceContainer, ct)
+			// Compare snapshots.
+			syncSnapshots, deleteSnapshots, err := instanceCompareSnapshots(sourceContainer, ct)
 			if err != nil {
 				return nil, err
 			}
 
-			// Delete extra snapshots
+			// Delete extra snapshots first.
 			for _, snap := range deleteSnapshots {
 				err := snap.Delete()
 				if err != nil {
@@ -623,10 +622,10 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 				}
 			}
 
-			// Only care about the snapshots that need updating
+			// Only care about the snapshots that need updating.
 			snapshots = syncSnapshots
 		} else {
-			// Get snapshots of source container
+			// Get snapshots of source instance.
 			snapshots, err = sourceContainer.Snapshots()
 			if err != nil {
 				ct.Delete()
@@ -638,7 +637,7 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 		for _, snap := range snapshots {
 			fields := strings.SplitN(snap.Name(), shared.SnapshotDelimiter, 2)
 
-			// Ensure that snapshot and parent container have the
+			// Ensure that snapshot and parent instance have the
 			// same storage pool in their local root disk device.
 			// If the root disk device for the snapshot comes from a
 			// profile on the new instance as well we don't need to
@@ -681,7 +680,7 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 				return nil, err
 			}
 
-			// Restore snapshot creation date
+			// Restore snapshot creation date.
 			err = s.Cluster.ContainerCreationUpdate(cs.ID(), snap.CreationDate())
 			if err != nil {
 				if !refresh {
@@ -691,11 +690,11 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 				return nil, err
 			}
 
-			csList = append(csList, &cs)
+			snapList = append(snapList, &cs)
 		}
 	}
 
-	// Now clone or refresh the storage
+	// Now clone or refresh the storage.
 	if refresh {
 		err = ct.Storage().ContainerRefresh(ct, sourceContainer, snapshots)
 		if err != nil {
@@ -713,7 +712,7 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	}
 
 	// Apply any post-storage configuration.
-	err = containerConfigureInternal(s, ct)
+	err = instanceConfigureInternal(s, ct)
 	if err != nil {
 		if !refresh {
 			ct.Delete()
@@ -723,9 +722,9 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	}
 
 	if !containerOnly {
-		for _, cs := range csList {
+		for _, snap := range snapList {
 			// Apply any post-storage configuration.
-			err = containerConfigureInternal(s, *cs)
+			err = instanceConfigureInternal(s, *snap)
 			if err != nil {
 				if !refresh {
 					ct.Delete()
@@ -1051,7 +1050,8 @@ func instanceCreateInternal(s *state.State, args db.InstanceArgs) (Instance, err
 	return inst, nil
 }
 
-func containerConfigureInternal(state *state.State, c Instance) error {
+// instanceConfigureInternal applies quota set in volatile "apply_quota" and writes a backup file.
+func instanceConfigureInternal(state *state.State, c Instance) error {
 	// Find the root device
 	rootDiskDeviceKey, rootDiskDevice, err := shared.GetRootDiskDevice(c.ExpandedDevices().CloneNative())
 	if err != nil {
@@ -1339,44 +1339,57 @@ func instanceLoad(s *state.State, args db.InstanceArgs, profiles []api.Profile)
 	return inst, nil
 }
 
-func containerCompareSnapshots(source Instance, target Instance) ([]Instance, []Instance, error) {
-	// Get the source snapshots
+// instanceCompareSnapshots returns a list of snapshots to sync to the target and a list of
+// snapshots to remove from the target. A snapshot will be marked as "to sync" if it either doesn't
+// exist in the target or its creation date is different to the source. A snapshot will be marked
+// as "to delete" if it doesn't exist in the source or creation date is different to the source.
+func instanceCompareSnapshots(source Instance, target Instance) ([]Instance, []Instance, error) {
+	// Get the source snapshots.
 	sourceSnapshots, err := source.Snapshots()
 	if err != nil {
 		return nil, nil, err
 	}
 
-	// Get the target snapshots
+	// Get the target snapshots.
 	targetSnapshots, err := target.Snapshots()
 	if err != nil {
 		return nil, nil, err
 	}
 
-	// Compare source and target
+	// Compare source and target.
 	sourceSnapshotsTime := map[string]time.Time{}
 	targetSnapshotsTime := map[string]time.Time{}
 
 	toDelete := []Instance{}
 	toSync := []Instance{}
 
+	// Generate a list of source snapshot creation dates.
 	for _, snap := range sourceSnapshots {
 		_, snapName, _ := shared.ContainerGetParentAndSnapshotName(snap.Name())
 
 		sourceSnapshotsTime[snapName] = snap.CreationDate()
 	}
 
+	// Generate a list of target snapshot creation times, if the source doesn't contain the
+	// the snapshot or the creation time is different on the source then add the target snapshot
+	// to the "to delete" list.
 	for _, snap := range targetSnapshots {
 		_, snapName, _ := shared.ContainerGetParentAndSnapshotName(snap.Name())
 
 		targetSnapshotsTime[snapName] = snap.CreationDate()
 		existDate, exists := sourceSnapshotsTime[snapName]
 		if !exists {
+			// Snapshot doesn't exist in source, mark it for deletion on target.
 			toDelete = append(toDelete, snap)
 		} else if existDate != snap.CreationDate() {
+			// Snapshot creation date is different in source, mark it for deletion on
+			// target.
 			toDelete = append(toDelete, snap)
 		}
 	}
 
+	// For each of the source snapshots, decide whether it needs to be synced or not based on
+	// whether it already exists in the target and whether the creation dates match.
 	for _, snap := range sourceSnapshots {
 		_, snapName, _ := shared.ContainerGetParentAndSnapshotName(snap.Name())
 

From 76a5ef29c3b46f441e56595eb29b418f1d0da528 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 10:47:21 +0000
Subject: [PATCH 2/5] lxd/containers/post: Adds instances field to response
 from createFromCopy

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/containers_post.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 976be3a618..9abf1a4727 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -612,7 +612,8 @@ func createFromCopy(d *Daemon, project string, req *api.InstancesPost) response.
 	}
 
 	resources := map[string][]string{}
-	resources["containers"] = []string{req.Name, req.Source.Source}
+	resources["instances"] = []string{req.Name, req.Source.Source}
+	resources["containers"] = resources["instances"] // Populate old field name.
 
 	op, err := operations.OperationCreate(d.State(), targetProject, operations.OperationClassTask, db.OperationContainerCreate, resources, nil, run, nil, nil)
 	if err != nil {

From e5b8f8fde8a05644d7e099255d18e49f2668b2be Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 12:04:37 +0000
Subject: [PATCH 3/5] lxd/container: instanceCreateAsCopy rename and revertion
 logic

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 81 +++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 46 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 626245a8ac..37ca7929e4 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -570,34 +570,44 @@ func instanceCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *o
 	return inst, nil
 }
 
-func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer Instance, containerOnly bool, refresh bool) (Instance, error) {
-	var ct Instance
+func instanceCreateAsCopy(s *state.State, args db.InstanceArgs, sourceInst Instance, instanceOnly bool, refresh bool) (Instance, error) {
+	var inst, revertInst Instance
 	var err error
 
+	defer func() {
+		if revertInst == nil {
+			return
+		}
+
+		revertInst.Delete()
+	}()
+
 	if refresh {
 		// Load the target instance.
-		ct, err = instanceLoadByProjectAndName(s, args.Project, args.Name)
+		inst, err = instanceLoadByProjectAndName(s, args.Project, args.Name)
 		if err != nil {
-			refresh = false
+			refresh = false // Instance doesn't exist, so switch to copy mode.
+		}
+
+		if inst.IsRunning() {
+			return nil, fmt.Errorf("Cannot refresh a running instance")
 		}
 	}
 
+	// If we are not in refresh mode, then create a new instance as we are in copy mode.
 	if !refresh {
 		// Create the instance.
-		ct, err = instanceCreateInternal(s, args)
+		inst, err = instanceCreateInternal(s, args)
 		if err != nil {
 			return nil, err
 		}
-	}
-
-	if refresh && ct.IsRunning() {
-		return nil, fmt.Errorf("Cannot refresh a running instance")
+		revertInst = inst
 	}
 
 	// At this point we have already figured out the parent container's root disk device so we
 	// can simply retrieve it from the expanded devices.
 	parentStoragePool := ""
-	parentExpandedDevices := ct.ExpandedDevices()
+	parentExpandedDevices := inst.ExpandedDevices()
 	parentLocalRootDiskDeviceKey, parentLocalRootDiskDevice, _ := shared.GetRootDiskDevice(parentExpandedDevices.CloneNative())
 	if parentLocalRootDiskDeviceKey != "" {
 		parentStoragePool = parentLocalRootDiskDevice["pool"]
@@ -606,10 +616,10 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 	snapList := []*Instance{}
 	var snapshots []Instance
 
-	if !containerOnly {
+	if !instanceOnly {
 		if refresh {
 			// Compare snapshots.
-			syncSnapshots, deleteSnapshots, err := instanceCompareSnapshots(sourceContainer, ct)
+			syncSnapshots, deleteSnapshots, err := instanceCompareSnapshots(sourceInst, inst)
 			if err != nil {
 				return nil, err
 			}
@@ -626,10 +636,8 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 			snapshots = syncSnapshots
 		} else {
 			// Get snapshots of source instance.
-			snapshots, err = sourceContainer.Snapshots()
+			snapshots, err = sourceInst.Snapshots()
 			if err != nil {
-				ct.Delete()
-
 				return nil, err
 			}
 		}
@@ -656,11 +664,11 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 				}
 			}
 
-			newSnapName := fmt.Sprintf("%s/%s", ct.Name(), fields[1])
-			csArgs := db.InstanceArgs{
+			newSnapName := fmt.Sprintf("%s/%s", inst.Name(), fields[1])
+			snapInstArgs := db.InstanceArgs{
 				Architecture: snap.Architecture(),
 				Config:       snap.LocalConfig(),
-				Type:         sourceContainer.Type(),
+				Type:         sourceInst.Type(),
 				Snapshot:     true,
 				Devices:      snapDevices,
 				Description:  snap.Description(),
@@ -671,71 +679,52 @@ func containerCreateAsCopy(s *state.State, args db.InstanceArgs, sourceContainer
 			}
 
 			// Create the snapshots.
-			cs, err := instanceCreateInternal(s, csArgs)
+			snapInst, err := instanceCreateInternal(s, snapInstArgs)
 			if err != nil {
-				if !refresh {
-					ct.Delete()
-				}
-
 				return nil, err
 			}
 
 			// Restore snapshot creation date.
-			err = s.Cluster.ContainerCreationUpdate(cs.ID(), snap.CreationDate())
+			err = s.Cluster.ContainerCreationUpdate(snapInst.ID(), snap.CreationDate())
 			if err != nil {
-				if !refresh {
-					ct.Delete()
-				}
-
 				return nil, err
 			}
 
-			snapList = append(snapList, &cs)
+			snapList = append(snapList, &snapInst)
 		}
 	}
 
 	// Now clone or refresh the storage.
 	if refresh {
-		err = ct.Storage().ContainerRefresh(ct, sourceContainer, snapshots)
+		err = inst.Storage().ContainerRefresh(inst, sourceInst, snapshots)
 		if err != nil {
 			return nil, err
 		}
 	} else {
-		err = ct.Storage().ContainerCopy(ct, sourceContainer, containerOnly)
+		err = inst.Storage().ContainerCopy(inst, sourceInst, instanceOnly)
 		if err != nil {
-			if !refresh {
-				ct.Delete()
-			}
-
 			return nil, err
 		}
 	}
 
 	// Apply any post-storage configuration.
-	err = instanceConfigureInternal(s, ct)
+	err = instanceConfigureInternal(s, inst)
 	if err != nil {
-		if !refresh {
-			ct.Delete()
-		}
-
 		return nil, err
 	}
 
-	if !containerOnly {
+	if !instanceOnly {
 		for _, snap := range snapList {
 			// Apply any post-storage configuration.
 			err = instanceConfigureInternal(s, *snap)
 			if err != nil {
-				if !refresh {
-					ct.Delete()
-				}
-
 				return nil, err
 			}
 		}
 	}
 
-	return ct, nil
+	revertInst = nil
+	return inst, nil
 }
 
 func containerCreateAsSnapshot(s *state.State, args db.InstanceArgs, sourceInstance Instance) (Instance, error) {

From 5eb355c0ace37a8a502061876f8175d680b943e6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 12:05:23 +0000
Subject: [PATCH 4/5] lxd/container: instanceCreateInternal comment

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

diff --git a/lxd/container.go b/lxd/container.go
index 37ca7929e4..55aaed404c 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -820,6 +820,7 @@ func containerCreateAsSnapshot(s *state.State, args db.InstanceArgs, sourceInsta
 	return c, nil
 }
 
+// instanceCreateInternal creates an instance record and storage volume record in the database.
 func instanceCreateInternal(s *state.State, args db.InstanceArgs) (Instance, error) {
 	// Set default values.
 	if args.Project == "" {

From 97cda101269d6892bda8fd3f2615a69c4c4494b0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 19 Nov 2019 12:05:39 +0000
Subject: [PATCH 5/5] lxd/containers/post: instanceCreateAsCopy usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/containers_post.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 9abf1a4727..dacc135db2 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -604,7 +604,7 @@ func createFromCopy(d *Daemon, project string, req *api.InstancesPost) response.
 
 	run := func(op *operations.Operation) error {
 		instanceOnly := req.Source.InstanceOnly || req.Source.ContainerOnly
-		_, err := containerCreateAsCopy(d.State(), args, source, instanceOnly, req.Source.Refresh)
+		_, err := instanceCreateAsCopy(d.State(), args, source, instanceOnly, req.Source.Refresh)
 		if err != nil {
 			return err
 		}


More information about the lxc-devel mailing list