[PATCH 1/3] blspec: refactor to prepare for booting by file path

[PATCH 1/3] blspec: refactor to prepare for booting by file path

[Ahmad Fatoum]

The blspec boot entry provider currently feeds the path into
blspec_scan_directory, which will fail to collect an boot entry when
given a regular file. In preparation for allowing to boot a specific
blspec file by path, refactor the code pertaining to a single file into
a blspec_scan_file function and move parse_nfs_url which applies equally
to files and directories into the callsite. There is one other callsite
of blspec_scan_directory in blspec_scan_cdev, but that one will never
handle NFS paths, so it's fine to skip calling parse_nfs_url there.

No functional change intended.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/blspec.c  | 112 ++++++++++++++++++++++++-----------------------
 include/blspec.h |   2 +
 2 files changed, 60 insertions(+), 54 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index 158fd1e9a28c..d798f6f38a4b 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -513,6 +513,54 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
 	return ret;
 }
 
+int blspec_scan_file(struct bootentries *bootentries, const char *root,
+		     const char *configname)
+{
+	char *devname = NULL, *hwdevname = NULL;
+	struct blspec_entry *entry;
+
+	if (blspec_have_entry(bootentries, configname))
+		return -EEXIST;
+
+	entry = blspec_entry_open(bootentries, configname);
+	if (IS_ERR(entry))
+		return PTR_ERR(entry);
+
+	entry->rootpath = xstrdup(root);
+	entry->configpath = xstrdup(configname);
+	entry->cdev = get_cdev_by_mountpath(root);
+
+	if (!entry_is_of_compatible(entry)) {
+		blspec_entry_free(&entry->entry);
+		return -ENODEV;
+	}
+
+	if (!entry_is_match_machine_id(entry)) {
+		blspec_entry_free(&entry->entry);
+		return -ENODEV;
+	}
+
+	if (entry->cdev && entry->cdev->dev) {
+		devname = xstrdup(dev_name(entry->cdev->dev));
+		if (entry->cdev->dev->parent)
+			hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
+	}
+
+	entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
+				       configname);
+	entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
+					    devname ? devname : "none",
+					    hwdevname ? hwdevname : "none");
+	free(devname);
+	free(hwdevname);
+
+	entry->entry.me.type = MENU_ENTRY_NORMAL;
+	entry->entry.release = blspec_entry_free;
+
+	bootentries_add_entry(bootentries, &entry->entry);
+	return 1;
+}
+
 /*
  * blspec_scan_directory - scan over a directory
  *
@@ -522,17 +570,11 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
  */
 int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 {
-	struct blspec_entry *entry;
 	DIR *dir;
 	struct dirent *d;
 	char *abspath;
 	int ret, found = 0;
 	const char *dirname = "loader/entries";
-	char *nfspath = NULL;
-
-	nfspath = parse_nfs_url(root);
-	if (!IS_ERR(nfspath))
-		root = nfspath;
 
 	pr_debug("%s: %s %s\n", __func__, root, dirname);
 
@@ -549,7 +591,6 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		char *configname;
 		struct stat s;
 		char *dot;
-		char *devname = NULL, *hwdevname = NULL;
 
 		if (*d->d_name == '.')
 			continue;
@@ -578,59 +619,15 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 			continue;
 		}
 
-		if (blspec_have_entry(bootentries, configname)) {
-			free(configname);
-			continue;
-		}
-
-		entry = blspec_entry_open(bootentries, configname);
-		if (IS_ERR(entry)) {
-			free(configname);
-			continue;
-		}
-
-		entry->rootpath = xstrdup(root);
-		entry->configpath = configname;
-		entry->cdev = get_cdev_by_mountpath(root);
-
-		if (!entry_is_of_compatible(entry)) {
-			blspec_entry_free(&entry->entry);
-			continue;
-		}
-
-		if (!entry_is_match_machine_id(entry)) {
-			blspec_entry_free(&entry->entry);
-			continue;
-		}
-
-		found++;
-
-		if (entry->cdev && entry->cdev->dev) {
-			devname = xstrdup(dev_name(entry->cdev->dev));
-			if (entry->cdev->dev->parent)
-				hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
-		}
-
-		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
-					       configname);
-		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
-						    devname ? devname : "none",
-						    hwdevname ? hwdevname : "none");
-		free(devname);
-		free(hwdevname);
-
-		entry->entry.me.type = MENU_ENTRY_NORMAL;
-		entry->entry.release = blspec_entry_free;
-
-		bootentries_add_entry(bootentries, &entry->entry);
+		ret = blspec_scan_file(bootentries, root, configname);
+		if (ret == 1)
+			found++;
 	}
 
 	ret = found;
 
 	closedir(dir);
 err_out:
-	if (!IS_ERR(nfspath))
-		free(nfspath);
 	free(abspath);
 
 	return ret;
@@ -846,9 +843,16 @@ static int blspec_bootentry_provider(struct bootentries *bootentries,
 		found += ret;
 
 	if (*name == '/' || !strncmp(name, "nfs://", 6)) {
+		char *nfspath = parse_nfs_url(name);
+		if (!IS_ERR(nfspath))
+			name = nfspath;
+
 		ret = blspec_scan_directory(bootentries, name);
 		if (ret > 0)
 			found += ret;
+
+		if (!IS_ERR(nfspath))
+			free(nfspath);
 	}
 
 	return found;
diff --git a/include/blspec.h b/include/blspec.h
index 37076cd47c97..8f2556f6620e 100644
--- a/include/blspec.h
+++ b/include/blspec.h
@@ -23,5 +23,7 @@ int blspec_scan_devices(struct bootentries *bootentries);
 int blspec_scan_device(struct bootentries *bootentries, struct device_d *dev);
 int blspec_scan_devicename(struct bootentries *bootentries, const char *devname);
 int blspec_scan_directory(struct bootentries *bootentries, const char *root);
+int blspec_scan_file(struct bootentries *bootentries, const char *root,
+		     const char *configname);
 
 #endif /* __LOADER_H__ */
-- 
2.30.2

[PATCH 2/3] blspec: iterate over entries in lexical order

[Ahmad Fatoum]

We already iterate over boot scripts in lexical order, so it makes sense
to do the same for bootspec entries. This way, it's stable which boot
entry will be taken, when multiple match.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/blspec.c | 43 ++++++++++---------------------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index d798f6f38a4b..cbaa211cb16f 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -15,6 +15,7 @@
 #include <libbb.h>
 #include <init.h>
 #include <bootm.h>
+#include <glob.h>
 #include <net.h>
 #include <fs.h>
 #include <of.h>
@@ -570,54 +571,30 @@ int blspec_scan_file(struct bootentries *bootentries, const char *root,
  */
 int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 {
-	DIR *dir;
-	struct dirent *d;
+	glob_t globb;
 	char *abspath;
 	int ret, found = 0;
 	const char *dirname = "loader/entries";
+	int i;
 
 	pr_debug("%s: %s %s\n", __func__, root, dirname);
 
-	abspath = basprintf("%s/%s", root, dirname);
+	abspath = basprintf("%s/%s/*.conf", root, dirname);
 
-	dir = opendir(abspath);
-	if (!dir) {
+	ret = glob(abspath, 0, NULL, &globb);
+	if (ret) {
 		pr_debug("%s: %s: %s\n", __func__, abspath, strerror(errno));
 		ret = -errno;
 		goto err_out;
 	}
 
-	while ((d = readdir(dir))) {
-		char *configname;
+	for (i = 0; i < globb.gl_pathc; i++) {
+		const char *configname = globb.gl_pathv[i];
 		struct stat s;
-		char *dot;
-
-		if (*d->d_name == '.')
-			continue;
-
-		configname = basprintf("%s/%s", abspath, d->d_name);
-
-		dot = strrchr(configname, '.');
-		if (!dot) {
-			free(configname);
-			continue;
-		}
-
-		if (strcmp(dot, ".conf")) {
-			free(configname);
-			continue;
-		}
 
 		ret = stat(configname, &s);
-		if (ret) {
-			free(configname);
+		if (ret || !S_ISREG(s.st_mode))
 			continue;
-		}
-
-		if (!S_ISREG(s.st_mode)) {
-			free(configname);
-			continue;
-		}
 
 		ret = blspec_scan_file(bootentries, root, configname);
 		if (ret == 1)
@@ -626,7 +603,7 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 
 	ret = found;
 
-	closedir(dir);
+	globfree(&globb);
 err_out:
 	free(abspath);
 
-- 
2.30.2

[PATCH 3/3] boot: allow booting by bootspec absolute path

[Ahmad Fatoum]

When multiple bootloader spec files are available, the first match in
lexical order will be the one to autoboot. Users can customize which one
to use interactively via boot -m, but no means to select a different by
default exists. Allow for this by having the boot command not only
accept a directory to search for bootloader spec entries in, but also
the path of a single bootloader spec file. This aligns it with what we
have for bootscripts, where both directories containing boot scripts
and the path to a specific boot script is understood.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig |  1 +
 commands/boot.c  |  1 +
 common/blspec.c  | 11 ++++++++++-
 common/boot.c    |  1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 69d76a9c8028..3e21dc4c0500 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -367,6 +367,7 @@ config CMD_BOOT
 	  BOOTSRC can be:
 	  - a filename under /env/boot/
 	  - a full path to a boot script
+	  - a full path to a bootspec entry
 	  - a device name
 	  - a partition name under /dev/
 	  - a full path to a directory which
diff --git a/commands/boot.c b/commands/boot.c
index 18f4e36ec733..3f67b3bec6b0 100644
--- a/commands/boot.c
+++ b/commands/boot.c
@@ -122,6 +122,7 @@ BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("BOOTSRC can be:")
 BAREBOX_CMD_HELP_TEXT("- a filename under /env/boot/")
 BAREBOX_CMD_HELP_TEXT("- a full path to a boot script")
+BAREBOX_CMD_HELP_TEXT("- a full path to a bootspec entry")
 BAREBOX_CMD_HELP_TEXT("- a device name")
 BAREBOX_CMD_HELP_TEXT("- a partition name under /dev/")
 BAREBOX_CMD_HELP_TEXT("- a full path to a directory which")
diff --git a/common/blspec.c b/common/blspec.c
index cbaa211cb16f..340752337f3c 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -527,6 +527,7 @@ int blspec_scan_file(struct bootentries *bootentries, const char *root,
 	if (IS_ERR(entry))
 		return PTR_ERR(entry);
 
+	root = root ?: get_mounted_path(configname);
 	entry->rootpath = xstrdup(root);
 	entry->configpath = xstrdup(configname);
 	entry->cdev = get_cdev_by_mountpath(root);
@@ -813,6 +814,7 @@ int blspec_scan_devicename(struct bootentries *bootentries, const char *devname)
 static int blspec_bootentry_provider(struct bootentries *bootentries,
 				     const char *name)
 {
+	struct stat s;
 	int ret, found = 0;
 
 	ret = blspec_scan_devicename(bootentries, name);
@@ -824,7 +826,14 @@ static int blspec_bootentry_provider(struct bootentries *bootentries,
 		if (!IS_ERR(nfspath))
 			name = nfspath;
 
-		ret = blspec_scan_directory(bootentries, name);
+		ret = stat(name, &s);
+		if (ret)
+			return found;
+
+		if (S_ISDIR(s.st_mode))
+			ret = blspec_scan_directory(bootentries, name);
+		else if (S_ISREG(s.st_mode) && strends(name, ".conf"))
+			ret = blspec_scan_file(bootentries, NULL, name);
 		if (ret > 0)
 			found += ret;
 
diff --git a/common/boot.c b/common/boot.c
index 8220b8d3fbd7..58bb02ac2548 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -272,6 +272,7 @@ int bootentry_register_provider(int (*fn)(struct bootentries *bootentries, const
  * name can be:
  * - a name of a boot script under /env/boot
  * - a full path of a boot script
+ * - a full path of a bootloader spec entry
  * - a device name
  * - a cdev name
  * - a full path of a directory containing bootloader spec entries
-- 
2.30.2

Re: [PATCH 1/3] blspec: refactor to prepare for booting by file path

[Sascha Hauer]

On Sun, Jul 24, 2022 at 08:48:05PM +0200, Ahmad Fatoum wrote:
> The blspec boot entry provider currently feeds the path into
> blspec_scan_directory, which will fail to collect an boot entry when
> given a regular file. In preparation for allowing to boot a specific
> blspec file by path, refactor the code pertaining to a single file into
> a blspec_scan_file function and move parse_nfs_url which applies equally
> to files and directories into the callsite. There is one other callsite
> of blspec_scan_directory in blspec_scan_cdev, but that one will never
> handle NFS paths, so it's fine to skip calling parse_nfs_url there.
> 
> No functional change intended.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/blspec.c  | 112 ++++++++++++++++++++++++-----------------------
>  include/blspec.h |   2 +
>  2 files changed, 60 insertions(+), 54 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/common/blspec.c b/common/blspec.c
> index 158fd1e9a28c..d798f6f38a4b 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -513,6 +513,54 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
>  	return ret;
>  }
>  
> +int blspec_scan_file(struct bootentries *bootentries, const char *root,
> +		     const char *configname)
> +{
> +	char *devname = NULL, *hwdevname = NULL;
> +	struct blspec_entry *entry;
> +
> +	if (blspec_have_entry(bootentries, configname))
> +		return -EEXIST;
> +
> +	entry = blspec_entry_open(bootentries, configname);
> +	if (IS_ERR(entry))
> +		return PTR_ERR(entry);
> +
> +	entry->rootpath = xstrdup(root);
> +	entry->configpath = xstrdup(configname);
> +	entry->cdev = get_cdev_by_mountpath(root);
> +
> +	if (!entry_is_of_compatible(entry)) {
> +		blspec_entry_free(&entry->entry);
> +		return -ENODEV;
> +	}
> +
> +	if (!entry_is_match_machine_id(entry)) {
> +		blspec_entry_free(&entry->entry);
> +		return -ENODEV;
> +	}
> +
> +	if (entry->cdev && entry->cdev->dev) {
> +		devname = xstrdup(dev_name(entry->cdev->dev));
> +		if (entry->cdev->dev->parent)
> +			hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> +	}
> +
> +	entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> +				       configname);
> +	entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> +					    devname ? devname : "none",
> +					    hwdevname ? hwdevname : "none");
> +	free(devname);
> +	free(hwdevname);
> +
> +	entry->entry.me.type = MENU_ENTRY_NORMAL;
> +	entry->entry.release = blspec_entry_free;
> +
> +	bootentries_add_entry(bootentries, &entry->entry);
> +	return 1;
> +}
> +
>  /*
>   * blspec_scan_directory - scan over a directory
>   *
> @@ -522,17 +570,11 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
>   */
>  int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  {
> -	struct blspec_entry *entry;
>  	DIR *dir;
>  	struct dirent *d;
>  	char *abspath;
>  	int ret, found = 0;
>  	const char *dirname = "loader/entries";
> -	char *nfspath = NULL;
> -
> -	nfspath = parse_nfs_url(root);
> -	if (!IS_ERR(nfspath))
> -		root = nfspath;
>  
>  	pr_debug("%s: %s %s\n", __func__, root, dirname);
>  
> @@ -549,7 +591,6 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  		char *configname;
>  		struct stat s;
>  		char *dot;
> -		char *devname = NULL, *hwdevname = NULL;
>  
>  		if (*d->d_name == '.')
>  			continue;
> @@ -578,59 +619,15 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  			continue;
>  		}
>  
> -		if (blspec_have_entry(bootentries, configname)) {
> -			free(configname);
> -			continue;
> -		}
> -
> -		entry = blspec_entry_open(bootentries, configname);
> -		if (IS_ERR(entry)) {
> -			free(configname);
> -			continue;
> -		}
> -
> -		entry->rootpath = xstrdup(root);
> -		entry->configpath = configname;
> -		entry->cdev = get_cdev_by_mountpath(root);
> -
> -		if (!entry_is_of_compatible(entry)) {
> -			blspec_entry_free(&entry->entry);
> -			continue;
> -		}
> -
> -		if (!entry_is_match_machine_id(entry)) {
> -			blspec_entry_free(&entry->entry);
> -			continue;
> -		}
> -
> -		found++;
> -
> -		if (entry->cdev && entry->cdev->dev) {
> -			devname = xstrdup(dev_name(entry->cdev->dev));
> -			if (entry->cdev->dev->parent)
> -				hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> -		}
> -
> -		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> -					       configname);
> -		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> -						    devname ? devname : "none",
> -						    hwdevname ? hwdevname : "none");
> -		free(devname);
> -		free(hwdevname);
> -
> -		entry->entry.me.type = MENU_ENTRY_NORMAL;
> -		entry->entry.release = blspec_entry_free;
> -
> -		bootentries_add_entry(bootentries, &entry->entry);
> +		ret = blspec_scan_file(bootentries, root, configname);
> +		if (ret == 1)
> +			found++;
>  	}
>  
>  	ret = found;
>  
>  	closedir(dir);
>  err_out:
> -	if (!IS_ERR(nfspath))
> -		free(nfspath);
>  	free(abspath);
>  
>  	return ret;
> @@ -846,9 +843,16 @@ static int blspec_bootentry_provider(struct bootentries *bootentries,
>  		found += ret;
>  
>  	if (*name == '/' || !strncmp(name, "nfs://", 6)) {
> +		char *nfspath = parse_nfs_url(name);
> +		if (!IS_ERR(nfspath))
> +			name = nfspath;
> +
>  		ret = blspec_scan_directory(bootentries, name);
>  		if (ret > 0)
>  			found += ret;
> +
> +		if (!IS_ERR(nfspath))
> +			free(nfspath);
>  	}
>  
>  	return found;
> diff --git a/include/blspec.h b/include/blspec.h
> index 37076cd47c97..8f2556f6620e 100644
> --- a/include/blspec.h
> +++ b/include/blspec.h
> @@ -23,5 +23,7 @@ int blspec_scan_devices(struct bootentries *bootentries);
>  int blspec_scan_device(struct bootentries *bootentries, struct device_d *dev);
>  int blspec_scan_devicename(struct bootentries *bootentries, const char *devname);
>  int blspec_scan_directory(struct bootentries *bootentries, const char *root);
> +int blspec_scan_file(struct bootentries *bootentries, const char *root,
> +		     const char *configname);
>  
>  #endif /* __LOADER_H__ */
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |