Patchwork [U-Boot,05/12] tools/env: Serialize calls to fw_*env

login
register
mail settings
Submitter Joe Hershberger
Date Aug. 17, 2012, 8:49 p.m.
Message ID <1345236586-19076-6-git-send-email-joe.hershberger@ni.com>
Download mbox | patch
Permalink /patch/178376/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Joe Hershberger - Aug. 17, 2012, 8:49 p.m.
Use a lock file at /var/lock/fw_printenv.lock.
Avoids seriously confusing the MTD driver.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 tools/env/fw_env_main.c | 59 +++++++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 22 deletions(-)
Mike Frysinger - Aug. 23, 2012, 3:33 a.m.
On Friday 17 August 2012 16:49:39 Joe Hershberger wrote:
> Use a lock file at /var/lock/fw_printenv.lock.

the lock should be per-MTD, not per-system.

> +	if (-1 == flock(lockfd, LOCK_EX)) {

i guess flock() on the fd returned from the mtd device itself doesn't work ?
-mike
Joe Hershberger - Oct. 3, 2012, 1:12 a.m.
Hi Mike,

Sorry for the delay... I've been distracted.

On Wed, Aug 22, 2012 at 10:33 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Friday 17 August 2012 16:49:39 Joe Hershberger wrote:
>> Use a lock file at /var/lock/fw_printenv.lock.
>
> the lock should be per-MTD, not per-system.

I'm not sure that's the case.  The idea is to prevent the tool from
running more than once at a time.  I don't think it matters which mtd
is being accessed as we want all the mtd accesses across the whole
operation to be atomic.

If you are talking about having two independent sets of environments
accessed at the same time, then there are other reasons (such as the
config file) that make that not possible for now, and it seems like
it's an out-there use-case.

>> +     if (-1 == flock(lockfd, LOCK_EX)) {
>
> i guess flock() on the fd returned from the mtd device itself doesn't work ?

Again.. the thought it to be atomic across all accesses.  We want to
avoid the read / modify / write problems.

Thanks,
-Joe

Patch

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index c654057..c855f4c 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -39,10 +39,13 @@ 
  *		  variable "name"
  */
 
+#include <fcntl.h>
+#include <getopt.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
-#include <getopt.h>
+#include <sys/file.h>
+#include <unistd.h>
 #include "fw_env.h"
 
 #define	CMD_PRINTENV	"fw_printenv"
@@ -81,13 +84,27 @@  void usage(void)
 	);
 }
 
-int
-main(int argc, char *argv[])
+int main(int argc, char *argv[])
 {
 	char *p;
 	char *cmdname = *argv;
 	char *script_file = NULL;
 	int c;
+	const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
+	int lockfd = -1;
+	int retval = EXIT_SUCCESS;
+
+	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC);
+	if (-1 == lockfd) {
+		fprintf(stderr, "Error opening lock file %s\n", lockname);
+		return EXIT_FAILURE;
+	}
+
+	if (-1 == flock(lockfd, LOCK_EX)) {
+		fprintf(stderr, "Error locking file %s\n", lockname);
+		close(lockfd);
+		return EXIT_FAILURE;
+	}
 
 	if ((p = strrchr (cmdname, '/')) != NULL) {
 		cmdname = p + 1;
@@ -104,38 +121,36 @@  main(int argc, char *argv[])
 			break;
 		case 'h':
 			usage();
-			return EXIT_SUCCESS;
+			goto exit;
 		default: /* '?' */
 			fprintf(stderr, "Try `%s --help' for more information."
 				"\n", cmdname);
-			return EXIT_FAILURE;
+			retval = EXIT_FAILURE;
+			goto exit;
 		}
 	}
 
-
 	if (strcmp(cmdname, CMD_PRINTENV) == 0) {
-
-		if (fw_printenv (argc, argv) != 0)
-			return EXIT_FAILURE;
-
-		return EXIT_SUCCESS;
-
+		if (fw_printenv(argc, argv) != 0)
+			retval = EXIT_FAILURE;
 	} else if (strcmp(cmdname, CMD_SETENV) == 0) {
 		if (!script_file) {
 			if (fw_setenv(argc, argv) != 0)
-				return EXIT_FAILURE;
+				retval = EXIT_FAILURE;
 		} else {
 			if (fw_parse_script(script_file) != 0)
-				return EXIT_FAILURE;
+				retval = EXIT_FAILURE;
 		}
-
-		return EXIT_SUCCESS;
-
+	} else {
+		fprintf(stderr,
+			"Identity crisis - may be called as `" CMD_PRINTENV
+			"' or as `" CMD_SETENV "' but not as `%s'\n",
+			cmdname);
+		retval = EXIT_FAILURE;
 	}
 
-	fprintf (stderr,
-		"Identity crisis - may be called as `" CMD_PRINTENV
-		"' or as `" CMD_SETENV "' but not as `%s'\n",
-		cmdname);
-	return EXIT_FAILURE;
+exit:
+	flock(lockfd, LOCK_UN);
+	close(lockfd);
+	return retval;
 }