[U-Boot] [PATCH] tools: env: Add support for direct read/write UBI volumes

["Lothar Waßmann" <LW@KARO-electronics.de>]

Hi,

On Tue, 14 Nov 2017 23:01:26 -0800 S. Lockwood-Childs wrote:
> Up to now we were able to read/write environment data from/to UBI
> volumes only indirectly by gluebi driver. This driver creates NAND MTD
> on top of UBI volumes, which is quite a workaroung for this use case.
> 
> Add support for direct read/write UBI volumes in order to not use
> obsolete gluebi driver.
> 
> Forward-ported from this patch:
> http://patchwork.ozlabs.org/patch/619305/
> 
> Original patch:
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> 
> Forward port:
> Signed-off-by: S. Lockwood-Childs <sjl@vctlabs.com>
> ---
>  tools/env/fw_env.c      | 255 +++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/env/fw_env.config |   8 ++
>  2 files changed, 261 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index ab06415..867fba5 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -25,6 +25,7 @@
>  #include <sys/ioctl.h>
>  #include <sys/stat.h>
>  #include <unistd.h>
> +#include <dirent.h>
>  
>  #ifdef MTD_OLD
>  # include <stdint.h>
> @@ -34,6 +35,8 @@
>  # include <mtd/mtd-user.h>
>  #endif
>  
> +#include <mtd/ubi-user.h>
> +
>  #include "fw_env_private.h"
>  #include "fw_env.h"
>  
> @@ -58,6 +61,7 @@ struct envdev_s {
>  	ulong erase_size;		/* device erase size */
>  	ulong env_sectors;		/* number of environment sectors */
>  	uint8_t mtd_type;		/* type of the MTD device */
> +	int is_ubi;			/* set if we use UBI volume */
>  };
>  
>  static struct envdev_s envdevices[2] =
> @@ -76,6 +80,7 @@ static int dev_current;
>  #define DEVESIZE(i)   envdevices[(i)].erase_size
>  #define ENVSECTORS(i) envdevices[(i)].env_sectors
>  #define DEVTYPE(i)    envdevices[(i)].mtd_type
> +#define IS_UBI(i)     envdevices[(i)].is_ubi
>  
>  #define CUR_ENVSIZE ENVSIZE(dev_current)
>  
> @@ -122,6 +127,228 @@ static unsigned char obsolete_flag = 0;
>  #define DEFAULT_ENV_INSTANCE_STATIC
>  #include <env_default.h>
>  
> +#define UBI_DEV_START "/dev/ubi"
> +#define UBI_SYSFS "/sys/class/ubi"
> +#define UBI_VOL_NAME_PATT "ubi%d_%d"
> +
> +static int is_ubi_devname(const char *devname)
> +{
> +	return !strncmp(devname, UBI_DEV_START, sizeof(UBI_DEV_START) - 1);
> +}
> +
> +static int ubi_check_volume_sysfs_name(const char *volume_sysfs_name,
> +				       const char *volname)
> +{
> +	char path[256];
>
/usr/include/linux/limits.h specifies the constant 'PATH_MAX' which
should be used here.

> +	FILE *file;
> +	char *name;
> +	int ret;
> +
> +	strcpy(path, UBI_SYSFS "/");
> +	strcat(path, volume_sysfs_name);
> +	strcat(path, "/name");
> +
> +	file = fopen(path, "r");
> +	if (!file)
> +		return -1;
> +
> +	ret = fscanf(file, "%ms", &name);
> +	fclose(file);
> +	if (ret <= 0 || !name) {
> +		fprintf(stderr,
> +			"Failed to read from file %s, ret = %d, name = %s\n",
> +			path, ret, name);
> +		return -1;
> +	}
> +
> +	if (!strcmp(name, volname)) {
> +		free(name);
> +		return 0;
> +	}
> +	free(name);
> +
> +	return -1;
> +}
> +
> +static int ubi_get_volnum_by_name(int devnum, const char *volname)
> +{
> +	DIR *sysfs_ubi;
> +	struct dirent *dirent;
> +	int ret;
> +	int tmp_devnum;
> +	int volnum;
> +
> +	sysfs_ubi = opendir(UBI_SYSFS);
> +	if (!sysfs_ubi)
> +		return -1;
> +
> +#ifdef DEBUG
> +	fprintf(stderr, "Looking for volume name \"%s\"\n", volname);
> +#endif
> +
> +	while (1) {
> +		dirent = readdir(sysfs_ubi);
> +		if (!dirent)
> +			return -1;
> +
> +		ret = sscanf(dirent->d_name, UBI_VOL_NAME_PATT,
> +			     &tmp_devnum, &volnum);
> +		if (ret == 2 && devnum == tmp_devnum) {
> +			if (ubi_check_volume_sysfs_name(dirent->d_name,
> +							volname) == 0)
> +				return volnum;
> +		}
> +	}
> +
> +	return -1;
>
This can never be reached.

> +}
> +
> +static int ubi_get_devnum_by_devname(const char *devname)
> +{
> +	int devnum;
> +	int ret;
> +
> +	ret = sscanf(devname + sizeof(UBI_DEV_START) - 1, "%d", &devnum);
> +	if (ret != 1)
> +		return -1;
> +
> +	return devnum;
> +}
> +
> +static const char *ubi_get_volume_devname(const char *devname,
> +					  const char *volname)
> +{
> +	char *volume_devname;
> +	int volnum;
> +	int devnum;
> +	int ret;
> +
> +	devnum = ubi_get_devnum_by_devname(devname);
> +	if (devnum < 0)
> +		return NULL;
> +
> +	volnum = ubi_get_volnum_by_name(devnum, volname);
> +	if (volnum < 0)
> +		return NULL;
> +
> +	ret = asprintf(&volume_devname, "%s_%d", devname, volnum);
> +	if (ret < 0)
> +		return NULL;
> +
> +#ifdef DEBUG
> +	fprintf(stderr, "Found ubi volume \"%s:%s\" -> %s\n",
> +		devname, volname, volume_devname);
> +#endif
> +
> +	return volume_devname;
> +}
> +
> +static void ubi_check_dev(unsigned int dev_id)
> +{
> +	char *devname = (char *)DEVNAME(dev_id);
> +	char *pname;
> +	const char *volname = NULL;
> +	const char *volume_devname;
> +
> +	if (!is_ubi_devname(DEVNAME(dev_id)))
> +		return;
> +
> +	IS_UBI(dev_id) = 1;
> +
> +	for (pname = devname; *pname != '\0'; pname++) {
> +		if (*pname == ':') {
> +			*pname = '\0';
> +			volname = pname + 1;
> +			break;
> +		}
> +	}
> +
> +	if (volname) {
> +		/* Let's find real volume device name */
> +		volume_devname = ubi_get_volume_devname(devname, volname);
> +		if (!volume_devname) {
> +			fprintf(stderr, "Didn't found ubi volume \"%s\"\n",
> +				volname);
> +			return;
> +		}
> +
> +		free(devname);
> +		DEVNAME(dev_id) = volume_devname;
> +	}
> +}
> +
> +static int ubi_update_start(int fd, int64_t bytes)
> +{
> +	if (ioctl(fd, UBI_IOCVOLUP, &bytes))
> +		return -1;
> +	return 0;
> +}
> +
> +static int ubi_read(int fd, void *buf, size_t count)
> +{
> +	ssize_t ret;
> +
> +	while (count > 0) {
> +		ret = read(fd, buf, count);
> +		if (ret > 0) {
> +			count -= ret;
> +			buf += ret;
> +
> +			continue;
> +		}
> +
> +		if (ret == 0) {
> +			/*
> +			 * Happens in case of too short volume data size. If we
> +			 * return error status we will fail it will be treated
> +			 * as UBI device error.
>
This sentence no sense.

> +			 *
> +			 * Leave catching this error to CRC check.
> +			 */
> +			fprintf(stderr, "Warning: end of data on ubi volume\n");
> +			return 0;
> +		} else if (errno == EBADF) {
> +			/*
> +			 * Happens in case of corrupted volume. The same as
> +			 * above, we cannot return error now, as we will still
> +			 * be able to successfully write environment later.
> +			 */
> +			fprintf(stderr, "Warning: corrupted volume?\n");
> +			return 0;
> +		} else if (errno == EINTR) {
> +			continue;
> +		}
> +
> +		fprintf(stderr, "Cannot read %u bytes from ubi volume, %s\n",
> +			(unsigned int)count, strerror(errno));
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ubi_write(int fd, const void *buf, size_t count)
> +{
> +	ssize_t ret;
> +
> +	while (count > 0) {
> +		ret = write(fd, buf, count);
> +		if (ret <= 0) {
> +			if (ret < 0 && errno == EINTR)
> +				continue;
> +
> +			fprintf(stderr, "Cannot write %u bytes to ubi volume\n",
> +				(unsigned int)count);
>
'%zu' is an appropriate format string for size_t values. No need for a
type cast.

> @@ -1347,8 +1587,12 @@ int fw_env_close(struct env_opts *opts)
>  static int check_device_config(int dev)
>  {
>  	struct stat st;
> +	int32_t lnum = 0;
>
This could be defined right before the ioctl, which is the only user of
this variable.

>  	int fd, rc = 0;
>  
> +	/* Fills in IS_UBI(), converts DEVNAME() with ubi volume name */
> +	ubi_check_dev(dev);
> +
>  	fd = open(DEVNAME(dev), O_RDONLY);
>  	if (fd < 0) {
>  		fprintf(stderr,
> @@ -1364,7 +1608,14 @@ static int check_device_config(int dev)
>  		goto err;
>  	}
>  
> -	if (S_ISCHR(st.st_mode)) {
> +	if (IS_UBI(dev)) {
> +		rc = ioctl(fd, UBI_IOCEBISMAP, &lnum);
> +		if (rc < 0) {
> +			fprintf(stderr, "Cannot get UBI information for %s\n",
> +				DEVNAME(dev));
> +			goto err;
> +		}
> +	} else if (S_ISCHR(st.st_mode)) {
>  		struct mtd_info_user mtdinfo;
>  		rc = ioctl(fd, MEMGETINFO, &mtdinfo);
>  		if (rc < 0) {


Lothar Waßmann