The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8147
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) === Introduces the concept of storage volume mount/unmount reference counting. When a storage volume is mounted for the first time, a global reference counter is created and incremented to 1. If other operations on the storage volume also need it while it is already mounted, then subsequent requests to mount the volume will just increment the reference counter again. Then once each operation has finished with the volume they unmount the volume, and the reference counter is decremented. If the reference counter is not yet at zero, then other operations are still using the volume and an `ErrInUse` error is returned. The last unmount request that detects the reference counter is at zero will actually perform the unmount of the volume. This work was done primarily to address the following group of issues related to copying ZFS VM snapshot as another instance: - https://github.com/lxc/lxd/issues/8113 - https://discuss.linuxcontainers.org/t/copying-vm-snapshot-to-other-host-not-working-attempts-to-create-a-container/9363 - https://discuss.linuxcontainers.org/t/lxc-copy-container-ok-lxc-copy-vm-fails-with-failed-to-mount-permission-denied/9237 Reference counting is required to solve these problems because in order to activate a ZFS block volume snapshot, the parent volume also needs to be activated. However the current storage interface didn't provide a clean way for the parent volume to be unmounted (if needed) once the snapshot volume was unmounted. By introducing reference counters we can just request that the parent volume always be unmounted when unmounting a snapshot, and if others are using it, the unmount request will fail. However reference counting also helps with several other classes of problems: - Instance file operations (push/pull/delete etc) while the instance is running. - Detecting when a custom volume is mounted in multiple running instances and deciding whether to unmount when one instance is stopped (which was causing shutdown delays and errors). In order to support reference counting, everywhere that mounts a volume needed to be reviewed to ensure that where appropriate the equivalent unmount request was made when the operation completed, otherwise the reference counter would not be decremented and the volume would be left mounted.
From 47904e03ef85f80fbfbfe7461314a3cdde36c84a Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 10 Nov 2020 11:34:46 +0000 Subject: [PATCH 01/39] lxd/storage/drivers/driver/zfs/volumes: Remove workarounds for snapshot volume mounting And unmount parent volume when unmounting snapshot volume (to undo parent volume activation). Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_zfs_volumes.go | 48 +++++++---------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go index 463c97114b..07ae48a883 100644 --- a/lxd/storage/drivers/driver_zfs_volumes.go +++ b/lxd/storage/drivers/driver_zfs_volumes.go @@ -1215,18 +1215,6 @@ func (d *zfs) RenameVolume(vol Volume, newVolName string, op *operations.Operati func (d *zfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *migration.VolumeSourceArgs, op *operations.Operation) error { // Handle simple rsync and block_and_rsync through generic. if volSrcArgs.MigrationType.FSType == migration.MigrationFSType_RSYNC || volSrcArgs.MigrationType.FSType == migration.MigrationFSType_BLOCK_AND_RSYNC { - // Before doing a generic volume migration, we need to ensure volume (or snap volume parent) is - // activated to avoid issues activating the snapshot volume device. - parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name()) - parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig) - ourMount, err := d.MountVolume(parentVol, op) - if err != nil { - return err - } - if ourMount { - defer d.UnmountVolume(parentVol, false, op) - } - return genericVFSMigrateVolume(d, d.state, vol, conn, volSrcArgs, op) } else if volSrcArgs.MigrationType.FSType != migration.MigrationFSType_ZFS { return ErrNotSupported @@ -1317,21 +1305,6 @@ func (d *zfs) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *mig func (d *zfs) BackupVolume(vol Volume, tarWriter *instancewriter.InstanceTarWriter, optimized bool, snapshots bool, op *operations.Operation) error { // Handle the non-optimized tarballs through the generic packer. if !optimized { - // For block volumes that are exporting snapshots, we need to activate parent volume first so that - // the snapshot volumes can have their devices accessible. - if vol.contentType == ContentTypeBlock && snapshots { - parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name()) - parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig) - ourMount, err := d.MountVolume(parentVol, op) - if err != nil { - return err - } - - if ourMount { - defer d.UnmountVolume(parentVol, false, op) - } - } - // Because the generic backup method will not take a consistent backup if files are being modified // as they are copied to the tarball, as ZFS allows us to take a quick snapshot without impacting // the parent volume we do so here to ensure the backup taken is consistent. @@ -1624,14 +1597,16 @@ func (d *zfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo var ourMountBlock, ourMountFs bool - // For block devices, we make them appear by enabling volmode=dev and snapdev=visible on the - // parent volume. If we have to enable this volmode=dev on the parent, then we will return ourMount true - // so that the caller knows to call UnmountVolumeSnapshot to undo this action, but if it is already set - // then we will return ourMount false, because we don't want to deactivate the parent volume's device if it - // is already in use. + // For block devices, we make them appear by enabling volmode=dev and snapdev=visible on the parent volume. if snapVol.contentType == ContentTypeBlock { + // Ensure snap volume parent is activated to avoid issues activating the snapshot volume device. parent, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.Name()) parentVol := NewVolume(d, d.Name(), snapVol.volType, snapVol.contentType, parent, snapVol.config, snapVol.poolConfig) + _, err = d.MountVolume(parentVol, op) + if err != nil { + return false, err + } + parentDataset := d.dataset(parentVol, false) // Check if parent already active. @@ -1640,7 +1615,7 @@ func (d *zfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo return false, err } - // Order is important here, the volmode=dev must be set before snapdev=visible otherwise + // Order is important here, the parent volmode=dev must be set before snapdev=visible otherwise // it won't take effect. if parentVolMode != "dev" { return false, fmt.Errorf("Parent block volume needs to be mounted first") @@ -1712,6 +1687,13 @@ func (d *zfs) UnmountVolumeSnapshot(snapVol Volume, op *operations.Operation) (b } d.logger.Debug("Deactivated ZFS snapshot volume", log.Ctx{"dev": snapshotDataset}) + + // Ensure snap volume parent is deactivated in case we activated it when mounting snapshot. + _, err = d.UnmountVolume(parentVol, false, op) + if err != nil { + return false, err + } + return true, nil } From 714c6d0b5946f53a4e4f78fe2df9d782befcf7be Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Tue, 10 Nov 2020 15:49:08 +0000 Subject: [PATCH 02/39] lxd/refcount: Adds ref counting package This maintains global ref counters under a mutex protected map. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/refcount/refcount.go | 52 ++++++++++++++++++++++++++++++++++ lxd/refcount/refcount_test.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 lxd/refcount/refcount.go create mode 100644 lxd/refcount/refcount_test.go diff --git a/lxd/refcount/refcount.go b/lxd/refcount/refcount.go new file mode 100644 index 0000000000..168ae54e65 --- /dev/null +++ b/lxd/refcount/refcount.go @@ -0,0 +1,52 @@ +package refcount + +import ( + "fmt" + "sync" +) + +var refCounters = map[string]uint{} + +// refCounterMutex is used to access refCounters safely. +var refCounterMutex sync.Mutex + +// Increment increases a refCounter by the value. If the ref counter doesn't exist, a new one is created. +// The counter's new value is returned. +func Increment(refCounter string, value uint) uint { + refCounterMutex.Lock() + defer refCounterMutex.Unlock() + + v := refCounters[refCounter] + oldValue := v + v = v + value + + if v < oldValue { + panic(fmt.Sprintf("Ref counter %q overflowed from %d to %d", refCounter, oldValue, v)) + } + + refCounters[refCounter] = v + + return v +} + +// Decrement decreases a refCounter by the value. If the ref counter doesn't exist, a new one is created. +// The counter's new value is returned. A counter cannot be decreased below zero. +func Decrement(refCounter string, value uint) uint { + refCounterMutex.Lock() + defer refCounterMutex.Unlock() + + v := refCounters[refCounter] + if v < value { + v = 0 + } else { + v = v - value + } + + if v == 0 { + delete(refCounters, refCounter) + } else { + refCounters[refCounter] = v + } + + return v +} diff --git a/lxd/refcount/refcount_test.go b/lxd/refcount/refcount_test.go new file mode 100644 index 0000000000..af72dce61f --- /dev/null +++ b/lxd/refcount/refcount_test.go @@ -0,0 +1,53 @@ +package refcount + +import ( + "fmt" +) + +func Example_Increment() { + refCounter1 := "testinc1" + fmt.Println(Increment(refCounter1, 1)) + fmt.Println(Increment(refCounter1, 1)) + fmt.Println(Increment(refCounter1, 2)) + + refCounter2 := "testinc2" + fmt.Println(Increment(refCounter2, 1)) + fmt.Println(Increment(refCounter2, 10)) + + // Test overflow panic. + defer func() { + if r := recover(); r != nil { + fmt.Printf("Recovered: %v\n", r) + } + }() + var maxUint uint = ^uint(0) + fmt.Println(Increment(refCounter2, maxUint)) + + // Output: 1 + // 2 + // 4 + // 1 + // 11 + // Recovered: Ref counter "testinc2" overflowed from 11 to 10 +} + +func Example_Decrement() { + refCounter1 := "testdec1" + fmt.Println(Increment(refCounter1, 10)) + fmt.Println(Decrement(refCounter1, 1)) + fmt.Println(Decrement(refCounter1, 2)) + + refCounter2 := "testdec2" + fmt.Println(Decrement(refCounter2, 1)) + fmt.Println(Increment(refCounter2, 1)) + fmt.Println(Decrement(refCounter2, 1)) + fmt.Println(Decrement(refCounter2, 1)) + + // Output: 10 + // 9 + // 7 + // 0 + // 1 + // 0 + // 0 +} From aed4cdb61fe8bdc5e322d20abc30b5cc653a7d48 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Wed, 11 Nov 2020 11:38:58 +0000 Subject: [PATCH 03/39] lxd/storage/drivers/volume: Adds ref counting functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/volume.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index e03873f1a7..ab897f6b6e 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -8,6 +8,7 @@ import ( "github.com/lxc/lxd/lxd/locking" "github.com/lxc/lxd/lxd/operations" + "github.com/lxc/lxd/lxd/refcount" "github.com/lxc/lxd/lxd/revert" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/units" @@ -134,9 +135,28 @@ func (v Volume) MountPath() string { return GetVolumeMountPath(v.pool, v.volType, v.name) } +// mountLockName returns the lock name to use for mount/unmount operations on a volume. +func (v Volume) mountLockName() string { + return OperationLockName("Mount", v.pool, v.volType, v.contentType, v.name) +} + // MountLock attempts to lock the mount lock for the volume and returns the UnlockFunc. func (v Volume) MountLock() locking.UnlockFunc { - return locking.Lock(OperationLockName("MountLock", v.pool, v.volType, v.contentType, v.name)) + return locking.Lock(v.mountLockName()) +} + +// MountRefCountIncrement increments the mount ref counter for the volume and returns the new value. +func (v Volume) MountRefCountIncrement() uint { + counter := refcount.Increment(v.mountLockName(), 0, 1) + v.driver.Logger().Error("tomp MountRefCountIncrement", "counter", v.mountLockName(), "counter", counter) + return counter +} + +// MountRefCountDecrement decrements the mount ref counter for the volume and returns the new value. +func (v Volume) MountRefCountDecrement() uint { + counter := refcount.Decrement(v.mountLockName(), 0, 1) + v.driver.Logger().Error("tomp MountRefCountDecrement", "counter", v.mountLockName(), "counter", counter) + return counter } // EnsureMountPath creates the volume's mount path if missing, then sets the correct permission for the type. From 8684ffc3d6b7e2ef5e74d0f067c7a47c80fabb0d Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 08:58:46 +0000 Subject: [PATCH 04/39] lxd/storage/pool/interface: Removes OurMount from MountInfo struct With ref counting we no longer expect the caller to only call equivalent unmount if it was "our mount", now we expect them to do it always and the ref counting will keep track of whether to actually unmount. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/pool_interface.go | 1 - 1 file changed, 1 deletion(-) diff --git a/lxd/storage/pool_interface.go b/lxd/storage/pool_interface.go index d836b5d3ac..2ee6876379 100644 --- a/lxd/storage/pool_interface.go +++ b/lxd/storage/pool_interface.go @@ -15,7 +15,6 @@ import ( // MountInfo represents info about the result of a mount operation. type MountInfo struct { - OurMount bool // Whether a mount was needed. DiskPath string // The location of the block disk (if supported). } From 834676b07ee3469252a9f942a5c54219512ce1d4 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 08:59:58 +0000 Subject: [PATCH 05/39] lxd/storage/pool/interface: Removes "our mount" bool return value from MountCustomVolume Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/pool_interface.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/storage/pool_interface.go b/lxd/storage/pool_interface.go index 2ee6876379..5423c89829 100644 --- a/lxd/storage/pool_interface.go +++ b/lxd/storage/pool_interface.go @@ -78,7 +78,7 @@ type Pool interface { DeleteCustomVolume(projectName string, volName string, op *operations.Operation) error GetCustomVolumeDisk(projectName string, volName string) (string, error) GetCustomVolumeUsage(projectName string, volName string) (int64, error) - MountCustomVolume(projectName string, volName string, op *operations.Operation) (bool, error) + MountCustomVolume(projectName string, volName string, op *operations.Operation) error UnmountCustomVolume(projectName string, volName string, op *operations.Operation) (bool, error) // Custom volume snapshots. From 7c300b4f662476cfa38dc8776f5a13fb991835a2 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:00:46 +0000 Subject: [PATCH 06/39] lxd/storage/drivers/interface: Removes "our mount" bool return value from MountVolume Ref counting keeps track of whether an unmount should proceed or not, so no need for indicating to caller whether they need to unmount or not. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/interface.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go index 7913b221e1..82450ad5e5 100644 --- a/lxd/storage/drivers/interface.go +++ b/lxd/storage/drivers/interface.go @@ -57,9 +57,8 @@ type Driver interface { SetVolumeQuota(vol Volume, size string, op *operations.Operation) error GetVolumeDiskPath(vol Volume) (string, error) - // MountVolume mounts a storage volume, returns true if we caused a new mount, false if - // already mounted. - MountVolume(vol Volume, op *operations.Operation) (bool, error) + // MountVolume mounts a storage volume (if not mounted) and increments reference counter. + MountVolume(vol Volume, op *operations.Operation) error // MountVolumeSnapshot mounts a storage volume snapshot as readonly, returns true if we // caused a new mount, false if already mounted. From aae0644b5d5eee9c0ffda8f8ad4c898376e4cfd0 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:01:47 +0000 Subject: [PATCH 07/39] lxd/storage/drivers/errors: Adds ErrInUse error For indicating when an unmount cannot proceed as volume is still in use. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/errors.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lxd/storage/drivers/errors.go b/lxd/storage/drivers/errors.go index 838452192e..d0acd43cd9 100644 --- a/lxd/storage/drivers/errors.go +++ b/lxd/storage/drivers/errors.go @@ -16,6 +16,9 @@ var ErrNotSupported = fmt.Errorf("Not supported") // ErrCannotBeShrunk is the "Cannot be shrunk" error var ErrCannotBeShrunk = fmt.Errorf("Cannot be shrunk") +// ErrInUse indicates operation cannot proceed as resource is in use. +var ErrInUse = fmt.Errorf("In use") + // ErrDeleteSnapshots is a special error used to tell the backend to delete more recent snapshots type ErrDeleteSnapshots struct { Snapshots []string From 0a709109a86f33a5406e8b4659d7b666807c7855 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:02:17 +0000 Subject: [PATCH 08/39] lxd/storage/drivers/drivers/mock: Updates MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/drivers_mock.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lxd/storage/drivers/drivers_mock.go b/lxd/storage/drivers/drivers_mock.go index b3967e5970..9f1c379179 100644 --- a/lxd/storage/drivers/drivers_mock.go +++ b/lxd/storage/drivers/drivers_mock.go @@ -141,10 +141,9 @@ func (d *mock) GetVolumeDiskPath(vol Volume) (string, error) { return "", nil } -// 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. -func (d *mock) MountVolume(vol Volume, op *operations.Operation) (bool, error) { - return false, nil +// MountVolume simulates mounting a volume. +func (d *mock) MountVolume(vol Volume, op *operations.Operation) error { + return nil } // UnmountVolume simulates unmounting a volume. As dir driver doesn't have volumes to unmount it From d2331dcf323ab866d91d1af31c01b4617b071982 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:03:26 +0000 Subject: [PATCH 09/39] lxd/storage/drivers/volume: Updates ref counting functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/volume.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index ab897f6b6e..938f2a2166 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -147,14 +147,14 @@ func (v Volume) MountLock() locking.UnlockFunc { // MountRefCountIncrement increments the mount ref counter for the volume and returns the new value. func (v Volume) MountRefCountIncrement() uint { - counter := refcount.Increment(v.mountLockName(), 0, 1) + counter := refcount.Increment(v.mountLockName(), 1) v.driver.Logger().Error("tomp MountRefCountIncrement", "counter", v.mountLockName(), "counter", counter) return counter } // MountRefCountDecrement decrements the mount ref counter for the volume and returns the new value. func (v Volume) MountRefCountDecrement() uint { - counter := refcount.Decrement(v.mountLockName(), 0, 1) + counter := refcount.Decrement(v.mountLockName(), 1) v.driver.Logger().Error("tomp MountRefCountDecrement", "counter", v.mountLockName(), "counter", counter) return counter } From 117309ac894c95b2e44c241af3e24313168975d3 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:03:56 +0000 Subject: [PATCH 10/39] lxd/storage/drivers/utils: Error quoting in shrinkFileSystem Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/drivers/utils.go b/lxd/storage/drivers/utils.go index 09e10212b7..c07f91928f 100644 --- a/lxd/storage/drivers/utils.go +++ b/lxd/storage/drivers/utils.go @@ -475,7 +475,7 @@ func shrinkFileSystem(fsType string, devPath string, vol Volume, byteSize int64) case "": // if not specified, default to ext4. fallthrough case "xfs": - return errors.Wrapf(ErrCannotBeShrunk, `Shrinking not supported for filesystem type "%s". A dump, mkfs, and restore are required`, fsType) + return errors.Wrapf(ErrCannotBeShrunk, "Shrinking not supported for filesystem type %q. A dump, mkfs, and restore are required", fsType) case "ext4": return vol.UnmountTask(func(op *operations.Operation) error { output, err := shared.RunCommand("e2fsck", "-f", "-y", devPath) @@ -516,7 +516,7 @@ func shrinkFileSystem(fsType string, devPath string, vol Volume, byteSize int64) return nil }, nil) default: - return errors.Wrapf(ErrCannotBeShrunk, `Shrinking not supported for filesystem type "%s"`, fsType) + return errors.Wrapf(ErrCannotBeShrunk, "Shrinking not supported for filesystem type %q", fsType) } } From 532ac1011bff937072342b1eee2b30540fb2de41 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:04:23 +0000 Subject: [PATCH 11/39] lxd/storage/drivers/driver/btrfs/volumes: Updates MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_btrfs_volumes.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lxd/storage/drivers/driver_btrfs_volumes.go b/lxd/storage/drivers/driver_btrfs_volumes.go index e695b1e20c..fb0a22653f 100644 --- a/lxd/storage/drivers/driver_btrfs_volumes.go +++ b/lxd/storage/drivers/driver_btrfs_volumes.go @@ -755,9 +755,8 @@ func (d *btrfs) GetVolumeDiskPath(vol Volume) (string, error) { return genericVFSGetVolumeDiskPath(vol) } -// MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns -// false indicating that there is no need to issue an unmount. -func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +// MountVolume simulates mounting a volume. +func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() @@ -766,11 +765,11 @@ func (d *btrfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom { err := vol.EnsureMountPath() if err != nil { - return false, err + return err } } - return false, nil + return nil } // UnmountVolume simulates unmounting a volume. From d3c54a0cc91f4cf5734ccf82013f9d45d7d1bcdc Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:05:10 +0000 Subject: [PATCH 12/39] lxd/storage/drivers/driver/ceph/volumes: Adds ref counting to MountVolume and UnmountVolume Also restructures those functions to align with other storage driver approach. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_ceph_volumes.go | 131 ++++++++++++--------- 1 file changed, 77 insertions(+), 54 deletions(-) diff --git a/lxd/storage/drivers/driver_ceph_volumes.go b/lxd/storage/drivers/driver_ceph_volumes.go index dac36c8047..a785d3a9a6 100644 --- a/lxd/storage/drivers/driver_ceph_volumes.go +++ b/lxd/storage/drivers/driver_ceph_volumes.go @@ -923,43 +923,47 @@ func (d *ceph) GetVolumeDiskPath(vol Volume) (string, error) { return "", ErrNotSupported } -// MountVolume simulates mounting a volume. -func (d *ceph) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +// MountVolume mounts a volume and increments ref counter. Please call UnmountVolume() when done with the volume. +func (d *ceph) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() - mountPath := vol.MountPath() - // Activate RBD volume if needed. - ourMount, devPath, err := d.getRBDMappedDevPath(vol, true) + _, devPath, err := d.getRBDMappedDevPath(vol, true) if err != nil { - return false, err + return err } - if vol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) { - err := vol.EnsureMountPath() - if err != nil { - return false, err - } - - RBDFilesystem := vol.ConfigBlockFilesystem() - mountFlags, mountOptions := resolveMountOptions(vol.ConfigBlockMountOptions()) - err = TryMount(devPath, mountPath, RBDFilesystem, mountFlags, mountOptions) - if err != nil { - return false, err - } - d.logger.Debug("Mounted RBD volume", log.Ctx{"dev": devPath, "path": mountPath, "options": mountOptions}) + if vol.contentType == ContentTypeFS { + mountPath := vol.MountPath() + if !shared.IsMountPoint(mountPath) { + err := vol.EnsureMountPath() + if err != nil { + return err + } - return true, nil - } + RBDFilesystem := vol.ConfigBlockFilesystem() + mountFlags, mountOptions := resolveMountOptions(vol.ConfigBlockMountOptions()) + err = TryMount(devPath, mountPath, RBDFilesystem, mountFlags, mountOptions) + if err != nil { + return err + } - // For VMs, mount the filesystem volume. - if vol.IsVMBlock() { - fsVol := vol.NewVMBlockFilesystemVolume() - return d.MountVolume(fsVol, op) + d.logger.Debug("Mounted RBD volume", log.Ctx{"dev": devPath, "path": mountPath, "options": mountOptions}) + } + } else if vol.contentType == ContentTypeBlock { + // For VMs, mount the filesystem volume. + if vol.IsVMBlock() { + fsVol := vol.NewVMBlockFilesystemVolume() + err = d.MountVolume(fsVol, op) + if err != nil { + return err + } + } } - return ourMount, nil + vol.MountRefCountIncrement() // From here on it is up to caller to call UnmountVolume() when done. + return nil } // UnmountVolume simulates unmounting a volume. @@ -968,41 +972,62 @@ func (d *ceph) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Opera unlock := vol.MountLock() defer unlock() + ourUnmount := false + refCount := vol.MountRefCountDecrement() + // Attempt to unmount the volume. - mountPath := vol.MountPath() - if vol.contentType == ContentTypeFS && shared.IsMountPoint(mountPath) { - err := TryUnmount(mountPath, unix.MNT_DETACH) - if err != nil { - return false, err - } - d.logger.Debug("Unmounted RBD volume", log.Ctx{"path": mountPath, "keepBlockDev": keepBlockDev}) + if vol.contentType == ContentTypeFS { + mountPath := vol.MountPath() + if shared.IsMountPoint(mountPath) { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } - // Attempt to unmap. - if !keepBlockDev { - err = d.rbdUnmapVolume(vol, true) + err := TryUnmount(mountPath, unix.MNT_DETACH) if err != nil { return false, err } - } + d.logger.Debug("Unmounted RBD volume", log.Ctx{"path": mountPath, "keepBlockDev": keepBlockDev}) - return true, nil - } + // Attempt to unmap. + if !keepBlockDev { + err = d.rbdUnmapVolume(vol, true) + if err != nil { + return false, err + } + } - if vol.contentType == ContentTypeBlock && !keepBlockDev { - // Attempt to unmap. - err := d.rbdUnmapVolume(vol, true) - if err != nil { - return false, err + ourUnmount = true + } + } else if vol.contentType == ContentTypeBlock { + // For VMs, unmount the filesystem volume. + if vol.IsVMBlock() { + fsVol := vol.NewVMBlockFilesystemVolume() + return d.UnmountVolume(fsVol, false, op) } - } - // For VMs, unmount the filesystem volume. - if vol.IsVMBlock() { - fsVol := vol.NewVMBlockFilesystemVolume() - return d.UnmountVolume(fsVol, false, op) + if !keepBlockDev { + // Check if device is currently mapped (but don't map if not). + _, devPath, _ := d.getRBDMappedDevPath(vol, false) + if devPath != "" && shared.PathExists(devPath) { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } + + // Attempt to unmap. + err := d.rbdUnmapVolume(vol, true) + if err != nil { + return false, err + } + + ourUnmount = true + } + } } - return false, nil + return ourUnmount, nil } // RenameVolume renames a volume and its snapshots. @@ -1059,13 +1084,11 @@ func (d *ceph) MigrateVolume(vol Volume, conn io.ReadWriteCloser, volSrcArgs *mi // activated to avoid issues activating the snapshot volume device. parent, _, _ := shared.InstanceGetParentAndSnapshotName(vol.Name()) parentVol := NewVolume(d, d.Name(), vol.volType, vol.contentType, parent, vol.config, vol.poolConfig) - ourMount, err := d.MountVolume(parentVol, op) + err := d.MountVolume(parentVol, op) if err != nil { return err } - if ourMount { - defer d.UnmountVolume(parentVol, false, op) - } + defer d.UnmountVolume(parentVol, false, op) return genericVFSMigrateVolume(d, d.state, vol, conn, volSrcArgs, op) } else if volSrcArgs.MigrationType.FSType != migration.MigrationFSType_RBD { From d20ea7daedb36d030277126a75d6167054073c9e Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:06:11 +0000 Subject: [PATCH 13/39] lxd/storage/drivers/driver/cephfs/volumes: Updates MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_cephfs_volumes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/drivers/driver_cephfs_volumes.go b/lxd/storage/drivers/driver_cephfs_volumes.go index c7fb7066bd..da8c9a055c 100644 --- a/lxd/storage/drivers/driver_cephfs_volumes.go +++ b/lxd/storage/drivers/driver_cephfs_volumes.go @@ -338,11 +338,11 @@ func (d *cephfs) GetVolumeDiskPath(vol Volume) (string, error) { } // MountVolume sets up the volume for use. -func (d *cephfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +func (d *cephfs) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() - return false, nil + return nil } // UnmountVolume clears any runtime state for the volume. From 3e91104e9cc07826d5f5c7abc24b53109f85070b Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:06:33 +0000 Subject: [PATCH 14/39] lxd/storage/drivers/driver/dir/volumes: Updates MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_dir_volumes.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lxd/storage/drivers/driver_dir_volumes.go b/lxd/storage/drivers/driver_dir_volumes.go index f391ea841f..c7ac627882 100644 --- a/lxd/storage/drivers/driver_dir_volumes.go +++ b/lxd/storage/drivers/driver_dir_volumes.go @@ -307,9 +307,8 @@ func (d *dir) GetVolumeDiskPath(vol Volume) (string, error) { return genericVFSGetVolumeDiskPath(vol) } -// MountVolume simulates mounting a volume. As the driver doesn't have volumes to mount it returns -// false indicating that there is no need to issue an unmount. -func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +// MountVolume simulates mounting a volume. +func (d *dir) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() @@ -318,11 +317,11 @@ func (d *dir) MountVolume(vol Volume, op *operations.Operation) (bool, error) { if !shared.PathExists(vol.MountPath()) || vol.volType != VolumeTypeCustom { err := vol.EnsureMountPath() if err != nil { - return false, err + return err } } - return false, nil + return nil } // UnmountVolume simulates unmounting a volume. From 1100f1740184a744578d0de16c67eef66b3e70a7 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:06:57 +0000 Subject: [PATCH 15/39] lxd/storage/drivers/driver/lvm/volumes: Adds ref counting to MountVolume and UnmountVolume Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_lvm_volumes.go | 126 +++++++++++++--------- 1 file changed, 74 insertions(+), 52 deletions(-) diff --git a/lxd/storage/drivers/driver_lvm_volumes.go b/lxd/storage/drivers/driver_lvm_volumes.go index a96ec08ad0..c247527d6d 100644 --- a/lxd/storage/drivers/driver_lvm_volumes.go +++ b/lxd/storage/drivers/driver_lvm_volumes.go @@ -431,90 +431,112 @@ func (d *lvm) GetVolumeDiskPath(vol Volume) (string, error) { return "", ErrNotSupported } -// MountVolume mounts a volume. Returns true if this volume was our mount. -func (d *lvm) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +// MountVolume mounts a volume and increments ref counter. Please call UnmountVolume() when done with the volume. +func (d *lvm) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() // Activate LVM volume if needed. volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, vol.contentType, vol.name) - activated, err := d.activateVolume(volDevPath) + _, err := d.activateVolume(volDevPath) if err != nil { - return false, err + return err } - // Check if already mounted. - mountPath := vol.MountPath() - if vol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) { - err = vol.EnsureMountPath() - if err != nil { - return false, err - } + if vol.contentType == ContentTypeFS { + // Check if already mounted. + mountPath := vol.MountPath() + if !shared.IsMountPoint(mountPath) { + err = vol.EnsureMountPath() + if err != nil { + return err + } - mountFlags, mountOptions := resolveMountOptions(vol.ConfigBlockMountOptions()) - err = TryMount(volDevPath, mountPath, vol.ConfigBlockFilesystem(), mountFlags, mountOptions) - if err != nil { - return false, errors.Wrapf(err, "Failed to mount LVM logical volume") + mountFlags, mountOptions := resolveMountOptions(vol.ConfigBlockMountOptions()) + err = TryMount(volDevPath, mountPath, vol.ConfigBlockFilesystem(), mountFlags, mountOptions) + if err != nil { + return errors.Wrapf(err, "Failed to mount LVM logical volume") + } + d.logger.Debug("Mounted logical volume", log.Ctx{"dev": volDevPath, "path": mountPath, "options": mountOptions}) + } + } else if vol.contentType == ContentTypeBlock { + // For VMs, mount the filesystem volume. + if vol.IsVMBlock() { + fsVol := vol.NewVMBlockFilesystemVolume() + err = d.MountVolume(fsVol, op) + if err != nil { + return err + } } - d.logger.Debug("Mounted logical volume", log.Ctx{"dev": volDevPath, "path": mountPath, "options": mountOptions}) - - return true, nil - } - - // For VMs, mount the filesystem volume. - if vol.IsVMBlock() { - fsVol := vol.NewVMBlockFilesystemVolume() - return d.MountVolume(fsVol, op) } - return activated, nil + vol.MountRefCountIncrement() // From here on it is up to caller to call UnmountVolume() when done. + return nil } -// UnmountVolume unmounts a volume. Returns true if we unmounted. -// keepBlockDev indicates if backing block device should be not be deactivated if volume is unmounted. +// UnmountVolume unmounts volume if mounted and not in use. Returns true if this unmounted the volume. +// keepBlockDev indicates if backing block device should be not be deactivated when volume is unmounted. func (d *lvm) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) { unlock := vol.MountLock() defer unlock() var err error + ourUnmount := false volDevPath := d.lvmDevPath(d.config["lvm.vg_name"], vol.volType, vol.contentType, vol.name) - mountPath := vol.MountPath() + refCount := vol.MountRefCountDecrement() // Check if already mounted. - 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, "keepBlockDev": keepBlockDev}) + if vol.contentType == ContentTypeFS { + mountPath := vol.MountPath() + if shared.IsMountPoint(mountPath) { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } - // We only deactivate filesystem volumes if an unmount was needed to better align with our - // unmount return value indicator. - if !keepBlockDev { - _, err = d.deactivateVolume(volDevPath) + 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, "keepBlockDev": keepBlockDev}) + + // We only deactivate filesystem volumes if an unmount was needed to better align with our + // unmount return value indicator. + if !keepBlockDev { + _, err = d.deactivateVolume(volDevPath) + if err != nil { + return false, err + } + } + + ourUnmount = true + } + } else if vol.contentType == ContentTypeBlock { + // For VMs, unmount the filesystem volume. + if vol.IsVMBlock() { + fsVol := vol.NewVMBlockFilesystemVolume() + _, err = d.UnmountVolume(fsVol, false, op) if err != nil { return false, err } } - return true, nil - } + if !keepBlockDev && shared.PathExists(volDevPath) { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } - deactivated := false - if vol.contentType == ContentTypeBlock && !keepBlockDev { - deactivated, err = d.deactivateVolume(volDevPath) - if err != nil { - return false, err - } - } + _, 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, false, op) + ourUnmount = true + } } - return deactivated, nil + return ourUnmount, nil } // RenameVolume renames a volume and its snapshots. From 7edc76401b0cd11028c1b025f2d70ada1f866e56 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:07:32 +0000 Subject: [PATCH 16/39] lxd/storage/drivers/driver/zfs/volumes: Adds ref counting to MountVolume and UnmountVolume Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/driver_zfs_volumes.go | 150 ++++++++++++---------- 1 file changed, 80 insertions(+), 70 deletions(-) diff --git a/lxd/storage/drivers/driver_zfs_volumes.go b/lxd/storage/drivers/driver_zfs_volumes.go index 07ae48a883..e9d053fb0f 100644 --- a/lxd/storage/drivers/driver_zfs_volumes.go +++ b/lxd/storage/drivers/driver_zfs_volumes.go @@ -411,7 +411,7 @@ func (d *zfs) CreateVolumeFromBackup(vol Volume, srcBackup backup.Info, srcData if vol.volType != VolumeTypeCustom { // The import requires a mounted volume, so mount it and have it unmounted as a post hook. - _, err = d.MountVolume(vol, op) + err = d.MountVolume(vol, op) if err != nil { return nil, nil, err } @@ -1041,118 +1041,128 @@ func (d *zfs) GetVolumeDiskPath(vol Volume) (string, error) { return "", fmt.Errorf("Could not locate a zvol for %s", d.dataset(vol, false)) } -// MountVolume simulates mounting a volume. -func (d *zfs) MountVolume(vol Volume, op *operations.Operation) (bool, error) { +// MountVolume mounts a volume and increments ref counter. Please call UnmountVolume() when done with the volume. +func (d *zfs) MountVolume(vol Volume, op *operations.Operation) error { unlock := vol.MountLock() defer unlock() - var err error - mountPath := vol.MountPath() dataset := d.dataset(vol, false) // Check if filesystem volume already mounted. - if vol.contentType == ContentTypeFS && !shared.IsMountPoint(mountPath) { - err := vol.EnsureMountPath() - if err != nil { - return false, err - } - - // Mount the dataset. - _, err = shared.RunCommand("zfs", "mount", dataset) - if err != nil { - return false, err - } - - d.logger.Debug("Mounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath}) - return true, nil - } + if vol.contentType == ContentTypeFS { + mountPath := vol.MountPath() + if !shared.IsMountPoint(mountPath) { + err := vol.EnsureMountPath() + if err != nil { + return err + } - var ourMountBlock, ourMountFs bool + // Mount the dataset. + _, err = shared.RunCommand("zfs", "mount", dataset) + if err != nil { + return err + } - // For block devices, we make them appear. - if vol.contentType == ContentTypeBlock { + d.logger.Debug("Mounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath}) + } + } else if vol.contentType == ContentTypeBlock { + // For block devices, we make them appear. // Check if already active. current, err := d.getDatasetProperty(d.dataset(vol, false), "volmode") if err != nil { - return false, err + return err } if current != "dev" { // Activate. err = d.setDatasetProperties(d.dataset(vol, false), "volmode=dev") if err != nil { - return false, err + return err } // Wait half a second to give udev a chance to kick in. time.Sleep(500 * time.Millisecond) d.logger.Debug("Activated ZFS volume", log.Ctx{"dev": dataset}) - ourMountBlock = true } - } - if vol.IsVMBlock() { - // For VMs, also mount the filesystem dataset. - fsVol := vol.NewVMBlockFilesystemVolume() - ourMountFs, err = d.MountVolume(fsVol, op) - if err != nil { - return false, err + if vol.IsVMBlock() { + // For VMs, also mount the filesystem dataset. + fsVol := vol.NewVMBlockFilesystemVolume() + err = d.MountVolume(fsVol, op) + if err != nil { + return err + } } } - // If we 'mounted' either block or filesystem volumes, this was our mount. - if ourMountFs || ourMountBlock { - return true, nil - } - - return false, nil + vol.MountRefCountIncrement() // From here on it is up to caller to call UnmountVolume() when done. + return nil } -// UnmountVolume simulates unmounting a volume. -// keepBlockDev indicates if backing block device should be not be deactivated if volume is unmounted. +// UnmountVolume unmounts volume if mounted and not in use. Returns true if this unmounted the volume. +// keepBlockDev indicates if backing block device should be not be deactivated when volume is unmounted. func (d *zfs) UnmountVolume(vol Volume, keepBlockDev bool, op *operations.Operation) (bool, error) { unlock := vol.MountLock() defer unlock() - mountPath := vol.MountPath() + ourUnmount := false dataset := d.dataset(vol, false) + refCount := vol.MountRefCountDecrement() - // For VMs, also mount the filesystem dataset. - if vol.IsVMBlock() { - fsVol := vol.NewVMBlockFilesystemVolume() - _, err := d.UnmountVolume(fsVol, false, op) - if err != nil { - return false, err - } - } + if vol.contentType == ContentTypeFS { + // Check if mounted. + mountPath := vol.MountPath() + if shared.IsMountPoint(mountPath) { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } - // For block devices, we make them disappear. - if vol.contentType == ContentTypeBlock && !keepBlockDev { - err := d.setDatasetProperties(dataset, "volmode=none") - if err != nil { - return false, err - } + // Unmount the dataset. + err := TryUnmount(mountPath, 0) + if err != nil { + return false, err + } - d.logger.Debug("Deactivated ZFS volume", log.Ctx{"dev": dataset}) + d.logger.Debug("Unmounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath}) + ourUnmount = true + } + } else if vol.contentType == ContentTypeBlock { + // For VMs, also mount the filesystem dataset. + if vol.IsVMBlock() { + fsVol := vol.NewVMBlockFilesystemVolume() + _, err := d.UnmountVolume(fsVol, false, op) + if err != nil { + return false, err + } + } - return false, nil - } + // For block devices, we make them disappear if active. + if !keepBlockDev { + current, err := d.getDatasetProperty(d.dataset(vol, false), "volmode") + if err != nil { + return false, err + } - // Check if still mounted. - if shared.IsMountPoint(mountPath) { - // Unmount the dataset. - err := TryUnmount(mountPath, 0) - if err != nil { - return false, err - } + if current == "dev" { + if refCount > 0 { + d.logger.Debug("Skipping unmount as in use", "refCount", refCount) + return false, ErrInUse + } - d.logger.Debug("Unmounted ZFS dataset", log.Ctx{"dev": dataset, "path": mountPath}) - return true, nil + err := d.setDatasetProperties(dataset, "volmode=none") + if err != nil { + return false, err + } + d.logger.Debug("Deactivated ZFS volume", log.Ctx{"dev": dataset}) + ourUnmount = true + } + } } - return false, nil + return ourUnmount, nil } // RenameVolume renames a volume and its snapshots. @@ -1602,7 +1612,7 @@ func (d *zfs) MountVolumeSnapshot(snapVol Volume, op *operations.Operation) (boo // Ensure snap volume parent is activated to avoid issues activating the snapshot volume device. parent, _, _ := shared.InstanceGetParentAndSnapshotName(snapVol.Name()) parentVol := NewVolume(d, d.Name(), snapVol.volType, snapVol.contentType, parent, snapVol.config, snapVol.poolConfig) - _, err = d.MountVolume(parentVol, op) + err = d.MountVolume(parentVol, op) if err != nil { return false, err } From 43f2280b1259391e751056b7cb67fa858cdb8c70 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:08:13 +0000 Subject: [PATCH 17/39] lxd/storage/drivers/generic/vfs: Updates genericVFSBackupUnpack to use new MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/generic_vfs.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lxd/storage/drivers/generic_vfs.go b/lxd/storage/drivers/generic_vfs.go index 755c0cddb8..8a4aecc3bd 100644 --- a/lxd/storage/drivers/generic_vfs.go +++ b/lxd/storage/drivers/generic_vfs.go @@ -771,7 +771,7 @@ func genericVFSBackupUnpack(d Driver, vol Volume, snapshots []string, srcData io revert.Add(func() { d.DeleteVolumeSnapshot(snapVol, op) }) } - ourMount, err := d.MountVolume(vol, op) + err = d.MountVolume(vol, op) if err != nil { return nil, nil, err } @@ -805,17 +805,12 @@ func genericVFSBackupUnpack(d Driver, vol Volume, snapshots []string, srcData io // backup restoration process). Create a post hook function that will be called at the end of the // backup restore process to unmount the volume if needed. postHook = func(vol Volume) error { - if ourMount { - d.UnmountVolume(vol, false, op) - } - + d.UnmountVolume(vol, false, op) return nil } } else { // For custom volumes unmount now, there is no post hook as there is no backup.yaml to generate. - if ourMount { - d.UnmountVolume(vol, false, op) - } + d.UnmountVolume(vol, false, op) } return postHook, revertExternal.Fail, nil From 5b7589b6fb3ac853c546ac38c9753c25efa4209d Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:08:37 +0000 Subject: [PATCH 18/39] lxd/storage/drivers/volume: Updates MountTask to use new MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/drivers/volume.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index 938f2a2166..b3dbb641a4 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -221,14 +221,11 @@ func (v Volume) MountTask(task func(mountPath string, op *operations.Operation) defer v.driver.UnmountVolumeSnapshot(v, op) } } else { - ourMount, err := v.driver.MountVolume(v, op) + err := v.driver.MountVolume(v, op) if err != nil { return err } - - if ourMount { - defer v.driver.UnmountVolume(v, false, op) - } + defer v.driver.UnmountVolume(v, false, op) } return task(v.MountPath(), op) From 04e5f079a2e019ca523a2179cb00f38f0bbd4b0f Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:09:02 +0000 Subject: [PATCH 19/39] lxd/storage/utils: Adds InstanceMount and InstanceUnmount and updates InstanceDiskBlockSize to use them Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/utils.go | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go index 97c781a543..1e2b7c9fab 100644 --- a/lxd/storage/utils.go +++ b/lxd/storage/utils.go @@ -944,31 +944,49 @@ func RenderSnapshotUsage(s *state.State, snapInst instance.Instance) func(respon } } -// InstanceDiskBlockSize returns the block device size for the instance's disk. -// This will mount the instance if not already mounted and will unmount at the end if needed. -func InstanceDiskBlockSize(pool Pool, inst instance.Instance, op *operations.Operation) (int64, error) { +// InstanceMount mounts an instance's storage volume (if not already mounted). +// Please call InstanceUnmount when finished. +func InstanceMount(pool Pool, inst instance.Instance, op *operations.Operation) (*MountInfo, error) { var err error var mountInfo *MountInfo if inst.IsSnapshot() { mountInfo, err = pool.MountInstanceSnapshot(inst, op) if err != nil { - return -1, err - } - - if mountInfo.OurMount { - defer pool.UnmountInstanceSnapshot(inst, op) + return nil, err } } else { mountInfo, err = pool.MountInstance(inst, op) if err != nil { - return -1, err + return nil, err } + } - if mountInfo.OurMount { - defer pool.UnmountInstance(inst, op) - } + return mountInfo, nil +} + +// InstanceUnmount unmounts an instance's storage volume (if not in use). Returns if we unmounted the volume. +func InstanceUnmount(pool Pool, inst instance.Instance, op *operations.Operation) (bool, error) { + var err error + var ourUnmount bool + + if inst.IsSnapshot() { + ourUnmount, err = pool.UnmountInstanceSnapshot(inst, op) + } else { + ourUnmount, err = pool.UnmountInstance(inst, op) + } + + return ourUnmount, err +} + +// InstanceDiskBlockSize returns the block device size for the instance's disk. +// This will mount the instance if not already mounted and will unmount at the end if needed. +func InstanceDiskBlockSize(pool Pool, inst instance.Instance, op *operations.Operation) (int64, error) { + mountInfo, err := InstanceMount(pool, inst, op) + if err != nil { + return -1, err } + defer InstanceUnmount(pool, inst, op) if mountInfo.DiskPath == "" { return -1, fmt.Errorf("No disk path available from mount") From 0215a3a4fceec3d06ec6d04ce747eecef357e7d1 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:09:49 +0000 Subject: [PATCH 20/39] lxd/storage/backend/mock: Removes OurMount Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/backend_mock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/backend_mock.go b/lxd/storage/backend_mock.go index 38e5174e82..33dc3a1890 100644 --- a/lxd/storage/backend_mock.go +++ b/lxd/storage/backend_mock.go @@ -128,7 +128,7 @@ func (b *mockBackend) SetInstanceQuota(inst instance.Instance, size string, op * } func (b *mockBackend) MountInstance(inst instance.Instance, op *operations.Operation) (*MountInfo, error) { - return &MountInfo{OurMount: true}, nil + return &MountInfo{}, nil } func (b *mockBackend) UnmountInstance(inst instance.Instance, op *operations.Operation) (bool, error) { @@ -152,7 +152,7 @@ func (b *mockBackend) RestoreInstanceSnapshot(inst instance.Instance, src instan } func (b *mockBackend) MountInstanceSnapshot(inst instance.Instance, op *operations.Operation) (*MountInfo, error) { - return &MountInfo{OurMount: true}, nil + return &MountInfo{}, nil } func (b *mockBackend) UnmountInstanceSnapshot(inst instance.Instance, op *operations.Operation) (bool, error) { From f4ea567e6bf7c4e8987bf1e216e8bbba19654c58 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:10:03 +0000 Subject: [PATCH 21/39] lxd/storage/backend/mock: Removes "our mount" bool return value from MountCustomVolume Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/backend_mock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/backend_mock.go b/lxd/storage/backend_mock.go index 33dc3a1890..bfc443fa29 100644 --- a/lxd/storage/backend_mock.go +++ b/lxd/storage/backend_mock.go @@ -211,8 +211,8 @@ func (b *mockBackend) GetCustomVolumeUsage(projectName string, volName string) ( return 0, nil } -func (b *mockBackend) MountCustomVolume(projectName string, volName string, op *operations.Operation) (bool, error) { - return true, nil +func (b *mockBackend) MountCustomVolume(projectName string, volName string, op *operations.Operation) error { + return nil } func (b *mockBackend) UnmountCustomVolume(projectName string, volName string, op *operations.Operation) (bool, error) { From 99092c75590228d3271c720c027721d03b8c21ad Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:10:39 +0000 Subject: [PATCH 22/39] lxd/storage/backend/lxd: Updates mount functions to remove OurMount and use new MountVolume signature Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/backend_lxd.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go index 4d323ef8cb..b942e1dfab 100644 --- a/lxd/storage/backend_lxd.go +++ b/lxd/storage/backend_lxd.go @@ -1636,7 +1636,7 @@ func (b *lxdBackend) MountInstance(inst instance.Instance, op *operations.Operat // Get the volume. vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf) - ourMount, err := b.driver.MountVolume(vol, op) + err = b.driver.MountVolume(vol, op) if err != nil { return nil, err } @@ -1647,7 +1647,6 @@ func (b *lxdBackend) MountInstance(inst instance.Instance, op *operations.Operat } mountInfo := &MountInfo{ - OurMount: ourMount, DiskPath: diskPath, } @@ -2015,7 +2014,7 @@ func (b *lxdBackend) MountInstanceSnapshot(inst instance.Instance, op *operation // Get the volume. vol := b.newVolume(volType, contentType, volStorageName, rootDiskConf) - ourMount, err := b.driver.MountVolumeSnapshot(vol, op) + _, err = b.driver.MountVolumeSnapshot(vol, op) if err != nil { return nil, err } @@ -2026,7 +2025,6 @@ func (b *lxdBackend) MountInstanceSnapshot(inst instance.Instance, op *operation } mountInfo := &MountInfo{ - OurMount: ourMount, DiskPath: diskPath, } @@ -3043,14 +3041,14 @@ func (b *lxdBackend) GetCustomVolumeUsage(projectName, volName string) (int64, e } // MountCustomVolume mounts a custom volume. -func (b *lxdBackend) MountCustomVolume(projectName, volName string, op *operations.Operation) (bool, error) { +func (b *lxdBackend) MountCustomVolume(projectName, volName string, op *operations.Operation) error { logger := logging.AddContext(b.logger, log.Ctx{"project": projectName, "volName": volName}) logger.Debug("MountCustomVolume started") defer logger.Debug("MountCustomVolume finished") _, volume, err := b.state.Cluster.GetLocalStoragePoolVolume(projectName, volName, db.StoragePoolVolumeTypeCustom, b.id) if err != nil { - return false, err + return err } // Get the volume name on storage. From f509c3320ebcf17bbb3409993370fc586cc34a26 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:11:06 +0000 Subject: [PATCH 23/39] lxd/storage/backend/lxd/patches: b.driver.MountVolume usage Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/storage/backend_lxd_patches.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/storage/backend_lxd_patches.go b/lxd/storage/backend_lxd_patches.go index ef5e373292..5d58fa83e5 100644 --- a/lxd/storage/backend_lxd_patches.go +++ b/lxd/storage/backend_lxd_patches.go @@ -129,7 +129,7 @@ func lxdPatchStorageRenameCustomVolumeAddProject(b *lxdBackend) error { if ourUnmount { logger.Infof("Mount custom volume %q in pool %q", newVolStorageName, b.Name()) - _, err = b.driver.MountVolume(newVol, nil) + err = b.driver.MountVolume(newVol, nil) if err != nil { return err } From 3ea1d4e597f7e8fd6da109b4ec09e79c75d98b3f Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 09:24:27 +0000 Subject: [PATCH 24/39] lxd/instance/drivers/driver: Unexports common restart function Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_common.go | 4 ++-- lxd/instance/drivers/driver_lxc.go | 2 +- lxd/instance/drivers/driver_qemu.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lxd/instance/drivers/driver_common.go b/lxd/instance/drivers/driver_common.go index d22d7eb733..a6736d94aa 100644 --- a/lxd/instance/drivers/driver_common.go +++ b/lxd/instance/drivers/driver_common.go @@ -96,8 +96,8 @@ func (c *common) DevPaths() []string { return c.devPaths } -// Restart handles instance restarts. -func (c *common) Restart(inst instance.Instance, timeout time.Duration) error { +// restart handles instance restarts. +func (c *common) restart(inst instance.Instance, timeout time.Duration) error { if timeout == 0 { err := inst.Stop(false) if err != nil { diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go index b02f212c3e..aa7c697f75 100644 --- a/lxd/instance/drivers/driver_lxc.go +++ b/lxd/instance/drivers/driver_lxc.go @@ -2827,7 +2827,7 @@ func (c *lxc) Shutdown(timeout time.Duration) error { // Restart restart the instance. func (c *lxc) Restart(timeout time.Duration) error { - return c.common.Restart(c, timeout) + return c.common.restart(c, timeout) } // onStopNS is triggered by LXC's stop hook once a container is shutdown but before the container's diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go index bda16f90ae..38c57cb9a2 100644 --- a/lxd/instance/drivers/driver_qemu.go +++ b/lxd/instance/drivers/driver_qemu.go @@ -648,7 +648,7 @@ func (vm *qemu) Shutdown(timeout time.Duration) error { // Restart restart the instance. func (vm *qemu) Restart(timeout time.Duration) error { - return vm.common.Restart(vm, timeout) + return vm.common.restart(vm, timeout) } func (vm *qemu) ovmfPath() string { From 02e0b1a0b48328174041d36e347376c8c5060c9d Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:01:07 +0000 Subject: [PATCH 25/39] lxd/instance/instance/interface: Removes deprecated StorageStart and StorageStop functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/instance_interface.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/lxd/instance/instance_interface.go b/lxd/instance/instance_interface.go index 3992870f35..c096230bf3 100644 --- a/lxd/instance/instance_interface.go +++ b/lxd/instance/instance_interface.go @@ -138,8 +138,6 @@ type Instance interface { // FIXME: Those should be internal functions. // Needed for migration for now. - StorageStart() (bool, error) - StorageStop() (bool, error) DeferTemplateApply(trigger string) error } From b0493ee648c86dab6b3848e96d2827b92ca7d8a0 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:01:30 +0000 Subject: [PATCH 26/39] lxd/instance/drivers/driver/common: Import ordering Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_common.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lxd/instance/drivers/driver_common.go b/lxd/instance/drivers/driver_common.go index a6736d94aa..f98bd1db69 100644 --- a/lxd/instance/drivers/driver_common.go +++ b/lxd/instance/drivers/driver_common.go @@ -2,13 +2,14 @@ package drivers import ( "errors" + "time" + "github.com/lxc/lxd/lxd/db" deviceConfig "github.com/lxc/lxd/lxd/device/config" "github.com/lxc/lxd/lxd/instance" "github.com/lxc/lxd/lxd/instance/instancetype" "github.com/lxc/lxd/lxd/state" "github.com/lxc/lxd/shared/api" - "time" ) // common provides structure common to all instance types. From 1c19889154cc3bc00d97e9eadca6a77f62d71fb9 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:02:41 +0000 Subject: [PATCH 27/39] lxd/instance/drivers/driver/lxc: Updates mount usage with ref counting in mind Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_lxc.go | 174 ++++++----------------------- 1 file changed, 34 insertions(+), 140 deletions(-) diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go index aa7c697f75..f1da2344d9 100644 --- a/lxd/instance/drivers/driver_lxc.go +++ b/lxd/instance/drivers/driver_lxc.go @@ -1412,9 +1412,7 @@ func (c *lxc) deviceAdd(deviceName string, rawConfig deviceConfig.Device) error return d.Add() } -// deviceStart loads a new device and calls its Start() function. After processing the runtime -// config returned from Start(), it also runs the device's Register() function irrespective of -// whether the container is running or not. +// deviceStart loads a new device and calls its Start() function. func (c *lxc) deviceStart(deviceName string, rawConfig deviceConfig.Device, isRunning bool) (*deviceConfig.RunConfig, error) { logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "type": rawConfig["type"], "project": c.Project(), "instance": c.Name()}) logger.Debug("Starting device") @@ -1967,8 +1965,6 @@ func (c *lxc) expandDevices(profiles []api.Profile) error { // Start functions func (c *lxc) startCommon() (string, []func() error, error) { - var ourStart bool - revert := revert.New() defer revert.Fail() @@ -2006,6 +2002,7 @@ func (c *lxc) startCommon() (string, []func() error, error) { return "", nil, errors.Wrap(err, "Set last ID map") } + mounted := false if !nextIdmap.Equals(diskIdmap) && !(diskIdmap == nil && c.state.OS.Shiftfs) { if shared.IsTrue(c.expandedConfig["security.protection.shift"]) { return "", nil, fmt.Errorf("Container is protected against filesystem shifting") @@ -2014,11 +2011,12 @@ func (c *lxc) startCommon() (string, []func() error, error) { logger.Debugf("Container idmap changed, remapping") c.updateProgress("Remapping container filesystem") - moutInfo, err := c.mount() + _, err = c.mount() if err != nil { return "", nil, errors.Wrap(err, "Storage start") } - ourStart = moutInfo.OurMount + mounted = true + revert.Add(func() { c.unmount() }) storageType, err := c.getStorageType() if err != nil { @@ -2034,9 +2032,6 @@ func (c *lxc) startCommon() (string, []func() error, error) { err = diskIdmap.UnshiftRootfs(c.RootfsPath(), nil) } if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } } @@ -2050,9 +2045,6 @@ func (c *lxc) startCommon() (string, []func() error, error) { err = nextIdmap.ShiftRootfs(c.RootfsPath(), nil) } if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } } @@ -2127,11 +2119,13 @@ func (c *lxc) startCommon() (string, []func() error, error) { } // Mount instance root volume. - mountInfo, err := c.mount() - if err != nil { - return "", nil, err + if !mounted { + _, err = c.mount() + if err != nil { + return "", nil, err + } + revert.Add(func() { c.unmount() }) } - ourStart = mountInfo.OurMount // Create the devices postStartHooks := []func() error{} @@ -2291,9 +2285,6 @@ func (c *lxc) startCommon() (string, []func() error, error) { // Set ownership to match container root currentIdmapset, err := c.CurrentIdmap() if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } @@ -2304,27 +2295,18 @@ func (c *lxc) startCommon() (string, []func() error, error) { err = os.Chown(c.Path(), int(uid), 0) if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } // We only need traversal by root in the container err = os.Chmod(c.Path(), 0100) if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } // Update the backup.yaml file err = c.UpdateBackupFile() if err != nil { - if ourStart { - c.unmount() - } return "", nil, err } @@ -2386,12 +2368,6 @@ func (c *lxc) Start(stateful bool) error { return errors.Wrap(err, "Common start logic") } - // Ensure that the container storage volume is mounted. - _, err = c.mount() - if err != nil { - return errors.Wrap(err, "Storage start") - } - ctxMap = log.Ctx{ "project": c.project, "name": c.name, @@ -2536,18 +2512,9 @@ func (c *lxc) onStart(_ map[string]string) error { // Make sure we can't call go-lxc functions by mistake c.fromHook = true - // Start the storage for this container - mountInfo, err := c.mount() - if err != nil { - return err - } - // Load the container AppArmor profile - err = apparmor.InstanceLoad(c.state, c) + err := apparmor.InstanceLoad(c.state, c) if err != nil { - if mountInfo.OurMount { - c.unmount() - } return err } @@ -2558,9 +2525,6 @@ func (c *lxc) onStart(_ map[string]string) error { err = c.templateApplyNow(c.localConfig[key]) if err != nil { apparmor.InstanceUnload(c.state, c) - if mountInfo.OurMount { - c.unmount() - } return err } @@ -2568,9 +2532,6 @@ func (c *lxc) onStart(_ map[string]string) error { err := c.state.Cluster.DeleteInstanceConfigKey(c.id, key) if err != nil { apparmor.InstanceUnload(c.state, c) - if mountInfo.OurMount { - c.unmount() - } return err } } @@ -2578,9 +2539,6 @@ func (c *lxc) onStart(_ map[string]string) error { err = c.templateApplyNow("start") if err != nil { apparmor.InstanceUnload(c.state, c) - if mountInfo.OurMount { - c.unmount() - } return err } @@ -3435,13 +3393,11 @@ func (c *lxc) Restore(sourceContainer instance.Instance, stateful bool) error { } // Ensure that storage is mounted for state path checks and for backup.yaml updates. - mountInfo, err := pool.MountInstance(c, nil) + _, err = pool.MountInstance(c, nil) if err != nil { return err } - if mountInfo.OurMount && !wasRunning { - defer pool.UnmountInstance(c, nil) - } + defer pool.UnmountInstance(c, nil) // Check for CRIU if necessary, before doing a bunch of filesystem manipulations. // Requires container be mounted to check StatePath exists. @@ -4681,14 +4637,12 @@ func (c *lxc) Export(w io.Writer, properties map[string]string) (api.ImageMetada logger.Info("Exporting instance", ctxMap) // Start the storage. - mountInfo, err := c.mount() + _, err := c.mount() if err != nil { logger.Error("Failed exporting instance", ctxMap) return meta, err } - if mountInfo.OurMount { - defer c.unmount() - } + defer c.unmount() // Get IDMap to unshift container as the tarball is created. idmap, err := c.DiskIdmap() @@ -4988,11 +4942,6 @@ func (c *lxc) Migrate(args *instance.CriuMigrationArgs) error { } if idmapset != nil { - mountInfo, err := c.mount() - if err != nil { - return err - } - storageType, err := c.getStorageType() if err != nil { return errors.Wrap(err, "Storage type") @@ -5005,15 +4954,8 @@ func (c *lxc) Migrate(args *instance.CriuMigrationArgs) error { } else { err = idmapset.ShiftRootfs(args.StateDir, nil) } - if mountInfo.OurMount { - _, err2 := c.unmount() - if err != nil { - return err - } - - if err2 != nil { - return err2 - } + if err != nil { + return err } } @@ -5297,15 +5239,11 @@ func (c *lxc) inheritInitPidFd() (int, *os.File) { // FileExists returns whether file exists inside instance. func (c *lxc) FileExists(path string) error { // Setup container storage if needed - var ourStart bool - var err error - if !c.IsRunning() { - mountInfo, err := c.mount() - if err != nil { - return err - } - ourStart = mountInfo.OurMount + _, err := c.mount() + if err != nil { + return err } + defer c.unmount() pidFdNr, pidFd := c.inheritInitPidFd() if pidFdNr >= 0 { @@ -5325,14 +5263,6 @@ func (c *lxc) FileExists(path string) error { path, ) - // Tear down container storage if needed - if !c.IsRunning() && ourStart { - _, err := c.unmount() - if err != nil { - return err - } - } - // Process forkcheckfile response if stderr != "" { if strings.HasPrefix(stderr, "error:") { @@ -5359,16 +5289,12 @@ func (c *lxc) FilePull(srcpath string, dstpath string) (int64, int64, os.FileMod op.Wait() } - var ourStart bool - var err error // Setup container storage if needed - if !c.IsRunning() { - mountInfo, err := c.mount() - if err != nil { - return -1, -1, 0, "", nil, err - } - ourStart = mountInfo.OurMount + _, err := c.mount() + if err != nil { + return -1, -1, 0, "", nil, err } + defer c.unmount() pidFdNr, pidFd := c.inheritInitPidFd() if pidFdNr >= 0 { @@ -5389,14 +5315,6 @@ func (c *lxc) FilePull(srcpath string, dstpath string) (int64, int64, os.FileMod dstpath, ) - // Tear down container storage if needed - if !c.IsRunning() && ourStart { - _, err := c.unmount() - if err != nil { - return -1, -1, 0, "", nil, err - } - } - uid := int64(-1) gid := int64(-1) mode := -1 @@ -5514,16 +5432,12 @@ func (c *lxc) FilePush(fileType string, srcpath string, dstpath string, uid int6 } } - var ourStart bool - var err error // Setup container storage if needed - if !c.IsRunning() { - mountInfo, err := c.mount() - if err != nil { - return err - } - ourStart = mountInfo.OurMount + _, err := c.mount() + if err != nil { + return err } + defer c.unmount() defaultMode := 0640 if fileType == "directory" { @@ -5557,14 +5471,6 @@ func (c *lxc) FilePush(fileType string, srcpath string, dstpath string, uid int6 write, ) - // Tear down container storage if needed - if !c.IsRunning() && ourStart { - _, err := c.unmount() - if err != nil { - return err - } - } - // Process forkgetfile response for _, line := range strings.Split(strings.TrimRight(stderr, "\n"), "\n") { if line == "" { @@ -5597,17 +5503,13 @@ func (c *lxc) FilePush(fileType string, srcpath string, dstpath string, uid int6 // FileRemove removes a file inside the instance. func (c *lxc) FileRemove(path string) error { var errStr string - var ourStart bool - var err error // Setup container storage if needed - if !c.IsRunning() { - mountInfo, err := c.mount() - if err != nil { - return err - } - ourStart = mountInfo.OurMount + _, err := c.mount() + if err != nil { + return err } + defer c.unmount() pidFdNr, pidFd := c.inheritInitPidFd() if pidFdNr >= 0 { @@ -5627,14 +5529,6 @@ func (c *lxc) FileRemove(path string) error { path, ) - // Tear down container storage if needed - if !c.IsRunning() && ourStart { - _, err := c.unmount() - if err != nil { - return err - } - } - // Process forkremovefile response for _, line := range strings.Split(strings.TrimRight(stderr, "\n"), "\n") { if line == "" { From 99e841423f81dd77f76e7ecc16690569c14f2afd Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:03:03 +0000 Subject: [PATCH 28/39] lxd/instance/drivers/driver/lxc: Removes deprecated StorageStart and StorageStop Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_lxc.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go index f1da2344d9..a00865208f 100644 --- a/lxd/instance/drivers/driver_lxc.go +++ b/lxd/instance/drivers/driver_lxc.go @@ -5973,16 +5973,6 @@ func (c *lxc) getStorageType() (string, error) { return pool.Driver().Info().Name, nil } -// StorageStart mounts the instance's rootfs volume. Deprecated. -func (c *lxc) StorageStart() (bool, error) { - mountInfo, err := c.mount() - if err != nil { - return false, err - } - - return mountInfo.OurMount, nil -} - // mount the instance's rootfs volume if needed. func (c *lxc) mount() (*storagePools.MountInfo, error) { pool, err := c.getStoragePool() @@ -6007,11 +5997,6 @@ func (c *lxc) mount() (*storagePools.MountInfo, error) { return mountInfo, nil } -// StorageStop unmounts the instance's rootfs volume. Deprecated. -func (c *lxc) StorageStop() (bool, error) { - return c.unmount() -} - // unmount the instance's rootfs volume if needed. func (c *lxc) unmount() (bool, error) { pool, err := c.getStoragePool() From a60c2ea669ab9ad682484bb04717c35a19dad046 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:03:50 +0000 Subject: [PATCH 29/39] lxd/instance/drivers/driver/qemu: Updates mount usage with ref counting in mind Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_qemu.go | 35 +++++++++-------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go index 38c57cb9a2..e2eed444a8 100644 --- a/lxd/instance/drivers/driver_qemu.go +++ b/lxd/instance/drivers/driver_qemu.go @@ -449,14 +449,11 @@ func (vm *qemu) unmount() (bool, error) { // generateAgentCert creates the necessary server key and certificate if needed. func (vm *qemu) generateAgentCert() (string, string, string, string, error) { // Mount the instance's config volume if needed. - mountInfo, err := vm.mount() + _, err := vm.mount() if err != nil { return "", "", "", "", err } - - if mountInfo.OurMount { - defer vm.unmount() - } + defer vm.unmount() agentCertFile := filepath.Join(vm.Path(), "agent.crt") agentKeyFile := filepath.Join(vm.Path(), "agent.key") @@ -1125,14 +1122,11 @@ func (vm *qemu) setupNvram() error { } // Mount the instance's config volume. - mountInfo, err := vm.mount() + _, err := vm.mount() if err != nil { return err } - - if mountInfo.OurMount { - defer vm.unmount() - } + defer vm.unmount() srcOvmfFile := filepath.Join(vm.ovmfPath(), "OVMF_VARS.fd") if vm.expandedConfig["security.secureboot"] == "" || shared.IsTrue(vm.expandedConfig["security.secureboot"]) { @@ -1231,9 +1225,7 @@ func (vm *qemu) deviceLoad(deviceName string, rawConfig deviceConfig.Device) (de return d, configCopy, err } -// deviceStart loads a new device and calls its Start() function. After processing the runtime -// config returned from Start(), it also runs the device's Register() function irrespective of -// whether the instance is running or not. +// deviceStart loads a new device and calls its Start() function. func (vm *qemu) deviceStart(deviceName string, rawConfig deviceConfig.Device, isRunning bool) (*deviceConfig.RunConfig, error) { logger := logging.AddContext(logger.Log, log.Ctx{"device": deviceName, "type": rawConfig["type"], "project": vm.Project(), "instance": vm.Name()}) logger.Debug("Starting device") @@ -1334,14 +1326,11 @@ func (vm *qemu) spicePath() string { // inside the VM's config volume so that it can be restricted by quota. func (vm *qemu) generateConfigShare() error { // Mount the instance's config volume if needed. - mountInfo, err := vm.mount() + _, err := vm.mount() if err != nil { return err } - - if mountInfo.OurMount { - defer vm.unmount() - } + defer vm.unmount() configDrivePath := filepath.Join(vm.Path(), "config") @@ -2667,13 +2656,11 @@ func (vm *qemu) Restore(source instance.Instance, stateful bool) error { } // Ensure that storage is mounted for backup.yaml updates. - mountInfo, err := pool.MountInstance(vm, nil) + _, err = pool.MountInstance(vm, nil) if err != nil { return err } - if mountInfo.OurMount && !wasRunning { - defer pool.UnmountInstance(vm, nil) - } + defer pool.UnmountInstance(vm, nil) // Restore the rootfs. err = pool.RestoreInstanceSnapshot(vm, source, nil) @@ -3718,9 +3705,7 @@ func (vm *qemu) Export(w io.Writer, properties map[string]string) (api.ImageMeta logger.Error("Failed exporting instance", ctxMap) return meta, err } - if mountInfo.OurMount { - defer vm.unmount() - } + defer vm.unmount() // Create the tarball. tarWriter := instancewriter.NewInstanceTarWriter(w, nil) From f9b91ba0f86bbaec90e93f00c5e5d7d9540aced6 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:04:08 +0000 Subject: [PATCH 30/39] lxd/instance/drivers/driver/qemu: Implements RegisterDevices Used for ref counting initially. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_qemu.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go index e2eed444a8..ff0eecee2c 100644 --- a/lxd/instance/drivers/driver_qemu.go +++ b/lxd/instance/drivers/driver_qemu.go @@ -1188,9 +1188,27 @@ func (vm *qemu) deviceVolatileSetFunc(devName string) func(save map[string]strin } } -// RegisterDevices is not used by VMs. +// RegisterDevices calls the Register() function on all of the instance's devices. func (vm *qemu) RegisterDevices() { - return + devices := vm.ExpandedDevices() + for _, dev := range devices.Sorted() { + d, _, err := vm.deviceLoad(dev.Name, dev.Config) + if err == device.ErrUnsupportedDevType { + continue + } + + if err != nil { + logger.Error("Failed to load device to register", log.Ctx{"err": err, "instance": vm.Name(), "device": dev.Name}) + continue + } + + // Check whether device wants to register for any events. + err = d.Register() + if err != nil { + logger.Error("Failed to register device", log.Ctx{"err": err, "instance": vm.Name(), "device": dev.Name}) + continue + } + } } // SaveConfigFile is not used by VMs. From af0eaedf1be3fc83a2d0343dedb80318f990b451 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:04:25 +0000 Subject: [PATCH 31/39] lxd/instance/drivers/driver/qemu: Removes deprecated StorageStart and StorageStop Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_qemu.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go index ff0eecee2c..c5504de77c 100644 --- a/lxd/instance/drivers/driver_qemu.go +++ b/lxd/instance/drivers/driver_qemu.go @@ -4705,16 +4705,6 @@ func (vm *qemu) SetOperation(op *operations.Operation) { vm.op = op } -// StorageStart deprecated. -func (vm *qemu) StorageStart() (bool, error) { - return false, storagePools.ErrNotImplemented -} - -// StorageStop deprecated. -func (vm *qemu) StorageStop() (bool, error) { - return false, storagePools.ErrNotImplemented -} - // DeferTemplateApply not used currently. func (vm *qemu) DeferTemplateApply(trigger string) error { err := vm.VolatileSet(map[string]string{"volatile.apply_template": trigger}) From 378f71a11c1cb5fc2ad73ee39e80705e479baac6 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:05:24 +0000 Subject: [PATCH 32/39] lxd/patches: Updates instance mount usage Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/patches.go | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/lxd/patches.go b/lxd/patches.go index d3ca662f9b..31bbb1b6ad 100644 --- a/lxd/patches.go +++ b/lxd/patches.go @@ -1523,15 +1523,12 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d err = func() error { // In case the new LVM logical volume for the container is not mounted mount it. if !shared.IsMountPoint(newContainerMntPoint) { - mountInfo, err := pool.MountInstance(ctStruct, nil) + _, err = pool.MountInstance(ctStruct, nil) if err != nil { logger.Errorf("Failed to mount new empty LVM logical volume for container %s: %s", ct, err) return err } - - if mountInfo.OurMount { - defer pool.UnmountInstance(ctStruct, nil) - } + defer pool.UnmountInstance(ctStruct, nil) } // Use rsync to fill the empty volume. @@ -1689,15 +1686,12 @@ func upgradeFromStorageTypeLvm(name string, d *Daemon, defaultPoolName string, d err = func() error { // In case the new LVM logical volume for the snapshot is not mounted mount it. if !shared.IsMountPoint(newSnapshotMntPoint) { - mountInfo, err := pool.MountInstanceSnapshot(csStruct, nil) + _, err = pool.MountInstanceSnapshot(csStruct, nil) if err != nil { logger.Errorf("Failed to mount new empty LVM logical volume for container %s: %s", cs, err) return err } - - if mountInfo.OurMount { - defer pool.UnmountInstanceSnapshot(csStruct, nil) - } + defer pool.UnmountInstanceSnapshot(csStruct, nil) } // Use rsync to fill the snapshot volume. @@ -3325,14 +3319,11 @@ func patchStorageApiPermissions(name string, d *Daemon) error { // Run task in anonymous function so as not to stack up defers. err = func() error { - ourMount, err := pool.MountCustomVolume(project.Default, vol, nil) + err = pool.MountCustomVolume(project.Default, vol, nil) if err != nil { return err } - - if ourMount { - defer pool.UnmountCustomVolume(project.Default, vol, nil) - } + defer pool.UnmountCustomVolume(project.Default, vol, nil) cuMntPoint := storageDrivers.GetVolumeMountPath(poolName, storageDrivers.VolumeTypeCustom, vol) err = os.Chmod(cuMntPoint, 0711) @@ -3355,26 +3346,30 @@ func patchStorageApiPermissions(name string, d *Daemon) error { for _, ct := range cRegular { // load the container from the database - ctStruct, err := instance.LoadByProjectAndName(d.State(), "default", ct) + inst, err := instance.LoadByProjectAndName(d.State(), project.Default, ct) if err != nil { return err } - ourMount, err := ctStruct.StorageStart() + // Start the storage if needed + pool, err := storagePools.GetPoolByInstance(d.State(), inst) if err != nil { return err } - if ctStruct.IsPrivileged() { - err = os.Chmod(ctStruct.Path(), 0700) - } else { - err = os.Chmod(ctStruct.Path(), 0711) + _, err = storagePools.InstanceMount(pool, inst, nil) + if err != nil { + return err } - if ourMount { - ctStruct.StorageStop() + if inst.IsPrivileged() { + err = os.Chmod(inst.Path(), 0700) + } else { + err = os.Chmod(inst.Path(), 0711) } + storagePools.InstanceUnmount(pool, inst, nil) + if err != nil && !os.IsNotExist(err) { return err } From 11e387a1d3baa82da3eb83cdfc2a3308668f7da5 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:06:19 +0000 Subject: [PATCH 33/39] lxd/instance/metadata: Removes use of c.StorageStart and c.StorageStop Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance_metadata.go | 50 ++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/lxd/instance_metadata.go b/lxd/instance_metadata.go index 53050d67f1..4d15a40869 100644 --- a/lxd/instance_metadata.go +++ b/lxd/instance_metadata.go @@ -15,6 +15,7 @@ import ( "github.com/lxc/lxd/lxd/instance" "github.com/lxc/lxd/lxd/response" + storagePools "github.com/lxc/lxd/lxd/storage" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" ) @@ -42,18 +43,21 @@ func containerMetadataGet(d *Daemon, r *http.Request) response.Response { if err != nil { return response.SmartError(err) } - metadataPath := filepath.Join(c.Path(), "metadata.yaml") // Start the storage if needed - ourStart, err := c.StorageStart() + pool, err := storagePools.GetPoolByInstance(d.State(), c) if err != nil { return response.SmartError(err) } - if ourStart { - defer c.StorageStop() + + _, err = storagePools.InstanceMount(pool, c, nil) + if err != nil { + return response.SmartError(err) } + defer storagePools.InstanceUnmount(pool, c, nil) // If missing, just return empty result + metadataPath := filepath.Join(c.Path(), "metadata.yaml") if !shared.PathExists(metadataPath) { return response.SyncResponse(true, api.ImageMetadata{}) } @@ -103,16 +107,18 @@ func containerMetadataPut(d *Daemon, r *http.Request) response.Response { if err != nil { return response.SmartError(err) } - metadataPath := filepath.Join(c.Path(), "metadata.yaml") // Start the storage if needed - ourStart, err := c.StorageStart() + pool, err := storagePools.GetPoolByInstance(d.State(), c) if err != nil { return response.SmartError(err) } - if ourStart { - defer c.StorageStop() + + _, err = storagePools.InstanceMount(pool, c, nil) + if err != nil { + return response.SmartError(err) } + defer storagePools.InstanceUnmount(pool, c, nil) // Read the new metadata metadata := api.ImageMetadata{} @@ -126,6 +132,7 @@ func containerMetadataPut(d *Daemon, r *http.Request) response.Response { return response.BadRequest(err) } + metadataPath := filepath.Join(c.Path(), "metadata.yaml") if err := ioutil.WriteFile(metadataPath, data, 0644); err != nil { return response.InternalError(err) } @@ -159,13 +166,16 @@ func containerMetadataTemplatesGet(d *Daemon, r *http.Request) response.Response } // Start the storage if needed - ourStart, err := c.StorageStart() + pool, err := storagePools.GetPoolByInstance(d.State(), c) if err != nil { return response.SmartError(err) } - if ourStart { - defer c.StorageStop() + + _, err = storagePools.InstanceMount(pool, c, nil) + if err != nil { + return response.SmartError(err) } + defer storagePools.InstanceUnmount(pool, c, nil) // Look at the request templateName := r.FormValue("path") @@ -253,13 +263,16 @@ func containerMetadataTemplatesPostPut(d *Daemon, r *http.Request) response.Resp } // Start the storage if needed - ourStart, err := c.StorageStart() + pool, err := storagePools.GetPoolByInstance(d.State(), c) if err != nil { return response.SmartError(err) } - if ourStart { - defer c.StorageStop() + + _, err = storagePools.InstanceMount(pool, c, nil) + if err != nil { + return response.SmartError(err) } + defer storagePools.InstanceUnmount(pool, c, nil) // Look at the request templateName := r.FormValue("path") @@ -326,13 +339,16 @@ func containerMetadataTemplatesDelete(d *Daemon, r *http.Request) response.Respo } // Start the storage if needed - ourStart, err := c.StorageStart() + pool, err := storagePools.GetPoolByInstance(d.State(), c) if err != nil { return response.SmartError(err) } - if ourStart { - defer c.StorageStop() + + _, err = storagePools.InstanceMount(pool, c, nil) + if err != nil { + return response.SmartError(err) } + defer storagePools.InstanceUnmount(pool, c, nil) // Look at the request templateName := r.FormValue("path") From 25d433201987c8605981daa3e5bfde5b56fe813c Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:06:39 +0000 Subject: [PATCH 34/39] lxd/instance/test: Removes use of c2.StorageStart Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/lxd/instance_test.go b/lxd/instance_test.go index 35a28e1bd7..5a87179b83 100644 --- a/lxd/instance_test.go +++ b/lxd/instance_test.go @@ -146,8 +146,6 @@ func (suite *containerTestSuite) TestContainer_LoadFromDB() { c2, err := instance.LoadByProjectAndName(state, "default", "testFoo") c2.IsRunning() suite.Req.Nil(err) - _, err = c2.StorageStart() - suite.Req.Nil(err) instanceDrivers.PrepareEqualTest(c, c2) suite.Exactly( From 000e2e9cd3d4a2eacc36a23411fb74c5d2a8157d Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:07:01 +0000 Subject: [PATCH 35/39] lxd/instance: Updates instanceCreateAsSnapshot to use updated mount functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lxd/instance.go b/lxd/instance.go index ff87a0edc2..00514630c8 100644 --- a/lxd/instance.go +++ b/lxd/instance.go @@ -415,13 +415,11 @@ func instanceCreateAsSnapshot(s *state.State, args db.InstanceArgs, sourceInstan } // Mount volume for backup.yaml writing. - mountInfo, err := pool.MountInstance(sourceInstance, op) + _, err = pool.MountInstance(sourceInstance, op) if err != nil { return nil, errors.Wrap(err, "Create instance snapshot (mount source)") } - if mountInfo.OurMount { - defer pool.UnmountInstance(sourceInstance, op) - } + defer pool.UnmountInstance(sourceInstance, op) // Attempt to update backup.yaml for instance. err = sourceInstance.UpdateBackupFile() From 3d3dd4e960aefa3a0ffae7baa13a02796d2877eb Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:07:27 +0000 Subject: [PATCH 36/39] lxd/devices: Register devices on all instance types Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/devices.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/devices.go b/lxd/devices.go index e72e125fc9..67d3def366 100644 --- a/lxd/devices.go +++ b/lxd/devices.go @@ -583,7 +583,7 @@ func deviceEventListener(s *state.State) { // devicesRegister calls the Register() function on all supported devices so they receive events. func devicesRegister(s *state.State) { - instances, err := instance.LoadNodeAll(s, instancetype.Container) + instances, err := instance.LoadNodeAll(s, instancetype.Any) if err != nil { logger.Error("Problem loading instances list", log.Ctx{"err": err}) return From 123c91f5e8453bd01e1c59b5513ac6b4e9f43f54 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:08:23 +0000 Subject: [PATCH 37/39] lxd/device/disk: Implements Register function This allows disk devices of running instances to register themselves with the mount ref counters when LXD starts. Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/device/disk.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/lxd/device/disk.go b/lxd/device/disk.go index d303a5d02e..04f5e73f95 100644 --- a/lxd/device/disk.go +++ b/lxd/device/disk.go @@ -245,6 +245,39 @@ func (d *disk) CanHotPlug() (bool, []string) { return true, []string{"limits.max", "limits.read", "limits.write", "size"} } +func (d *disk) Register() error { + d.logger.Debug("Initialising mounted disk ref counter") + + if d.config["path"] == "/" { + pool, err := storagePools.GetPoolByInstance(d.state, d.inst) + if err != nil { + return err + } + + _, err = pool.MountInstance(d.inst, nil) + if err != nil { + return err + } + } else if d.config["path"] != "/" && d.config["source"] != "" && d.config["pool"] != "" { + pool, err := storagePools.GetPoolByName(d.state, d.config["pool"]) + if err != nil { + return err + } + + storageProjectName, err := project.StorageVolumeProject(d.state.Cluster, d.inst.Project(), db.StoragePoolVolumeTypeCustom) + if err != nil { + return err + } + + err = pool.MountCustomVolume(storageProjectName, d.config["source"], nil) + if err != nil { + return err + } + } + + return nil +} + // Start is run when the device is added to the instance. func (d *disk) Start() (*deviceConfig.RunConfig, error) { err := d.validateEnvironment() From 88f124a3a92ec7452e418124cfc50dc1af3d6d5b Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:09:10 +0000 Subject: [PATCH 38/39] lxd/device/disk: Updates mount function usage in mountPoolVolume Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/device/disk.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lxd/device/disk.go b/lxd/device/disk.go index 04f5e73f95..12355ddfcb 100644 --- a/lxd/device/disk.go +++ b/lxd/device/disk.go @@ -883,14 +883,11 @@ func (d *disk) mountPoolVolume(reverter *revert.Reverter) (string, error) { return "", err } - ourMount, err := pool.MountCustomVolume(storageProjectName, volumeName, nil) + err = pool.MountCustomVolume(storageProjectName, volumeName, nil) if err != nil { return "", errors.Wrapf(err, "Failed mounting storage volume %q of type %q on storage pool %q", volumeName, volumeTypeName, pool.Name()) } - - if ourMount { - reverter.Add(func() { pool.UnmountCustomVolume(storageProjectName, volumeName, nil) }) - } + reverter.Add(func() { pool.UnmountCustomVolume(storageProjectName, volumeName, nil) }) _, vol, err := d.state.Cluster.GetLocalStoragePoolVolume(storageProjectName, volumeName, db.StoragePoolVolumeTypeCustom, pool.ID()) if err != nil { From d07256b717ceaa34e26adda74c582654e424c6f6 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Thu, 12 Nov 2020 10:10:23 +0000 Subject: [PATCH 39/39] lxd/daemon/storage: Updates mount function usage for daemon custom volume functions Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/daemon_storage.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lxd/daemon_storage.go b/lxd/daemon_storage.go index 8369606854..c1d85688b7 100644 --- a/lxd/daemon_storage.go +++ b/lxd/daemon_storage.go @@ -53,7 +53,7 @@ func daemonStorageMount(s *state.State) error { } // Mount volume. - _, err = pool.MountCustomVolume(project.Default, volumeName, nil) + err = pool.MountCustomVolume(project.Default, volumeName, nil) if err != nil { return errors.Wrapf(err, "Failed to mount storage volume %q", source) } @@ -119,13 +119,11 @@ func daemonStorageValidate(s *state.State, target string) error { } // Mount volume. - ourMount, err := pool.MountCustomVolume(project.Default, volumeName, nil) + err = pool.MountCustomVolume(project.Default, volumeName, nil) if err != nil { return errors.Wrapf(err, "Failed to mount storage volume %q", target) } - if ourMount { - defer pool.UnmountCustomVolume(project.Default, volumeName, nil) - } + defer pool.UnmountCustomVolume(project.Default, volumeName, nil) // Validate volume is empty (ignore lost+found). volStorageName := project.StorageVolume(project.Default, volumeName) @@ -249,7 +247,7 @@ func daemonStorageMove(s *state.State, storageType string, target string) error } // Mount volume. - _, err = pool.MountCustomVolume(project.Default, volumeName, nil) + err = pool.MountCustomVolume(project.Default, volumeName, nil) if err != nil { return errors.Wrapf(err, "Failed to mount storage volume %q", target) }
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel