public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: steved@redhat.com
Cc: chris.mason@oracle.com, linux-nfs@vger.kernel.org
Subject: [PATCH 16/24] libnsm.a: Factor atomic write code out of nsm_get_state()
Date: Thu, 14 Jan 2010 12:31:11 -0500	[thread overview]
Message-ID: <20100114173111.26079.38685.stgit@localhost.localdomain> (raw)
In-Reply-To: <20100114172457.26079.66627.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

We're about to use the same logic (mktemp, write, rename) for
other new purposes, so pull it out into its own function.

This change also addresses a latent bug: O_TRUNC is now used when
creating the temporary file.  This eliminates the possibility of
getting stale data in the temp file, if somehow a previous "atomic
write" was interrupted and didn't remove the temporary file.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/nsm/file.c |  134 +++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 89 insertions(+), 45 deletions(-)

diff --git a/support/nsm/file.c b/support/nsm/file.c
index fc3241a..10769d9 100644
--- a/support/nsm/file.c
+++ b/support/nsm/file.c
@@ -195,6 +195,94 @@ nsm_make_pathname(const char *directory)
 	return path;
 }
 
+/*
+ * Returns a dynamically allocated, '\0'-terminated buffer
+ * containing an appropriate pathname, or NULL if an error
+ * occurs.  Caller must free the returned result with free(3).
+ */
+__attribute_malloc__
+static char *
+nsm_make_temp_pathname(const char *pathname)
+{
+	size_t size;
+	char *path;
+	int len;
+
+	size = strlen(pathname) + sizeof(".new") + 2;
+	if (size > PATH_MAX)
+		return NULL;
+
+	path = malloc(size);
+	if (path == NULL)
+		return NULL;
+
+	len = snprintf(path, size, "%s.new", pathname);
+	if (error_check(len, size)) {
+		free(path);
+		return NULL;
+	}
+
+	return path;
+}
+
+/*
+ * Use "mktemp, write, rename" to update the contents of a file atomically.
+ *
+ * Returns true if completely successful, or false if some error occurred.
+ */
+static _Bool
+nsm_atomic_write(const char *path, const void *buf, const size_t buflen)
+{
+	_Bool result = false;
+	ssize_t len;
+	char *temp;
+	int fd;
+
+	temp = nsm_make_temp_pathname(path);
+	if (temp == NULL) {
+		xlog(L_ERROR, "Failed to create new path for %s", path);
+		goto out;
+	}
+
+	fd = open(temp, O_CREAT | O_TRUNC | O_SYNC | O_WRONLY, 0644);
+	if (fd == -1) {
+		xlog(L_ERROR, "Failed to create %s: %m", temp);
+		goto out;
+	}
+
+	len = write(fd, buf, buflen);
+	if (exact_error_check(len, buflen)) {
+		xlog(L_ERROR, "Failed to write %s: %m", temp);
+		(void)close(fd);
+		(void)unlink(temp);
+		goto out;
+	}
+
+	if (close(fd) == -1) {
+		xlog(L_ERROR, "Failed to close %s: %m", temp);
+		(void)unlink(temp);
+		goto out;
+	}
+
+	if (rename(temp, path) == -1) {
+		xlog(L_ERROR, "Failed to rename %s -> %s: %m",
+				temp, path);
+		(void)unlink(temp);
+		goto out;
+	}
+
+	/* Ostensibly, a sync(2) is not needed here because
+	 * open(O_CREAT), write(O_SYNC), and rename(2) are
+	 * already synchronous with persistent storage, for
+	 * any file system we care about. */
+
+	result = true;
+
+out:
+	free(temp);
+	return result;
+}
+
 /**
  * nsm_setup_pathnames - set up pathname
  * @progname: C string containing name of program, for error messages
@@ -326,7 +414,6 @@ nsm_get_state(_Bool update)
 	int fd, state = 0;
 	ssize_t result;
 	char *path = NULL;
-	char *newpath = NULL;
 
 	path = nsm_make_pathname(NSM_STATE_FILE);
 	if (path == NULL) {
@@ -365,54 +452,11 @@ update:
 
 	if (update) {
 		state += 2;
-
-		newpath = nsm_make_pathname(NSM_STATE_FILE ".new");
-		if (newpath == NULL) {
-			xlog(L_ERROR,
-			  "Failed to create path for " NSM_STATE_FILE ".new");
-			state = 0;
-			goto out;
-		}
-
-		fd = open(newpath, O_CREAT | O_SYNC | O_WRONLY, 0644);
-		if (fd == -1) {
-			xlog(L_ERROR, "Failed to create %s: %m", newpath);
-			state = 0;
-			goto out;
-		}
-
-		result = write(fd, &state, sizeof(state));
-		if (exact_error_check(result, sizeof(state))) {
-			xlog(L_ERROR, "Failed to write %s: %m", newpath);
-			(void)close(fd);
-			(void)unlink(newpath);
-			state = 0;
-			goto out;
-		}
-
-		if (close(fd) == -1) {
-			xlog(L_ERROR, "Failed to close %s: %m", newpath);
-			(void)unlink(newpath);
-			state = 0;
-			goto out;
-		}
-
-		if (rename(newpath, path) == -1) {
-			xlog(L_ERROR, "Failed to rename %s -> %s: %m",
-					newpath, path);
-			(void)unlink(newpath);
+		if (!nsm_atomic_write(path, &state, sizeof(state)))
 			state = 0;
-			goto out;
-		}
-
-		/* Ostensibly, a sync(2) is not needed here because
-		 * open(O_CREAT), write(O_SYNC), and rename(2) are
-		 * already synchronous with persistent storage, for
-		 * any file system we care about. */
 	}
 
 out:
-	free(newpath);
 	free(path);
 	return state;
 }


  parent reply	other threads:[~2010-01-14 17:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-14 17:28 [PATCH 00/24] Remaining IPv6 patches for statd Chuck Lever
     [not found] ` <20100114172457.26079.66627.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-01-14 17:28   ` [PATCH 01/24] libnsm.a: Add RPC construction helper functions Chuck Lever
2010-01-14 17:29   ` [PATCH 02/24] sm-notify: Replace RPC code Chuck Lever
2010-01-14 17:29   ` [PATCH 03/24] statd: Update rmtcall.c Chuck Lever
2010-01-14 17:29   ` [PATCH 04/24] sm-notify: factor socket creation out of notify() Chuck Lever
2010-01-14 17:29   ` [PATCH 05/24] sm-notify: Support creating a PF_INET6 socket in smn_create_socket() Chuck Lever
2010-01-14 17:29   ` [PATCH 06/24] sm-notify: IPv6 support in reserved port binding " Chuck Lever
2010-01-14 17:29   ` [PATCH 07/24] sm-notify: Use getaddrinfo(3) to create bind address " Chuck Lever
2010-01-14 17:30   ` [PATCH 08/24] sm-notify: Support IPv6 DNS lookups in smn_lookup Chuck Lever
2010-01-14 17:30   ` [PATCH 09/24] nfs-utils: Collect socket address helpers into one location Chuck Lever
2010-01-14 17:30   ` [PATCH 10/24] statd: Introduce statd version of matchhostname() Chuck Lever
2010-01-14 17:30   ` [PATCH 11/24] statd: add nsm_present_address() API Chuck Lever
2010-01-14 17:30   ` [PATCH 12/24] statd: add IPv6 support in sm_notify_1_svc() Chuck Lever
2010-01-14 17:30   ` [PATCH 13/24] statd: Support IPv6 is caller_is_localhost() Chuck Lever
2010-01-14 17:30   ` [PATCH 14/24] statd: Support IPv6 in sm_simu_crash_1_svc Chuck Lever
2010-01-14 17:31   ` [PATCH 15/24] sm-notify: Save mon_name and my_name strings Chuck Lever
2010-01-14 17:31   ` Chuck Lever [this message]
2010-01-14 17:31   ` [PATCH 17/24] libnsm.a: Add support for multiple lines in monitor record files Chuck Lever
2010-01-14 17:31   ` [PATCH 18/24] statd: Add API to canonicalize mon_names Chuck Lever
2010-01-14 17:31   ` [PATCH 19/24] statd: Support IPv6 in sm_mon_1_svc() Chuck Lever
2010-01-14 17:31   ` [PATCH 20/24] statd: Support IPv6 in sm_stat_1_svc() Chuck Lever
2010-01-14 17:31   ` [PATCH 21/24] statd: Remove NL_ADDR() macro Chuck Lever
2010-01-14 17:32   ` [PATCH 22/24] libnsm.a: retain CAP_NET_BIND when dropping privileges Chuck Lever
2010-01-14 17:32   ` [PATCH 23/24] statd: Support TI-RPC statd listener Chuck Lever
2010-01-14 17:32   ` [PATCH 24/24] statd: update rpc.statd(8) and sm-notify(8) to reflect IPv6 support Chuck Lever
2010-01-16 13:22   ` [PATCH 00/24] Remaining IPv6 patches for statd Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100114173111.26079.38685.stgit@localhost.localdomain \
    --to=chuck.lever@oracle.com \
    --cc=chris.mason@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=steved@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox