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 <[email protected]>
> ---
> 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
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
