[U-Boot] [PATCH 2/2] tools: env: Implement atomic replace for filesystem

[Stefano Babic]

Hi alex,

On 08/03/2018 12:52, Alex Kiernan wrote:
> If the U-Boot environment is stored in a regular file and redundant
> operation isn't set, then write to a temporary file and perform an
> atomic rename.
> 

Even if it is not explicitely set (IMHO it should), this code can
be used as library and linked to own application. That means that
concurrency can happens. I mean:

> Signed-off-by: Alex Kiernan <alex.kiernan at gmail.com>
> ---
> 
>  tools/env/fw_env.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 78 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 2df3504..b814c4e 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -14,6 +14,7 @@
>  #include <errno.h>
>  #include <env_flags.h>
>  #include <fcntl.h>
> +#include <libgen.h>
>  #include <linux/fs.h>
>  #include <linux/stringify.h>
>  #include <ctype.h>
> @@ -1229,9 +1230,46 @@ static int flash_read (int fd)
>  	return 0;
>  }
>  
> +static int flash_open_tempfile(const char **dname, const char **target_temp)
> +{
> +	char *dup_name = strdup(DEVNAME (dev_current));
> +	char *temp_name = NULL;
> +	int rc = -1;
> +
> +	if (!dup_name)
> +		return -1;
> +
> +	*dname = dirname(dup_name);
> +	if (!*dname)
> +		goto err;
> +
> +	rc = asprintf(&temp_name, "%s/uboot.tmp", *dname);

Filename is fixed - should we not use mkstemp ?

> +	if (rc == -1)
> +		goto err;
> +
> +	rc = open(temp_name, O_RDWR | O_CREAT | O_TRUNC, 0700);
> +	if (rc == -1) {
> +		/* fall back to in place write */
> +		free(temp_name);
> +	} else {
> +		*target_temp = temp_name;
> +		/* deliberately leak dup_name as dname /might/ point into
> +		 * it and we need it for our caller
> +		 */
> +		dup_name = NULL;
> +	}
> +
> +err:
> +	if (dup_name)
> +		free(dup_name);
> +
> +	return rc;
> +}
> +
>  static int flash_io_write (int fd_current)
>  {
> -	int fd_target, rc, dev_target;
> +	int fd_target = -1, rc, dev_target;
> +	const char *dname, *target_temp = NULL;
>  
>  	if (HaveRedundEnv) {
>  		/* switch to next partition for writing */
> @@ -1247,8 +1285,17 @@ static int flash_io_write (int fd_current)
>  			goto exit;
>  		}
>  	} else {
> +		struct stat sb;
> +
> +		if (fstat(fd_current, &sb) == 0 && S_ISREG(sb.st_mode)) {
> +			/* if any part of flash_open_tempfile() fails we fall
> +			 * back to in-place writes
> +			 */
> +			fd_target = flash_open_tempfile(&dname, &target_temp);
> +		}
>  		dev_target = dev_current;
> -		fd_target = fd_current;
> +		if (fd_target == -1)
> +			fd_target = fd_current;
>  	}
>  
>  	rc = flash_write (fd_current, fd_target, dev_target);
> @@ -1260,7 +1307,7 @@ static int flash_io_write (int fd_current)
>  			 DEVNAME (dev_current), strerror (errno));
>  	}
>  
> -	if (HaveRedundEnv) {
> +	if (fd_current != fd_target) {
>  		if (fsync(fd_target) &&
>  		    !(errno == EINVAL || errno == EROFS)) {
>  			fprintf (stderr,
> @@ -1275,6 +1322,34 @@ static int flash_io_write (int fd_current)
>  				 strerror (errno));
>  			rc = -1;
>  		}
> +
> +		if (target_temp) {
> +			int dir_fd;
> +
> +			dir_fd = open(dname, O_DIRECTORY | O_RDONLY);
> +			if (dir_fd == -1)
> +				fprintf (stderr,
> +					 "Can't open %s: %s\n",
> +					 dname, strerror (errno));
> +
> +			if (rename(target_temp, DEVNAME(dev_target))) {
> +				fprintf (stderr,
> +					 "rename failed %s => %s: %s\n",
> +					 target_temp, DEVNAME(dev_target),
> +					 strerror (errno));
> +				rc = -1;
> +			}
> +
> +			if (dir_fd != -1 && fsync(dir_fd))
> +				fprintf (stderr,
> +					 "fsync failed on %s: %s\n",
> +					 dname, strerror (errno));
> +
> +			if (dir_fd != -1 && close(dir_fd))
> +				fprintf (stderr,
> +					 "I/O error on %s: %s\n",
> +					 dname, strerror (errno));
> +		}
>  	}
>  exit:
>  	return rc;
> 

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================