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

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) ===
Moves the DB logic from the legacy storage functions directly into the API (which is similar to existing functions that do DB lookups and modifications for simple cases). This way the legacy storage function can be removed and it does fewer DB queries.

Includes https://github.com/lxc/lxd/pull/6371
From 1a57754ff99999efe1ec25d5c273b7a2ae2749b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 23 Oct 2019 16:13:10 +0100
Subject: [PATCH 01/16] lxc/storage/volumes: Links storagePoolVolumeTypePatch
 to new storage pkg

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/storage_volumes.go | 64 ++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index df2cec6f48..ac5c2cd43f 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -986,23 +986,13 @@ func storagePoolVolumeTypeImagePut(d *Daemon, r 
*http.Request) response.Response
 // /1.0/storage-pools/{pool}/volumes/{type}/{name}
 func storagePoolVolumeTypePatch(d *Daemon, r *http.Request, volumeTypeName 
string) response.Response {
        // Get the name of the storage volume.
-       var volumeName string
-       fields := strings.Split(mux.Vars(r)["name"], "/")
+       volumeName := mux.Vars(r)["name"]
 
-       if len(fields) == 3 && fields[1] == "snapshots" {
-               // Handle volume snapshots
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[2])
-       } else if len(fields) > 1 {
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[1])
-       } else if len(fields) > 0 {
-               // Handle volume
-               volumeName = fields[0]
-       } else {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
%s", mux.Vars(r)["name"]))
+       if shared.IsSnapshot(volumeName) {
+               return response.BadRequest(fmt.Errorf("Invalid volume name"))
        }
 
-       // Get the name of the storage pool the volume is supposed to be
-       // attached to.
+       // Get the name of the storage pool the volume is supposed to be 
attached to.
        poolName := mux.Vars(r)["pool"]
 
        // Convert the volume type name to our internal integer representation.
@@ -1010,14 +1000,14 @@ func storagePoolVolumeTypePatch(d *Daemon, r 
*http.Request, volumeTypeName strin
        if err != nil {
                return response.BadRequest(err)
        }
+
        // Check that the storage volume type is valid.
        if !shared.IntInSlice(volumeType, supportedVolumeTypes) {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
-       // Get the ID of the storage pool the storage volume is supposed to be
-       // attached to.
-       poolID, pool, err := d.cluster.StoragePoolGet(poolName)
+       // Get the ID of the storage pool the storage volume is supposed to be 
attached to.
+       poolID, poolRow, err := d.cluster.StoragePoolGet(poolName)
        if err != nil {
                return response.SmartError(err)
        }
@@ -1033,13 +1023,13 @@ func storagePoolVolumeTypePatch(d *Daemon, r 
*http.Request, volumeTypeName strin
        }
 
        // Get the existing storage volume.
-       _, volume, err := d.cluster.StoragePoolNodeVolumeGetType(volumeName, 
volumeType, poolID)
+       _, vol, err := d.cluster.StoragePoolNodeVolumeGetType(volumeName, 
volumeType, poolID)
        if err != nil {
                return response.SmartError(err)
        }
 
-       // Validate the ETag
-       etag := []interface{}{volumeName, volume.Type, volume.Config}
+       // Validate the ETag.
+       etag := []interface{}{volumeName, vol.Type, vol.Config}
 
        err = util.EtagCheck(r, etag)
        if err != nil {
@@ -1055,22 +1045,36 @@ func storagePoolVolumeTypePatch(d *Daemon, r 
*http.Request, volumeTypeName strin
                req.Config = map[string]string{}
        }
 
-       for k, v := range volume.Config {
+       // Merge current config with requested changes.
+       for k, v := range vol.Config {
                _, ok := req.Config[k]
                if !ok {
                        req.Config[k] = v
                }
        }
 
-       // Validate the configuration
-       err = storagePools.VolumeValidateConfig(volumeName, req.Config, pool)
-       if err != nil {
-               return response.BadRequest(err)
-       }
+       // Check if we can load new storage layer for pool driver type.
+       pool, err := storagePools.GetPoolByName(d.State(), poolName)
+       if err != storageDrivers.ErrUnknownDriver {
+               if err != nil {
+                       return response.SmartError(err)
+               }
 
-       err = storagePoolVolumeUpdate(d.State(), poolName, volumeName, 
volumeType, req.Description, req.Config)
-       if err != nil {
-               return response.SmartError(err)
+               err = pool.UpdateCustomVolume(vol.Name, req.Description, 
req.Config, nil)
+               if err != nil {
+                       return response.SmartError(err)
+               }
+       } else {
+               // Validate the configuration.
+               err = storagePools.VolumeValidateConfig(volumeName, req.Config, 
poolRow)
+               if err != nil {
+                       return response.BadRequest(err)
+               }
+
+               err = storagePoolVolumeUpdate(d.State(), poolName, volumeName, 
volumeType, req.Description, req.Config)
+               if err != nil {
+                       return response.SmartError(err)
+               }
        }
 
        return response.EmptySyncResponse

From c2e5372a7e693ab048ceb724aea73f5be24b8f34 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:29:14 +0000
Subject: [PATCH 02/16] lxd/storage/volumes/utils: Links
 storagePoolVolumeUsedByRunningContainersWithProfilesGet to storage pkg

As VolumeUsedByInstancesWithProfiles

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

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index 2d9bdc5bf1..c70e8acb8a 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -39,6 +39,10 @@ const (
 var supportedVolumeTypesExceptImages = []int{storagePoolVolumeTypeContainer, 
storagePoolVolumeTypeCustom}
 var supportedVolumeTypes = append(supportedVolumeTypesExceptImages, 
storagePoolVolumeTypeImage)
 
+func init() {
+       storagePools.VolumeUsedByInstancesWithProfiles = 
storagePoolVolumeUsedByRunningContainersWithProfilesGet
+}
+
 func storagePoolVolumeTypeNameToAPIEndpoint(volumeTypeName string) (string, 
error) {
        switch volumeTypeName {
        case storagePoolVolumeTypeNameContainer:

From f2be5e80787d668cd2a97fbb922b25b40ad4d40f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:30:00 +0000
Subject: [PATCH 03/16] lxd/storage/utils: Adds
 VolumeUsedByInstancesWithProfiles

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 5f9d890497..695812239d 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -28,6 +28,9 @@ var baseDirectories = []string{
        "virtual-machines-snapshots",
 }
 
+// VolumeUsedByInstancesWithProfiles returns a slice containing the names of 
instances using a volume.
+var VolumeUsedByInstancesWithProfiles func(s *state.State, poolName string, 
volumeName string, volumeTypeName string, runningOnly bool) ([]string, error)
+
 func createStorageStructure(path string) error {
        for _, name := range baseDirectories {
                err := os.MkdirAll(filepath.Join(path, name), 0711)

From d0f630c8559ec6ae47a9c96aec05f2a3a70365d6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:31:03 +0000
Subject: [PATCH 04/16] lxd/storage/volumes: Links storagePoolVolumeTypePut to
 storage pkg

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/storage_volumes.go | 100 ++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 32 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index ac5c2cd43f..e4cd291be9 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -876,21 +876,14 @@ func storagePoolVolumeTypeImageGet(d *Daemon, r 
*http.Request) response.Response
 }
 
 // /1.0/storage-pools/{pool}/volumes/{type}/{name}
+// This function does allow limited functionality for non-custom volume types, 
specifically you
+// can modify the volume's description only.
 func storagePoolVolumeTypePut(d *Daemon, r *http.Request, volumeTypeName 
string) response.Response {
        // Get the name of the storage volume.
-       var volumeName string
-       fields := strings.Split(mux.Vars(r)["name"], "/")
+       volumeName := mux.Vars(r)["name"]
 
-       if len(fields) == 3 && fields[1] == "snapshots" {
-               // Handle volume snapshots
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[2])
-       } else if len(fields) > 1 {
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[1])
-       } else if len(fields) > 0 {
-               // Handle volume
-               volumeName = fields[0]
-       } else {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
%s", mux.Vars(r)["name"]))
+       if shared.IsSnapshot(volumeName) {
+               return response.BadRequest(fmt.Errorf("Invalid volume name"))
        }
 
        // Get the name of the storage pool the volume is supposed to be
@@ -907,7 +900,7 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, 
volumeTypeName string)
                return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
        }
 
-       poolID, pool, err := d.cluster.StoragePoolGet(poolName)
+       poolID, poolRow, err := d.cluster.StoragePoolGet(poolName)
        if err != nil {
                return response.SmartError(err)
        }
@@ -923,13 +916,13 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, 
volumeTypeName string)
        }
 
        // Get the existing storage volume.
-       _, volume, err := d.cluster.StoragePoolNodeVolumeGetType(volumeName, 
volumeType, poolID)
+       _, vol, err := d.cluster.StoragePoolNodeVolumeGetType(volumeName, 
volumeType, poolID)
        if err != nil {
                return response.SmartError(err)
        }
 
        // Validate the ETag
-       etag := []interface{}{volumeName, volume.Type, volume.Config}
+       etag := []interface{}{volumeName, vol.Type, vol.Config}
 
        err = util.EtagCheck(r, etag)
        if err != nil {
@@ -941,30 +934,73 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, 
volumeTypeName string)
                return response.BadRequest(err)
        }
 
-       if req.Restore != "" {
-               ctsUsingVolume, err := 
storagePoolVolumeUsedByRunningContainersWithProfilesGet(d.State(), poolName, 
volume.Name, storagePoolVolumeTypeNameCustom, true)
+       // Check if we can load new storage layer for pool driver type.
+       pool, err := storagePools.GetPoolByName(d.State(), poolName)
+       if err != storageDrivers.ErrUnknownDriver {
                if err != nil {
-                       return response.InternalError(err)
+                       return response.SmartError(err)
                }
 
-               if len(ctsUsingVolume) != 0 {
-                       return response.BadRequest(fmt.Errorf("Cannot restore 
custom volume used by running containers"))
-               }
+               if volumeType == db.StoragePoolVolumeTypeCustom {
+                       // Restore custom volume from snapshot if requested. 
This should occur first
+                       // before applying config changes so that changes are 
applied to the
+                       // restored volume.
+                       if req.Restore != "" {
+                               err = pool.RestoreCustomVolume(vol.Name, 
req.Restore, nil)
+                               if err != nil {
+                                       return response.SmartError(err)
+                               }
+                       }
 
-               err = storagePoolVolumeRestore(d.State(), poolName, volumeName, 
volumeType, req.Restore)
-               if err != nil {
-                       return response.SmartError(err)
+                       // Handle update requests.
+                       err = pool.UpdateCustomVolume(vol.Name, 
req.Description, req.Config, nil)
+                       if err != nil {
+                               return response.SmartError(err)
+                       }
+               } else {
+                       // You are only allowed to modify the description for 
non-custom volumes.
+                       // This is a special case because the rootfs devices do 
not provide a way
+                       // to update a non-custom volume's description.
+                       if len(req.Config) > 0 {
+                               return response.BadRequest(fmt.Errorf("Only 
description can be modified for volume type %s", volumeTypeName))
+                       }
+
+                       // There is a bug in the lxc client 
(lxc/storage_volume.go#L829-L865) which
+                       // means that modifying a snapshot's description gets 
routed here rather
+                       // than the dedicated snapshot editing route. So need 
to handle snapshot
+                       // volumes here too.
+                       err = d.cluster.StoragePoolVolumeUpdate(vol.Name, 
volumeType, poolID, req.Description, req.Config)
+                       if err != nil {
+                               return response.SmartError(err)
+                       }
                }
        } else {
-               // Validate the configuration
-               err = storagePools.VolumeValidateConfig(volumeName, req.Config, 
pool)
-               if err != nil {
-                       return response.BadRequest(err)
-               }
 
-               err = storagePoolVolumeUpdate(d.State(), poolName, volumeName, 
volumeType, req.Description, req.Config)
-               if err != nil {
-                       return response.SmartError(err)
+               if req.Restore != "" {
+                       ctsUsingVolume, err := 
storagePoolVolumeUsedByRunningContainersWithProfilesGet(d.State(), poolName, 
vol.Name, storagePoolVolumeTypeNameCustom, true)
+                       if err != nil {
+                               return response.InternalError(err)
+                       }
+
+                       if len(ctsUsingVolume) != 0 {
+                               return response.BadRequest(fmt.Errorf("Cannot 
restore custom volume used by running containers"))
+                       }
+
+                       err = storagePoolVolumeRestore(d.State(), poolName, 
volumeName, volumeType, req.Restore)
+                       if err != nil {
+                               return response.SmartError(err)
+                       }
+               } else {
+                       // Validate the configuration
+                       err = storagePools.VolumeValidateConfig(volumeName, 
req.Config, poolRow)
+                       if err != nil {
+                               return response.BadRequest(err)
+                       }
+
+                       err = storagePoolVolumeUpdate(d.State(), poolName, 
volumeName, volumeType, req.Description, req.Config)
+                       if err != nil {
+                               return response.SmartError(err)
+                       }
                }
        }
 

From 6fdb2cd283edaa6f5e8214fe1adf43c35d1b0b8b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:33:17 +0000
Subject: [PATCH 05/16] lxd/storage/drivers/interface: Updates function
 definitions

- Adds UpdateVolume
- Updates ValidateVolume
- Fixes typo

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

diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 13f071a6c2..7e4fadd28f 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -35,6 +35,7 @@ type Driver interface {
        CreateVolumeFromCopy(vol Volume, srcVol Volume, copySnapshots bool, op 
*operations.Operation) error
        DeleteVolume(volType VolumeType, volName string, op 
*operations.Operation) error
        RenameVolume(volType VolumeType, volName string, newName string, op 
*operations.Operation) error
+       UpdateVolume(vol Volume, changedKeys map[string]struct{}) error
 
        // MountVolume mounts a storage volume, returns true if we caused a new 
mount, false if
        // already mounted.
@@ -42,7 +43,7 @@ type Driver interface {
 
        // MountVolumeSnapshot mounts a storage volume snapshot as readonly, 
returns true if we
        // caused a new mount, false if already mounted.
-       MountVolumeSnapshot(volType VolumeType, VolName, snapshotName string, 
op *operations.Operation) (bool, error)
+       MountVolumeSnapshot(volType VolumeType, volName, snapshotName string, 
op *operations.Operation) (bool, error)
 
        // UnmountVolume unmounts a storage volume, returns true if unmounted, 
false if was not
        // mounted.

From fcd3d9989a6aeb94b0930022755d986711b82328 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:35:03 +0000
Subject: [PATCH 06/16] lxd/storage/backend: UpdateCustomVolume and
 RestoreCustomVolume

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

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 68c39a6baf..e2a33f178f 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -5,6 +5,7 @@ import (
        "io"
        "os"
        "regexp"
+       "strings"
 
        "github.com/lxc/lxd/lxd/db"
        "github.com/lxc/lxd/lxd/migration"
@@ -515,16 +516,22 @@ func (b *lxdBackend) RenameCustomVolume(volName string, 
newVolName string, op *o
        return nil
 }
 
-// UpdateCustomVolume applies the supplied config to the volume.
+// UpdateCustomVolume applies the supplied config to the custom volume.
 func (b *lxdBackend) UpdateCustomVolume(volName, newDesc string, newConfig 
map[string]string, op *operations.Operation) error {
+       if shared.IsSnapshot(volName) {
+               return fmt.Errorf("Volume name cannot be a snapshot")
+       }
+
+       vol := b.newVolume(drivers.VolumeTypeCustom, drivers.ContentTypeFS, 
volName, newConfig)
+
        // Validate config.
-       err := b.driver.ValidateVolume(b.newVolume(drivers.VolumeTypeCustom, 
drivers.ContentTypeFS, volName, newConfig), false)
+       err := b.driver.ValidateVolume(vol, false)
        if err != nil {
                return err
        }
 
        // Get current config to compare what has changed.
-       _, _, err = 
b.state.Cluster.StoragePoolNodeVolumeGetTypeByProject("default", volName, 
db.StoragePoolVolumeTypeCustom, b.ID())
+       _, curVol, err := 
b.state.Cluster.StoragePoolNodeVolumeGetTypeByProject("default", volName, 
db.StoragePoolVolumeTypeCustom, b.ID())
        if err != nil {
                if err == db.ErrNoSuchObject {
                        return fmt.Errorf("Volume doesn't exist")
@@ -533,7 +540,71 @@ func (b *lxdBackend) UpdateCustomVolume(volName, newDesc 
string, newConfig map[s
                return err
        }
 
-       return ErrNotImplemented
+       // Diff the configurations.
+       changedKeys := make(map[string]struct{})
+       userOnly := true
+       for key := range curVol.Config {
+               if curVol.Config[key] != newConfig[key] {
+                       if !strings.HasPrefix(key, "user.") {
+                               userOnly = false
+                       }
+
+                       changedKeys[key] = struct{}{}
+               }
+       }
+
+       for key := range newConfig {
+               if curVol.Config[key] != newConfig[key] {
+                       if !strings.HasPrefix(key, "user.") {
+                               userOnly = false
+                       }
+
+                       changedKeys[key] = struct{}{}
+               }
+       }
+
+       // Apply config changes if there are any.
+       if len(changedKeys) != 0 {
+               if !userOnly {
+                       err = b.driver.UpdateVolume(vol, changedKeys)
+                       if err != nil {
+                               return err
+                       }
+               }
+       }
+
+       // Check that security.unmapped and security.shifted aren't set 
together.
+       if shared.IsTrue(newConfig["security.unmapped"]) && 
shared.IsTrue(newConfig["security.shifted"]) {
+               return fmt.Errorf("security.unmapped and security.shifted are 
mutually exclusive")
+       }
+
+       // Confirm that no instances are running when changing shifted state.
+       if newConfig["security.shifted"] != curVol.Config["security.shifted"] {
+               usingVolume, err := VolumeUsedByInstancesWithProfiles(b.state, 
b.Name(), volName, db.StoragePoolVolumeTypeNameCustom, true)
+               if err != nil {
+                       return err
+               }
+
+               if len(usingVolume) != 0 {
+                       return fmt.Errorf("Cannot modify shifting with running 
containers using the volume")
+               }
+       }
+
+       // Unset idmap keys if volume is unmapped.
+       if shared.IsTrue(newConfig["security.unmapped"]) {
+               delete(newConfig, "volatile.idmap.last")
+               delete(newConfig, "volatile.idmap.next")
+       }
+
+       // Update the database if something changed.
+       if len(changedKeys) != 0 || newDesc != curVol.Description {
+               err = b.state.Cluster.StoragePoolVolumeUpdate(volName, 
db.StoragePoolVolumeTypeCustom, b.ID(), newDesc, newConfig)
+               if err != nil {
+                       return err
+               }
+       }
+
+       return nil
 }
 
 // DeleteCustomVolume removes a custom volume and its snapshots.
@@ -707,3 +778,30 @@ func (b *lxdBackend) DeleteCustomVolumeSnapshot(volName 
string, op *operations.O
 
        return nil
 }
+
+// RestoreCustomVolume restores a custom volume from a snapshot.
+func (b *lxdBackend) RestoreCustomVolume(volName string, srcSnapshotName 
string, op *operations.Operation) error {
+       if shared.IsSnapshot(volName) {
+               return fmt.Errorf("Volume cannot be snapshot")
+       }
+
+       if shared.IsSnapshot(srcSnapshotName) {
+               return fmt.Errorf("Invalid snapshot name")
+       }
+
+       usingVolume, err := VolumeUsedByInstancesWithProfiles(b.state, 
b.Name(), volName, db.StoragePoolVolumeTypeNameCustom, true)
+       if err != nil {
+               return err
+       }
+
+       if len(usingVolume) != 0 {
+               return fmt.Errorf("Cannot restore custom volume used by running 
instances")
+       }
+
+       err = b.driver.RestoreVolume(b.newVolume(drivers.VolumeTypeCustom, 
drivers.ContentTypeFS, volName, nil), srcSnapshotName, op)
+       if err != nil {
+               return err
+       }
+
+       return nil
+}

From 5d4689ee86d0183f4ac62278dedf05a8ceb534b9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 30 Oct 2019 15:36:13 +0000
Subject: [PATCH 07/16] lxd/storage/drivers/driver/dir: Adds UpdateVolume
 function

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

diff --git a/lxd/storage/drivers/driver_dir.go 
b/lxd/storage/drivers/driver_dir.go
index 61cfb9deb4..cfc0a26a0b 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -459,6 +459,25 @@ func (d *dir) VolumeSnapshots(volType VolumeType, volName 
string, op *operations
        return snapshots, nil
 }
 
+// UpdateVolume applies config changes to the volume.
+func (d *dir) UpdateVolume(vol Volume, changedKeys map[string]struct{}) error {
+       if _, found := changedKeys["size"]; found {
+               volID, err := d.getVolID(vol.volType, vol.name)
+               if err != nil {
+                       return err
+               }
+
+               // Set the quota if specified in volConfig or pool config.
+               err = d.setQuota(vol.MountPath(), volID, vol.config["size"])
+               if err != nil {
+                       return err
+               }
+
+       }
+
+       return nil
+}
+
 // RenameVolume renames a volume and its snapshots.
 func (d *dir) RenameVolume(volType VolumeType, volName string, newVolName 
string, op *operations.Operation) error {
        vol := NewVolume(d, d.name, volType, ContentTypeFS, volName, nil)

From 1cfd07b42237dd2c86d389fe0f2895016d00ff8c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:34:13 +0000
Subject: [PATCH 08/16] lxd/storage/volumes: Consistent casing on error
 messages

- Moves volume name from URL extraction a common function.

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

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index e4cd291be9..0277b83c03 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -174,7 +174,7 @@ func storagePoolVolumesTypeGet(d *Daemon, r *http.Request) 
response.Response {
        }
        // Check that the storage volume type is valid.
        if !shared.IntInSlice(volumeType, supportedVolumeTypes) {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
        // Retrieve ID of the storage pool (and check if the storage pool
@@ -794,25 +794,32 @@ func storagePoolVolumeTypeImagePost(d *Daemon, r 
*http.Request) response.Respons
        return storagePoolVolumeTypePost(d, r, "image")
 }
 
+// storageGetVolumeNameFromURL retrieves the volume name from the URL name 
segment.
+func storageGetVolumeNameFromURL(r *http.Request) (string, error) {
+       fields := strings.Split(mux.Vars(r)["name"], "/")
+
+       if len(fields) == 3 && fields[1] == "snapshots" {
+               // Handle volume snapshots.
+               return fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[2]), nil
+       } else if len(fields) > 1 {
+               return fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[1]), nil
+       } else if len(fields) > 0 {
+               // Handle volume.
+               return fields[0], nil
+       }
+
+       return "", fmt.Errorf("Invalid storage volume %s", mux.Vars(r)["name"])
+}
+
 // /1.0/storage-pools/{pool}/volumes/{type}/{name}
 // Get storage volume of a given volume type on a given storage pool.
 func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, volumeTypeName 
string) response.Response {
        project := projectParam(r)
 
        // Get the name of the storage volume.
-       var volumeName string
-       fields := strings.Split(mux.Vars(r)["name"], "/")
-
-       if len(fields) == 3 && fields[1] == "snapshots" {
-               // Handle volume snapshots
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[2])
-       } else if len(fields) > 1 {
-               volumeName = fmt.Sprintf("%s%s%s", fields[0], 
shared.SnapshotDelimiter, fields[1])
-       } else if len(fields) > 0 {
-               // Handle volume
-               volumeName = fields[0]
-       } else {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
%s", mux.Vars(r)["name"]))
+       volumeName, err := storageGetVolumeNameFromURL(r)
+       if err != nil {
+               return response.BadRequest(err)
        }
 
        // Get the name of the storage pool the volume is supposed to be
@@ -826,7 +833,7 @@ func storagePoolVolumeTypeGet(d *Daemon, r *http.Request, 
volumeTypeName string)
        }
        // Check that the storage volume type is valid.
        if !shared.IntInSlice(volumeType, supportedVolumeTypes) {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
        // Get the ID of the storage pool the storage volume is supposed to be
@@ -880,10 +887,9 @@ func storagePoolVolumeTypeImageGet(d *Daemon, r 
*http.Request) response.Response
 // can modify the volume's description only.
 func storagePoolVolumeTypePut(d *Daemon, r *http.Request, volumeTypeName 
string) response.Response {
        // Get the name of the storage volume.
-       volumeName := mux.Vars(r)["name"]
-
-       if shared.IsSnapshot(volumeName) {
-               return response.BadRequest(fmt.Errorf("Invalid volume name"))
+       volumeName, err := storageGetVolumeNameFromURL(r)
+       if err != nil {
+               return response.BadRequest(err)
        }
 
        // Get the name of the storage pool the volume is supposed to be
@@ -895,9 +901,10 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, 
volumeTypeName string)
        if err != nil {
                return response.BadRequest(err)
        }
+
        // Check that the storage volume type is valid.
        if !shared.IntInSlice(volumeType, supportedVolumeTypes) {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
        poolID, poolRow, err := d.cluster.StoragePoolGet(poolName)
@@ -1150,7 +1157,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r 
*http.Request, volumeTypeName stri
        }
        // Check that the storage volume type is valid.
        if !shared.IntInSlice(volumeType, supportedVolumeTypes) {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
        resp := ForwardedResponseIfTargetIsRemote(d, r)

From f4903257cef79b91fcff4af221c25faf67e9955a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:36:04 +0000
Subject: [PATCH 09/16] lxd/storage/utils: Consistent casing on error messages

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

diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index 695812239d..acc5e6069c 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -400,7 +400,7 @@ func VolumeTypeNameToType(volumeTypeName string) (int, 
error) {
                return db.StoragePoolVolumeTypeCustom, nil
        }
 
-       return -1, fmt.Errorf("invalid storage volume type name")
+       return -1, fmt.Errorf("Invalid storage volume type name")
 }
 
 // VolumeTypeToDBType converts volume type to internal code.
@@ -414,7 +414,7 @@ func VolumeTypeToDBType(volType drivers.VolumeType) (int, 
error) {
                return db.StoragePoolVolumeTypeCustom, nil
        }
 
-       return -1, fmt.Errorf("invalid storage volume type")
+       return -1, fmt.Errorf("Invalid storage volume type")
 }
 
 // VolumeDBCreate creates a volume in the database.

From 343a5f65e1fed5563b8ca37ed407f747181866f3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:36:20 +0000
Subject: [PATCH 10/16] lxd/storage/interfaces: Adds RestoreCustomVolume

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

diff --git a/lxd/storage/interfaces.go b/lxd/storage/interfaces.go
index d1d3cbdbba..17eeab8409 100644
--- a/lxd/storage/interfaces.go
+++ b/lxd/storage/interfaces.go
@@ -85,6 +85,7 @@ type Pool interface {
        CreateCustomVolumeSnapshot(volName string, newSnapshotName string, op 
*operations.Operation) error
        RenameCustomVolumeSnapshot(volName string, newSnapshotName string, op 
*operations.Operation) error
        DeleteCustomVolumeSnapshot(volName string, op *operations.Operation) 
error
+       RestoreCustomVolume(volName string, srcSnapshotName string, op 
*operations.Operation) error
 
        // Custom volume migration.
        MigrationTypes(contentType drivers.ContentType) []migration.Type

From dc0b69f68e24ab7db828c6037c0fefdb68024526 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:36:37 +0000
Subject: [PATCH 11/16] lxd/storage/drivers/interface: Adds RestoreVolume

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

diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 7e4fadd28f..c10fd183b2 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -57,6 +57,7 @@ type Driver interface {
        DeleteVolumeSnapshot(volType VolumeType, volName string, snapshotName 
string, op *operations.Operation) error
        RenameVolumeSnapshot(volType VolumeType, volName string, snapshotName 
string, newSnapshotName string, op *operations.Operation) error
        VolumeSnapshots(volType VolumeType, volName string, op 
*operations.Operation) ([]string, error)
+       RestoreVolume(vol Volume, srcSnapshotName string, op 
*operations.Operation) error
 
        // Migration.
        MigrationTypes(contentType ContentType) []migration.Type

From 6ccbb077bbe2ea5d12f12150cfb91f0a9466f920 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:36:54 +0000
Subject: [PATCH 12/16] lxd/storage/drivers/driver/dir: Implements
 RestoreVolume

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

diff --git a/lxd/storage/drivers/driver_dir.go 
b/lxd/storage/drivers/driver_dir.go
index cfc0a26a0b..78cd2efde0 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -550,6 +550,25 @@ func (d *dir) RenameVolume(volType VolumeType, volName 
string, newVolName string
        return nil
 }
 
+// RestoreVolume restores a volume from a snapshot.
+func (d *dir) RestoreVolume(vol Volume, srcSnapshotName string, op 
*operations.Operation) error {
+       srcPath := GetVolumeMountPath(d.name, vol.volType, 
GetSnapshotVolumeName(vol.name, srcSnapshotName))
+       if !shared.PathExists(srcPath) {
+               return fmt.Errorf("Snapshot not found")
+       }
+
+       volPath := vol.MountPath()
+
+       // Restore using rsync.
+       bwlimit := d.config["rsync.bwlimit"]
+       output, err := rsync.LocalCopy(srcPath, volPath, bwlimit, true)
+       if err != nil {
+               return fmt.Errorf("Failed to rsync volume: %s: %s", 
string(output), err)
+       }
+
+       return nil
+}
+
 // DeleteVolume deletes a volume of the storage device. If any snapshots of 
the volume remain then
 // this function will return an error.
 func (d *dir) DeleteVolume(volType VolumeType, volName string, op 
*operations.Operation) error {

From 945f1517832838f40aa88083a44ab1bde7478a91 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 10:37:08 +0000
Subject: [PATCH 13/16] lxd/storage/backend/mock: Adds RestoreCustomVolume

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

diff --git a/lxd/storage/backend_mock.go b/lxd/storage/backend_mock.go
index 86562bc73e..86e18253bd 100644
--- a/lxd/storage/backend_mock.go
+++ b/lxd/storage/backend_mock.go
@@ -203,3 +203,7 @@ func (b *mockBackend) RenameCustomVolumeSnapshot(volName 
string, newName string,
 func (b *mockBackend) DeleteCustomVolumeSnapshot(volName string, op 
*operations.Operation) error {
        return nil
 }
+
+func (b *mockBackend) RestoreCustomVolume(volName string, srcSnapshotName 
string, op *operations.Operation) error {
+       return nil
+}

From b19d65b65342381cb37a137e8c342c171cb6c6df Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 11:51:29 +0000
Subject: [PATCH 14/16] lxd/storage/volumes: Makes storagePoolVolumeTypePut
 logic consistent with storagePoolVolumeSnapshotTypePut

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/storage_volumes.go | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 0277b83c03..1f7f31cea4 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -976,9 +976,13 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request, 
volumeTypeName string)
                        // means that modifying a snapshot's description gets 
routed here rather
                        // than the dedicated snapshot editing route. So need 
to handle snapshot
                        // volumes here too.
-                       err = d.cluster.StoragePoolVolumeUpdate(vol.Name, 
volumeType, poolID, req.Description, req.Config)
-                       if err != nil {
-                               return response.SmartError(err)
+
+                       // Update the database if description changed.
+                       if req.Description != vol.Description {
+                               err = 
d.cluster.StoragePoolVolumeUpdate(vol.Name, volumeType, poolID, 
req.Description, vol.Config)
+                               if err != nil {
+                                       response.SmartError(err)
+                               }
                        }
                }
        } else {

From d2ff2481af9e820231756b45f4a6c39e57306d32 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 11:52:08 +0000
Subject: [PATCH 15/16] lxd/storage/volumes/snapshot: Moves
 storagePoolVolumeSnapshotTypePut DB logic

Moves DB logic from legacy storage functions into API directly (as just DB 
updates) as its just a description update.

This will then work with both old and new storage layers.

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

diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index f38af7fef8..16794ab836 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -398,6 +398,7 @@ func storagePoolVolumeSnapshotTypeGet(d *Daemon, r 
*http.Request) response.Respo
        return response.SyncResponseETag(true, &snapshot, etag)
 }
 
+// storagePoolVolumeSnapshotTypePut allows a snapshot's description to be 
changed.
 func storagePoolVolumeSnapshotTypePut(d *Daemon, r *http.Request) 
response.Response {
        // Get the name of the storage pool the volume is supposed to be
        // attached to.
@@ -420,7 +421,7 @@ func storagePoolVolumeSnapshotTypePut(d *Daemon, r 
*http.Request) response.Respo
 
        // Check that the storage volume type is valid.
        if volumeType != storagePoolVolumeTypeCustom {
-               return response.BadRequest(fmt.Errorf("invalid storage volume 
type %s", volumeTypeName))
+               return response.BadRequest(fmt.Errorf("Invalid storage volume 
type %s", volumeTypeName))
        }
 
        resp := ForwardedResponseIfTargetIsRemote(d, r)
@@ -439,13 +440,13 @@ func storagePoolVolumeSnapshotTypePut(d *Daemon, r 
*http.Request) response.Respo
                return resp
        }
 
-       _, volume, err := 
d.cluster.StoragePoolNodeVolumeGetType(fullSnapshotName, volumeType, poolID)
+       _, vol, err := d.cluster.StoragePoolNodeVolumeGetType(fullSnapshotName, 
volumeType, poolID)
        if err != nil {
                return response.SmartError(err)
        }
 
        // Validate the ETag
-       etag := []interface{}{snapshotName, volume.Description, volume.Config}
+       etag := []interface{}{snapshotName, vol.Description, vol.Config}
        err = util.EtagCheck(r, etag)
        if err != nil {
                return response.PreconditionFailed(err)
@@ -457,22 +458,22 @@ func storagePoolVolumeSnapshotTypePut(d *Daemon, r 
*http.Request) response.Respo
                return response.BadRequest(err)
        }
 
-       var do func(*operations.Operation) error
-       var opDescription db.OperationType
-       do = func(op *operations.Operation) error {
-               err = storagePoolVolumeSnapshotUpdate(d.State(), poolName, 
volume.Name, volumeType, req.Description)
-               if err != nil {
-                       return err
+       do := func(op *operations.Operation) error {
+               // Update the database if description changed.
+               if req.Description != vol.Description {
+                       err = d.cluster.StoragePoolVolumeUpdate(vol.Name, 
volumeType, poolID, req.Description, vol.Config)
+                       if err != nil {
+                               return err
+                       }
                }
 
-               opDescription = db.OperationVolumeSnapshotDelete
                return nil
        }
 
        resources := map[string][]string{}
        resources["storage_volume_snapshots"] = []string{volumeName}
 
-       op, err := operations.OperationCreate(d.State(), "", 
operations.OperationClassTask, opDescription, resources, nil, do, nil, nil)
+       op, err := operations.OperationCreate(d.State(), "", 
operations.OperationClassTask, db.OperationSnapshotUpdate, resources, nil, do, 
nil, nil)
        if err != nil {
                return response.InternalError(err)
        }

From 8faca9d6a42cd6c6fe402412cc08c0b66214d69b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 31 Oct 2019 11:53:28 +0000
Subject: [PATCH 16/16] lxd/storage/volumes/utils: Removes unused
 storagePoolVolumeSnapshotUpdate

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/storage_volumes_utils.go | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index c70e8acb8a..ab7791e742 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -207,28 +207,6 @@ func storagePoolVolumeUpdate(state *state.State, poolName 
string, volumeName str
        return nil
 }
 
-func storagePoolVolumeSnapshotUpdate(state *state.State, poolName string, 
volumeName string, volumeType int, newDescription string) error {
-       s, err := storagePoolVolumeInit(state, "default", poolName, volumeName, 
volumeType)
-       if err != nil {
-               return err
-       }
-
-       oldWritable := s.GetStoragePoolVolumeWritable()
-       oldDescription := oldWritable.Description
-
-       poolID, err := state.Cluster.StoragePoolGetID(poolName)
-       if err != nil {
-               return err
-       }
-
-       // Update the database if something changed
-       if newDescription != oldDescription {
-               return state.Cluster.StoragePoolVolumeUpdate(volumeName, 
volumeType, poolID, newDescription, oldWritable.Config)
-       }
-
-       return nil
-}
-
 func storagePoolVolumeUsedByContainersGet(s *state.State, project, poolName 
string, volumeName string) ([]string, error) {
        insts, err := instanceLoadByProject(s, project)
        if err != nil {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to