From c85b10fabed3f156ebccb81bd9193f09dc8c629a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Fri, 19 Jul 2024 14:10:54 -0400 Subject: [PATCH 1/2] incusd/storage/lvm: Require an exclusive lock during snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- .../server/storage/drivers/driver_lvm_utils.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/server/storage/drivers/driver_lvm_utils.go b/internal/server/storage/drivers/driver_lvm_utils.go index 9bde7526092..475e2988fd0 100644 --- a/internal/server/storage/drivers/driver_lvm_utils.go +++ b/internal/server/storage/drivers/driver_lvm_utils.go @@ -433,6 +433,23 @@ func (d *lvm) createLogicalVolumeSnapshot(vgName string, srcVol Volume, snapVol revert := revert.New() defer revert.Fail() + // If clustered, we need to acquire exclusive write access for this operation. + if d.clustered && !makeThinLv { + parent, _, _ := api.GetParentAndSnapshotName(srcVol.Name()) + volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], srcVol.volType, srcVol.contentType, parent) + + if util.PathExists(volDevPath) { + _, err := subprocess.RunCommand("lvchange", "--activate", "ey", "--ignoreactivationskip", volDevPath) + if err != nil { + return "", fmt.Errorf("Failed to acquire exclusive lock on LVM logical volume %q: %w", volDevPath, err) + } + + defer func() { + _, _ = subprocess.RunCommand("lvchange", "--activate", "sy", "--ignoreactivationskip", volDevPath) + }() + } + } + _, err = subprocess.TryRunCommand("lvcreate", args...) if err != nil { return "", err From 64966418de3372ac2c563c58fe56852987b5ad75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Fri, 19 Jul 2024 14:38:03 -0400 Subject: [PATCH 2/2] incusd/storage/lvm: Properly handle activation during resize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LVM resize requires the volume be fully deactivated. For some reason our LVM logic would always fully activate the volume, which is needed for shrinking but was still doing it during growth. Note that this is only needed when not using thin pools so this is likely why this slipped through the cracks. Signed-off-by: Stéphane Graber --- .../storage/drivers/driver_lvm_volumes.go | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/internal/server/storage/drivers/driver_lvm_volumes.go b/internal/server/storage/drivers/driver_lvm_volumes.go index a926c8d9ab4..6050f975bf5 100644 --- a/internal/server/storage/drivers/driver_lvm_volumes.go +++ b/internal/server/storage/drivers/driver_lvm_volumes.go @@ -442,16 +442,6 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op l := d.logger.AddContext(logger.Ctx{"dev": volDevPath, "size": fmt.Sprintf("%db", sizeBytes)}) - // Activate volume if needed. - activated, err := d.activateVolume(vol) - if err != nil { - return err - } - - if activated { - defer func() { _, _ = d.deactivateVolume(vol) }() - } - inUse := vol.MountInUse() // Resize filesystem if needed. @@ -467,6 +457,12 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op return ErrInUse // We don't allow online shrinking of filesytem volumes. } + // Activate volume if needed. + _, err := d.activateVolume(vol) + if err != nil { + return err + } + // Shrink filesystem first. // Pass allowUnsafeResize to allow disabling of filesystem resize safety checks. // We do this as a separate step rather than passing -r to lvresize in resizeLogicalVolume @@ -474,6 +470,13 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op // otherwise by passing -f to lvresize (required for other reasons) this would then pass // -f onto resize2fs as well. err = shrinkFileSystem(fsType, volDevPath, vol, sizeBytes, allowUnsafeResize) + if err != nil { + _, _ = d.deactivateVolume(vol) + return err + } + + // Deactivate the volume if needed. + _, err = d.deactivateVolume(vol) if err != nil { return err } @@ -492,6 +495,16 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op return err } + // Activate the volume if needed. + _, err := d.activateVolume(vol) + if err != nil { + return err + } + + defer func() { + _, _ = d.deactivateVolume(vol) + }() + // Grow the filesystem to fill block device. err = growFileSystem(fsType, volDevPath, vol) if err != nil { @@ -518,6 +531,16 @@ func (d *lvm) SetVolumeQuota(vol Volume, size string, allowUnsafeResize bool, op return err } + // Activate the volume if needed. + _, err := d.activateVolume(vol) + if err != nil { + return err + } + + defer func() { + _, _ = d.deactivateVolume(vol) + }() + // Move the VM GPT alt header to end of disk if needed (not needed in unsafe resize mode as it is // expected the caller will do all necessary post resize actions themselves). if vol.IsVMBlock() && !allowUnsafeResize {