[lxc-devel] [lxd/master] Storage Pool Load By Instance

tomponline on Github lxc-bot at linuxcontainers.org
Tue Nov 5 09:20:03 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 935 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191105/cebfd625/attachment.bin>
-------------- next part --------------
From 70b0bc0ff0c1a6a978c5dc04b5de5d353fe00032 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 08:54:10 +0000
Subject: [PATCH 1/4] lxd/storage/drivers/driver/dir: Comment grammar
 consistency

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/driver_dir.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lxd/storage/drivers/driver_dir.go b/lxd/storage/drivers/driver_dir.go
index 0062e75798..d95bc00f1a 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -304,13 +304,13 @@ func (d *dir) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser, vol
 				return err
 			}
 
-			// Create the snapshot itself
+			// Create the snapshot itself.
 			err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
 			if err != nil {
 				return err
 			}
 
-			// Setup the revert
+			// Setup the revert.
 			snapPath := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
 			revertPaths = append(revertPaths, snapPath)
 		}
@@ -389,20 +389,20 @@ func (d *dir) CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool
 			for _, srcSnapshot := range srcSnapshots {
 				_, snapName, _ := shared.ContainerGetParentAndSnapshotName(srcSnapshot.name)
 
-				// Mount the source snapshot
+				// Mount the source snapshot.
 				err = srcSnapshot.MountTask(func(srcMountPath string, op *operations.Operation) error {
-					// Copy the snapshot
+					// Copy the snapshot.
 					_, err = rsync.LocalCopy(srcMountPath, mountPath, bwlimit, true)
 					return err
 				}, op)
 
-				// Create the snapshot itself
+				// Create the snapshot itself.
 				err = d.CreateVolumeSnapshot(vol.volType, vol.name, snapName, op)
 				if err != nil {
 					return err
 				}
 
-				// Setup the revert
+				// Setup the revert.
 				snapPath := GetVolumeMountPath(d.name, vol.volType, GetSnapshotVolumeName(vol.name, snapName))
 				revertPaths = append(revertPaths, snapPath)
 			}

From 0bbcae61d5e0722e15078a199e3cf7360db29f3a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 09:08:58 +0000
Subject: [PATCH 2/4] lxd/storage/load: Renames GetPoolByInstanceName to
 GetPoolByInstance

- Adds additional check to GetPoolByInstance that checks that the Instance is of a type that is compatible with the underlying storage pool driver.
- If not it returns storageDrivers.ErrNotImplemented (for consistency with storageDrivers.ErrUnknownDriver where the pool's driver itself is not implemented).
- This allows the caller to decide to fallback to the old storage layer if either the pool's driver is not known or the driver doesn't implement support for the instance's type.
- This allows for operations to cleanly fail when we introduce VM support rather than have unexpected failures.
- It also allows for new storage drivers to be partially implemented, such that they only implement custom volume functionality, allowing quicker iteration and testing of new drivers.

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

diff --git a/lxd/storage/load.go b/lxd/storage/load.go
index d2f1e24072..5249f9a32e 100644
--- a/lxd/storage/load.go
+++ b/lxd/storage/load.go
@@ -60,6 +60,7 @@ func volIDFuncMake(state *state.State, poolID int64) func(volType drivers.Volume
 }
 
 // CreatePool creates a new storage pool on disk and returns a Pool interface.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is returned.
 func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePool, op *operations.Operation) (Pool, error) {
 	// Sanity checks.
 	if dbPool == nil {
@@ -106,6 +107,7 @@ func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePool, op *o
 }
 
 // GetPoolByName retrieves the pool from the database by its name and returns a Pool interface.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is returned.
 func GetPoolByName(state *state.State, name string) (Pool, error) {
 	// Handle mock requests.
 	if MockBackend {
@@ -146,12 +148,32 @@ func GetPoolByName(state *state.State, name string) (Pool, error) {
 	return &pool, nil
 }
 
-// GetPoolByInstanceName retrieves the pool from the database using the instance's project and name.
-func GetPoolByInstanceName(s *state.State, projectName, instanceName string) (Pool, error) {
-	poolName, err := s.Cluster.ContainerPool(projectName, instanceName)
+// GetPoolByInstance retrieves the pool from the database using the instance's pool.
+// If the pool's driver is not recognised then drivers.ErrUnknownDriver is returned. If the pool's
+// driver does not support the instance's type then drivers.ErrNotImplemented is returned.
+func GetPoolByInstance(s *state.State, inst Instance) (Pool, error) {
+	poolName, err := s.Cluster.ContainerPool(inst.Project(), inst.Name())
 	if err != nil {
 		return nil, err
 	}
 
-	return GetPoolByName(s, poolName)
+	pool, err := GetPoolByName(s, poolName)
+	if err != nil {
+		return nil, err
+	}
+
+	volType, err := InstanceTypeToVolumeType(inst.Type())
+	if err != nil {
+		return nil, err
+	}
+
+	for _, supportedType := range pool.Driver().Info().VolumeTypes {
+		if supportedType == volType {
+			return pool, nil
+		}
+	}
+
+	// Return drivers not implemented error for consistency with predefined errors returned by
+	// GetPoolByName (which can return drivers.ErrUnknownDriver).
+	return nil, drivers.ErrNotImplemented
 }

From 90639b9c3c875e0b26246a2dd71ab10e09041f3f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 09:12:34 +0000
Subject: [PATCH 3/4] lxd/container: Updates use of
 storagePools.GetPoolByInstance and fallback for container types

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

diff --git a/lxd/container.go b/lxd/container.go
index 8f52cc1a8f..b841fc7e17 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -406,7 +406,7 @@ func containerCreateEmptySnapshot(s *state.State, args db.InstanceArgs) (contain
 func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *operations.Operation) (container, error) {
 	s := d.State()
 
-	// Get the image properties
+	// Get the image properties.
 	_, img, err := s.Cluster.ImageGet(args.Project, hash, false, false)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Fetch image %s from database", hash)
@@ -428,8 +428,7 @@ func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *
 		return nil, errors.Wrapf(err, "Locate image %s in the cluster", hash)
 	}
 	if nodeAddress != "" {
-		// The image is available from another node, let's try to
-		// import it.
+		// The image is available from another node, let's try to import it.
 		logger.Debugf("Transferring image %s from node %s", hash, nodeAddress)
 		client, err := cluster.Connect(nodeAddress, d.endpoints.NetworkCert(), false)
 		if err != nil {
@@ -449,14 +448,14 @@ func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *
 		}
 	}
 
-	// Set the "image.*" keys
+	// Set the "image.*" keys.
 	if img.Properties != nil {
 		for k, v := range img.Properties {
 			args.Config[fmt.Sprintf("image.%s", k)] = v
 		}
 	}
 
-	// Set the BaseImage field (regardless of previous value)
+	// Set the BaseImage field (regardless of previous value).
 	args.BaseImage = hash
 
 	// Create the container
@@ -472,8 +471,8 @@ func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *
 	}
 
 	// Check if we can load new storage layer for pool driver type.
-	pool, err := storagePools.GetPoolByInstanceName(d.State(), c.Project(), c.Name())
-	if err != storageDrivers.ErrUnknownDriver {
+	pool, err := storagePools.GetPoolByInstance(d.State(), c)
+	if err != storageDrivers.ErrUnknownDriver && err != storageDrivers.ErrNotImplemented {
 		if err != nil {
 			return nil, errors.Wrap(err, "Load instance storage pool")
 		}
@@ -482,7 +481,7 @@ func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *
 		if err != nil {
 			return nil, errors.Wrap(err, "Create instance from image")
 		}
-	} else {
+	} else if c.Type() == instancetype.Container {
 		metadata := make(map[string]interface{})
 		var tracker *ioprogress.ProgressTracker
 		if op != nil {
@@ -493,15 +492,17 @@ func containerCreateFromImage(d *Daemon, args db.InstanceArgs, hash string, op *
 				}}
 		}
 
-		// Now create the storage from an image
+		// Now create the storage from an image.
 		err = c.Storage().ContainerCreateFromImage(c, hash, tracker)
 		if err != nil {
 			c.Delete()
 			return nil, errors.Wrap(err, "Create container from image")
 		}
+	} else {
+		return nil, fmt.Errorf("Instance type not supported")
 	}
 
-	// Apply any post-storage configuration
+	// Apply any post-storage configuration.
 	err = containerConfigureInternal(c)
 	if err != nil {
 		c.Delete()

From 435d598bc9206be776fef4601b047d25a9c25258 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 5 Nov 2019 09:12:51 +0000
Subject: [PATCH 4/4] lxd/storage/drivers/errors: Removes unused error

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/storage/drivers/errors.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lxd/storage/drivers/errors.go b/lxd/storage/drivers/errors.go
index 7cca047d15..56d1bdab3c 100644
--- a/lxd/storage/drivers/errors.go
+++ b/lxd/storage/drivers/errors.go
@@ -4,9 +4,6 @@ import (
 	"fmt"
 )
 
-// ErrNilValue is the "Nil value provided" error
-var ErrNilValue = fmt.Errorf("Nil value provided")
-
 // ErrNotImplemented is the "Not implemented" error
 var ErrNotImplemented = fmt.Errorf("Not implemented")
 


More information about the lxc-devel mailing list