[lxc-devel] [lxd/master] Port storage pool creation to new logic
stgraber on Github
lxc-bot at linuxcontainers.org
Wed Nov 6 06:24:50 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191105/a45c3f4b/attachment.bin>
-------------- next part --------------
From b840d3f61edddfd219c07fe13253b5067ec8184b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 6 Nov 2019 01:21:26 -0500
Subject: [PATCH 1/3] lxd/storage/dir: Add check for bad source path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/storage/drivers/driver_dir.go | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lxd/storage/drivers/driver_dir.go b/lxd/storage/drivers/driver_dir.go
index a80c2ccb12..4e581b9054 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -6,6 +6,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
+ "strings"
"golang.org/x/sys/unix"
@@ -51,6 +52,12 @@ func (d *dir) Create() error {
return fmt.Errorf("Source path '%s' doesn't exist", d.config["source"])
}
+ // Check that if within LXD_DIR, we're at our expected spot.
+ cleanSource := filepath.Clean(d.config["source"])
+ if strings.HasPrefix(cleanSource, shared.VarPath()) && cleanSource != GetPoolMountPath(d.name) {
+ return fmt.Errorf("Source path '%s' is within the LXD directory")
+ }
+
// Check that the path is currently empty.
isEmpty, err := shared.PathIsEmpty(d.config["source"])
if err != nil {
From d334b7ad17f10f6e53ebe89317fb42b5c66c0832 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 6 Nov 2019 01:22:34 -0500
Subject: [PATCH 2/3] lxd/storage: Add localOnly handling of create/delete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/storage/backend_lxd.go | 24 ++++++++++++++++++------
lxd/storage/backend_mock.go | 2 +-
lxd/storage/interfaces.go | 2 +-
lxd/storage/load.go | 4 ++--
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 1e8b81b2a8..3fcc152513 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -57,7 +57,7 @@ func (b *lxdBackend) MigrationTypes(contentType drivers.ContentType) []migration
}
// create creates the storage pool layout on the storage device.
-func (b *lxdBackend) create(dbPool *api.StoragePool, op *operations.Operation) error {
+func (b *lxdBackend) create(dbPool *api.StoragePoolsPost, localOnly bool, op *operations.Operation) error {
logger := logging.AddContext(b.logger, log.Ctx{"args": dbPool})
logger.Debug("create started")
defer logger.Debug("created finished")
@@ -71,6 +71,11 @@ func (b *lxdBackend) create(dbPool *api.StoragePool, op *operations.Operation) e
return err
}
+ // If dealing with a remote storage pool, we're done now
+ if b.driver.Info().Remote && localOnly {
+ return nil
+ }
+
// Undo the storage path create if there is an error.
defer func() {
if !revertPath {
@@ -122,20 +127,27 @@ func (b *lxdBackend) GetResources() (*api.ResourcesStoragePool, error) {
}
// Delete removes the pool.
-func (b *lxdBackend) Delete(op *operations.Operation) error {
+func (b *lxdBackend) Delete(localOnly bool, op *operations.Operation) error {
logger := logging.AddContext(b.logger, nil)
logger.Debug("Delete started")
defer logger.Debug("Delete finished")
// Delete the low-level storage.
- err := b.driver.Delete(op)
- if err != nil {
- return err
+ if !localOnly || !b.driver.Info().Remote {
+ err := b.driver.Delete(op)
+ if err != nil {
+ return err
+ }
+ } else {
+ _, err := b.driver.Unmount()
+ if err != nil {
+ return err
+ }
}
// Delete the mountpoint.
path := shared.VarPath("storage-pools", b.name)
- err = os.Remove(path)
+ err := os.Remove(path)
if err != nil {
return err
}
diff --git a/lxd/storage/backend_mock.go b/lxd/storage/backend_mock.go
index a0ddb34707..7e8520d4cc 100644
--- a/lxd/storage/backend_mock.go
+++ b/lxd/storage/backend_mock.go
@@ -46,7 +46,7 @@ func (b *mockBackend) GetResources() (*api.ResourcesStoragePool, error) {
return nil, nil
}
-func (b *mockBackend) Delete(op *operations.Operation) error {
+func (b *mockBackend) Delete(localOnly bool, op *operations.Operation) error {
return nil
}
diff --git a/lxd/storage/interfaces.go b/lxd/storage/interfaces.go
index cc7d249a55..bd3d0d7cf5 100644
--- a/lxd/storage/interfaces.go
+++ b/lxd/storage/interfaces.go
@@ -33,7 +33,7 @@ type Pool interface {
Driver() drivers.Driver
GetResources() (*api.ResourcesStoragePool, error)
- Delete(op *operations.Operation) error
+ Delete(localOnly bool, op *operations.Operation) error
Mount() (bool, error)
Unmount() (bool, error)
diff --git a/lxd/storage/load.go b/lxd/storage/load.go
index 5249f9a32e..ee3df9df32 100644
--- a/lxd/storage/load.go
+++ b/lxd/storage/load.go
@@ -61,7 +61,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) {
+func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePoolsPost, localOnly bool, op *operations.Operation) (Pool, error) {
// Sanity checks.
if dbPool == nil {
return nil, ErrNilValue
@@ -98,7 +98,7 @@ func CreatePool(state *state.State, poolID int64, dbPool *api.StoragePool, op *o
pool.logger = logger
// Create the pool itself on the storage device..
- err = pool.create(dbPool, op)
+ err = pool.create(dbPool, localOnly, op)
if err != nil {
return nil, err
}
From afa14dbb147ef3b6522fee9fd0fd3706d230cf8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Wed, 6 Nov 2019 01:23:52 -0500
Subject: [PATCH 3/3] lxd/storage: Switch Create to new logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/api_internal.go | 2 +-
lxd/storage_pools.go | 26 +++++---
lxd/storage_pools_utils.go | 127 ++++++++++++++++++++++++-------------
3 files changed, 102 insertions(+), 53 deletions(-)
diff --git a/lxd/api_internal.go b/lxd/api_internal.go
index 46393aba2c..cdf6dbc1b9 100644
--- a/lxd/api_internal.go
+++ b/lxd/api_internal.go
@@ -506,7 +506,7 @@ func internalImport(d *Daemon, r *http.Request) response.Response {
if poolErr == db.ErrNoSuchObject {
// Create the storage pool db entry if it doesn't exist.
- err := storagePoolDBCreate(d.State(), containerPoolName, "",
+ _, err := storagePoolDBCreate(d.State(), containerPoolName, "",
backup.Pool.Driver, backup.Pool.Config)
if err != nil {
err = errors.Wrap(err, "Create storage pool database entry")
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index 6634c4c240..cd9e33b2e7 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -117,11 +117,17 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
if err != nil {
return response.BadRequest(err)
}
- err = doStoragePoolCreateInternal(
- d.State(), req.Name, req.Description, req.Driver, req.Config, true)
+
+ poolID, err := d.cluster.StoragePoolGetID(req.Name)
+ if err != nil {
+ return response.NotFound(err)
+ }
+
+ err = storagePoolCreateLocal(d.State(), poolID, req, true)
if err != nil {
return response.SmartError(err)
}
+
return resp
}
@@ -136,8 +142,7 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
// No targetNode was specified and we're either a single-node
// cluster or not clustered at all, so create the storage
// pool immediately.
- err = storagePoolCreateInternal(
- d.State(), req.Name, req.Description, req.Driver, req.Config)
+ err = storagePoolCreateGlobal(d.State(), req)
} else {
// No targetNode was specified and we're clustered, so finalize the
// config in the db and actually create the pool on all nodes.
@@ -146,8 +151,8 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
if err != nil {
return response.InternalError(err)
}
- return resp
+ return resp
}
// A targetNode was specified, let's just define the node's storage
@@ -171,6 +176,7 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
if err == db.ErrAlreadyDefined {
return response.BadRequest(fmt.Errorf("The storage pool already defined on node %s", targetNode))
}
+
return response.SmartError(err)
}
@@ -189,9 +195,12 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost) error {
// configs and insert the global config.
var configs map[string]map[string]string
var nodeName string
+ var poolID int64
err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
+ var err error
+
// Check that the pool was defined at all.
- poolID, err := tx.StoragePoolID(req.Name)
+ poolID, err = tx.StoragePoolID(req.Name)
if err != nil {
return err
}
@@ -223,12 +232,13 @@ func storagePoolsPostCluster(d *Daemon, req api.StoragePoolsPost) error {
for key, value := range configs[nodeName] {
nodeReq.Config[key] = value
}
+
err = storagePoolValidate(req.Name, req.Driver, req.Config)
if err != nil {
return err
}
- err = doStoragePoolCreateInternal(
- d.State(), req.Name, req.Description, req.Driver, req.Config, false)
+
+ err = storagePoolCreateLocal(d.State(), poolID, req, false)
if err != nil {
return err
}
diff --git a/lxd/storage_pools_utils.go b/lxd/storage_pools_utils.go
index 3da2e8d30e..3a846e98da 100644
--- a/lxd/storage_pools_utils.go
+++ b/lxd/storage_pools_utils.go
@@ -7,8 +7,10 @@ import (
"github.com/lxc/lxd/lxd/db"
"github.com/lxc/lxd/lxd/state"
- driver "github.com/lxc/lxd/lxd/storage"
+ 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"
"github.com/lxc/lxd/shared/version"
)
@@ -40,7 +42,7 @@ func storagePoolUpdate(state *state.State, name, newDescription string, newConfi
}
}()
- changedConfig, userOnly := driver.ConfigDiff(oldConfig, newConfig)
+ changedConfig, userOnly := storagePools.ConfigDiff(oldConfig, newConfig)
// Apply config changes if there are any
if len(changedConfig) != 0 {
newWritable.Description = newDescription
@@ -159,11 +161,11 @@ func profilesUsingPoolGetNames(db *db.Cluster, poolName string) ([]string, error
return usedBy, nil
}
-func storagePoolDBCreate(s *state.State, poolName, poolDescription string, driver string, config map[string]string) error {
+func storagePoolDBCreate(s *state.State, poolName, poolDescription string, driver string, config map[string]string) (int64, error) {
// Check that the storage pool does not already exist.
_, err := s.Cluster.StoragePoolGetID(poolName)
if err == nil {
- return fmt.Errorf("The storage pool already exists")
+ return -1, fmt.Errorf("The storage pool already exists")
}
// Make sure that we don't pass a nil to the next function.
@@ -172,27 +174,27 @@ func storagePoolDBCreate(s *state.State, poolName, poolDescription string, drive
}
err = storagePoolValidate(poolName, driver, config)
if err != nil {
- return err
+ return -1, err
}
// Fill in the defaults
err = storagePoolFillDefault(poolName, driver, config)
if err != nil {
- return err
+ return -1, err
}
// Create the database entry for the storage pool.
- _, err = dbStoragePoolCreateAndUpdateCache(s.Cluster, poolName, poolDescription, driver, config)
+ id, err := dbStoragePoolCreateAndUpdateCache(s.Cluster, poolName, poolDescription, driver, config)
if err != nil {
- return fmt.Errorf("Error inserting %s into database: %s", poolName, err)
+ return -1, fmt.Errorf("Error inserting %s into database: %s", poolName, err)
}
- return nil
+ return id, nil
}
func storagePoolValidate(poolName string, driverName string, config map[string]string) error {
// Check if the storage pool name is valid.
- err := driver.ValidName(poolName)
+ err := storagePools.ValidName(poolName)
if err != nil {
return err
}
@@ -206,11 +208,13 @@ func storagePoolValidate(poolName string, driverName string, config map[string]s
return nil
}
-func storagePoolCreateInternal(state *state.State, poolName, poolDescription string, driver string, config map[string]string) error {
- err := storagePoolDBCreate(state, poolName, poolDescription, driver, config)
+func storagePoolCreateGlobal(state *state.State, req api.StoragePoolsPost) error {
+ // Create the database entry.
+ id, err := storagePoolDBCreate(state, req.Name, req.Description, req.Driver, req.Config)
if err != nil {
return err
}
+
// Define a function which reverts everything. Defer this function
// so that it doesn't need to be explicitly called in every failing
// return path. Track whether or not we want to undo the changes
@@ -220,40 +224,77 @@ func storagePoolCreateInternal(state *state.State, poolName, poolDescription str
if !tryUndo {
return
}
- dbStoragePoolDeleteAndUpdateCache(state.Cluster, poolName)
+
+ dbStoragePoolDeleteAndUpdateCache(state.Cluster, req.Name)
}()
- err = doStoragePoolCreateInternal(state, poolName, poolDescription, driver, config, false)
- tryUndo = err != nil
- return err
-}
-// This performs all non-db related work needed to create the pool.
-func doStoragePoolCreateInternal(state *state.State, poolName, poolDescription string, driverName string, config map[string]string, isNotification bool) error {
- tryUndo := true
- s, err := storagePoolInit(state, poolName)
+ err = storagePoolCreateLocal(state, id, req, false)
if err != nil {
return err
}
- // If this is a clustering notification for a ceph storage, we don't
- // want this node to actually create the pool, as it's already been
- // done by the node that triggered this notification. We just need to
- // create the storage pool directory.
- if s, ok := s.(*storageCeph); ok && isNotification {
- volumeMntPoint := driver.GetStoragePoolVolumeMountPoint(s.pool.Name, s.volume.Name)
- return os.MkdirAll(volumeMntPoint, 0711)
+ tryUndo = false
+ return nil
+}
- }
- err = s.StoragePoolCreate()
- if err != nil {
- return err
- }
- defer func() {
- if !tryUndo {
- return
+// This performs all non-db related work needed to create the pool.
+func storagePoolCreateLocal(state *state.State, id int64, req api.StoragePoolsPost, isNotification bool) error {
+ tryUndo := true
+
+ // Make a copy of the req for later diff.
+ var updatedConfig map[string]string
+ var updatedReq api.StoragePoolsPost
+ shared.DeepCopy(&req, &updatedReq)
+
+ // Attempt to create using the new storage pool logic.
+ pool, err := storagePools.CreatePool(state, id, &updatedReq, isNotification, nil)
+ if err != storageDrivers.ErrUnknownDriver {
+ if err != nil {
+ return err
+ }
+
+ // Record the updated config.
+ updatedConfig = updatedReq.Config
+
+ // Setup revert function.
+ defer func() {
+ if !tryUndo {
+ return
+ }
+
+ pool.Delete(isNotification, nil)
+ }()
+ } else {
+ // Load the old storage struct
+ s, err := storagePoolInit(state, req.Name)
+ if err != nil {
+ return err
}
- s.StoragePoolDelete()
- }()
+
+ // If this is a clustering notification for a ceph storage, we don't
+ // want this node to actually create the pool, as it's already been
+ // done by the node that triggered this notification. We just need to
+ // create the storage pool directory.
+ if s, ok := s.(*storageCeph); ok && isNotification {
+ volumeMntPoint := storagePools.GetStoragePoolVolumeMountPoint(s.pool.Name, s.volume.Name)
+ return os.MkdirAll(volumeMntPoint, 0711)
+ }
+
+ // Create the pool
+ err = s.StoragePoolCreate()
+ if err != nil {
+ return err
+ }
+
+ updatedConfig = s.GetStoragePoolWritable().Config
+
+ defer func() {
+ if !tryUndo {
+ return
+ }
+ s.StoragePoolDelete()
+ }()
+ }
// In case the storage pool config was changed during the pool creation,
// we need to update the database to reflect this change. This can e.g.
@@ -261,13 +302,12 @@ func doStoragePoolCreateInternal(state *state.State, poolName, poolDescription s
// to the path the user gave us and update the config in the storage
// callback. So diff the config here to see if something like this has
// happened.
- postCreateConfig := s.GetStoragePoolWritable().Config
- configDiff, _ := driver.ConfigDiff(config, postCreateConfig)
+ configDiff, _ := storagePools.ConfigDiff(req.Config, updatedConfig)
if len(configDiff) > 0 {
// Create the database entry for the storage pool.
- err = state.Cluster.StoragePoolUpdate(poolName, poolDescription, postCreateConfig)
+ err = state.Cluster.StoragePoolUpdate(req.Name, req.Description, updatedConfig)
if err != nil {
- return fmt.Errorf("Error inserting %s into database: %s", poolName, err)
+ return fmt.Errorf("Error inserting %s into database: %s", req.Name, err)
}
}
@@ -277,8 +317,7 @@ func doStoragePoolCreateInternal(state *state.State, poolName, poolDescription s
return nil
}
-// Helper around the low-level DB API, which also updates the driver names
-// cache.
+// Helper around the low-level DB API, which also updates the driver names cache.
func dbStoragePoolCreateAndUpdateCache(db *db.Cluster, poolName string, poolDescription string, poolDriver string, poolConfig map[string]string) (int64, error) {
id, err := db.StoragePoolCreate(poolName, poolDescription, poolDriver, poolConfig)
if err != nil {
More information about the lxc-devel
mailing list