[PATCH] ieee1275/ofdisk: retry on open and read failure

[PATCH] ieee1275/ofdisk: retry on open and read failure

[Diego Domingos]

Sometimes, when booting from a very busy SAN, the access to the
disk can fail and then grub will eventually drop to grub prompt.
This scenario is more frequent when deploying many machines at
the same time using the same SAN.
This patch aims to force the ofdisk module to retry the open or
read function after it fails. We use MAX_RETRIES to specify the
amount of times it will try to access the disk before it
definitely fails.

---
 grub-core/disk/ieee1275/ofdisk.c | 27 +++++++++++++++++++++------
 include/grub/ieee1275/ofdisk.h   |  8 ++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
index d887d4b..777ae63 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c
@@ -225,7 +225,9 @@ dev_iterate (const struct grub_ieee1275_devalias *alias)
       char *buf, *bufptr;
       unsigned i;
 
-      if (grub_ieee1275_open (alias->path, &ihandle))
+
+      RETRY_IEEE1275_OFDISK_OPEN(alias->path, &ihandle)
+      if (! ihandle)
 	return;
 
       /* This method doesn't need memory allocation for the table. Open
@@ -305,7 +307,9 @@ dev_iterate (const struct grub_ieee1275_devalias *alias)
           return;
         }
 
-      if (grub_ieee1275_open (alias->path, &ihandle))
+      RETRY_IEEE1275_OFDISK_OPEN(alias->path, &ihandle)
+
+      if (! ihandle)
         {
           grub_free (buf);
           grub_free (table);
@@ -555,7 +559,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t sector)
       last_ihandle = 0;
       last_devpath = NULL;
 
-      grub_ieee1275_open (disk->data, &last_ihandle);
+      RETRY_IEEE1275_OFDISK_OPEN(disk->data, &last_ihandle)
       if (! last_ihandle)
 	return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device");
       last_devpath = disk->data;      
@@ -582,12 +586,23 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
     return err;
   grub_ieee1275_read (last_ihandle, buf, size  << disk->log_sector_size,
 		      &actual);
-  if (actual != (grub_ssize_t) (size  << disk->log_sector_size))
+  int i = 0;
+  while(actual != (grub_ssize_t) (size  << disk->log_sector_size)){
+    if (i>10){
     return grub_error (GRUB_ERR_READ_ERROR, N_("failure reading sector 0x%llx "
 					       "from `%s'"),
 		       (unsigned long long) sector,
 		       disk->name);
-
+    }
+    grub_dprintf("ofdisk","Read failed. Retrying...\n");
+    last_devpath = NULL;
+    err = grub_ofdisk_prepare (disk, sector);
+    if (err)
+      return err;
+    grub_ieee1275_read (last_ihandle, buf, size  << disk->log_sector_size,
+                      &actual);
+    i++;
+  }
   return 0;
 }
 
@@ -704,7 +719,7 @@ grub_ofdisk_get_block_size (const char *device, grub_uint32_t *block_size,
   last_ihandle = 0;
   last_devpath = NULL;
 
-  grub_ieee1275_open (device, &last_ihandle);
+  RETRY_IEEE1275_OFDISK_OPEN (device, &last_ihandle)
   if (! last_ihandle)
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device");
 
diff --git a/include/grub/ieee1275/ofdisk.h b/include/grub/ieee1275/ofdisk.h
index 2f69e3f..124e297 100644
--- a/include/grub/ieee1275/ofdisk.h
+++ b/include/grub/ieee1275/ofdisk.h
@@ -22,4 +22,12 @@
 extern void grub_ofdisk_init (void);
 extern void grub_ofdisk_fini (void);
 
+#define MAX_RETRIES 20
+
+
+#define RETRY_IEEE1275_OFDISK_OPEN(device, last_ihandle) unsigned retry_i=0;for(retry_i=0; retry_i < MAX_RETRIES; retry_i++){ \
+						if(!grub_ieee1275_open(device, last_ihandle)) \
+						break; \
+						grub_dprintf("ofdisk","Opening disk %s failed. Retrying...\n",device); }
+
 #endif /* ! GRUB_INIT_HEADER */
-- 
2.27.0

[PATCH] ieee1275/ofdisk: retry on open and read failure

[Mukesh Kumar Chaurasiya]

Sometimes, when booting from a very busy SAN, the access to the
disk can fail and then grub will eventually drop to grub prompt.
This scenario is more frequent when deploying many machines at
the same time using the same SAN.
This patch aims to force the ofdisk module to retry the open or
read function after it fails. We use MAX_RETRIES to specify the
amount of times it will try to access the disk before it
definitely fails.

Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.vnet.ibm.com>
---
 grub-core/disk/ieee1275/ofdisk.c | 65 +++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
index c6cba0c8a..f4183a531 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c
@@ -24,6 +24,9 @@
 #include <grub/ieee1275/ofdisk.h>
 #include <grub/i18n.h>
 #include <grub/time.h>
+#include <grub/env.h>
+
+#define RETRY_DEFAULT_TIMEOUT 15000
 
 static char *last_devpath;
 static grub_ieee1275_ihandle_t last_ihandle;
@@ -452,7 +455,7 @@ compute_dev_path (const char *name)
 }
 
 static grub_err_t
-grub_ofdisk_open (const char *name, grub_disk_t disk)
+grub_ofdisk_open_real (const char *name, grub_disk_t disk)
 {
   grub_ieee1275_phandle_t dev;
   char *devpath;
@@ -525,6 +528,41 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
   return 0;
 }
 
+static grub_uint64_t
+grub_ofdisk_disk_timeout(void)
+{
+   if(grub_env_get("ofdisk_retry_timeout") != NULL)
+     {
+	grub_uint64_t retry = grub_strtoul(grub_env_get("ofdisk_retry_timeout"), 0, 10);
+	if(retry)
+	  return retry;
+     }
+
+   return RETRY_DEFAULT_TIMEOUT;
+}
+
+static grub_err_t
+grub_ofdisk_open (const char *name, grub_disk_t disk)
+{
+  grub_err_t err;
+  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
+
+ retry:
+  err = grub_ofdisk_open_real (name, disk);
+
+  if (err == GRUB_ERR_UNKNOWN_DEVICE)
+    {
+      if (grub_get_time_ms () < timeout)
+        {
+          grub_dprintf ("ofdisk","Failed to open disk %s. Retrying...\n", name);
+          grub_errno = GRUB_ERR_NONE;
+          goto retry;
+	}
+    }
+
+  return err;
+}
+
 static void
 grub_ofdisk_close (grub_disk_t disk)
 {
@@ -568,7 +606,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t sector)
 }
 
 static grub_err_t
-grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
+grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
 		  grub_size_t size, char *buf)
 {
   grub_err_t err;
@@ -587,6 +625,29 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
   return 0;
 }
 
+static grub_err_t
+grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
+		  grub_size_t size, char *buf)
+{
+  grub_err_t err;
+  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
+
+ retry:
+  err = grub_ofdisk_read_real (disk, sector, size, buf);
+
+  if (err == GRUB_ERR_READ_ERROR)
+    {
+      if (grub_get_time_ms () < timeout)
+        {
+          grub_dprintf ("ofdisk","Failed to read disk %s. Retrying...\n", (char*)disk->data);
+          grub_errno = GRUB_ERR_NONE;
+          goto retry;
+	}
+    }
+
+  return err;
+}
+
 static grub_err_t
 grub_ofdisk_write (grub_disk_t disk, grub_disk_addr_t sector,
 		   grub_size_t size, const char *buf)
-- 
2.31.1

Re: [PATCH] ieee1275/ofdisk: retry on open and read failure

[Michael Chang]

On Tue, Mar 28, 2023 at 11:00:01AM +0530, Mukesh Kumar Chaurasiya wrote:
> Sometimes, when booting from a very busy SAN, the access to the
> disk can fail and then grub will eventually drop to grub prompt.
> This scenario is more frequent when deploying many machines at
> the same time using the same SAN.
> This patch aims to force the ofdisk module to retry the open or
> read function after it fails. We use MAX_RETRIES to specify the
> amount of times it will try to access the disk before it
> definitely fails.

To clarify this is a continuation of previous patch [1]. Obviously the
count of retries, MAX_RETRIES, has been replaced by a timeout,
RETRY_DEFAULT_TIMEOUT, which is 15000 milliseconds or fifteen seconds.
It appears that the description was not updated accordingly and needs to
be amended.

[1] https://www.mail-archive.com/grub-devel@gnu.org/msg32174.html

Thanks,
Michael

> 
> Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.vnet.ibm.com>
> ---
>  grub-core/disk/ieee1275/ofdisk.c | 65 +++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..f4183a531 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -24,6 +24,9 @@
>  #include <grub/ieee1275/ofdisk.h>
>  #include <grub/i18n.h>
>  #include <grub/time.h>
> +#include <grub/env.h>
> +
> +#define RETRY_DEFAULT_TIMEOUT 15000
>  
>  static char *last_devpath;
>  static grub_ieee1275_ihandle_t last_ihandle;
> @@ -452,7 +455,7 @@ compute_dev_path (const char *name)
>  }
>  
>  static grub_err_t
> -grub_ofdisk_open (const char *name, grub_disk_t disk)
> +grub_ofdisk_open_real (const char *name, grub_disk_t disk)
>  {
>    grub_ieee1275_phandle_t dev;
>    char *devpath;
> @@ -525,6 +528,41 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>    return 0;
>  }
>  
> +static grub_uint64_t
> +grub_ofdisk_disk_timeout(void)
> +{
> +   if(grub_env_get("ofdisk_retry_timeout") != NULL)
> +     {
> +	grub_uint64_t retry = grub_strtoul(grub_env_get("ofdisk_retry_timeout"), 0, 10);
> +	if(retry)
> +	  return retry;
> +     }
> +
> +   return RETRY_DEFAULT_TIMEOUT;
> +}
> +
> +static grub_err_t
> +grub_ofdisk_open (const char *name, grub_disk_t disk)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_open_real (name, disk);
> +
> +  if (err == GRUB_ERR_UNKNOWN_DEVICE)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to open disk %s. Retrying...\n", name);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;
> +	}
> +    }
> +
> +  return err;
> +}
> +
>  static void
>  grub_ofdisk_close (grub_disk_t disk)
>  {
> @@ -568,7 +606,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t sector)
>  }
>  
>  static grub_err_t
> -grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
>  		  grub_size_t size, char *buf)
>  {
>    grub_err_t err;
> @@ -587,6 +625,29 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
>    return 0;
>  }
>  
> +static grub_err_t
> +grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +		  grub_size_t size, char *buf)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_read_real (disk, sector, size, buf);
> +
> +  if (err == GRUB_ERR_READ_ERROR)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to read disk %s. Retrying...\n", (char*)disk->data);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;
> +	}
> +    }
> +
> +  return err;
> +}
> +
>  static grub_err_t
>  grub_ofdisk_write (grub_disk_t disk, grub_disk_addr_t sector,
>  		   grub_size_t size, const char *buf)
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ieee1275/ofdisk: retry on open and read failure

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

Michael Chang via Grub-devel <grub-devel@gnu.org> writes:

> On Tue, Mar 28, 2023 at 11:00:01AM +0530, Mukesh Kumar Chaurasiya wrote:
>> Sometimes, when booting from a very busy SAN, the access to the
>> disk can fail and then grub will eventually drop to grub prompt.
>> This scenario is more frequent when deploying many machines at
>> the same time using the same SAN.
>> This patch aims to force the ofdisk module to retry the open or
>> read function after it fails. We use MAX_RETRIES to specify the
>> amount of times it will try to access the disk before it
>> definitely fails.
>
> To clarify this is a continuation of previous patch [1]. Obviously the
> count of retries, MAX_RETRIES, has been replaced by a timeout,
> RETRY_DEFAULT_TIMEOUT, which is 15000 milliseconds or fifteen seconds.
> It appears that the description was not updated accordingly and needs to
> be amended.
>
> [1] https://www.mail-archive.com/grub-devel@gnu.org/msg32174.html

We carry that ^ patch basically as-is downstream.  This proposed patch
seems rather different: there's environment logic, more functions, etc..
As Michael says, it would be helpful if what's happening here could be
clarified - especially since the description mentions MAX_RETRIES from
Diego's patch, but there's no mention of Diego's authorship in the
commit message, and no MAX_RETRIES in the code...

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

[PATCH V2] ieee1275/ofdisk: retry on open and read failure

[Mukesh Kumar Chaurasiya]

Sometimes, when booting from a very busy SAN, the access to the
disk can fail and then grub will eventually drop to grub prompt.
This scenario is more frequent when deploying many machines at
the same time using the same SAN.
This patch aims to force the ofdisk module to retry the open or
read function after it fails. We use DEFAULT_RETRY_TIMEOUT, which
is 15000ms or 15 seconds to specify the time it'll retry to
access the disk before it definitely fails. The timeout can
be changed by setting the environment variable
ofdisk_retry_timeout.

Signed-off-by: Diego Domingos <diegodo@linux.vnet.ibm.com>
Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.vnet.ibm.com>
---
 grub-core/disk/ieee1275/ofdisk.c | 65 +++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
index c6cba0c8a..f4183a531 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c
@@ -24,6 +24,9 @@
 #include <grub/ieee1275/ofdisk.h>
 #include <grub/i18n.h>
 #include <grub/time.h>
+#include <grub/env.h>
+
+#define RETRY_DEFAULT_TIMEOUT 15000
 
 static char *last_devpath;
 static grub_ieee1275_ihandle_t last_ihandle;
@@ -452,7 +455,7 @@ compute_dev_path (const char *name)
 }
 
 static grub_err_t
-grub_ofdisk_open (const char *name, grub_disk_t disk)
+grub_ofdisk_open_real (const char *name, grub_disk_t disk)
 {
   grub_ieee1275_phandle_t dev;
   char *devpath;
@@ -525,6 +528,41 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
   return 0;
 }
 
+static grub_uint64_t
+grub_ofdisk_disk_timeout(void)
+{
+   if(grub_env_get("ofdisk_retry_timeout") != NULL)
+     {
+	grub_uint64_t retry = grub_strtoul(grub_env_get("ofdisk_retry_timeout"), 0, 10);
+	if(retry)
+	  return retry;
+     }
+
+   return RETRY_DEFAULT_TIMEOUT;
+}
+
+static grub_err_t
+grub_ofdisk_open (const char *name, grub_disk_t disk)
+{
+  grub_err_t err;
+  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
+
+ retry:
+  err = grub_ofdisk_open_real (name, disk);
+
+  if (err == GRUB_ERR_UNKNOWN_DEVICE)
+    {
+      if (grub_get_time_ms () < timeout)
+        {
+          grub_dprintf ("ofdisk","Failed to open disk %s. Retrying...\n", name);
+          grub_errno = GRUB_ERR_NONE;
+          goto retry;
+	}
+    }
+
+  return err;
+}
+
 static void
 grub_ofdisk_close (grub_disk_t disk)
 {
@@ -568,7 +606,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t sector)
 }
 
 static grub_err_t
-grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
+grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
 		  grub_size_t size, char *buf)
 {
   grub_err_t err;
@@ -587,6 +625,29 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
   return 0;
 }
 
+static grub_err_t
+grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
+		  grub_size_t size, char *buf)
+{
+  grub_err_t err;
+  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
+
+ retry:
+  err = grub_ofdisk_read_real (disk, sector, size, buf);
+
+  if (err == GRUB_ERR_READ_ERROR)
+    {
+      if (grub_get_time_ms () < timeout)
+        {
+          grub_dprintf ("ofdisk","Failed to read disk %s. Retrying...\n", (char*)disk->data);
+          grub_errno = GRUB_ERR_NONE;
+          goto retry;
+	}
+    }
+
+  return err;
+}
+
 static grub_err_t
 grub_ofdisk_write (grub_disk_t disk, grub_disk_addr_t sector,
 		   grub_size_t size, const char *buf)
-- 
2.31.1

Re: [PATCH V2] ieee1275/ofdisk: retry on open and read failure

[Daniel Kiper]

First of all, please start new thread when you post next version of the patch.

On Wed, Mar 29, 2023 at 11:00:13AM +0530, Mukesh Kumar Chaurasiya wrote:
> Sometimes, when booting from a very busy SAN, the access to the
> disk can fail and then grub will eventually drop to grub prompt.
> This scenario is more frequent when deploying many machines at
> the same time using the same SAN.
> This patch aims to force the ofdisk module to retry the open or
> read function after it fails. We use DEFAULT_RETRY_TIMEOUT, which
> is 15000ms or 15 seconds to specify the time it'll retry to
> access the disk before it definitely fails. The timeout can
> be changed by setting the environment variable
> ofdisk_retry_timeout.
>
> Signed-off-by: Diego Domingos <diegodo@linux.vnet.ibm.com>
> Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.vnet.ibm.com>
> ---
>  grub-core/disk/ieee1275/ofdisk.c | 65 +++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..f4183a531 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -24,6 +24,9 @@
>  #include <grub/ieee1275/ofdisk.h>
>  #include <grub/i18n.h>
>  #include <grub/time.h>
> +#include <grub/env.h>
> +
> +#define RETRY_DEFAULT_TIMEOUT 15000
>
>  static char *last_devpath;
>  static grub_ieee1275_ihandle_t last_ihandle;
> @@ -452,7 +455,7 @@ compute_dev_path (const char *name)
>  }
>
>  static grub_err_t
> -grub_ofdisk_open (const char *name, grub_disk_t disk)
> +grub_ofdisk_open_real (const char *name, grub_disk_t disk)
>  {
>    grub_ieee1275_phandle_t dev;
>    char *devpath;
> @@ -525,6 +528,41 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>    return 0;
>  }
>
> +static grub_uint64_t
> +grub_ofdisk_disk_timeout(void)

s/grub_ofdisk_disk_timeout(void)/grub_ofdisk_disk_timeout (void)/

> +{
> +   if(grub_env_get("ofdisk_retry_timeout") != NULL)

if (grub_env_get ("ofdisk_retry_timeout") != NULL)

> +     {
> +	grub_uint64_t retry = grub_strtoul(grub_env_get("ofdisk_retry_timeout"), 0, 10);

Please move variable declaration to the beginning of the function.

And you blindly assume grub_strtoul() call will not fail. This is not
true. Please fix it. The commit ac8a37dda (net/http: Allow use of
non-standard TCP/IP ports) is good example how it should be done.

And please document ofdisk_retry_timeout in the docs/grub.texi.
I think ofdisk_retry_timeout should be expressed in seconds.

> +	if(retry)
> +	  return retry;
> +     }
> +
> +   return RETRY_DEFAULT_TIMEOUT;

In general please stick to the GRUB coding style [1]. You can also take
a look at grub-core/kern/efi/sb.c.

> +}
> +
> +static grub_err_t
> +grub_ofdisk_open (const char *name, grub_disk_t disk)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_open_real (name, disk);
> +
> +  if (err == GRUB_ERR_UNKNOWN_DEVICE)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to open disk %s. Retrying...\n", name);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;

Please use while() or for() instead of goto.

And are you sure you want retry at full speed? Should not you introduce,
e.g., 1s delay between subsequent grub_ofdisk_open_real() calls?

> +	}
> +    }
> +
> +  return err;
> +}
> +
>  static void
>  grub_ofdisk_close (grub_disk_t disk)
>  {
> @@ -568,7 +606,7 @@ grub_ofdisk_prepare (grub_disk_t disk, grub_disk_addr_t sector)
>  }
>
>  static grub_err_t
> -grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
>  		  grub_size_t size, char *buf)
>  {
>    grub_err_t err;
> @@ -587,6 +625,29 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
>    return 0;
>  }
>
> +static grub_err_t
> +grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +		  grub_size_t size, char *buf)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + grub_ofdisk_disk_timeout();
> +
> + retry:
> +  err = grub_ofdisk_read_real (disk, sector, size, buf);
> +
> +  if (err == GRUB_ERR_READ_ERROR)
> +    {
> +      if (grub_get_time_ms () < timeout)
> +        {
> +          grub_dprintf ("ofdisk","Failed to read disk %s. Retrying...\n", (char*)disk->data);
> +          grub_errno = GRUB_ERR_NONE;
> +          goto retry;

Same comments as above...

Daniel

[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style