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

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 modifies the LVM driver to be more like the ZFS driver in that it will not activate all LXD logical volumes (in so much as entries will not appear in `/dev`) until the volumes are needed, and will deactivate them when they are not needed.

This uses the `--setactivationskip y` option in LVM to mark a volume as one that should not be automatically activated when the volume group is scanned.

For non-thinpool volumes, any snapshots are activated and deactivated by LVM when the parent is activated/deactivated. However LVM does not allow a non-thin snapshot to be activated without the parent already being activated (similar to ZFS) so we also detect this and activate the parent volume when needed.

- [ ] Container testing
- [ ] VM testing
- [ ] Custom volume testing
- [ ] Storage patch to set `setactivationskip` to `y` on existing volumes
From 0c80e5d572eca9d6f149422e0649d885aa87a01e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:16:12 +0100
Subject: [PATCH 01/13] lxd/storage/drivers/driver/lvm/utils: Adds
 activateVolume and deactivateVolume functions

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go 
b/lxd/storage/drivers/driver_lvm_utils.go
index 4c6f543528..1b5191eeb8 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -737,3 +737,31 @@ func (d *lvm) thinPoolVolumeUsage(volDevPath string) 
(uint64, uint64, error) {
 
        return totalSize, usedSize, nil
 }
+
+// activateVolume activates an LVM logical volume if not already present. 
Returns true if activated, false if not.
+func (d *lvm) activateVolume(volDevPath string) (bool, error) {
+       if !shared.PathExists(volDevPath) {
+               _, err := shared.RunCommand("lvchange", "--activate", "y", 
"--ignoreactivationskip", volDevPath)
+               if err != nil {
+                       return false, errors.Wrapf(err, "Failed to activate LVM 
logical volume %q", volDevPath)
+               }
+               d.logger.Debug("Activated logical volume", log.Ctx{"dev": 
volDevPath})
+               return true, nil
+       }
+
+       return false, nil
+}
+
+// deactivateVolume deactivates an LVM logical volume if present. Returns true 
if deactivated, false if not.
+func (d *lvm) deactivateVolume(volDevPath string) (bool, error) {
+       if shared.PathExists(volDevPath) {
+               _, err := shared.RunCommand("lvchange", "--activate", "n", 
"--ignoreactivationskip", volDevPath)
+               if err != nil {
+                       return false, errors.Wrapf(err, "Failed to deactivate 
LVM logical volume %q", volDevPath)
+               }
+               d.logger.Debug("Deactivated logical volume", log.Ctx{"dev": 
volDevPath})
+               return true, nil
+       }
+
+       return false, nil
+}

From 1e59e3b34ef4156624ce3c8a8c343721db5154ba Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:17:06 +0100
Subject: [PATCH 02/13] lxd/storage/drivers/driver/lvm/utils: Set
 --setactivationskip on in createLogicalVolume

So that volume devices are not automatically activated when the storage pool is 
started.

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go 
b/lxd/storage/drivers/driver_lvm_utils.go
index 1b5191eeb8..cbc68b2fe5 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -390,6 +390,20 @@ func (d *lvm) createLogicalVolume(vgName, thinPoolName 
string, vol Volume, makeT
                }
        }
 
+       isRecent, err := d.lvmVersionIsAtLeast(lvmVersion, "2.02.99")
+       if err != nil {
+               return errors.Wrapf(err, "Error checking LVM version")
+       }
+
+       if isRecent {
+               // Disable auto activation of volume on LVM versions that 
support it.
+               // Must be done after volume create so that zeroing and 
signature wiping can take place.
+               _, err := shared.RunCommand("lvchange", "--setactivationskip", 
"y", volDevPath)
+               if err != nil {
+                       return errors.Wrapf(err, "Failed to set activation skip 
on LVM logical volume %q", volDevPath)
+               }
+       }
+
        d.logger.Debug("Logical volume created", log.Ctx{"vg_name": vgName, 
"lv_name": lvFullName, "size": fmt.Sprintf("%db", lvSizeBytes), "fs": 
d.volumeFilesystem(vol)})
        return nil
 }

From f4292aa87e2e164c0f4ad089c4a673135187821b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:21:01 +0100
Subject: [PATCH 03/13] lxd/storage/drivers/driver/lvm/utils: Set
 --setactivationskip on in createLogicalVolumeSnapshot

So that snapshot volumes are not automatically activated.

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go 
b/lxd/storage/drivers/driver_lvm_utils.go
index cbc68b2fe5..cb18f4bbcb 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -421,7 +421,7 @@ func (d *lvm) createLogicalVolumeSnapshot(vgName string, 
srcVol, snapVol Volume,
        args := []string{"-n", snapLvName, "-s", srcVolDevPath}
 
        if isRecent {
-               args = append(args, "-kn")
+               args = append(args, "--setactivationskip", "y")
        }
 
        // If the source is not a thin volume the size needs to be specified.
@@ -457,15 +457,6 @@ func (d *lvm) createLogicalVolumeSnapshot(vgName string, 
srcVol, snapVol Volume,
        })
 
        targetVolDevPath := d.lvmDevPath(vgName, snapVol.volType, 
snapVol.contentType, snapVol.name)
-       if makeThinLv {
-               // Snapshots of thin logical volumes can be directly activated.
-               // Normal snapshots will complain about changing the origin 
(Which they never do.),
-               // so skip the activation since the logical volume will be 
automatically activated anyway.
-               _, err := shared.TryRunCommand("lvchange", "-ay", 
targetVolDevPath)
-               if err != nil {
-                       return "", err
-               }
-       }
 
        revert.Success()
        return targetVolDevPath, nil

From a84a4a4ec9ab9c5a9c30d8a922552bea06057f2e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:21:50 +0100
Subject: [PATCH 04/13] lxd/storage/drivers/driver/lvm/utils: Activate volume
 in copyThinpoolVolume when regeneration FS UUID

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

diff --git a/lxd/storage/drivers/driver_lvm_utils.go 
b/lxd/storage/drivers/driver_lvm_utils.go
index cb18f4bbcb..9a6092ce45 100644
--- a/lxd/storage/drivers/driver_lvm_utils.go
+++ b/lxd/storage/drivers/driver_lvm_utils.go
@@ -639,6 +639,11 @@ func (d *lvm) copyThinpoolVolume(vol, srcVol Volume, 
srcSnapshots []Volume, refr
                // volumes with the same UUID to be mounted at the same time). 
This should be done before volume
                // resize as some filesystems will need to mount the filesystem 
to resize.
                if renegerateFilesystemUUIDNeeded(d.volumeFilesystem(vol)) {
+                       _, err = d.activateVolume(volDevPath)
+                       if err != nil {
+                               return err
+                       }
+
                        d.logger.Debug("Regenerating filesystem UUID", 
log.Ctx{"dev": volDevPath, "fs": d.volumeFilesystem(vol)})
                        err = regenerateFilesystemUUID(d.volumeFilesystem(vol), 
volDevPath)
                        if err != nil {

From a79aed91a06a71db92d29839f3ef3a4f4711b628 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:22:37 +0100
Subject: [PATCH 05/13] lxd/storage/drivers/driver/lvm: Dont activate all
 volumes on pool mount

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

diff --git a/lxd/storage/drivers/driver_lvm.go 
b/lxd/storage/drivers/driver_lvm.go
index 1061c25e04..dbf3c6b5f4 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -501,12 +501,7 @@ func (d *lvm) Mount() (bool, error) {
                        return false, err
                }
                defer loopFile.Close()
-       }
-
-       // Activate volume group so that it's device is added to /dev.
-       _, err := shared.TryRunCommand("vgchange", "-ay", 
d.config["lvm.vg_name"])
-       if err != nil {
-               return false, err
+               return true, nil
        }
 
        return false, nil

From b63bd23241c26d092371e9cba60b28b0687c7715 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:23:30 +0100
Subject: [PATCH 06/13] lxd/storage/drivers/driver/lvm/volumes: Activate volume
 before generic copy in CreateVolumeFromCopy

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index a001ba1561..fffbda8320 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -137,6 +137,18 @@ func (d *lvm) CreateVolumeFromCopy(vol, srcVol Volume, 
copySnapshots bool, op *o
                return nil
        }
 
+       // Before doing a generic volume copy, we need to ensure volume (or 
snap volume parent) is activated to
+       // avoid failing with warnings about changing the origin of the 
snapshot when trying to activate it.
+       parent, _, _ := shared.InstanceGetParentAndSnapshotName(srcVol.Name())
+       volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], srcVol.volType, 
srcVol.contentType, parent)
+       activated, err := d.activateVolume(volDevPath)
+       if err != nil {
+               return err
+       }
+       if activated {
+               defer d.deactivateVolume(volDevPath)
+       }
+
        // Otherwise run the generic copy.
        return genericVFSCopyVolume(d, nil, vol, srcVol, srcSnapshots, false, 
op)
 }

From 211f88993457b17c69b64d3983669b5c9090dc85 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:23:57 +0100
Subject: [PATCH 07/13] lxd/storage/drivers/driver/lvm/volumes: Activate volume
 in SetVolumeQuota

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index fffbda8320..0ce1a37da2 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -366,6 +366,15 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, op 
*operations.Operation)
 
        logCtx := log.Ctx{"dev": volDevPath, "size": fmt.Sprintf("%db", 
sizeBytes)}
 
+       // Activate volume if needed.
+       activated, err := d.activateVolume(volDevPath)
+       if err != nil {
+               return err
+       }
+       if activated {
+               defer d.deactivateVolume(volDevPath)
+       }
+
        // Resize filesystem if needed.
        if vol.contentType == ContentTypeFS {
                if sizeBytes < oldSizeBytes {

From f09f9bf6c64798bd3139d18e31fd69bf8f678351 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:26:05 +0100
Subject: [PATCH 08/13] lxd/storage/drivers/driver/lvm/volumes: Activate volume
 in MountVolume

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index 0ce1a37da2..66399090fe 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -436,19 +436,26 @@ func (d *lvm) GetVolumeDiskPath(vol Volume) (string, 
error) {
        return "", ErrNotSupported
 }
 
-// MountVolume simulates mounting a volume. As dir driver doesn't have volumes 
to mount it returns
-// false indicating that there is no need to issue an unmount.
+// MountVolume mounts a volume. Returns true if this volume was our mount.
 func (d *lvm) MountVolume(vol Volume, op *operations.Operation) (bool, error) {
-       mountPath := vol.MountPath()
+       var err error
+       activated := false
+
+       // Activate LVM volume if needed.
+       volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, 
vol.contentType, vol.name)
+       activated, err = d.activateVolume(volDevPath)
+       if err != nil {
+               return false, err
+       }
 
        // Check if already mounted.
+       mountPath := vol.MountPath()
        if vol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) {
-               err := vol.EnsureMountPath()
+               err = vol.EnsureMountPath()
                if err != nil {
                        return false, err
                }
 
-               volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], 
vol.volType, vol.contentType, vol.name)
                mountFlags, mountOptions := 
resolveMountOptions(d.volumeMountOptions(vol))
                err = TryMount(volDevPath, mountPath, d.volumeFilesystem(vol), 
mountFlags, mountOptions)
                if err != nil {
@@ -465,7 +472,7 @@ func (d *lvm) MountVolume(vol Volume, op 
*operations.Operation) (bool, error) {
                return d.MountVolume(fsVol, op)
        }
 
-       return false, nil
+       return activated, nil
 }
 
 // UnmountVolume simulates unmounting a volume. As dir driver doesn't have 
volumes to unmount it

From 57c982d60c3305240189206cc62a4586e039e8fc Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:27:19 +0100
Subject: [PATCH 09/13] lxd/storage/drivers/driver/lvm/volumes: Deactivate
 volume in UnmountVolume

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index 66399090fe..8449e9e1dd 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -475,23 +475,45 @@ func (d *lvm) MountVolume(vol Volume, op 
*operations.Operation) (bool, error) {
        return activated, nil
 }
 
-// UnmountVolume simulates unmounting a volume. As dir driver doesn't have 
volumes to unmount it
-// returns false indicating the volume was already unmounted.
+// UnmountVolume unmounts a volume. Returns true if we unmounted.
 func (d *lvm) UnmountVolume(vol Volume, op *operations.Operation) (bool, 
error) {
+       var err error
+       volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, 
vol.contentType, vol.name)
        mountPath := vol.MountPath()
 
        // Check if already mounted.
-       if shared.IsMountPoint(mountPath) {
-               err := TryUnmount(mountPath, 0)
+       if vol.contentType == ContentTypeFS && shared.IsMountPoint(mountPath) {
+               err = TryUnmount(mountPath, 0)
                if err != nil {
                        return false, errors.Wrapf(err, "Failed to unmount LVM 
logical volume")
                }
                d.logger.Debug("Unmounted logical volume", log.Ctx{"path": 
mountPath})
 
+               // We only deactivate filesystem volumes if an unmount was 
needed to better align with our
+               // unmount return value indicator.
+               _, err = d.deactivateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+
                return true, nil
        }
 
-       return false, nil
+       deactivated := false
+       if vol.contentType == ContentTypeBlock {
+               deactivated, err = d.deactivateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+       }
+
+       // For VMs, unmount the filesystem volume.
+       if vol.IsVMBlock() {
+               fsVol := vol.NewVMBlockFilesystemVolume()
+               return d.UnmountVolume(fsVol, op)
+       }
+
+       return deactivated, nil
 }
 
 // RenameVolume renames a volume and its snapshots.

From 3472d062b2031e0bdef1173c7ab98a837180de1d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:27:47 +0100
Subject: [PATCH 10/13] lxd/storage/drivers/driver/lvm/volumes: Acticate volume
 before generic migrate in MigrateVolume

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index 8449e9e1dd..570f6e666a 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -590,6 +590,18 @@ func (d *lvm) RenameVolume(vol Volume, newVolName string, 
op *operations.Operati
 
 // MigrateVolume sends a volume for migration.
 func (d *lvm) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs 
*migration.VolumeSourceArgs, op *operations.Operation) error {
+       // Before doing a generic volume migration, we need to ensure volume 
(or snap volume parent) is activated
+       // to avoid failing with warnings about changing the origin of the 
snapshot when trying to activate it.
+       parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name())
+       volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, 
vol.contentType, parent)
+       activated, err := d.activateVolume(volDevPath)
+       if err != nil {
+               return err
+       }
+       if activated {
+               defer d.deactivateVolume(volDevPath)
+       }
+
        return genericVFSMigrateVolume(d, d.state, vol, conn, volSrcArgs, op)
 }
 

From fd374e5bdbf7615c7b36ea3a58a3eb8b7fa8b508 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:28:41 +0100
Subject: [PATCH 11/13] lxd/storage/drivers/driver/lvm/volumes: Activate volume
 in MountVolumeSnapshot

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index 570f6e666a..ad44269a54 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -708,6 +708,7 @@ func (d *lvm) DeleteVolumeSnapshot(snapVol Volume, op 
*operations.Operation) err
 
 // MountVolumeSnapshot sets up a read-only mount on top of the snapshot to 
avoid accidental modifications.
 func (d *lvm) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) 
(bool, error) {
+       var err error
        mountPath := snapVol.MountPath()
 
        // Check if already mounted.
@@ -715,7 +716,7 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
                revert := revert.New()
                defer revert.Fail()
 
-               err := snapVol.EnsureMountPath()
+               err = snapVol.EnsureMountPath()
                if err != nil {
                        return false, err
                }
@@ -733,13 +734,14 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
                // we do not want to modify a snapshot in case it is corrupted 
for some reason, so at mount time
                // we take another snapshot of the snapshot, regenerate the 
temporary snapshot's UUID and then
                // mount that.
-               if renegerateFilesystemUUIDNeeded(d.volumeFilesystem(snapVol)) {
+               regenerateFSUUID := 
renegerateFilesystemUUIDNeeded(d.volumeFilesystem(snapVol))
+               if regenerateFSUUID {
                        // Instantiate a new volume to be the temporary 
writable snapshot.
                        tmpVolName := fmt.Sprintf("%s%s", snapVol.name, 
tmpVolSuffix)
                        tmpVol := NewVolume(d, d.name, snapVol.volType, 
snapVol.contentType, tmpVolName, snapVol.config, snapVol.poolConfig)
 
                        // Create writable snapshot from source snapshot named 
with a tmpVolSuffix suffix.
-                       _, err := 
d.createLogicalVolumeSnapshot(d.config["lvm.vg_name"], snapVol, tmpVol, false, 
d.usesThinpool())
+                       _, err = 
d.createLogicalVolumeSnapshot(d.config["lvm.vg_name"], snapVol, tmpVol, false, 
d.usesThinpool())
                        if err != nil {
                                return false, errors.Wrapf(err, "Error creating 
temporary LVM logical volume snapshot")
                        }
@@ -748,8 +750,20 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
                                
d.removeLogicalVolume(d.lvmDevPath(d.config["lvm.vg_name"], tmpVol.volType, 
tmpVol.contentType, tmpVol.name))
                        })
 
-                       tmpVolDevPath := d.lvmDevPath(d.config["lvm.vg_name"], 
tmpVol.volType, tmpVol.contentType, tmpVol.name)
-                       tmpVolFsType := d.volumeFilesystem(tmpVol)
+                       // We are going to mount the temporary volume instead.
+                       mountVol = tmpVol
+               }
+
+               volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], 
mountVol.volType, mountVol.contentType, mountVol.name)
+
+               // Activate volume if needed.
+               _, err = d.activateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+
+               if regenerateFSUUID {
+                       tmpVolFsType := d.volumeFilesystem(mountVol)
 
                        // When mounting XFS filesystems temporarily we can use 
the nouuid option rather than fully
                        // regenerating the filesystem UUID.
@@ -759,19 +773,15 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
                                        mountOptions += ",nouuid"
                                }
                        } else {
-                               d.logger.Debug("Regenerating filesystem UUID", 
log.Ctx{"dev": tmpVolDevPath, "fs": d.volumeFilesystem(tmpVol)})
-                               err = 
regenerateFilesystemUUID(d.volumeFilesystem(tmpVol), tmpVolDevPath)
+                               d.logger.Debug("Regenerating filesystem UUID", 
log.Ctx{"dev": volDevPath, "fs": tmpVolFsType})
+                               err = 
regenerateFilesystemUUID(d.volumeFilesystem(mountVol), volDevPath)
                                if err != nil {
                                        return false, err
                                }
                        }
-
-                       // We are going to mount the temporary volume instead.
-                       mountVol = tmpVol
                }
 
                // Finally attempt to mount the volume that needs mounting.
-               volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], 
mountVol.volType, mountVol.contentType, mountVol.name)
                err = TryMount(volDevPath, mountPath, 
d.volumeFilesystem(mountVol), mountFlags|unix.MS_RDONLY, mountOptions)
                if err != nil {
                        return false, errors.Wrapf(err, "Failed to mount LVM 
snapshot volume")
@@ -782,13 +792,24 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
                return true, nil
        }
 
+       activated := false
+       if snapVol.contentType == ContentTypeBlock {
+               volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], 
snapVol.volType, snapVol.contentType, snapVol.name)
+
+               // Activate volume if needed.
+               activated, err = d.activateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+       }
+
        // For VMs, mount the filesystem volume.
        if snapVol.IsVMBlock() {
                fsVol := snapVol.NewVMBlockFilesystemVolume()
                return d.MountVolumeSnapshot(fsVol, op)
        }
 
-       return false, nil
+       return activated, nil
 }
 
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a 
snapshot.

From 9fd213561bdb32ced9746350c247b8265005e7d3 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:30:02 +0100
Subject: [PATCH 12/13] lxd/storage/drivers/driver/lvm/volumes: Deactivate
 volume in UnmountVolumeSnapshot

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index ad44269a54..a2dc7ff8e1 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -815,11 +815,13 @@ func (d *lvm) MountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (boo
 // UnmountVolumeSnapshot removes the read-only mount placed on top of a 
snapshot.
 // If a temporary snapshot volume exists then it will attempt to remove it.
 func (d *lvm) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) 
(bool, error) {
+       var err error
+       volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], snapVol.volType, 
snapVol.contentType, snapVol.name)
        mountPath := snapVol.MountPath()
 
        // Check if already mounted.
        if snapVol.contentType == ContentTypeFS && 
shared.IsMountPoint(mountPath) {
-               err := TryUnmount(mountPath, 0)
+               err = TryUnmount(mountPath, 0)
                if err != nil {
                        return false, errors.Wrapf(err, "Failed to unmount LVM 
snapshot volume")
                }
@@ -840,16 +842,31 @@ func (d *lvm) UnmountVolumeSnapshot(snapVol Volume, op 
*operations.Operation) (b
                        }
                }
 
+               // We only deactivate filesystem volumes if an unmount was 
needed to better align with our
+               // unmount return value indicator.
+               _, err = d.deactivateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+
                return true, nil
        }
 
+       deactivated := false
+       if snapVol.contentType == ContentTypeBlock {
+               deactivated, err = d.deactivateVolume(volDevPath)
+               if err != nil {
+                       return false, err
+               }
+       }
+
        // For VMs, unmount the filesystem volume.
        if snapVol.IsVMBlock() {
                fsVol := snapVol.NewVMBlockFilesystemVolume()
                return d.UnmountVolumeSnapshot(fsVol, op)
        }
 
-       return false, nil
+       return deactivated, nil
 }
 
 // VolumeSnapshots returns a list of snapshots for the volume.

From 527048b8359a37d0cafa5c57cc65c7330e0d105d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 5 May 2020 14:30:25 +0100
Subject: [PATCH 13/13] lxd/storage/drivers/driver/lvm/volumes: Activate volume
 before FS UUID regen in RestoreVolume

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

diff --git a/lxd/storage/drivers/driver_lvm_volumes.go 
b/lxd/storage/drivers/driver_lvm_volumes.go
index a2dc7ff8e1..ed280d4f96 100644
--- a/lxd/storage/drivers/driver_lvm_volumes.go
+++ b/lxd/storage/drivers/driver_lvm_volumes.go
@@ -984,6 +984,11 @@ func (d *lvm) RestoreVolume(vol Volume, snapshotName 
string, op *operations.Oper
 
                // If the volume's filesystem needs to have its UUID 
regenerated to allow mount then do so now.
                if vol.contentType == ContentTypeFS && 
renegerateFilesystemUUIDNeeded(d.volumeFilesystem(vol)) {
+                       _, err = d.activateVolume(volDevPath)
+                       if err != nil {
+                               return err
+                       }
+
                        d.logger.Debug("Regenerating filesystem UUID", 
log.Ctx{"dev": volDevPath, "fs": d.volumeFilesystem(vol)})
                        err = regenerateFilesystemUUID(d.volumeFilesystem(vol), 
volDevPath)
                        if err != nil {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to