The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7824
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) ===
Hot on the heals of https://github.com/lxc/lxd/pull/7822 this PR removes the cache added in that last PR as the cache invalidation logic (when combined with the required locking) was getting complicated.
It occurred to me that the "is remote" property is a static property of the driver and not related to the version probing.
So this PR decouples the two concepts and simplifies the `RemoteDriverNames` function.
From 46394281d452f56c88b228f6baf809695d05f19e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:50:51 +0100
Subject: [PATCH 1/8] lxd/storage/drivers/interface: Adds isRemote function
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/interface.go | 1 +
1 file changed, 1 insertion(+)
diff --git a/lxd/storage/drivers/interface.go b/lxd/storage/drivers/interface.go
index 312e127f39..4aba1a134c 100644
--- a/lxd/storage/drivers/interface.go
+++ b/lxd/storage/drivers/interface.go
@@ -18,6 +18,7 @@ type driver interface {
init(state *state.State, name string, config map[string]string, logger
logger.Logger, volIDFunc func(volType VolumeType, volName string) (int64,
error), commonRules *Validators)
load() error
+ isRemote() bool
}
// Driver represents a low-level storage driver.
From d26252973b6cafbd4cb4bd84b4e12a08fdd6e821 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:51:09 +0100
Subject: [PATCH 2/8] lxd/storage/drivers/driver/common: Adds isRemote()
function that returns false
As most storage drivers are not remote.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_common.go | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lxd/storage/drivers/driver_common.go
b/lxd/storage/drivers/driver_common.go
index 1c297f2d75..843f5652f0 100644
--- a/lxd/storage/drivers/driver_common.go
+++ b/lxd/storage/drivers/driver_common.go
@@ -33,6 +33,11 @@ func (d *common) init(state *state.State, name string,
config map[string]string,
d.logger = logger
}
+// isRemote returns false indicating this driver does not use remote storage.
+func (d *common) isRemote() bool {
+ return false
+}
+
// validatePool validates a pool config against common rules and optional
driver specific rules.
func (d *common) validatePool(config map[string]string, driverRules
map[string]func(value string) error) error {
checkedFields := map[string]struct{}{}
From 4885f8f683f8364b35396a2905ddf4dbd3c3562f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:51:51 +0100
Subject: [PATCH 3/8] lxd/storage/drivers/driver: Updates non-remote driver's
Info() function to call d.isRemote()
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_btrfs.go | 2 +-
lxd/storage/drivers/driver_dir.go | 2 +-
lxd/storage/drivers/driver_lvm.go | 2 +-
lxd/storage/drivers/driver_zfs.go | 2 +-
lxd/storage/drivers/drivers_mock.go | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lxd/storage/drivers/driver_btrfs.go
b/lxd/storage/drivers/driver_btrfs.go
index 1a420f47cd..988e741277 100644
--- a/lxd/storage/drivers/driver_btrfs.go
+++ b/lxd/storage/drivers/driver_btrfs.go
@@ -77,7 +77,7 @@ func (d *btrfs) Info() Info {
OptimizedBackups: true,
OptimizedBackupHeader: true,
PreservesInodes: !d.state.OS.RunningInUserNS,
- Remote: false,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: false,
RunningQuotaResize: true,
diff --git a/lxd/storage/drivers/driver_dir.go
b/lxd/storage/drivers/driver_dir.go
index a9ea65e131..906024f17b 100644
--- a/lxd/storage/drivers/driver_dir.go
+++ b/lxd/storage/drivers/driver_dir.go
@@ -38,7 +38,7 @@ func (d *dir) Info() Info {
Version: "1",
OptimizedImages: false,
PreservesInodes: false,
- Remote: false,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: false,
RunningQuotaResize: true,
diff --git a/lxd/storage/drivers/driver_lvm.go
b/lxd/storage/drivers/driver_lvm.go
index 2082cc8cdf..f68d537cd8 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -91,7 +91,7 @@ func (d *lvm) Info() Info {
Version: lvmVersion,
OptimizedImages: d.usesThinpool(), // Only thinpool pools
support optimized images.
PreservesInodes: false,
- Remote: false,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: true,
RunningQuotaResize: false,
diff --git a/lxd/storage/drivers/driver_zfs.go
b/lxd/storage/drivers/driver_zfs.go
index 73be97127d..387a351f5f 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -108,7 +108,7 @@ func (d *zfs) Info() Info {
OptimizedImages: true,
OptimizedBackups: true,
PreservesInodes: true,
- Remote: false,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: false,
RunningQuotaResize: true,
diff --git a/lxd/storage/drivers/drivers_mock.go
b/lxd/storage/drivers/drivers_mock.go
index 229ed179a4..9d12ac3dee 100644
--- a/lxd/storage/drivers/drivers_mock.go
+++ b/lxd/storage/drivers/drivers_mock.go
@@ -26,7 +26,7 @@ func (d *mock) Info() Info {
Version: "1",
OptimizedImages: false,
PreservesInodes: false,
- Remote: false,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: false,
RunningQuotaResize: true,
From 64e6e6191aa5c4a61148bc48e99d70e90d1b2521 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:52:36 +0100
Subject: [PATCH 4/8] lxd/storage/drivers/ceph: Implements isRemote function
for ceph and cephfs
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph.go | 5 +++++
lxd/storage/drivers/driver_cephfs.go | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/lxd/storage/drivers/driver_ceph.go
b/lxd/storage/drivers/driver_ceph.go
index f0b97bcdce..afd49e8193 100644
--- a/lxd/storage/drivers/driver_ceph.go
+++ b/lxd/storage/drivers/driver_ceph.go
@@ -64,6 +64,11 @@ func (d *ceph) load() error {
return nil
}
+// isRemote returns true indicating this driver uses remote storage.
+func (d *ceph) isRemote() bool {
+ return true
+}
+
// Info returns info about the driver and its environment.
func (d *ceph) Info() Info {
return Info{
diff --git a/lxd/storage/drivers/driver_cephfs.go
b/lxd/storage/drivers/driver_cephfs.go
index 39af63e0f4..f07ac2140f 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -63,6 +63,11 @@ func (d *cephfs) load() error {
return nil
}
+// isRemote returns true indicating this driver uses remote storage.
+func (d *cephfs) isRemote() bool {
+ return true
+}
+
// Info returns the pool driver information.
func (d *cephfs) Info() Info {
return Info{
From 667345ab35cc99a2f03085b1abf5d23c928efba1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:53:03 +0100
Subject: [PATCH 5/8] drivers isRemote Info
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_ceph.go | 2 +-
lxd/storage/drivers/driver_cephfs.go | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/storage/drivers/driver_ceph.go
b/lxd/storage/drivers/driver_ceph.go
index afd49e8193..f13f1a3c0e 100644
--- a/lxd/storage/drivers/driver_ceph.go
+++ b/lxd/storage/drivers/driver_ceph.go
@@ -76,7 +76,7 @@ func (d *ceph) Info() Info {
Version: cephVersion,
OptimizedImages: true,
PreservesInodes: false,
- Remote: true,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom,
VolumeTypeImage, VolumeTypeContainer, VolumeTypeVM},
BlockBacking: true,
RunningQuotaResize: false,
diff --git a/lxd/storage/drivers/driver_cephfs.go
b/lxd/storage/drivers/driver_cephfs.go
index f07ac2140f..d275212c45 100644
--- a/lxd/storage/drivers/driver_cephfs.go
+++ b/lxd/storage/drivers/driver_cephfs.go
@@ -75,7 +75,7 @@ func (d *cephfs) Info() Info {
Version: cephfsVersion,
OptimizedImages: false,
PreservesInodes: false,
- Remote: true,
+ Remote: d.isRemote(),
VolumeTypes: []VolumeType{VolumeTypeCustom},
BlockBacking: false,
RunningQuotaResize: true,
From 3a196eb27a964342558d0230928706feb6187a1e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:54:13 +0100
Subject: [PATCH 6/8] lxd/storage/drivers/load: Removes SupportedDrivers
caching and updates comment
No need for caching now (and more important no need for cache invalidation!) as
RemoteDriverNames() is being changed to not need to load the driver.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/load.go | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/lxd/storage/drivers/load.go b/lxd/storage/drivers/load.go
index c458714d95..a480403252 100644
--- a/lxd/storage/drivers/load.go
+++ b/lxd/storage/drivers/load.go
@@ -46,18 +46,10 @@ func Load(state *state.State, driverName string, name
string, config map[string]
return d, nil
}
-// supportedDrivers cache of supported drivers to avoid inspecting the storage
layer every time.
-var supportedDrivers []Info
-
-// SupportedDrivers returns a list of supported storage drivers.
+// SupportedDrivers returns a list of supported storage drivers by loading
each storage driver and running its
+// compatibility inspection process. This can take a long time if a driver is
not supported.
func SupportedDrivers(s *state.State) []Info {
- // Return cached list if available.
- if supportedDrivers != nil {
- return supportedDrivers
- }
-
- // Initialise fresh cache and populate.
- supportedDrivers = make([]Info, 0, len(drivers))
+ supportedDrivers := make([]Info, 0, len(drivers))
for driverName := range drivers {
driver, err := Load(s, driverName, "", nil, nil, nil, nil)
From b464a28a44f781069ab7d9c612fc3035ef361873 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:55:02 +0100
Subject: [PATCH 7/8] lxd/storage/drivers/load: Simplifies RemoteDriverNames to
use the isRemote function
No need to go through overhead of loading driver and doing supported checks.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/load.go | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/lxd/storage/drivers/load.go b/lxd/storage/drivers/load.go
index a480403252..38e41058f6 100644
--- a/lxd/storage/drivers/load.go
+++ b/lxd/storage/drivers/load.go
@@ -74,16 +74,15 @@ func AllDriverNames() []string {
}
// RemoteDriverNames returns a list of remote storage driver names.
-func RemoteDriverNames(s *state.State) func() []string {
- return func() []string {
- var remoteDriverNames []string
-
- for _, driver := range SupportedDrivers(s) {
- if driver.Remote {
- remoteDriverNames = append(remoteDriverNames,
driver.Name)
- }
+func RemoteDriverNames() []string {
+ driverNames := make([]string, 0, len(drivers))
+ for driverName, driverFunc := range drivers {
+ if !driverFunc().isRemote() {
+ continue
}
- return remoteDriverNames
+ driverNames = append(driverNames, driverName)
}
+
+ return driverNames
}
From 607f7cf70295aa5f726d5a82b344a0e5893ab8d8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 27 Aug 2020 17:55:34 +0100
Subject: [PATCH 8/8] lxd/daemon: storageDrivers.RemoteDriverNames usage
simplifcation
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/daemon.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/daemon.go b/lxd/daemon.go
index cafe77ef02..c040e332d3 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -828,7 +828,7 @@ func (d *Daemon) init() error {
}
// Have the db package determine remote storage drivers
- db.StorageRemoteDriverNames =
storageDrivers.RemoteDriverNames(d.State())
+ db.StorageRemoteDriverNames = storageDrivers.RemoteDriverNames
/* Open the cluster database */
for {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
