Re: [PATCH] flatiron Move ringid store and load from totem library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ACK

Chrissie

On 30/05/14 17:54, Jan Friesse wrote:
Functions for storing and loading ring id was in the totem library. This
causes problem, what to do when it's impossible to load or store ring
id. Easy solution seemed to be assert, but sadly this makes hard for
user to find out what happened (because corosync was just aborted and
logsys didn't flush)

Solution is to move these functions to main.c, where is much easier to
handle error.

Path also introduces get_run_dir function.

Run dir (LOCALSTATEDIR/lib/corosync) was hardcoded thru whole codebase.
Totemsrp was trying to create and chdir into it, but also
takes into account environment variable COROSYNC_RUN_DIR creating
inconsistency.

get_run_dir correctly returns COROSYNC_RUN_DIR (when set) or
LOCALSTATEDIR/lib/corosync. This is now used by all functions instead of
hardcoded string.

All occurrences of mkdir/chdir are removed from totemsrp and chdir is
now called in main function.

This makes libtotem free of any file system operations.

(backported from master "Introduce get_run_dir function" and "Move
  ringid store and load from totem library" patches)

Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
---
  exec/main.c                    |  102 +++++++++++++++++++++++++++++++++--
  exec/totemsrp.c                |  115 +++++++--------------------------------
  exec/util.c                    |   21 +++++++
  exec/util.h                    |    6 ++
  include/corosync/totem/totem.h |   20 +++++--
  5 files changed, 159 insertions(+), 105 deletions(-)

diff --git a/exec/main.c b/exec/main.c
index 9afcd13..2e4fd02 100644
--- a/exec/main.c
+++ b/exec/main.c
@@ -231,8 +231,11 @@ static int corosync_exit_dispatch_fn (
  static void corosync_blackbox_write_to_file (void)
  {
  	int saved_errno;
+	char fname[PATH_MAX];

-	if (logsys_log_rec_store (LOCALSTATEDIR "/lib/corosync/fdata")) {
+	snprintf(fname, PATH_MAX, "%s/fdata", get_run_dir());
+
+	if (logsys_log_rec_store (fname)) {
  		saved_errno = errno;
  		LOGSYS_PERROR(saved_errno, LOGSYS_LEVEL_ERROR, "Can't store blackbox file");
  	}
@@ -983,6 +986,87 @@ int main_mcast (
  	return (totempg_groups_mcast_joined (corosync_group_handle, iovec, iov_len, guarantee));
  }

+static void corosync_ring_id_create_or_load (
+	struct memb_ring_id *memb_ring_id,
+	const struct totem_ip_address *addr)
+{
+	int fd;
+	int res = 0;
+	char filename[PATH_MAX];
+
+	snprintf (filename, sizeof(filename), "%s/ringid_%s",
+		get_run_dir(), totemip_print (addr));
+	fd = open (filename, O_RDONLY, 0700);
+	/*
+	 * If file can be opened and read, read the ring id
+	 */
+	if (fd != -1) {
+		res = read (fd, &memb_ring_id->seq, sizeof (uint64_t));
+		close (fd);
+	}
+	/*
+	 * If file could not be opened or read, create a new ring id
+	 */
+	if ((fd == -1) || (res != sizeof (uint64_t))) {
+		memb_ring_id->seq = 0;
+		umask(0);
+		fd = open (filename, O_CREAT|O_RDWR, 0700);
+		if (fd != -1) {
+			res = write (fd, &memb_ring_id->seq, sizeof (uint64_t));
+			close (fd);
+			if (res == -1) {
+				LOGSYS_PERROR (errno, LOGSYS_LEVEL_ERROR,
+					"Couldn't write ringid file '%s'", filename);
+
+				corosync_exit_error (AIS_DONE_STORE_RINGID);
+			}
+		} else {
+			LOGSYS_PERROR (errno, LOGSYS_LEVEL_ERROR,
+				"Couldn't create ringid file '%s'", filename);
+
+			corosync_exit_error (AIS_DONE_STORE_RINGID);
+		}
+	}
+
+	totemip_copy(&memb_ring_id->rep, addr);
+	assert (!totemip_zero_check(&memb_ring_id->rep));
+}
+
+static void corosync_ring_id_store (
+	const struct memb_ring_id *memb_ring_id,
+	const struct totem_ip_address *addr)
+{
+	char filename[PATH_MAX];
+	int fd;
+	int res;
+
+	snprintf (filename, sizeof(filename), "%s/ringid_%s",
+		get_run_dir(), totemip_print (addr));
+
+	fd = open (filename, O_WRONLY, 0777);
+	if (fd == -1) {
+		fd = open (filename, O_CREAT|O_RDWR, 0777);
+	}
+	if (fd == -1) {
+		LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR,
+			"Couldn't store new ring id %llx to stable storage",
+			memb_ring_id->seq);
+
+		corosync_exit_error (AIS_DONE_STORE_RINGID);
+	}
+	log_printf (LOGSYS_LEVEL_DEBUG,
+		"Storing new sequence id for ring %llx", memb_ring_id->seq);
+	res = write (fd, &memb_ring_id->seq, sizeof(memb_ring_id->seq));
+	close (fd);
+	if (res != sizeof(memb_ring_id->seq)) {
+		LOGSYS_PERROR(errno, LOGSYS_LEVEL_ERROR,
+			"Couldn't store new ring id %llx to stable storage",
+			memb_ring_id->seq);
+
+		corosync_exit_error (AIS_DONE_STORE_RINGID);
+	}
+}
+
  int message_source_is_local (const mar_message_source_t *source)
  {
  	int ret = 0;
@@ -1654,7 +1738,6 @@ int main (int argc, char **argv, char **envp)
  	int res, ch;
  	int background, setprio;
  	struct stat stat_out;
-	char corosync_lib_dir[PATH_MAX];
  	hdb_handle_t object_runtime_handle;
  	enum e_ais_done flock_err;
  	struct scheduler_pause_timeout_data scheduler_pause_timeout_data;
@@ -1802,10 +1885,16 @@ int main (int argc, char **argv, char **envp)
  	/*
  	 * Make sure required directory is present
  	 */
-	sprintf (corosync_lib_dir, "%s/lib/corosync", LOCALSTATEDIR);
-	res = stat (corosync_lib_dir, &stat_out);
+	res = stat (get_run_dir(), &stat_out);
  	if ((res == -1) || (res == 0 && !S_ISDIR(stat_out.st_mode))) {
-		log_printf (LOGSYS_LEVEL_ERROR, "Required directory not present %s.  Please create it.\n", corosync_lib_dir);
+		log_printf (LOGSYS_LEVEL_ERROR, "Required directory not present %s.  Please create it.\n", get_run_dir());
+		corosync_exit_error (AIS_DONE_DIR_NOT_PRESENT);
+	}
+
+	res = chdir(get_run_dir());
+	if (res == -1) {
+		log_printf (LOGSYS_LEVEL_ERROR, "Cannot chdir to run directory %s.  "
+		    "Please make sure it has correct context and rights.", get_run_dir());
  		corosync_exit_error (AIS_DONE_DIR_NOT_PRESENT);
  	}

@@ -1827,6 +1916,9 @@ int main (int argc, char **argv, char **envp)
  		corosync_exit_error (AIS_DONE_MAINCONFIGREAD);
  	}

+	totem_config.totem_memb_ring_id_create_or_load = corosync_ring_id_create_or_load;
+	totem_config.totem_memb_ring_id_store = corosync_ring_id_store;
+
  	totem_config.totem_logging_configuration = totem_logging_configuration;
  	totem_config.totem_logging_configuration.log_subsys_id =
  		_logsys_subsys_create ("TOTEM");
diff --git a/exec/totemsrp.c b/exec/totemsrp.c
index e383f7e..9aa94d9 100644
--- a/exec/totemsrp.c
+++ b/exec/totemsrp.c
@@ -462,6 +462,14 @@ struct totemsrp_instance {
  	void (*totemsrp_waiting_trans_ack_cb_fn) (
  		int waiting_trans_ack);

+	void (*memb_ring_id_create_or_load) (
+		struct memb_ring_id *memb_ring_id,
+		const struct totem_ip_address *addr);
+
+	void (*memb_ring_id_store) (
+		const struct memb_ring_id *memb_ring_id,
+		const struct totem_ip_address *addr);
+
  	int global_seqno;

  	int my_token_held;
@@ -577,8 +585,6 @@ static int srp_addr_equal (const struct srp_addr *a, const struct srp_addr *b);

  static void memb_leave_message_send (struct totemsrp_instance *instance);

-static void memb_ring_id_create_or_load (struct totemsrp_instance *, struct memb_ring_id *);
-
  static void token_callbacks_execute (struct totemsrp_instance *instance, enum totem_callback_token_type type);
  static void memb_state_gather_enter (struct totemsrp_instance *instance, int gather_from);
  static void messages_deliver_to_app (struct totemsrp_instance *instance, int skip, unsigned int end_point);
@@ -586,7 +592,7 @@ static int orf_token_mcast (struct totemsrp_instance *instance, struct orf_token
  	int fcc_mcasts_allowed);
  static void messages_free (struct totemsrp_instance *instance, unsigned int token_aru);

-static void memb_ring_id_set_and_store (struct totemsrp_instance *instance,
+static void memb_ring_id_set (struct totemsrp_instance *instance,
  	const struct memb_ring_id *ring_id);
  static void target_set_completed (void *context);
  static void memb_state_commit_token_update (struct totemsrp_instance *instance);
@@ -632,8 +638,6 @@ struct message_handlers totemsrp_message_handlers = {
  	}
  };

-static const char *rundir = NULL;
-
  #define log_printf(level, format, args...)				\
  do {									\
  	instance->totemsrp_log_printf (					\
@@ -789,28 +793,12 @@ int totemsrp_initialize (
  		int waiting_trans_ack))
  {
  	struct totemsrp_instance *instance;
-	unsigned int res;

  	instance = malloc (sizeof (struct totemsrp_instance));
  	if (instance == NULL) {
  		goto error_exit;
  	}

-	rundir = getenv ("COROSYNC_RUN_DIR");
-	if (rundir == NULL) {
-		rundir = LOCALSTATEDIR "/lib/corosync";
-	}
-
-	res = mkdir (rundir, 0700);
-	if (res == -1 && errno != EEXIST) {
-		goto error_destroy;
-	}
-
-	res = chdir (rundir);
-	if (res == -1) {
-		goto error_destroy;
-	}
-
  	totemsrp_instance_initialize (instance);

  	instance->totemsrp_waiting_trans_ack_cb_fn = waiting_trans_ack_cb_fn;
@@ -834,6 +822,12 @@ int totemsrp_initialize (
  	instance->totemsrp_log_printf = totem_config->totem_logging_configuration.log_printf;

  	/*
+	 * Configure totem store and load functions
+	 */
+	instance->memb_ring_id_create_or_load = totem_config->totem_memb_ring_id_create_or_load;
+	instance->memb_ring_id_store = totem_config->totem_memb_ring_id_store;
+
+	/*
  	 * Initialize local variables for totemsrp
  	 */
  	totemip_copy (&instance->mcast_address, &totem_config->interfaces[0].mcast_addr);
@@ -978,9 +972,6 @@ int totemsrp_initialize (
  	*srp_context = instance;
  	return (0);

-error_destroy:
-	free (instance);
-
  error_exit:
  	return (-1);
  }
@@ -1991,7 +1982,8 @@ static void memb_state_commit_enter (

  	instance->memb_timer_state_gather_consensus_timeout = 0;

-	memb_ring_id_set_and_store (instance, &instance->commit_token->ring_id);
+	memb_ring_id_set (instance, &instance->commit_token->ring_id);
+	instance->memb_ring_id_store (&instance->my_ring_id, &instance->my_id.addr[0]);

  	instance->token_ring_id_seq = instance->my_ring_id.seq;

@@ -3187,79 +3179,12 @@ static void memb_merge_detect_transmit (struct totemsrp_instance *instance)
  		sizeof (struct memb_merge_detect));
  }

-static void memb_ring_id_create_or_load (
-	struct totemsrp_instance *instance,
-	struct memb_ring_id *memb_ring_id)
-{
-	int fd;
-	int res = 0;
-	char filename[PATH_MAX];
-
-	snprintf (filename, sizeof(filename), "%s/ringid_%s",
-		rundir, totemip_print (&instance->my_id.addr[0]));
-	fd = open (filename, O_RDONLY, 0700);
-	/*
-	 * If file can be opened and read, read the ring id
-	 */
-	if (fd != -1) {
-		res = read (fd, &memb_ring_id->seq, sizeof (uint64_t));
-		close (fd);
-	}
-	/*
- 	 * If file could not be opened or read, create a new ring id
- 	 */
-	if ((fd == -1) || (res != sizeof (uint64_t))) {
-		memb_ring_id->seq = 0;
-		umask(0);
-		fd = open (filename, O_CREAT|O_RDWR, 0700);
-		if (fd != -1) {
-			res = write (fd, &memb_ring_id->seq, sizeof (uint64_t));
-			close (fd);
-			if (res == -1) {
-				LOGSYS_PERROR (errno, instance->totemsrp_log_level_warning,
-					"Couldn't write ringid file '%s'", filename);
-			}
-		} else {
-			LOGSYS_PERROR (errno, instance->totemsrp_log_level_warning,
-				"Couldn't create ringid file '%s'", filename);
-		}
-	}
-
-	totemip_copy(&memb_ring_id->rep, &instance->my_id.addr[0]);
-	assert (!totemip_zero_check(&memb_ring_id->rep));
-	instance->token_ring_id_seq = memb_ring_id->seq;
-}
-
-static void memb_ring_id_set_and_store (
+static void memb_ring_id_set (
  	struct totemsrp_instance *instance,
  	const struct memb_ring_id *ring_id)
  {
-	char filename[256];
-	int fd;
-	int res;

  	memcpy (&instance->my_ring_id, ring_id, sizeof (struct memb_ring_id));
-
-	snprintf (filename, sizeof(filename), "%s/ringid_%s",
-		rundir, totemip_print (&instance->my_id.addr[0]));
-
-	fd = open (filename, O_WRONLY, 0777);
-	if (fd == -1) {
-		fd = open (filename, O_CREAT|O_RDWR, 0777);
-	}
-	if (fd == -1) {
-		LOGSYS_PERROR(errno, instance->totemsrp_log_level_warning,
-			"Couldn't store new ring id %llx to stable storage",
-			instance->my_ring_id.seq);
-		assert (0);
-		return;
-	}
-	log_printf (instance->totemsrp_log_level_debug,
-		"Storing new sequence id for ring %llx\n", instance->my_ring_id.seq);
-	//assert (fd > 0);
-	res = write (fd, &instance->my_ring_id.seq, sizeof (unsigned long long));
-	assert (res == sizeof (unsigned long long));
-	close (fd);
  }

  int totemsrp_callback_token_create (
@@ -4505,7 +4430,9 @@ void main_iface_change_fn (
  	totemip_copy (&instance->my_memb_list[0].addr[iface_no], iface_addr);

  	if (instance->iface_changes++ == 0) {
-		memb_ring_id_create_or_load (instance, &instance->my_ring_id);
+		instance->memb_ring_id_create_or_load (&instance->my_ring_id,
+		    &instance->my_id.addr[0]);
+		instance->token_ring_id_seq = instance->my_ring_id.seq;
  		log_printf (
  			instance->totemsrp_log_level_debug,
  			"Created or loaded sequence id %llx.%s for this ring.\n",
diff --git a/exec/util.c b/exec/util.c
index fe0acec..27d42a3 100644
--- a/exec/util.c
+++ b/exec/util.c
@@ -177,3 +177,24 @@ int cs_name_tisEqual (cs_name_t *str1, char *str2) {
  		return 0;
  	}
  }
+
+const char *get_run_dir(void)
+{
+	static char path[PATH_MAX] = {'\0'};
+	char *env_run_dir;
+	int res;
+
+	if (path[0] == '\0') {
+		env_run_dir = getenv("COROSYNC_RUN_DIR");
+
+		if (env_run_dir != NULL && env_run_dir[0] != '\0') {
+			res = snprintf(path, PATH_MAX, "%s", getenv("COROSYNC_RUN_DIR"));
+		} else {
+			res = snprintf(path, PATH_MAX, "%s/%s", LOCALSTATEDIR, "lib/corosync");
+		}
+
+		assert(res < PATH_MAX);
+	}
+
+	return (path);
+}
diff --git a/exec/util.h b/exec/util.h
index 263919b..766157b 100644
--- a/exec/util.h
+++ b/exec/util.h
@@ -64,6 +64,7 @@ enum e_ais_done {
  	AIS_DONE_AQUIRE_LOCK = 17,
  	AIS_DONE_ALREADY_RUNNING = 18,
  	AIS_DONE_STD_TO_NULL_REDIR = 19,
+	AIS_DONE_STORE_RINGID = 21,
  };

  static inline cs_error_t hdb_error_to_cs (int res)		\
@@ -104,4 +105,9 @@ extern int cs_name_tisEqual (cs_name_t *str1, char *str2);
  const char * short_service_name_get(uint32_t service_id,
  				    char *buf, size_t buf_size);

+/*
+ * Return run directory (ether COROSYNC_RUN_DIR env or LOCALSTATEDIR/lib/corosync)
+ */
+const char *get_run_dir(void);
+
  #endif /* UTIL_H_DEFINED */
diff --git a/include/corosync/totem/totem.h b/include/corosync/totem/totem.h
index aa4c5d7..419df9b 100644
--- a/include/corosync/totem/totem.h
+++ b/include/corosync/totem/totem.h
@@ -98,6 +98,12 @@ typedef enum {
  	TOTEM_TRANSPORT_RDMA = 2
  } totem_transport_t;

+#define MEMB_RING_ID
+struct memb_ring_id {
+	struct totem_ip_address rep;
+	unsigned long long seq;
+} __attribute__((packed));
+
  struct totem_config {
  	int version;

@@ -190,6 +196,14 @@ struct totem_config {
  	totem_transport_t transport_number;

  	unsigned int miss_count_const;
+
+	void (*totem_memb_ring_id_create_or_load) (
+	    struct memb_ring_id *memb_ring_id,
+	    const struct totem_ip_address *addr);
+
+	void (*totem_memb_ring_id_store) (
+	    const struct memb_ring_id *memb_ring_id,
+	    const struct totem_ip_address *addr);
  };

  #define TOTEM_CONFIGURATION_TYPE
@@ -209,12 +223,6 @@ enum totem_event_type {
  	TOTEM_EVENT_NEW_MSG,
  };

-#define MEMB_RING_ID
-struct memb_ring_id {
-	struct totem_ip_address rep;
-	unsigned long long seq;
-} __attribute__((packed));
-
  typedef struct {
  	hdb_handle_t handle;
  	int is_dirty;


_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss




[Index of Archives]     [Linux Clusters]     [Corosync Project]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Linux Kernel]     [Linux SCSI]     [X.Org]

  Powered by Linux