[block] Retry any SAN device operation

The SCSI layer currently implements a retry loop in order to retry
commands that fail due to spurious "error" conditions such as "power
on occurred".  Move this retry loop to the generic SAN device layer:
this allow for retries due to other transient error conditions such as
an iSCSI target having dropped the connection due to inactivity.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/core/sanboot.c

@@ -64,6 +64,16 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  */
 #define SAN_COMMAND_TIMEOUT ( 15 * TICKS_PER_SEC )
 
+/**
+ * Number of times to retry commands
+ *
+ * We may need to retry commands.  For example, the underlying
+ * connection may be closed by the SAN target due to an inactivity
+ * timeout, or the SAN target may return pointless "error" messages
+ * such as "SCSI power-on occurred".
+ */
+#define SAN_COMMAND_MAX_RETRIES 10
+
 /** List of SAN devices */
 LIST_HEAD ( san_devices );
 
@@ -331,36 +341,42 @@ sandev_command ( struct san_device *sandev,
 		 int ( * command ) ( struct san_device *sandev,
 				     const union san_command_params *params ),
 		 const union san_command_params *params ) {
+	unsigned int retries;
 	int rc;
 
 	/* Sanity check */
 	assert ( ! timer_running ( &sandev->timer ) );
 
+	/* (Re)try command */
+	for ( retries = 0 ; retries < SAN_COMMAND_MAX_RETRIES ; retries++ ) {
+
 		/* Reopen block device if applicable */
 		if ( sandev_needs_reopen ( sandev ) &&
 		     ( ( rc = sandev_reopen ( sandev ) ) != 0 ) ) {
-	     goto err_reopen;
+			continue;
 		}
 
 		/* Start expiry timer */
 		start_timer_fixed ( &sandev->timer, SAN_COMMAND_TIMEOUT );
 
 		/* Initiate command */
-	if ( ( rc = command ( sandev, params ) ) != 0 )
+		if ( ( rc = command ( sandev, params ) ) != 0 ) {
-		goto err_op;
+			stop_timer ( &sandev->timer );
+			continue;
+		}
 
 		/* Wait for command to complete */
 		while ( timer_running ( &sandev->timer ) )
 			step();
 
-	/* Collect return status */
+		/* Exit on success */
-	rc = sandev->command_rc;
+		if ( ( rc = sandev->command_rc ) == 0 )
+			return 0;
+	}
 
-	return rc;
+	/* Sanity check */
+	assert ( ! timer_running ( &sandev->timer ) );
 
- err_op:
-	stop_timer ( &sandev->timer );
- err_reopen:
 	return rc;
 }
 

src/drivers/block/scsi.c

@@ -40,9 +40,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  *
  */
 
-/** Maximum number of command retries */
-#define SCSICMD_MAX_RETRIES 10
-
 /* Error numbers generated by SCSI sense data */
 #define EIO_NO_SENSE __einfo_error ( EINFO_EIO_NO_SENSE )
 #define EINFO_EIO_NO_SENSE \
@@ -283,9 +280,6 @@ struct scsi_command {
 	/** Command tag */
 	uint32_t tag;
 
-	/** Retry count */
-	unsigned int retries;
-
 	/** Private data */
 	uint8_t priv[0];
 };
@@ -449,28 +443,11 @@ static int scsicmd_command ( struct scsi_command *scsicmd ) {
  * @v rc		Reason for close
  */
 static void scsicmd_done ( struct scsi_command *scsicmd, int rc ) {
-	struct scsi_device *scsidev = scsicmd->scsidev;
 
 	/* Restart SCSI interface */
 	intf_restart ( &scsicmd->scsi, rc );
 
-	/* SCSI targets have an annoying habit of returning occasional
+	/* Hand over to the command completion handler */
-	 * pointless "error" messages such as "power-on occurred", so
-	 * we have to be prepared to retry commands.
-	 */
-	if ( ( rc != 0 ) && ( scsicmd->retries++ < SCSICMD_MAX_RETRIES ) ) {
-		/* Retry command */
-		DBGC ( scsidev, "SCSI %p tag %08x failed: %s\n",
-		       scsidev, scsicmd->tag, strerror ( rc ) );
-		DBGC ( scsidev, "SCSI %p tag %08x retrying (retry %d)\n",
-		       scsidev, scsicmd->tag, scsicmd->retries );
-		if ( ( rc = scsicmd_command ( scsicmd ) ) == 0 )
-			return;
-	}
-
-	/* If we didn't (successfully) reissue the command, hand over
-	 * to the command completion handler.
-	 */
 	scsicmd->type->done ( scsicmd, rc );
 }