libflash/blocklevel: Add keep_alive parameter

Currently the file backend will keep a file descriptor open until the
structure is destroyed. This is undesirable for daemons and long running
processes that only have a very occasional need to access the file. Keeping
the file open requires users of blocklevel to close and reinit their
structures every time, doing is isn't disastrous but can easily be
managed at the interface level.

This has been done at the blocklevel_device interface so as to provide
minimal changes to arch_flash_init().

At the moment there isn't a need for the actually flash driver to be able
to do this, this remains unimplmented there.

Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

5 files changed, 150 insertions, 26 deletions

diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
index 83823c5..9591194 100644
--- a/libflash/blocklevel.c
+++ b/libflash/blocklevel.c
@@ -58,6 +58,25 @@ static int ecc_protected(struct blocklevel_device *bl, uint32_t pos, uint32_t le
 	return 0;
 }
 
+static int reacquire(struct blocklevel_device *bl)
+{
+	if (!bl->keep_alive && bl->reacquire)
+		return bl->reacquire(bl);
+	return 0;
+}
+
+static int release(struct blocklevel_device *bl)
+{
+	int rc = 0;
+	if (!bl->keep_alive && bl->release) {
+		/* This is the error return path a lot, preserve errno */
+		int err = errno;
+		rc = bl->release(bl);
+		errno = err;
+	}
+	return rc;
+}
+
 int blocklevel_read(struct blocklevel_device *bl, uint32_t pos, void *buf, uint32_t len)
 {
 	int rc;
@@ -69,14 +88,21 @@ int blocklevel_read(struct blocklevel_device *bl, uint32_t pos, void *buf, uint3
 		return FLASH_ERR_PARM_ERROR;
 	}
 
+	rc = reacquire(bl);
+	if (rc)
+		return rc;
+
 	if (!ecc_protected(bl, pos, len)) {
-		return bl->read(bl, pos, buf, len);
+		rc = bl->read(bl, pos, buf, len);
+		release(bl);
+		return rc;
 	}
 
 	buffer = malloc(ecc_len);
 	if (!buffer) {
 		errno = ENOMEM;
-		return FLASH_ERR_MALLOC_FAILED;
+		rc = FLASH_ERR_MALLOC_FAILED;
+		goto out;
 	}
 
 	rc = bl->read(bl, pos, buffer, ecc_len);
@@ -89,6 +115,7 @@ int blocklevel_read(struct blocklevel_device *bl, uint32_t pos, void *buf, uint3
 	}
 
 out:
+	release(bl);
 	free(buffer);
 	return rc;
 }
@@ -104,14 +131,21 @@ int blocklevel_write(struct blocklevel_device *bl, uint32_t pos, const void *buf
 		return FLASH_ERR_PARM_ERROR;
 	}
 
+	rc = reacquire(bl);
+	if (rc)
+		return rc;
+
 	if (!ecc_protected(bl, pos, len)) {
-		return bl->write(bl, pos, buf, len);
+		rc =  bl->write(bl, pos, buf, len);
+		release(bl);
+		return rc;
 	}
 
 	buffer = malloc(ecc_len);
 	if (!buffer) {
 		errno = ENOMEM;
-		return FLASH_ERR_MALLOC_FAILED;
+		rc = FLASH_ERR_MALLOC_FAILED;
+		goto out;
 	}
 
 	if (memcpy_to_ecc(buffer, buf, len)) {
@@ -119,14 +153,18 @@ int blocklevel_write(struct blocklevel_device *bl, uint32_t pos, const void *buf
 		rc = FLASH_ERR_ECC_INVALID;
 		goto out;
 	}
+
 	rc = bl->write(bl, pos, buffer, ecc_len);
+
 out:
+	release(bl);
 	free(buffer);
 	return rc;
 }
 
 int blocklevel_erase(struct blocklevel_device *bl, uint32_t pos, uint32_t len)
 {
+	int rc;
 	if (!bl || !bl->erase) {
 		errno = EINVAL;
 		return FLASH_ERR_PARM_ERROR;
@@ -139,7 +177,15 @@ int blocklevel_erase(struct blocklevel_device *bl, uint32_t pos, uint32_t len)
 		return FLASH_ERR_ERASE_BOUNDARY;
 	}
 
-	return bl->erase(bl, pos, len);
+	rc = reacquire(bl);
+	if (rc)
+		return rc;
+
+	rc = bl->erase(bl, pos, len);
+
+	release(bl);
+
+	return rc;
 }
 
 int blocklevel_get_info(struct blocklevel_device *bl, const char **name, uint32_t *total_size,
@@ -152,6 +198,10 @@ int blocklevel_get_info(struct blocklevel_device *bl, const char **name, uint32_
 		return FLASH_ERR_PARM_ERROR;
 	}
 
+	rc = reacquire(bl);
+	if (rc)
+		return rc;
+
 	rc = bl->get_info(bl, name, total_size, erase_granule);
 
 	/* Check the validity of what we are being told */
@@ -159,6 +209,8 @@ int blocklevel_get_info(struct blocklevel_device *bl, const char **name, uint32_
 		fprintf(stderr, "blocklevel_get_info: WARNING: erase_granule (0x%08x) and erase_mask"
 				" (0x%08x) don't match\n", *erase_granule, bl->erase_mask + 1);
 
+	release(bl);
+
 	return rc;
 }
 
@@ -231,9 +283,13 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint32_t pos, const voi
 	if (!erase_buf) {
 		errno = ENOMEM;
 		rc = FLASH_ERR_MALLOC_FAILED;
-		goto out;
+		goto out_free;
 	}
 
+	rc = reacquire(bl);
+	if (rc)
+		goto out_free;
+
 	while (len > 0) {
 		uint32_t erase_block = pos & ~(erase_size - 1);
 		uint32_t block_offset = pos & (erase_size - 1);
@@ -264,6 +320,8 @@ int blocklevel_smart_write(struct blocklevel_device *bl, uint32_t pos, const voi
 	}
 
 out:
+	release(bl);
+out_free:
 	free(write_buf_start);
 	free(erase_buf);
 	return rc;
diff --git a/libflash/blocklevel.h b/libflash/blocklevel.h
index e9a1978..9f4285e 100644
--- a/libflash/blocklevel.h
+++ b/libflash/blocklevel.h
@@ -17,6 +17,7 @@
 #define __LIBFLASH_BLOCKLEVEL_H
 
 #include <stdint.h>
+#include <stdbool.h>
 
 struct bl_prot_range {
 	uint32_t start;
@@ -39,6 +40,8 @@ enum blocklevel_flags {
  */
 struct blocklevel_device {
 	void *priv;
+	int (*reacquire)(struct blocklevel_device *bl);
+	int (*release)(struct blocklevel_device *bl);
 	int (*read)(struct blocklevel_device *bl, uint32_t pos, void *buf, uint32_t len);
 	int (*write)(struct blocklevel_device *bl, uint32_t pos, const void *buf, uint32_t len);
 	int (*erase)(struct blocklevel_device *bl, uint32_t pos, uint32_t len);
@@ -49,6 +52,7 @@ struct blocklevel_device {
 	 * Keep the erase mask so that blocklevel_erase() can do sanity checking
 	 */
 	uint32_t erase_mask;
+	bool keep_alive;
 	enum blocklevel_flags flags;
 
 	struct blocklevel_range ecc_prot;
diff --git a/libflash/file.c b/libflash/file.c
index 0dbe610..72e2da9 100644
--- a/libflash/file.c
+++ b/libflash/file.c
@@ -14,11 +14,10 @@
  * limitations under the License.
  */
 #define _GNU_SOURCE
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/types.h>
-#include <unistd.h>
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -36,14 +35,35 @@
 struct file_data {
 	int fd;
 	char *name;
+	char *path;
 	struct blocklevel_device bl;
 };
 
+static int file_release(struct blocklevel_device *bl)
+{
+	struct file_data *file_data = container_of(bl, struct file_data, bl);
+	close(file_data->fd);
+	file_data->fd = -1;
+	return 0;
+}
+
+static int file_reacquire(struct blocklevel_device *bl)
+{
+	struct file_data *file_data = container_of(bl, struct file_data, bl);
+	int fd;
+
+	fd = open(file_data->path, O_RDWR);
+	if (fd == -1)
+		return FLASH_ERR_PARM_ERROR;
+	file_data->fd = fd;
+	return 0;
+}
+
 static int file_read(struct blocklevel_device *bl, uint32_t pos, void *buf, uint32_t len)
 {
 	struct file_data *file_data = container_of(bl, struct file_data, bl);
-	int count = 0;
-	int rc;
+	int rc, count = 0;
+
 	rc = lseek(file_data->fd, pos, SEEK_SET);
 	/* errno should remain set */
 	if (rc != pos)
@@ -54,6 +74,7 @@ static int file_read(struct blocklevel_device *bl, uint32_t pos, void *buf, uint
 		/* errno should remain set */
 		if (rc == -1)
 			return FLASH_ERR_BAD_READ;
+
 		count += rc;
 	}
 
@@ -64,12 +85,11 @@ static int file_write(struct blocklevel_device *bl, uint32_t dst, const void *sr
 		uint32_t len)
 {
 	struct file_data *file_data = container_of(bl, struct file_data, bl);
-	int count = 0;
-	int rc;
+	int rc, count = 0;
 
 	rc = lseek(file_data->fd, dst, SEEK_SET);
 	/* errno should remain set */
-	if (rc == -1)
+	if (rc != dst)
 		return FLASH_ERR_PARM_ERROR;
 
 	while (count < len) {
@@ -77,6 +97,7 @@ static int file_write(struct blocklevel_device *bl, uint32_t dst, const void *sr
 		/* errno should remain set */
 		if (rc == -1)
 			return FLASH_ERR_VERIFY_FAILURE;
+
 		count += rc;
 	}
 
@@ -94,7 +115,7 @@ static int file_erase(struct blocklevel_device *bl, uint32_t dst, uint32_t len)
 {
 	unsigned long long int d = ULLONG_MAX;
 	int i = 0;
-	int rc = 0;
+	int rc;
 
 	while (len - i > 0) {
 		rc = file_write(bl, dst + i, &d, len - i > sizeof(d) ? sizeof(d) : len - i);
@@ -114,7 +135,10 @@ static int mtd_erase(struct blocklevel_device *bl, uint32_t dst, uint32_t len)
 		.length = len
 	};
 
-	return ioctl(file_data->fd, MEMERASE, &erase_info) == -1 ? -1 : 0;
+	if (ioctl(file_data->fd, MEMERASE, &erase_info) == -1)
+		return FLASH_ERR_PARM_ERROR;
+
+	return 0;
 }
 
 static int get_info_name(struct file_data *file_data, char **name)
@@ -152,15 +176,16 @@ static int get_info_name(struct file_data *file_data, char **name)
 }
 
 
-static int mtd_get_info(struct blocklevel_device *bl, const char **name, uint32_t *total_size,
-		uint32_t *erase_granule)
+static int mtd_get_info(struct blocklevel_device *bl, const char **name,
+		uint32_t *total_size, uint32_t *erase_granule)
 {
 	struct file_data *file_data = container_of(bl, struct file_data, bl);
 	struct mtd_info_user mtd_info;
 	int rc;
 
-	if (ioctl(file_data->fd, MEMGETINFO, &mtd_info) == -1)
-			return FLASH_ERR_BAD_READ;
+	rc = ioctl(file_data->fd, MEMGETINFO, &mtd_info);
+	if (rc == -1)
+		return FLASH_ERR_BAD_READ;
 
 	if (total_size)
 		*total_size = mtd_info.size;
@@ -178,8 +203,8 @@ static int mtd_get_info(struct blocklevel_device *bl, const char **name, uint32_
 	return 0;
 }
 
-static int file_get_info(struct blocklevel_device *bl, const char **name, uint32_t *total_size,
-		uint32_t *erase_granule)
+static int file_get_info(struct blocklevel_device *bl, const char **name,
+		uint32_t *total_size, uint32_t *erase_granule)
 {
 	struct file_data *file_data = container_of(bl, struct file_data, bl);
 	struct stat st;
@@ -214,17 +239,26 @@ int file_init(int fd, struct blocklevel_device **bl)
 
 	*bl = NULL;
 
-	file_data = malloc(sizeof(struct file_data));
+	file_data = calloc(1, sizeof(struct file_data));
 	if (!file_data)
 		return FLASH_ERR_MALLOC_FAILED;
-	memset(file_data, 0, sizeof(*file_data));
 
 	file_data->fd = fd;
+	file_data->bl.reacquire = &file_reacquire;
+	file_data->bl.release = &file_release;
 	file_data->bl.read = &file_read;
 	file_data->bl.write = &file_write;
 	file_data->bl.erase = &file_erase;
 	file_data->bl.get_info = &file_get_info;
 	file_data->bl.erase_mask = 0;
+
+	/*
+	 * If the blocklevel_device is only inited with file_init() then keep
+	 * alive is assumed, as fd will change otherwise and this may break
+	 * callers assumptions.
+	 */
+	file_data->bl.keep_alive = 1;
+
 	/*
 	 * Unfortunately not all file descriptors are created equal...
 	 * Here we check to see if the file descriptor is to an MTD device, in
@@ -252,9 +286,12 @@ out:
 	return FLASH_ERR_PARM_ERROR;
 }
 
-int file_init_path(const char *path, int *r_fd, struct blocklevel_device **bl)
+int file_init_path(const char *path, int *r_fd, bool keep_alive,
+		struct blocklevel_device **bl)
 {
 	int fd, rc;
+	char *path_ptr = NULL;
+	struct file_data *file_data;
 
 	if (!path || !bl)
 		return FLASH_ERR_PARM_ERROR;
@@ -263,14 +300,32 @@ int file_init_path(const char *path, int *r_fd, struct blocklevel_device **bl)
 	if (fd == -1)
 		return FLASH_ERR_PARM_ERROR;
 
+	/*
+	 * strdup() first so don't have to deal with malloc failure after
+	 * file_init()
+	 */
+	path_ptr = strdup(path);
+	if (!path_ptr) {
+		rc = FLASH_ERR_MALLOC_FAILED;
+		goto out;
+	}
+
 	rc = file_init(fd, bl);
 	if (rc)
-		close(fd);
+		goto out;
+
+	file_data = container_of(*bl, struct file_data, bl);
+	file_data->bl.keep_alive = keep_alive;
+	file_data->path = path_ptr;
 
 	if (r_fd)
 		*r_fd = fd;
 
 	return rc;
+out:
+	free(path_ptr);
+	close(fd);
+	return rc;
 }
 
 void file_exit(struct blocklevel_device *bl)
@@ -280,6 +335,7 @@ void file_exit(struct blocklevel_device *bl)
 		free(bl->ecc_prot.prot);
 		file_data = container_of(bl, struct file_data, bl);
 		free(file_data->name);
+		free(file_data->path);
 		free(file_data);
 	}
 }
diff --git a/libflash/file.h b/libflash/file.h
index a9a89fe..c8e58a6 100644
--- a/libflash/file.h
+++ b/libflash/file.h
@@ -17,6 +17,8 @@
 #ifndef __LIBFLASH_FILE_H
 #define __LIBFLASH_FILE_H
 
+#include <stdbool.h>
+
 #include "blocklevel.h"
 
 /*
@@ -34,7 +36,7 @@ void file_exit(struct blocklevel_device *bl);
  * Because file_exit() doesn't close the file descriptor, file_init_path()
  * makes it available.
  */
-int file_init_path(const char *path, int *fd, struct blocklevel_device **bl);
+int file_init_path(const char *path, int *fd, bool keep_alive, struct blocklevel_device **bl);
 
 /*
  * file_exit_close is a convenience wrapper which will close the open
diff --git a/libflash/libflash.c b/libflash/libflash.c
index 69e809e..50dc54d 100644
--- a/libflash/libflash.c
+++ b/libflash/libflash.c
@@ -835,6 +835,10 @@ bail:
 		return rc;
 	}
 
+	/* The flash backend doesn't support reiniting it */
+	c->bl.keep_alive = true;
+	c->bl.reacquire = NULL;
+	c->bl.release = NULL;
 	c->bl.read = &flash_read;
 	c->bl.write = &flash_smart_write;
 	c->bl.erase = &flash_erase;