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