* [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.