WIP

Signed-off-by: Derek Su <[email protected]>

pkg/spdk/engine.go

@@ -2138,52 +2138,60 @@ func (e *Engine) Resume(spdkClient *spdkclient.Client) (err error) {
 }
 
 // SwitchOverTarget function in the Engine struct is responsible for switching the engine's target to a new address.
-func (e *Engine) SwitchOverTarget(spdkClient *spdkclient.Client, targetAddress string) (err error) {
-	e.log.Infof("Switching over engine to target address %s", targetAddress)
+func (e *Engine) SwitchOverTarget(spdkClient *spdkclient.Client, newTargetAddress string) (err error) {
+	e.log.Infof("Switching over engine to target address %s", newTargetAddress)
 
 	currentTargetAddress := ""
 
+	podIP, err := commonnet.GetIPForPod()
+	if err != nil {
+		return errors.Wrapf(err, "failed to get IP for pod for engine %s target switchover", e.Name)
+	}
+
+	newTargetIP, newTargetPort, err := splitHostPort(newTargetAddress)
+	if err != nil {
+		return errors.Wrapf(err, "failed to split target address %s for engine %s target switchover", newTargetAddress, e.Name)
+	}
+
 	e.Lock()
 	defer func() {
 		e.Unlock()
 
 		if err != nil {
-			e.log.WithError(err).Warnf("Failed to switch over engine to target address %s", targetAddress)
+			e.log.WithError(err).Warnf("Failed to switch over engine to target address %s", newTargetAddress)
 
 			if disconnected, errCheck := e.isTargetDisconnected(); errCheck != nil {
-				e.log.WithError(errCheck).Warnf("Failed to check if target %s is disconnected", targetAddress)
+				e.log.WithError(errCheck).Warnf("Failed to check if target %s is disconnected", newTargetAddress)
 			} else if disconnected {
-				if errConnect := e.connectTarget(currentTargetAddress); errConnect != nil {
-					e.log.WithError(errConnect).Warnf("Failed to connect target back to %s", currentTargetAddress)
-				} else {
-					e.log.Infof("Connected target back to %s", currentTargetAddress)
-
-					if errReload := e.reloadDevice(); errReload != nil {
-						e.log.WithError(errReload).Warnf("Failed to reload device mapper")
+				if currentTargetAddress != "" {
+					if errConnect := e.connectTarget(currentTargetAddress); errConnect != nil {
+						e.log.WithError(errConnect).Warnf("Failed to connect target back to %s", currentTargetAddress)
 					} else {
-						e.log.Infof("Reloaded device mapper for connecting target back to %s", currentTargetAddress)
+						e.log.Infof("Connected target back to %s", currentTargetAddress)
+
+						if errReload := e.reloadDevice(); errReload != nil {
+							e.log.WithError(errReload).Warnf("Failed to reload device mapper")
+						} else {
+							e.log.Infof("Reloaded device mapper for connecting target back to %s", currentTargetAddress)
+						}
 					}
 				}
 			}
 		} else {
 			e.ErrorMsg = ""
 
-			e.log.Infof("Switched over target to %s", targetAddress)
+			e.log.Infof("Switched over target to %s", newTargetAddress)
 		}
 
 		e.UpdateCh <- nil
 	}()
 
-	podIP, err := commonnet.GetIPForPod()
-	if err != nil {
-		return err
-	}
-
 	initiator, err := nvme.NewInitiator(e.VolumeName, e.Nqn, nvme.HostProc)
 	if err != nil {
 		return errors.Wrapf(err, "failed to create initiator for engine %s target switchover", e.Name)
 	}
 
+	// Check if the engine is suspended before target switchover.
 	suspended, err := initiator.IsSuspended()
 	if err != nil {
 		return errors.Wrapf(err, "failed to check if engine %s is suspended", e.Name)
@@ -2192,43 +2200,42 @@ func (e *Engine) SwitchOverTarget(spdkClient *spdkclient.Client, targetAddress s
 		return fmt.Errorf("engine %s must be suspended before target switchover", e.Name)
 	}
 
+	// Load NVMe device info before target switchover.
 	if err := initiator.LoadNVMeDeviceInfo(initiator.TransportAddress, initiator.TransportServiceID, initiator.SubsystemNQN); err != nil {
 		if !nvme.IsValidNvmeDeviceNotFound(err) {
 			return errors.Wrapf(err, "failed to load NVMe device info for engine %s target switchover", e.Name)
 		}
 	}
 
 	currentTargetAddress = net.JoinHostPort(initiator.TransportAddress, initiator.TransportServiceID)
-	if e.isSwitchOverTargetRequired(currentTargetAddress, targetAddress) {
+	if isSwitchOverTargetRequired(currentTargetAddress, newTargetAddress) {
 		if currentTargetAddress != "" {
 			if err := e.disconnectTarget(currentTargetAddress); err != nil {
-				return err
+				return errors.Wrapf(err, "failed to disconnect target %s for engine %s", currentTargetAddress, e.Name)
 			}
 		}
 
-		if err := e.connectTarget(targetAddress); err != nil {
-			return err
+		if err := e.connectTarget(newTargetAddress); err != nil {
+			return errors.Wrapf(err, "failed to connect target %s for engine %s", newTargetAddress, e.Name)
 		}
 	}
 
 	// Replace IP and Port with the new target address.
-	// No need to update TargetIP and TargetPort, because target is not delete yet.
-	targetIP, targetPort, err := splitHostPort(targetAddress)
-	if err != nil {
-		return errors.Wrapf(err, "failed to split target address %s", targetAddress)
-	}
-
-	e.IP = targetIP
-	e.Port = targetPort
+	// No need to update TargetIP, because old target is not delete yet.
+	e.IP = newTargetIP
+	e.Port = newTargetPort
 
-	if targetIP == podIP {
-		e.TargetPort = targetPort
+	if newTargetIP == podIP {
+		e.TargetPort = newTargetPort
 		e.StandbyTargetPort = 0
+	} else {
+		e.StandbyTargetPort = e.TargetPort
+		e.TargetPort = 0
 	}
 
 	e.log.Info("Reloading device mapper after target switchover")
 	if err := e.reloadDevice(); err != nil {
-		return err
+		return errors.Wrapf(err, "failed to reload device mapper after engine %s target switchover", e.Name)
 	}
 
 	return nil
@@ -2394,7 +2401,7 @@ func (e *Engine) DeleteTarget(spdkClient *spdkclient.Client, superiorPortAllocat
 	return nil
 }
 
-func (e *Engine) isSwitchOverTargetRequired(oldTargetAddress, newTargetAddress string) bool {
+func isSwitchOverTargetRequired(oldTargetAddress, newTargetAddress string) bool {
 	return oldTargetAddress != newTargetAddress
 }