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