* [PATCH v2] ieee1275: support added for multiple nvme bootpaths
@ 2025-05-19 11:04 Avnish Chouhan
2025-05-26 19:30 ` Daniel Kiper
0 siblings, 1 reply; 7+ messages in thread
From: Avnish Chouhan @ 2025-05-19 11:04 UTC (permalink / raw)
To: grub-devel; +Cc: brking, meghanaprakash, Avnish Chouhan
This patch sets mupltiple NVMe boot-devices for more robust boot.
Scenario where NVMe multipaths are available, all the available bootpaths (Max 5)
will be added as the boot-device.
Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
---
grub-core/osdep/unix/platform.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
grub-core/osdep/linux/ofpath.c | 6 +++---
include/grub/util/install.h | 3 +++
include/grub/util/ofpath.h | 4 ++++
4 files changed, 123 insertions(+), 4 deletion(-)
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index 55b8f40..124e0ed 100644
--- a/grub-core/osdep/unix/platform.c
+++ b/grub-core/osdep/unix/platform.c
@@ -28,6 +28,8 @@
#include <dirent.h>
#include <string.h>
#include <errno.h>
+#include <grub/util/ofpath.h>
+#define BOOTDEV_BUFFER 1000
static char *
get_ofpathname (const char *dev)
@@ -176,6 +178,105 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
return ret;
}
+
+char *
+add_multiple_nvme_bootdevices (const char *install_device)
+{
+ char *sysfs_path, *nvme_ns, *ptr, *non_splitter_path;
+ unsigned int nsid;
+ char *multipath_boot, *ofpath, *ext_dir;
+ struct dirent *ep, *splitter_ep;
+ DIR *dp, *splitter_dp;
+ char *cntl_id, *dirR1, *dirR2, *splitter_info_path;
+ int is_FC = 0, is_splitter = 0;
+
+ nvme_ns = grub_strstr (install_device, "nvme");
+ nsid = of_path_get_nvme_nsid (nvme_ns);
+ if (nsid == 0)
+ return NULL;
+
+ sysfs_path = nvme_get_syspath (nvme_ns);
+ ofpath = xasprintf ("%s",get_ofpathname (nvme_ns));
+
+ if (grub_strstr (ofpath, "fibre-channel"))
+ {
+ strcat (sysfs_path, "/device");
+ is_FC = 1;
+ }
+ else
+ {
+ strcat (sysfs_path, "/subsystem");
+ is_FC = 0;
+ }
+ if (is_FC == 0)
+ {
+ cntl_id = grub_strstr (nvme_ns, "e");
+ dirR1 = xasprintf ("nvme%c",cntl_id[1]);
+
+ splitter_info_path = xasprintf ("%s%s%s", "/sys/block/", nvme_ns, "/device");
+ splitter_dp = opendir (splitter_info_path);
+ if (!splitter_dp)
+ return NULL;
+
+ while ((splitter_ep = readdir (splitter_dp)) != NULL)
+ {
+ if (grub_strstr (splitter_ep->d_name, "nvme"))
+ {
+ if (grub_strstr (splitter_ep->d_name, dirR1))
+ continue;
+
+ ext_dir = grub_strstr (splitter_ep->d_name, "e");
+ if (!(grub_strstr (ext_dir, "n")))
+ {
+ dirR2 = xasprintf("%s", splitter_ep->d_name);
+ is_splitter = 1;
+ break;
+ }
+ }
+ }
+ closedir (splitter_dp);
+ }
+ sysfs_path = xrealpath (sysfs_path);
+ dp = opendir (sysfs_path);
+ if (!dp)
+ return NULL;
+
+ ptr = multipath_boot = xmalloc (BOOTDEV_BUFFER);
+ if (is_splitter == 0 && is_FC == 0)
+ {
+ non_splitter_path = xasprintf ("%s%s%x:1 ", get_ofpathname (dirR1), "/namespace@", nsid);
+ strncpy (ptr, non_splitter_path, strlen (non_splitter_path));
+ ptr += strlen (non_splitter_path);
+ free (non_splitter_path);
+ }
+ else
+ {
+ while ((ep = readdir (dp)) != NULL)
+ {
+ char *path;
+ if (grub_strstr (ep->d_name, "nvme"))
+ {
+ if (is_FC == 0 && !grub_strstr (ep->d_name, dirR1) && !grub_strstr (ep->d_name, dirR2))
+ continue;
+ path = xasprintf ("%s%s%x ", get_ofpathname (ep->d_name), "/namespace@", nsid);
+ if ((strlen (multipath_boot) + strlen (path)) > BOOTDEV_BUFFER)
+ {
+ grub_util_warn (_("Maximum five entries are allowed in the bootlist"));
+ free (path);
+ break;
+ }
+ strncpy (ptr, path, strlen (path));
+ ptr += strlen (path);
+ free (path);
+ }
+ }
+ }
+ *--ptr = '\0';
+ closedir (dp);
+
+ return multipath_boot;
+}
+
void
grub_install_register_ieee1275 (int is_prep, const char *install_device,
int partno, const char *relpath)
@@ -215,8 +316,19 @@ grub_install_register_ieee1275 (int is_prep, const char *install_device,
}
*ptr = '\0';
}
+ else if (grub_strstr (install_device, "nvme"))
+ {
+ boot_device = add_multiple_nvme_bootdevices (install_device);
+ }
else
- boot_device = get_ofpathname (install_device);
+ {
+ boot_device = get_ofpathname (install_device);
+ if (grub_strstr (boot_device, "nvme-of"))
+ {
+ free (boot_device);
+ boot_device = add_multiple_nvme_bootdevices (install_device);
+ }
+ }
if (grub_util_exec ((const char * []){ "nvsetenv", "boot-device",
boot_device, NULL }))
diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index 7158c8c..48f11c9 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -209,7 +209,7 @@ find_obppath (const char *sysfs_path_orig)
}
}
-static char *
+char *
xrealpath (const char *in)
{
char *out;
@@ -224,7 +224,7 @@ xrealpath (const char *in)
return out;
}
-static char *
+char *
block_device_get_sysfs_path_and_link(const char *devicenode)
{
char *rpath;
@@ -684,7 +684,7 @@ of_path_get_nvme_nsid (const char* devname)
return nsid;
}
-static char *
+char *
nvme_get_syspath (const char *nvmedev)
{
char *sysfs_path;
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 51f3b13..a67e225 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -235,6 +235,9 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
const char *efifile_path,
const char *efi_distributor);
+char *
+add_multiple_nvme_bootdevices (const char *install_device);
+
void
grub_install_register_ieee1275 (int is_prep, const char *install_device,
int partno, const char *relpath);
diff --git a/include/grub/util/ofpath.h b/include/grub/util/ofpath.h
index 5962322..78e78e7 100644
--- a/include/grub/util/ofpath.h
+++ b/include/grub/util/ofpath.h
@@ -30,5 +30,9 @@ int add_filename_to_pile (char *filename, struct ofpath_files_list_root* root);
void find_file (char* filename, char* directory, struct ofpath_files_list_root* root, int max_depth, int depth);
char* of_find_fc_host (char* host_wwpn);
void free_ofpath_files_list (struct ofpath_files_list_root* root);
+char* nvme_get_syspath (const char *nvmedev);
+char* block_device_get_sysfs_path_and_link (const char *devicenode);
+char* xrealpath (const char *in);
+unsigned int of_path_get_nvme_nsid (const char* devname);
#endif /* ! GRUB_OFPATH_MACHINE_UTIL_HEADER */
--
2.39.3
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-19 11:04 [PATCH v2] ieee1275: support added for multiple nvme bootpaths Avnish Chouhan
@ 2025-05-26 19:30 ` Daniel Kiper
2025-05-28 11:20 ` Avnish Chouhan
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kiper @ 2025-05-26 19:30 UTC (permalink / raw)
To: Avnish Chouhan; +Cc: grub-devel, brking, meghanaprakash
On Mon, May 19, 2025 at 04:34:34PM +0530, Avnish Chouhan wrote:
> This patch sets mupltiple NVMe boot-devices for more robust boot.
> Scenario where NVMe multipaths are available, all the available bootpaths (Max 5)
> will be added as the boot-device.
>
> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
> ---
> grub-core/osdep/unix/platform.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> grub-core/osdep/linux/ofpath.c | 6 +++---
> include/grub/util/install.h | 3 +++
> include/grub/util/ofpath.h | 4 ++++
> 4 files changed, 123 insertions(+), 4 deletion(-)
>
> diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
> index 55b8f40..124e0ed 100644
> --- a/grub-core/osdep/unix/platform.c
> +++ b/grub-core/osdep/unix/platform.c
> @@ -28,6 +28,8 @@
> #include <dirent.h>
> #include <string.h>
> #include <errno.h>
> +#include <grub/util/ofpath.h>
Please add empty line here.
> +#define BOOTDEV_BUFFER 1000
>
> static char *
> get_ofpathname (const char *dev)
> @@ -176,6 +178,105 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
> return ret;
> }
>
> +
> +char *
> +add_multiple_nvme_bootdevices (const char *install_device)
> +{
> + char *sysfs_path, *nvme_ns, *ptr, *non_splitter_path;
> + unsigned int nsid;
> + char *multipath_boot, *ofpath, *ext_dir;
> + struct dirent *ep, *splitter_ep;
> + DIR *dp, *splitter_dp;
> + char *cntl_id, *dirR1, *dirR2, *splitter_info_path;
> + int is_FC = 0, is_splitter = 0;
Please use bool type here.
> + nvme_ns = grub_strstr (install_device, "nvme");
grub_strstr() may return NULL. If it is not possible here then it should
be explained.
> + nsid = of_path_get_nvme_nsid (nvme_ns);
> + if (nsid == 0)
> + return NULL;
> +
> + sysfs_path = nvme_get_syspath (nvme_ns);
> + ofpath = xasprintf ("%s",get_ofpathname (nvme_ns));
Missing space after ",".
> + if (grub_strstr (ofpath, "fibre-channel"))
> + {
> + strcat (sysfs_path, "/device");
> + is_FC = 1;
> + }
> + else
> + {
> + strcat (sysfs_path, "/subsystem");
> + is_FC = 0;
> + }
> + if (is_FC == 0)
> + {
> + cntl_id = grub_strstr (nvme_ns, "e");
Again, missing NULL check and s/grub_strstr/grub_strchr/...
> + dirR1 = xasprintf ("nvme%c",cntl_id[1]);
> +
> + splitter_info_path = xasprintf ("%s%s%s", "/sys/block/", nvme_ns, "/device");
... xasprintf ("/sys/block/%s/device", nvme_ns);
> + splitter_dp = opendir (splitter_info_path);
> + if (!splitter_dp)
> + return NULL;
> +
> + while ((splitter_ep = readdir (splitter_dp)) != NULL)
> + {
> + if (grub_strstr (splitter_ep->d_name, "nvme"))
> + {
> + if (grub_strstr (splitter_ep->d_name, dirR1))
> + continue;
> +
> + ext_dir = grub_strstr (splitter_ep->d_name, "e");
s/grub_strstr/grub_strchr/
Missing NULL check too...
> + if (!(grub_strstr (ext_dir, "n")))
s/grub_strstr/grub_strchr/
grub_strchr (ext_dir, 'n') == NULL
> + {
> + dirR2 = xasprintf("%s", splitter_ep->d_name);
Something is off with indention...
> + is_splitter = 1;
> + break;
> + }
> + }
> + }
> + closedir (splitter_dp);
> + }
> + sysfs_path = xrealpath (sysfs_path);
> + dp = opendir (sysfs_path);
> + if (!dp)
> + return NULL;
> +
> + ptr = multipath_boot = xmalloc (BOOTDEV_BUFFER);
> + if (is_splitter == 0 && is_FC == 0)
> + {
> + non_splitter_path = xasprintf ("%s%s%x:1 ", get_ofpathname (dirR1), "/namespace@", nsid);
... xasprintf ("%s/namespace@%x:1 ", get_ofpathname (dirR1), nsid);
> + strncpy (ptr, non_splitter_path, strlen (non_splitter_path));
> + ptr += strlen (non_splitter_path);
> + free (non_splitter_path);
> + }
> + else
> + {
> + while ((ep = readdir (dp)) != NULL)
> + {
> + char *path;
Please add empty line here...
> + if (grub_strstr (ep->d_name, "nvme"))
grub_strstr (ep->d_name, "nvme") != NULL
> + {
> + if (is_FC == 0 && !grub_strstr (ep->d_name, dirR1) && !grub_strstr (ep->d_name, dirR2))
... grub_strstr (ep->d_name, dirR1) == NULL && grub_strstr (ep->d_name, dirR2) == NULL ...
> + continue;
> + path = xasprintf ("%s%s%x ", get_ofpathname (ep->d_name), "/namespace@", nsid);
... xasprintf ("%s/namespace@%x:1 ", get_ofpathname (ep->d_name), nsid);
> + if ((strlen (multipath_boot) + strlen (path)) > BOOTDEV_BUFFER)
> + {
> + grub_util_warn (_("Maximum five entries are allowed in the bootlist"));
> + free (path);
> + break;
> + }
> + strncpy (ptr, path, strlen (path));
> + ptr += strlen (path);
> + free (path);
> + }
> + }
> + }
> + *--ptr = '\0';
> + closedir (dp);
> +
> + return multipath_boot;
> +}
> +
> void
> grub_install_register_ieee1275 (int is_prep, const char *install_device,
> int partno, const char *relpath)
> @@ -215,8 +316,19 @@ grub_install_register_ieee1275 (int is_prep, const char *install_device,
> }
> *ptr = '\0';
> }
> + else if (grub_strstr (install_device, "nvme"))
> + {
> + boot_device = add_multiple_nvme_bootdevices (install_device);
> + }
> else
> - boot_device = get_ofpathname (install_device);
> + {
> + boot_device = get_ofpathname (install_device);
> + if (grub_strstr (boot_device, "nvme-of"))
> + {
> + free (boot_device);
> + boot_device = add_multiple_nvme_bootdevices (install_device);
> + }
> + }
>
> if (grub_util_exec ((const char * []){ "nvsetenv", "boot-device",
> boot_device, NULL }))
> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> index 7158c8c..48f11c9 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -209,7 +209,7 @@ find_obppath (const char *sysfs_path_orig)
> }
> }
>
> -static char *
> +char *
> xrealpath (const char *in)
> {
> char *out;
> @@ -224,7 +224,7 @@ xrealpath (const char *in)
> return out;
> }
>
> -static char *
> +char *
You do not need this change.
> block_device_get_sysfs_path_and_link(const char *devicenode)
> {
> char *rpath;
> @@ -684,7 +684,7 @@ of_path_get_nvme_nsid (const char* devname)
> return nsid;
> }
>
> -static char *
> +char *
> nvme_get_syspath (const char *nvmedev)
> {
> char *sysfs_path;
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 51f3b13..a67e225 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -235,6 +235,9 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
> const char *efifile_path,
> const char *efi_distributor);
>
> +char *
> +add_multiple_nvme_bootdevices (const char *install_device);
> +
> void
> grub_install_register_ieee1275 (int is_prep, const char *install_device,
> int partno, const char *relpath);
> diff --git a/include/grub/util/ofpath.h b/include/grub/util/ofpath.h
> index 5962322..78e78e7 100644
> --- a/include/grub/util/ofpath.h
> +++ b/include/grub/util/ofpath.h
> @@ -30,5 +30,9 @@ int add_filename_to_pile (char *filename, struct ofpath_files_list_root* root);
> void find_file (char* filename, char* directory, struct ofpath_files_list_root* root, int max_depth, int depth);
> char* of_find_fc_host (char* host_wwpn);
> void free_ofpath_files_list (struct ofpath_files_list_root* root);
> +char* nvme_get_syspath (const char *nvmedev);
> +char* block_device_get_sysfs_path_and_link (const char *devicenode);
Please drop this declaration.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-26 19:30 ` Daniel Kiper
@ 2025-05-28 11:20 ` Avnish Chouhan
2025-05-28 17:56 ` Daniel Kiper
0 siblings, 1 reply; 7+ messages in thread
From: Avnish Chouhan @ 2025-05-28 11:20 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel, brking, meghanaprakash
Hi Daniel,
Thank you so much for a review!
On 2025-05-27 01:00, Daniel Kiper wrote:
> On Mon, May 19, 2025 at 04:34:34PM +0530, Avnish Chouhan wrote:
>> This patch sets mupltiple NVMe boot-devices for more robust boot.
>> Scenario where NVMe multipaths are available, all the available
>> bootpaths (Max 5)
>> will be added as the boot-device.
>>
>> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
>> ---
>> grub-core/osdep/unix/platform.c | 114
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> grub-core/osdep/linux/ofpath.c | 6 +++---
>> include/grub/util/install.h | 3 +++
>> include/grub/util/ofpath.h | 4 ++++
>> 4 files changed, 123 insertions(+), 4 deletion(-)
>>
>> diff --git a/grub-core/osdep/unix/platform.c
>> b/grub-core/osdep/unix/platform.c
>> index 55b8f40..124e0ed 100644
>> --- a/grub-core/osdep/unix/platform.c
>> +++ b/grub-core/osdep/unix/platform.c
>> @@ -28,6 +28,8 @@
>> #include <dirent.h>
>> #include <string.h>
>> #include <errno.h>
>> +#include <grub/util/ofpath.h>
>
> Please add empty line here.
Sure!
>
>> +#define BOOTDEV_BUFFER 1000
>>
>> static char *
>> get_ofpathname (const char *dev)
>> @@ -176,6 +178,105 @@ grub_install_register_efi (grub_device_t
>> efidir_grub_dev,
>> return ret;
>> }
>>
>> +
>> +char *
>> +add_multiple_nvme_bootdevices (const char *install_device)
>> +{
>> + char *sysfs_path, *nvme_ns, *ptr, *non_splitter_path;
>> + unsigned int nsid;
>> + char *multipath_boot, *ofpath, *ext_dir;
>> + struct dirent *ep, *splitter_ep;
>> + DIR *dp, *splitter_dp;
>> + char *cntl_id, *dirR1, *dirR2, *splitter_info_path;
>> + int is_FC = 0, is_splitter = 0;
>
> Please use bool type here.
Sure!
>
>> + nvme_ns = grub_strstr (install_device, "nvme");
>
> grub_strstr() may return NULL. If it is not possible here then it
> should
> be explained.
>
NULL check not required here as this function works only on
install_device as "/dev/nvme3n1p1". I'll add the comment in the next
version!
>> + nsid = of_path_get_nvme_nsid (nvme_ns);
>> + if (nsid == 0)
>> + return NULL;
>> +
>> + sysfs_path = nvme_get_syspath (nvme_ns);
>> + ofpath = xasprintf ("%s",get_ofpathname (nvme_ns));
>
> Missing space after ",".
Sure, I'll add a space after ','.
>
>> + if (grub_strstr (ofpath, "fibre-channel"))
>> + {
>> + strcat (sysfs_path, "/device");
>> + is_FC = 1;
>> + }
>> + else
>> + {
>> + strcat (sysfs_path, "/subsystem");
>> + is_FC = 0;
>> + }
>> + if (is_FC == 0)
>> + {
>> + cntl_id = grub_strstr (nvme_ns, "e");
>
> Again, missing NULL check and s/grub_strstr/grub_strchr/...
>
NULL check not required as explained above. I'll change grub_strstr to
grub_strchr.
>> + dirR1 = xasprintf ("nvme%c",cntl_id[1]);
>> +
>> + splitter_info_path = xasprintf ("%s%s%s", "/sys/block/",
>> nvme_ns, "/device");
>
> ... xasprintf ("/sys/block/%s/device", nvme_ns);
>
Sure!
>> + splitter_dp = opendir (splitter_info_path);
>> + if (!splitter_dp)
>> + return NULL;
>> +
>> + while ((splitter_ep = readdir (splitter_dp)) != NULL)
>> + {
>> + if (grub_strstr (splitter_ep->d_name, "nvme"))
>> + {
>> + if (grub_strstr (splitter_ep->d_name, dirR1))
>> + continue;
>> +
>> + ext_dir = grub_strstr (splitter_ep->d_name, "e");
>
> s/grub_strstr/grub_strchr/
>
Sure!
> Missing NULL check too...
>
Not required as the check above "if (grub_strstr (splitter_ep->d_name,
"nvme"))".
>> + if (!(grub_strstr (ext_dir, "n")))
>
> s/grub_strstr/grub_strchr/
>
Sure, I'll change this!
> grub_strchr (ext_dir, 'n') == NULL
>
>> + {
>> + dirR2 = xasprintf("%s", splitter_ep->d_name);
>
> Something is off with indention...
Sure, I'll check and correct this!
>
>> + is_splitter = 1;
>> + break;
>> + }
>> + }
>> + }
>> + closedir (splitter_dp);
>> + }
>> + sysfs_path = xrealpath (sysfs_path);
>> + dp = opendir (sysfs_path);
>> + if (!dp)
>> + return NULL;
>> +
>> + ptr = multipath_boot = xmalloc (BOOTDEV_BUFFER);
>> + if (is_splitter == 0 && is_FC == 0)
>> + {
>> + non_splitter_path = xasprintf ("%s%s%x:1 ", get_ofpathname
>> (dirR1), "/namespace@", nsid);
>
> ... xasprintf ("%s/namespace@%x:1 ", get_ofpathname (dirR1), nsid);
>
>> + strncpy (ptr, non_splitter_path, strlen (non_splitter_path));
>> + ptr += strlen (non_splitter_path);
>> + free (non_splitter_path);
>> + }
>> + else
>> + {
>> + while ((ep = readdir (dp)) != NULL)
>> + {
>> + char *path;
>
> Please add empty line here...
>
Sure.
>> + if (grub_strstr (ep->d_name, "nvme"))
>
> grub_strstr (ep->d_name, "nvme") != NULL
>
>> + {
>> + if (is_FC == 0 && !grub_strstr (ep->d_name, dirR1) &&
>> !grub_strstr (ep->d_name, dirR2))
>
> ... grub_strstr (ep->d_name, dirR1) == NULL && grub_strstr
> (ep->d_name, dirR2) == NULL ...
>
>> + continue;
>> + path = xasprintf ("%s%s%x ", get_ofpathname
>> (ep->d_name), "/namespace@", nsid);
>
> ... xasprintf ("%s/namespace@%x:1 ", get_ofpathname (ep->d_name),
> nsid);
>
I'll change as suggested here and in other places too!
>> + if ((strlen (multipath_boot) + strlen (path)) >
>> BOOTDEV_BUFFER)
>> + {
>> + grub_util_warn (_("Maximum five entries are allowed
>> in the bootlist"));
>> + free (path);
>> + break;
>> + }
>> + strncpy (ptr, path, strlen (path));
>> + ptr += strlen (path);
>> + free (path);
>> + }
>> + }
>> + }
>> + *--ptr = '\0';
>> + closedir (dp);
>> +
>> + return multipath_boot;
>> +}
>> +
>> void
>> grub_install_register_ieee1275 (int is_prep, const char
>> *install_device,
>> int partno, const char *relpath)
>> @@ -215,8 +316,19 @@ grub_install_register_ieee1275 (int is_prep,
>> const char *install_device,
>> }
>> *ptr = '\0';
>> }
>> + else if (grub_strstr (install_device, "nvme"))
>> + {
>> + boot_device = add_multiple_nvme_bootdevices (install_device);
>> + }
>> else
>> - boot_device = get_ofpathname (install_device);
>> + {
>> + boot_device = get_ofpathname (install_device);
>> + if (grub_strstr (boot_device, "nvme-of"))
>> + {
>> + free (boot_device);
>> + boot_device = add_multiple_nvme_bootdevices
>> (install_device);
>> + }
>> + }
>>
>> if (grub_util_exec ((const char * []){ "nvsetenv", "boot-device",
>> boot_device, NULL }))
>> diff --git a/grub-core/osdep/linux/ofpath.c
>> b/grub-core/osdep/linux/ofpath.c
>> index 7158c8c..48f11c9 100644
>> --- a/grub-core/osdep/linux/ofpath.c
>> +++ b/grub-core/osdep/linux/ofpath.c
>> @@ -209,7 +209,7 @@ find_obppath (const char *sysfs_path_orig)
>> }
>> }
>>
>> -static char *
>> +char *
>> xrealpath (const char *in)
>> {
>> char *out;
>> @@ -224,7 +224,7 @@ xrealpath (const char *in)
>> return out;
>> }
>>
>> -static char *
>> +char *
>
> You do not need this change.
>
We need this function as this is used by the function
"of_path_get_nvme_nsid" we are using.
>> block_device_get_sysfs_path_and_link(const char *devicenode)
>> {
>> char *rpath;
>> @@ -684,7 +684,7 @@ of_path_get_nvme_nsid (const char* devname)
>> return nsid;
>> }
>>
>> -static char *
>> +char *
>> nvme_get_syspath (const char *nvmedev)
>> {
>> char *sysfs_path;
>> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>> index 51f3b13..a67e225 100644
>> --- a/include/grub/util/install.h
>> +++ b/include/grub/util/install.h
>> @@ -235,6 +235,9 @@ grub_install_register_efi (grub_device_t
>> efidir_grub_dev,
>> const char *efifile_path,
>> const char *efi_distributor);
>>
>> +char *
>> +add_multiple_nvme_bootdevices (const char *install_device);
>> +
>> void
>> grub_install_register_ieee1275 (int is_prep, const char
>> *install_device,
>> int partno, const char *relpath);
>> diff --git a/include/grub/util/ofpath.h b/include/grub/util/ofpath.h
>> index 5962322..78e78e7 100644
>> --- a/include/grub/util/ofpath.h
>> +++ b/include/grub/util/ofpath.h
>> @@ -30,5 +30,9 @@ int add_filename_to_pile (char *filename, struct
>> ofpath_files_list_root* root);
>> void find_file (char* filename, char* directory, struct
>> ofpath_files_list_root* root, int max_depth, int depth);
>> char* of_find_fc_host (char* host_wwpn);
>> void free_ofpath_files_list (struct ofpath_files_list_root* root);
>> +char* nvme_get_syspath (const char *nvmedev);
>> +char* block_device_get_sysfs_path_and_link (const char *devicenode);
>
> Please drop this declaration.
>
Same as explained above!
> Daniel
Regards,
Avnish Chouhan
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-28 11:20 ` Avnish Chouhan
@ 2025-05-28 17:56 ` Daniel Kiper
2025-05-29 6:29 ` Avnish Chouhan
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kiper @ 2025-05-28 17:56 UTC (permalink / raw)
To: Avnish Chouhan; +Cc: grub-devel, brking, meghanaprakash
On Wed, May 28, 2025 at 04:50:00PM +0530, Avnish Chouhan wrote:
> Hi Daniel,
>
> Thank you so much for a review!
>
> On 2025-05-27 01:00, Daniel Kiper wrote:
> > On Mon, May 19, 2025 at 04:34:34PM +0530, Avnish Chouhan wrote:
[...]
> > > diff --git a/grub-core/osdep/linux/ofpath.c
> > > b/grub-core/osdep/linux/ofpath.c
> > > index 7158c8c..48f11c9 100644
> > > --- a/grub-core/osdep/linux/ofpath.c
> > > +++ b/grub-core/osdep/linux/ofpath.c
> > > @@ -209,7 +209,7 @@ find_obppath (const char *sysfs_path_orig)
> > > }
> > > }
> > >
> > > -static char *
> > > +char *
> > > xrealpath (const char *in)
> > > {
> > > char *out;
> > > @@ -224,7 +224,7 @@ xrealpath (const char *in)
> > > return out;
> > > }
> > >
> > > -static char *
> > > +char *
> >
> > You do not need this change.
>
> We need this function as this is used by the function
> "of_path_get_nvme_nsid" we are using.
The block_device_get_sysfs_path_and_link() does not seem called from
this patch. So, probably this change belongs to another one.
> > > block_device_get_sysfs_path_and_link(const char *devicenode)
> > > {
> > > char *rpath;
> > > @@ -684,7 +684,7 @@ of_path_get_nvme_nsid (const char* devname)
> > > return nsid;
> > > }
> > >
> > > -static char *
> > > +char *
> > > nvme_get_syspath (const char *nvmedev)
> > > {
> > > char *sysfs_path;
> > > diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> > > index 51f3b13..a67e225 100644
> > > --- a/include/grub/util/install.h
> > > +++ b/include/grub/util/install.h
> > > @@ -235,6 +235,9 @@ grub_install_register_efi (grub_device_t
> > > efidir_grub_dev,
> > > const char *efifile_path,
> > > const char *efi_distributor);
> > >
> > > +char *
> > > +add_multiple_nvme_bootdevices (const char *install_device);
> > > +
> > > void
> > > grub_install_register_ieee1275 (int is_prep, const char
> > > *install_device,
> > > int partno, const char *relpath);
> > > diff --git a/include/grub/util/ofpath.h b/include/grub/util/ofpath.h
> > > index 5962322..78e78e7 100644
> > > --- a/include/grub/util/ofpath.h
> > > +++ b/include/grub/util/ofpath.h
> > > @@ -30,5 +30,9 @@ int add_filename_to_pile (char *filename, struct
> > > ofpath_files_list_root* root);
> > > void find_file (char* filename, char* directory, struct
> > > ofpath_files_list_root* root, int max_depth, int depth);
> > > char* of_find_fc_host (char* host_wwpn);
> > > void free_ofpath_files_list (struct ofpath_files_list_root* root);
> > > +char* nvme_get_syspath (const char *nvmedev);
> > > +char* block_device_get_sysfs_path_and_link (const char *devicenode);
> >
> > Please drop this declaration.
>
> Same as explained above!
As above...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-28 17:56 ` Daniel Kiper
@ 2025-05-29 6:29 ` Avnish Chouhan
2025-05-29 16:22 ` Daniel Kiper
0 siblings, 1 reply; 7+ messages in thread
From: Avnish Chouhan @ 2025-05-29 6:29 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel, brking, meghanaprakash
Hi Daniel,
I believe we need this change. We are using this function
"of_path_get_nvme_nsid" in the patch which is defined in other file, and
this "of_path_get_nvme_nsid" calls the function
"block_device_get_sysfs_path_and_link". If we don't define this in the
header file, we get this error below.
Thank you!
*****
In file included from grub-core/osdep/ofpath.c:2:
grub-core/osdep/linux/ofpath.c:228:1: error: no previous prototype for
‘block_device_get_sysfs_path_and_link’ [-Werror=missing-prototypes]
228 | block_device_get_sysfs_path_and_link(const char *devicenode)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:10855: grub-core/osdep/grub_mkrescue-ofpath.o]
Error 1
make[2]: Leaving directory '/root/splitupgrub/grub'
make[1]: *** [Makefile:12412: check-recursive] Error 1
make[1]: Leaving directory '/root/splitupgrub/grub'
make: *** [Makefile:13491: check] Error 2
*****
Regards,
Avnish Chouhan
On 2025-05-28 23:26, Daniel Kiper wrote:
> On Wed, May 28, 2025 at 04:50:00PM +0530, Avnish Chouhan wrote:
>> Hi Daniel,
>>
>> Thank you so much for a review!
>>
>> On 2025-05-27 01:00, Daniel Kiper wrote:
>> > On Mon, May 19, 2025 at 04:34:34PM +0530, Avnish Chouhan wrote:
>
> [...]
>
>> > > diff --git a/grub-core/osdep/linux/ofpath.c
>> > > b/grub-core/osdep/linux/ofpath.c
>> > > index 7158c8c..48f11c9 100644
>> > > --- a/grub-core/osdep/linux/ofpath.c
>> > > +++ b/grub-core/osdep/linux/ofpath.c
>> > > @@ -209,7 +209,7 @@ find_obppath (const char *sysfs_path_orig)
>> > > }
>> > > }
>> > >
>> > > -static char *
>> > > +char *
>> > > xrealpath (const char *in)
>> > > {
>> > > char *out;
>> > > @@ -224,7 +224,7 @@ xrealpath (const char *in)
>> > > return out;
>> > > }
>> > >
>> > > -static char *
>> > > +char *
>> >
>> > You do not need this change.
>>
>> We need this function as this is used by the function
>> "of_path_get_nvme_nsid" we are using.
>
> The block_device_get_sysfs_path_and_link() does not seem called from
> this patch. So, probably this change belongs to another one.
>
>> > > block_device_get_sysfs_path_and_link(const char *devicenode)
>> > > {
>> > > char *rpath;
>> > > @@ -684,7 +684,7 @@ of_path_get_nvme_nsid (const char* devname)
>> > > return nsid;
>> > > }
>> > >
>> > > -static char *
>> > > +char *
>> > > nvme_get_syspath (const char *nvmedev)
>> > > {
>> > > char *sysfs_path;
>> > > diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>> > > index 51f3b13..a67e225 100644
>> > > --- a/include/grub/util/install.h
>> > > +++ b/include/grub/util/install.h
>> > > @@ -235,6 +235,9 @@ grub_install_register_efi (grub_device_t
>> > > efidir_grub_dev,
>> > > const char *efifile_path,
>> > > const char *efi_distributor);
>> > >
>> > > +char *
>> > > +add_multiple_nvme_bootdevices (const char *install_device);
>> > > +
>> > > void
>> > > grub_install_register_ieee1275 (int is_prep, const char
>> > > *install_device,
>> > > int partno, const char *relpath);
>> > > diff --git a/include/grub/util/ofpath.h b/include/grub/util/ofpath.h
>> > > index 5962322..78e78e7 100644
>> > > --- a/include/grub/util/ofpath.h
>> > > +++ b/include/grub/util/ofpath.h
>> > > @@ -30,5 +30,9 @@ int add_filename_to_pile (char *filename, struct
>> > > ofpath_files_list_root* root);
>> > > void find_file (char* filename, char* directory, struct
>> > > ofpath_files_list_root* root, int max_depth, int depth);
>> > > char* of_find_fc_host (char* host_wwpn);
>> > > void free_ofpath_files_list (struct ofpath_files_list_root* root);
>> > > +char* nvme_get_syspath (const char *nvmedev);
>> > > +char* block_device_get_sysfs_path_and_link (const char *devicenode);
>> >
>> > Please drop this declaration.
>>
>> Same as explained above!
>
> As above...
>
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-29 6:29 ` Avnish Chouhan
@ 2025-05-29 16:22 ` Daniel Kiper
2025-06-03 6:03 ` Avnish Chouhan
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kiper @ 2025-05-29 16:22 UTC (permalink / raw)
To: Avnish Chouhan; +Cc: grub-devel, brking, meghanaprakash
Hi Avnish,
On Thu, May 29, 2025 at 11:59:27AM +0530, Avnish Chouhan wrote:
> Hi Daniel,
>
> I believe we need this change. We are using this function
> "of_path_get_nvme_nsid" in the patch which is defined in other file, and
> this "of_path_get_nvme_nsid" calls the function
> "block_device_get_sysfs_path_and_link". If we don't define this in the
> header file, we get this error below.
> Thank you!
>
> *****
>
> In file included from grub-core/osdep/ofpath.c:2:
> grub-core/osdep/linux/ofpath.c:228:1: error: no previous prototype for
> ‘block_device_get_sysfs_path_and_link’ [-Werror=missing-prototypes]
> 228 | block_device_get_sysfs_path_and_link(const char *devicenode)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If you drop both changes then everything should be OK.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ieee1275: support added for multiple nvme bootpaths
2025-05-29 16:22 ` Daniel Kiper
@ 2025-06-03 6:03 ` Avnish Chouhan
0 siblings, 0 replies; 7+ messages in thread
From: Avnish Chouhan @ 2025-06-03 6:03 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel, brking, meghanaprakash
Hi Daniel,
Yes, you are correct. I'll do the change as suggested.
Thank you!
Regards,
Avnish Chouhan
On 2025-05-29 21:52, Daniel Kiper wrote:
> Hi Avnish,
>
> On Thu, May 29, 2025 at 11:59:27AM +0530, Avnish Chouhan wrote:
>> Hi Daniel,
>>
>> I believe we need this change. We are using this function
>> "of_path_get_nvme_nsid" in the patch which is defined in other file,
>> and
>> this "of_path_get_nvme_nsid" calls the function
>> "block_device_get_sysfs_path_and_link". If we don't define this in the
>> header file, we get this error below.
>> Thank you!
>>
>> *****
>>
>> In file included from grub-core/osdep/ofpath.c:2:
>> grub-core/osdep/linux/ofpath.c:228:1: error: no previous prototype for
>> ‘block_device_get_sysfs_path_and_link’ [-Werror=missing-prototypes]
>> 228 | block_device_get_sysfs_path_and_link(const char *devicenode)
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> If you drop both changes then everything should be OK.
>
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-03 6:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-19 11:04 [PATCH v2] ieee1275: support added for multiple nvme bootpaths Avnish Chouhan
2025-05-26 19:30 ` Daniel Kiper
2025-05-28 11:20 ` Avnish Chouhan
2025-05-28 17:56 ` Daniel Kiper
2025-05-29 6:29 ` Avnish Chouhan
2025-05-29 16:22 ` Daniel Kiper
2025-06-03 6:03 ` Avnish Chouhan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.