The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6334
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) ===
This PR comes from https://github.com/lxc/lxd/pull/6285, but to make that PR smaller I am breaking it into parts.
This PR changes the return type of `StoragePoolVolumeSnapshotsGetType` to return a slice of `StorageVolumeArgs` structs so that getting a list of snapshots with their descriptions only requires a single query, rather than n+1 for queries for the number of snapshots a volume has.
Also it explicitly defines the return order of snapshots as this is important for migration, rather than implicitly depend on sqlite returning in creation order.
From 1c404d9fb75f87ee8dd6a5520cd04bd9186c385b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Mon, 14 Oct 2019 12:27:34 +0100
Subject: [PATCH 1/3] lxd/db/storage/pools: StoragePoolVolumeSnapshotsGetType
returns StorageVolumeArgs slice
Updates StoragePoolVolumeSnapshotsGetType to return a slice of
StorageVolumeArgs with Name and Description populated.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/storage_pools.go | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index c0ed91eed2..75d20fc250 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -776,22 +776,26 @@ SELECT storage_volumes.name
// StoragePoolVolumeSnapshotsGetType get all snapshots of a storage volume
// attached to a given storage pool of a given volume type, on the given node.
-func (c *Cluster) StoragePoolVolumeSnapshotsGetType(volumeName string,
volumeType int, poolID int64) ([]string, error) {
- result := []string{}
+func (c *Cluster) StoragePoolVolumeSnapshotsGetType(volumeName string,
volumeType int, poolID int64) ([]StorageVolumeArgs, error) {
+ result := []StorageVolumeArgs{}
regexp := volumeName + shared.SnapshotDelimiter
length := len(regexp)
- query := "SELECT name FROM storage_volumes WHERE storage_pool_id=? AND
node_id=? AND type=? AND snapshot=? AND SUBSTR(name,1,?)=?"
+ query := "SELECT name, description FROM storage_volumes WHERE
storage_pool_id=? AND node_id=? AND type=? AND snapshot=? AND
SUBSTR(name,1,?)=?"
inargs := []interface{}{poolID, c.nodeID, volumeType, true, length,
regexp}
- outfmt := []interface{}{volumeName}
-
+ typeGuide := StorageVolumeArgs{} // StorageVolume struct used to guide
the types expected.
+ outfmt := []interface{}{typeGuide.Name, typeGuide.Description}
dbResults, err := queryScan(c.db, query, inargs, outfmt)
if err != nil {
return result, err
}
for _, r := range dbResults {
- result = append(result, r[0].(string))
+ row := StorageVolumeArgs{
+ Name: r[0].(string),
+ Description: r[1].(string),
+ }
+ result = append(result, row)
}
return result, nil
From 8f7d9c3e2d0fbaf48e9e1a477df06c5e24bb07a6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 22 Oct 2019 11:51:24 +0100
Subject: [PATCH 2/3] lxd/db/storage/pools: Makes
StoragePoolVolumeSnapshotsGetType return in volume ID order
Adds explicit order by id to SQL statement and comment to add clarity around
the expectations of this function's return values.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/storage_pools.go | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lxd/db/storage_pools.go b/lxd/db/storage_pools.go
index 75d20fc250..ade8291d1c 100644
--- a/lxd/db/storage_pools.go
+++ b/lxd/db/storage_pools.go
@@ -776,12 +776,17 @@ SELECT storage_volumes.name
// StoragePoolVolumeSnapshotsGetType get all snapshots of a storage volume
// attached to a given storage pool of a given volume type, on the given node.
+// Returns snapshots slice ordered by volume ID (i.e effectively the order
they were created).
func (c *Cluster) StoragePoolVolumeSnapshotsGetType(volumeName string,
volumeType int, poolID int64) ([]StorageVolumeArgs, error) {
result := []StorageVolumeArgs{}
regexp := volumeName + shared.SnapshotDelimiter
length := len(regexp)
- query := "SELECT name, description FROM storage_volumes WHERE
storage_pool_id=? AND node_id=? AND type=? AND snapshot=? AND
SUBSTR(name,1,?)=?"
+ // ORDER BY id is important here as the users of this function can
expect that the results
+ // will be returned in the order that the snapshots were created. This
is specifically used
+ // during migration to ensure that the storage engines can re-create
snapshots using the
+ // correct deltas.
+ query := "SELECT name, description FROM storage_volumes WHERE
storage_pool_id=? AND node_id=? AND type=? AND snapshot=? AND
SUBSTR(name,1,?)=? ORDER BY id"
inargs := []interface{}{poolID, c.nodeID, volumeType, true, length,
regexp}
typeGuide := StorageVolumeArgs{} // StorageVolume struct used to guide
the types expected.
outfmt := []interface{}{typeGuide.Name, typeGuide.Description}
From 057314a219b8e10eea5250b423a5368a3c3eee5d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 22 Oct 2019 12:12:33 +0100
Subject: [PATCH 3/3] lxd: Updates use of StoragePoolVolumeSnapshotsGetType
return type change
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/migrate_storage_volumes.go | 6 +++---
lxd/storage_btrfs.go | 6 +++---
lxd/storage_ceph.go | 2 +-
lxd/storage_ceph_utils.go | 4 ++--
lxd/storage_cephfs.go | 4 ++--
lxd/storage_dir.go | 4 ++--
lxd/storage_lvm.go | 2 +-
lxd/storage_migration.go | 6 +++---
lxd/storage_volumes.go | 4 ++--
lxd/storage_volumes_snapshot.go | 4 ++--
lxd/storage_volumes_utils.go | 4 ++--
lxd/storage_zfs.go | 4 ++--
12 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/lxd/migrate_storage_volumes.go b/lxd/migrate_storage_volumes.go
index 1c492dedb2..57b8428525 100644
--- a/lxd/migrate_storage_volumes.go
+++ b/lxd/migrate_storage_volumes.go
@@ -62,14 +62,14 @@ func (s *migrationSourceWs) DoStorage(migrateOp
*operations.Operation) error {
if err == nil {
poolID, err := state.Cluster.StoragePoolGetID(pool.Name)
if err == nil {
- for _, name := range snaps {
- _, snapVolume, err :=
state.Cluster.StoragePoolNodeVolumeGetType(name, storagePoolVolumeTypeCustom,
poolID)
+ for _, snap := range snaps {
+ _, snapVolume, err :=
state.Cluster.StoragePoolNodeVolumeGetType(snap.Name,
storagePoolVolumeTypeCustom, poolID)
if err != nil {
continue
}
snapshots = append(snapshots,
volumeSnapshotToProtobuf(snapVolume))
- _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(name)
+ _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
snapshotNames = append(snapshotNames,
snapName)
}
}
diff --git a/lxd/storage_btrfs.go b/lxd/storage_btrfs.go
index 8690f2a8ae..22196a68a7 100644
--- a/lxd/storage_btrfs.go
+++ b/lxd/storage_btrfs.go
@@ -780,7 +780,7 @@ func (s *storageBtrfs) StoragePoolVolumeRename(newName
string) error {
}
for _, vol := range volumes {
- _, snapshotName, _ :=
shared.ContainerGetParentAndSnapshotName(vol)
+ _, snapshotName, _ :=
shared.ContainerGetParentAndSnapshotName(vol.Name)
oldVolumeName := fmt.Sprintf("%s%s%s", s.volume.Name,
shared.SnapshotDelimiter, snapshotName)
newVolumeName := fmt.Sprintf("%s%s%s", newName,
shared.SnapshotDelimiter, snapshotName)
@@ -2903,7 +2903,7 @@ func (s *storageBtrfs) doCrossPoolVolumeCopy(sourcePool
string, sourceName strin
}
for _, snap := range snapshots {
- srcSnapshotMntPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(sourcePool, snap)
+ srcSnapshotMntPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(sourcePool, snap.Name)
_, err = rsync.LocalCopy(srcSnapshotMntPoint,
destVolumeMntPoint, bwlimit, true)
if err != nil {
@@ -2912,7 +2912,7 @@ func (s *storageBtrfs) doCrossPoolVolumeCopy(sourcePool
string, sourceName strin
}
// create snapshot
- _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
+ _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
err = s.doVolumeSnapshotCreate(s.pool.Name,
s.volume.Name, fmt.Sprintf("%s/%s", s.volume.Name, snapOnlyName))
if err != nil {
diff --git a/lxd/storage_ceph.go b/lxd/storage_ceph.go
index b6c4d5e29d..948505f12d 100644
--- a/lxd/storage_ceph.go
+++ b/lxd/storage_ceph.go
@@ -428,7 +428,7 @@ func (s *storageCeph) StoragePoolVolumeDelete() error {
}
for _, snap := range snapshots {
- err := s.doPoolVolumeSnapshotDelete(snap)
+ err := s.doPoolVolumeSnapshotDelete(snap.Name)
if err != nil {
return err
}
diff --git a/lxd/storage_ceph_utils.go b/lxd/storage_ceph_utils.go
index 2b3d9bed36..10b197346f 100644
--- a/lxd/storage_ceph_utils.go
+++ b/lxd/storage_ceph_utils.go
@@ -1949,8 +1949,8 @@ func (s *storageCeph) doCrossPoolVolumeCopy(source
*api.StorageVolumeSource) err
if !source.VolumeOnly {
for _, snap := range snapshots {
- _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
- srcSnapshotMntPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(source.Pool, snap)
+ _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
+ srcSnapshotMntPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(source.Pool, snap.Name)
_, err = rsync.LocalCopy(srcSnapshotMntPoint,
dstVolumeMntPoint, bwlimit, true)
if err != nil {
diff --git a/lxd/storage_cephfs.go b/lxd/storage_cephfs.go
index 120a6100b1..e60150b22c 100644
--- a/lxd/storage_cephfs.go
+++ b/lxd/storage_cephfs.go
@@ -805,8 +805,8 @@ func (s *storageCephFs) StoragePoolVolumeCopy(source
*api.StorageVolumeSource) e
}
for _, snap := range snapshots {
- _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
- err = s.copyVolume(source.Pool, snap,
fmt.Sprintf("%s/%s", s.volume.Name, snapOnlyName))
+ _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
+ err = s.copyVolume(source.Pool, snap.Name,
fmt.Sprintf("%s/%s", s.volume.Name, snapOnlyName))
if err != nil {
return err
}
diff --git a/lxd/storage_dir.go b/lxd/storage_dir.go
index da290132c5..b32e6b2c2f 100644
--- a/lxd/storage_dir.go
+++ b/lxd/storage_dir.go
@@ -1403,8 +1403,8 @@ func (s *storageDir) StoragePoolVolumeCopy(source
*api.StorageVolumeSource) erro
}
for _, snap := range snapshots {
- _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
- err = s.copyVolumeSnapshot(source.Pool, snap,
fmt.Sprintf("%s/%s", s.volume.Name, snapOnlyName))
+ _, snapOnlyName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
+ err = s.copyVolumeSnapshot(source.Pool, snap.Name,
fmt.Sprintf("%s/%s", s.volume.Name, snapOnlyName))
if err != nil {
return err
}
diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index a3d408f6d2..0bd5337229 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -2231,7 +2231,7 @@ func (s *storageLvm) StoragePoolVolumeCopy(source
*api.StorageVolumeSource) erro
}
for _, snap := range snapshots {
- err = s.copyVolumeSnapshot(source.Pool, snap)
+ err = s.copyVolumeSnapshot(source.Pool, snap.Name)
if err != nil {
return err
}
diff --git a/lxd/storage_migration.go b/lxd/storage_migration.go
index 7bb005266d..6ae562c309 100644
--- a/lxd/storage_migration.go
+++ b/lxd/storage_migration.go
@@ -68,10 +68,10 @@ func (s rsyncStorageSourceDriver) SendStorageVolume(conn
*websocket.Conn, op *op
}
for _, snap := range snapshots {
- wrapper := StorageProgressReader(op, "fs_progress",
snap)
- path :=
driver.GetStoragePoolVolumeSnapshotMountPoint(pool.Name, snap)
+ wrapper := StorageProgressReader(op, "fs_progress",
snap.Name)
+ path :=
driver.GetStoragePoolVolumeSnapshotMountPoint(pool.Name, snap.Name)
path = shared.AddSlash(path)
- logger.Debugf("Starting to send storage volume snapshot
%s on storage pool %s from %s", snap, pool.Name, path)
+ logger.Debugf("Starting to send storage volume snapshot
%s on storage pool %s from %s", snap.Name, pool.Name, path)
err = rsync.Send(volume.Name, path, conn, wrapper,
s.rsyncFeatures, bwlimit, state.OS.ExecPath)
if err != nil {
diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index b3ab72ec23..7cf8c954da 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -1016,7 +1016,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r
*http.Request, volumeTypeName stri
switch volumeType {
case storagePoolVolumeTypeCustom:
- var snapshots []string
+ var snapshots []db.StorageVolumeArgs
// Delete storage volume snapshots
snapshots, err =
d.cluster.StoragePoolVolumeSnapshotsGetType(volumeName, volumeType, poolID)
@@ -1025,7 +1025,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r
*http.Request, volumeTypeName stri
}
for _, snapshot := range snapshots {
- s, err := storagePoolVolumeInit(d.State(), project,
poolName, snapshot, volumeType)
+ s, err := storagePoolVolumeInit(d.State(), project,
poolName, snapshot.Name, volumeType)
if err != nil {
return response.NotFound(err)
}
diff --git a/lxd/storage_volumes_snapshot.go b/lxd/storage_volumes_snapshot.go
index bd77b92bb6..b44e86cdd0 100644
--- a/lxd/storage_volumes_snapshot.go
+++ b/lxd/storage_volumes_snapshot.go
@@ -201,7 +201,7 @@ func storagePoolVolumeSnapshotsTypeGet(d *Daemon, r
*http.Request) response.Resp
resultString := []string{}
resultMap := []*api.StorageVolumeSnapshot{}
for _, volume := range volumes {
- _, snapshotName, _ :=
shared.ContainerGetParentAndSnapshotName(volume)
+ _, snapshotName, _ :=
shared.ContainerGetParentAndSnapshotName(volume.Name)
if !recursion {
apiEndpoint, err :=
storagePoolVolumeTypeToAPIEndpoint(volumeType)
@@ -210,7 +210,7 @@ func storagePoolVolumeSnapshotsTypeGet(d *Daemon, r
*http.Request) response.Resp
}
resultString = append(resultString,
fmt.Sprintf("/%s/storage-pools/%s/volumes/%s/%s/snapshots/%s",
version.APIVersion, poolName, apiEndpoint, volumeName, snapshotName))
} else {
- _, vol, err :=
d.cluster.StoragePoolNodeVolumeGetType(volume, volumeType, poolID)
+ _, vol, err :=
d.cluster.StoragePoolNodeVolumeGetType(volume.Name, volumeType, poolID)
if err != nil {
continue
}
diff --git a/lxd/storage_volumes_utils.go b/lxd/storage_volumes_utils.go
index aa02763399..ac9fecd908 100644
--- a/lxd/storage_volumes_utils.go
+++ b/lxd/storage_volumes_utils.go
@@ -647,7 +647,7 @@ func storagePoolVolumeCreateInternal(state *state.State,
poolName string, vol *a
}
for _, snap := range snapshots {
- _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(snap)
+ _, snapName, _ :=
shared.ContainerGetParentAndSnapshotName(snap.Name)
_, err :=
storagePoolVolumeSnapshotCopyInternal(state, poolName, vol, snapName)
if err != nil {
return err
@@ -731,7 +731,7 @@ func storagePoolVolumeSnapshotDBCreateInternal(state
*state.State, dbArgs *db.St
}
// storagePoolVolumeSnapshotsGet returns a list of snapshots of the form
<volume>/<snapshot-name>.
-func storagePoolVolumeSnapshotsGet(s *state.State, pool string, volume string,
volType int) ([]string, error) {
+func storagePoolVolumeSnapshotsGet(s *state.State, pool string, volume string,
volType int) ([]db.StorageVolumeArgs, error) {
poolID, err := s.Cluster.StoragePoolGetID(pool)
if err != nil {
return nil, err
diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index a124173704..0ddead6d96 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -669,7 +669,7 @@ func (s *storageZfs) StoragePoolVolumeUpdate(writable
*api.StorageVolumePut, cha
return err
}
- if volumes[len(volumes)-1] != fmt.Sprintf("%s/%s",
s.volume.Name, writable.Restore) {
+ if volumes[len(volumes)-1].Name != fmt.Sprintf("%s/%s",
s.volume.Name, writable.Restore) {
return fmt.Errorf("ZFS can only restore from the latest
snapshot. Delete newer snapshots or copy the snapshot into a new volume
instead")
}
@@ -2834,7 +2834,7 @@ func (s *storageZfs) doCrossPoolStorageVolumeCopy(source
*api.StorageVolumeSourc
if !source.VolumeOnly {
for _, snap := range snapshots {
- srcMountPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(source.Pool, snap)
+ srcMountPoint :=
driver.GetStoragePoolVolumeSnapshotMountPoint(source.Pool, snap.Name)
_, err = rsync.LocalCopy(srcMountPoint, dstMountPoint,
bwlimit, true)
if err != nil {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
