Storage: Powerflex use Connect and Disconnect for NVMe/TCP

doc/.custom_wordlist.txt

@@ -228,6 +228,7 @@ SDN
 SDNs
 SDS
 SDT
+SDTs
 SeaBIOS
 Seccomp
 SELinux

doc/metadata.txt

@@ -5851,9 +5851,8 @@ If you want to specify the storage pool via its name, also set {config:option}`s
 ```
 
 ```{config:option} powerflex.sdt storage-powerflex-pool-conf
-:defaultdesc: "one of the SDT"
 :scope: "global"
-:shortdesc: "PowerFlex NVMe/TCP SDT"
+:shortdesc: "Comma separated list of PowerFlex NVMe/TCP SDTs"
 :type: "string"
 
 ```

lxd/metadata/configuration.json

@@ -6545,10 +6545,9 @@
 					},
 					{
 						"powerflex.sdt": {
-							"defaultdesc": "one of the SDT",
 							"longdesc": "",
 							"scope": "global",
-							"shortdesc": "PowerFlex NVMe/TCP SDT",
+							"shortdesc": "Comma separated list of PowerFlex NVMe/TCP SDTs",
 							"type": "string"
 						}
 					},

lxd/patches.go

@@ -94,6 +94,7 @@ var patches = []patch{
 	{name: "entity_type_instance_snapshot_on_delete_trigger_typo_fix", stage: patchPreLoadClusterConfig, run: patchEntityTypeInstanceSnapshotOnDeleteTriggerTypoFix},
 	{name: "instance_remove_volatile_last_state_ip_addresses", stage: patchPostDaemonStorage, run: patchInstanceRemoveVolatileLastStateIPAddresses},
 	{name: "entity_type_identity_certificate_split", stage: patchPreLoadClusterConfig, run: patchSplitIdentityCertificateEntityTypes},
+	{name: "storage_unset_powerflex_sdt_setting", stage: patchPostDaemonStorage, run: patchUnsetPowerFlexSDTSetting},
 }
 
 type patch struct {
@@ -920,7 +921,7 @@ func patchZfsSetContentTypeUserProperty(name string, d *Daemon) error {
 
 			zfsVolName := fmt.Sprintf("%s/%s/%s", poolName, storageDrivers.VolumeTypeCustom, project.StorageVolume(vol.Project, vol.Name))
 
-			_, err = shared.RunCommand("zfs", "set", fmt.Sprintf("lxd:content_type=%s", vol.ContentType), zfsVolName)
+			_, err = shared.RunCommandContext(d.shutdownCtx, "zfs", "set", fmt.Sprintf("lxd:content_type=%s", vol.ContentType), zfsVolName)
 			if err != nil {
 				logger.Debug("Failed setting lxd:content_type property", logger.Ctx{"name": zfsVolName, "err": err})
 			}
@@ -1432,4 +1433,16 @@ UPDATE OR REPLACE auth_groups_permissions
 	return nil
 }
 
+// patchUnsetPowerFlexSDTSetting unsets the powerflex.sdt setting from all storage pools configs.
+// The address used inside the config key was populated for all PowerFlex storage pools using the nvme mode.
+// The single address was used together with the "nvme connect-all" command to discover the remaining SDTs to connect to all of them.
+// Unsetting this key, discovering all SDTs from PowerFlex REST API and connecting to all of them using
+// the "nvme connect" command has the exact same effect.
+func patchUnsetPowerFlexSDTSetting(_ string, d *Daemon) error {
+	_, err := d.State().DB.Cluster.DB().ExecContext(d.shutdownCtx, `
+DELETE FROM storage_pools_config WHERE key = "powerflex.sdt"
+	`)
+	return err
+}
+
 // Patches end here

lxd/storage/connectors/connector.go

@@ -41,10 +41,7 @@ type Connector interface {
 	QualifiedName() (string, error)
 	LoadModules() error
 	Connect(ctx context.Context, targetQN string, targetAddrs ...string) (revert.Hook, error)
-	ConnectAll(ctx context.Context, targetAddr string) error
 	Disconnect(targetQN string) error
-	DisconnectAll() error
-	SessionID(targetQN string) (string, error)
 	findSession(targetQN string) (*session, error)
 }
 

lxd/storage/connectors/connector_iscsi.go

@@ -134,11 +134,6 @@ func (c *connectorISCSI) Connect(ctx context.Context, targetQN string, targetAdd
 	return connect(ctx, c, targetQN, targetAddresses, connectFunc)
 }
 
-// ConnectAll establishes a connection with all targets available on the given address.
-func (c *connectorISCSI) ConnectAll(ctx context.Context, targetAddr string) error {
-	return fmt.Errorf("ConnectAll not implemented")
-}
-
 // Disconnect terminates a connection with the target.
 func (c *connectorISCSI) Disconnect(targetQN string) error {
 	// Find an existing iSCSI session.
@@ -175,16 +170,6 @@ func (c *connectorISCSI) Disconnect(targetQN string) error {
 	return nil
 }
 
-// DisconnectAll terminates all connections with all targets.
-func (c *connectorISCSI) DisconnectAll() error {
-	return fmt.Errorf("DisconnectAll not implemented")
-}
-
-// SessionID returns the ID of an existing session.
-func (c *connectorISCSI) SessionID(targetQN string) (string, error) {
-	return "", fmt.Errorf("SessionID not implemented")
-}
-
 // findSession returns an active iSCSI session that matches the given targetQN.
 // If the session is not found, nil session is returned.
 //

lxd/storage/connectors/connector_nvme.go

@@ -29,7 +29,7 @@ func (c *connectorNVMe) Type() string {
 // Version returns the version of the NVMe CLI.
 func (c *connectorNVMe) Version() (string, error) {
 	// Detect and record the version of the NVMe CLI.
-	out, err := shared.RunCommand("nvme", "version")
+	out, err := shared.RunCommandContext(context.Background(), "nvme", "version")
 	if err != nil {
 		return "", fmt.Errorf("Failed to get nvme-cli version: %w", err)
 	}
@@ -85,21 +85,6 @@ func (c *connectorNVMe) Connect(ctx context.Context, targetQN string, targetAddr
 	return connect(ctx, c, targetQN, targetAddresses, connectFunc)
 }
 
-// ConnectAll establishes a connection with all targets available on the given address.
-func (c *connectorNVMe) ConnectAll(ctx context.Context, targetAddr string) error {
-	hostNQN, err := c.QualifiedName()
-	if err != nil {
-		return err
-	}
-
-	_, err = shared.RunCommandContext(ctx, "nvme", "connect-all", "--transport", "tcp", "--traddr", targetAddr, "--hostnqn", hostNQN, "--hostid", c.serverUUID)
-	if err != nil {
-		return fmt.Errorf("Failed to connect to any target on %q via NVMe: %w", targetAddr, err)
-	}
-
-	return nil
-}
-
 // Disconnect terminates a connection with the target.
 func (c *connectorNVMe) Disconnect(targetQN string) error {
 	// Find an existing NVMe session.
@@ -122,27 +107,6 @@ func (c *connectorNVMe) Disconnect(targetQN string) error {
 	return nil
 }
 
-// DisconnectAll terminates all connections with all targets.
-func (c *connectorNVMe) DisconnectAll() error {
-	_, err := shared.RunCommand("nvme", "disconnect-all")
-	if err != nil {
-		return fmt.Errorf("Failed disconnecting from NVMe targets: %w", err)
-	}
-
-	return nil
-}
-
-// SessionID returns the identifier of a session that matches the targetQN.
-// If no session is found, an empty string is returned.
-func (c *connectorNVMe) SessionID(targetQN string) (string, error) {
-	session, err := c.findSession(targetQN)
-	if err != nil || session == nil {
-		return "", err
-	}
-
-	return session.id, nil
-}
-
 // findSession returns an active NVMe subsystem (referred to as session for
 // consistency across connectors) that matches the given targetQN.
 // If the session is not found, nil is returned.

lxd/storage/connectors/connector_sdc.go

@@ -2,6 +2,9 @@ package connectors
 
 import (
 	"context"
+	"fmt"
+
+	"github.com/dell/goscaleio"
 
 	"github.com/canonical/lxd/shared/revert"
 )
@@ -22,8 +25,14 @@ func (c *connectorSDC) Version() (string, error) {
 	return "", nil
 }
 
-// LoadModules returns true. SDC does not require any kernel modules to be loaded.
+// LoadModules checks if the respective SDC kernel module got already loaded outside of LXD.
+// It doesn't try to load the module as LXD doesn't have any control over it.
 func (c *connectorSDC) LoadModules() error {
+	ok := goscaleio.DrvCfgIsSDCInstalled()
+	if !ok {
+		return fmt.Errorf("SDC kernel module is not loaded")
+	}
+
 	return nil
 }
 
@@ -32,33 +41,17 @@ func (c *connectorSDC) QualifiedName() (string, error) {
 	return "", nil
 }
 
-// SessionID returns an empty string and no error, as connections are handled by SDC.
-func (c *connectorSDC) SessionID(targetQN string) (string, error) {
-	return "", nil
-}
-
 // Connect does nothing. Connections are fully handled by SDC.
 func (c *connectorSDC) Connect(ctx context.Context, targetQN string, targetAddresses ...string) (revert.Hook, error) {
 	// Nothing to do. Connection is handled by Dell SDC.
 	return revert.New().Fail, nil
 }
 
-// ConnectAll does nothing. Connections are fully handled by SDC.
-func (c *connectorSDC) ConnectAll(ctx context.Context, targetAddr string) error {
-	// Nothing to do. Connection is handled by Dell SDC.
-	return nil
-}
-
 // Disconnect does nothing. Connections are fully handled by SDC.
 func (c *connectorSDC) Disconnect(targetQN string) error {
 	return nil
 }
 
-// DisconnectAll does nothing. Connections are fully handled by SDC.
-func (c *connectorSDC) DisconnectAll() error {
-	return nil
-}
-
 func (c *connectorSDC) findSession(targetQN string) (*session, error) {
 	return nil, nil
 }

lxd/storage/drivers/driver_powerflex.go

@@ -1,8 +1,10 @@
 package drivers
 
 import (
+	"errors"
 	"fmt"
 	"strings"
+	"sync"
 
 	"github.com/dell/goscaleio"
 
@@ -42,6 +44,15 @@ type powerflex struct {
 	// Holds the SDC GUID of this specific host.
 	// Use powerflex.getHostGUID() to retrieve the actual value.
 	sdcGUID string
+
+	// Holds the lock for sdcGUID.
+	sdcGUIDLock sync.Mutex
+
+	// Holds the targetQN used by the SDTs.
+	nvmeTargetQN string
+
+	// Holds the lock for nvmeTargetQN.
+	nvmeTargetQNLock sync.Mutex
 }
 
 // load is used to run one-time action per-driver rather than per-pool.
@@ -127,6 +138,9 @@ func (d *powerflex) FillConfig() error {
 			d.config["powerflex.mode"] = connectors.TypeNVME
 		} else if goscaleio.DrvCfgIsSDCInstalled() {
 			d.config["powerflex.mode"] = connectors.TypeSDC
+		} else {
+			// Fail if no PowerFlex mode can be discovered.
+			return errors.New("Failed to discover PowerFlex mode")
 		}
 	}
 
@@ -150,52 +164,18 @@ func (d *powerflex) Create() error {
 	// Since those aren't any cluster member specific keys the general validation
 	// rules allow empty strings in order to create the pending storage pools.
 	if d.config["powerflex.pool"] == "" {
-		return fmt.Errorf("The powerflex.pool cannot be empty")
+		return errors.New("The powerflex.pool cannot be empty")
 	}
 
 	if d.config["powerflex.gateway"] == "" {
-		return fmt.Errorf("The powerflex.gateway cannot be empty")
+		return errors.New("The powerflex.gateway cannot be empty")
 	}
 
-	client := d.client()
-
-	switch d.config["powerflex.mode"] {
-	case connectors.TypeNVME:
-		// Discover one of the storage pools SDT services.
-		if d.config["powerflex.sdt"] == "" {
-			pool, err := d.resolvePool()
-			if err != nil {
-				return err
-			}
-
-			relations, err := client.getProtectionDomainSDTRelations(pool.ProtectionDomainID)
-			if err != nil {
-				return err
-			}
-
-			if len(relations) == 0 {
-				return fmt.Errorf("Failed to retrieve at least one SDT for the given storage pool: %q", pool.ID)
-			}
-
-			if len(relations[0].IPList) == 0 {
-				return fmt.Errorf("Failed to retrieve IP from SDT: %q", relations[0].Name)
-			}
-
-			d.config["powerflex.sdt"] = relations[0].IPList[0].IP
-		}
-
-	case connectors.TypeSDC:
+	if d.config["powerflex.mode"] == connectors.TypeSDC {
+		// In case the SDC mode is used the SDTs cannot be set.
 		if d.config["powerflex.sdt"] != "" {
-			return fmt.Errorf("The powerflex.sdt config key is specific to the NVMe/TCP mode")
+			return fmt.Errorf("The %q config key is specific to the %q mode", "powerflex.sdt", connectors.TypeNVME)
 		}
-
-		if !goscaleio.DrvCfgIsSDCInstalled() {
-			return fmt.Errorf("PowerFlex SDC is not available on the host")
-		}
-
-	default:
-		// Fail if no PowerFlex mode can be discovered.
-		return fmt.Errorf("Failed to discover PowerFlex mode")
 	}
 
 	return nil
@@ -272,10 +252,9 @@ func (d *powerflex) Validate(config map[string]string) error {
 		//
 		// ---
 		//  type: string
-		//  defaultdesc: one of the SDT
-		//  shortdesc: PowerFlex NVMe/TCP SDT
+		//  shortdesc: Comma separated list of PowerFlex NVMe/TCP SDTs
 		//  scope: global
-		"powerflex.sdt": validate.Optional(validate.IsNetworkAddress),
+		"powerflex.sdt": validate.Optional(validate.IsListOf(validate.IsNetworkAddress)),
 		// lxdmeta:generate(entities=storage-powerflex; group=pool-conf; key=powerflex.clone_copy)
 		// If this option is set to `true`, PowerFlex makes a non-sparse copy when creating a snapshot of an instance or custom volume.
 		// See {ref}`storage-powerflex-limitations` for more information.
@@ -307,7 +286,7 @@ func (d *powerflex) Validate(config map[string]string) error {
 	// Ensure powerflex.mode cannot be changed to avoid leaving volume mappings
 	// and to prevent disturbing running instances.
 	if oldMode != "" && oldMode != newMode {
-		return fmt.Errorf("PowerFlex mode cannot be changed")
+		return errors.New("PowerFlex mode cannot be changed")
 	}
 
 	// Check if the selected PowerFlex mode is supported on this node.
@@ -322,6 +301,8 @@ func (d *powerflex) Validate(config map[string]string) error {
 			return fmt.Errorf("PowerFlex mode %q is not supported: %w", newMode, err)
 		}
 
+		// In case of NVMe this will actually try to load the respective kernel modules.
+		// In case of SDC it will check if the kernel module got loaded outside of LXD.
 		err = connector.LoadModules()
 		if err != nil {
 			return fmt.Errorf("PowerFlex mode %q is not supported due to missing kernel modules: %w", newMode, err)