[lxc-devel] [lxd/master] Storage: Mount reference counting
tomponline on Github
lxc-bot at linuxcontainers.org
Thu Nov 12 10:30:41 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 2618 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201112/69cac206/attachment-0001.bin>
-------------- next part --------------
From 47904e03ef85f80fbfbfe7461314a3cdde36c84a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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.parrott at 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)
}
More information about the lxc-devel
mailing list