From 5f6a40827e8ff4b4723308a439320117b6ef3ede Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 5 Sep 2013 15:33:24 +0200
Subject: [PATCH 01/11] mysql: Ensure connections are properly released in
 multi-threaded environments

---
 .../plugins/mysql/mysql_database.c            | 37 ++++++++++++-------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/libstrongswan/plugins/mysql/mysql_database.c b/src/libstrongswan/plugins/mysql/mysql_database.c
index 8bd64692c..fdb6fd684 100644
--- a/src/libstrongswan/plugins/mysql/mysql_database.c
+++ b/src/libstrongswan/plugins/mysql/mysql_database.c
@@ -101,9 +101,11 @@ struct conn_t {
 /**
  * Release a mysql connection
  */
-static void conn_release(conn_t *conn)
+static void conn_release(private_mysql_database_t *this, conn_t *conn)
 {
+	this->mutex->lock(this->mutex);
 	conn->in_use = FALSE;
+	this->mutex->unlock(this->mutex);
 }
 
 /**
@@ -197,9 +199,10 @@ static conn_t *conn_get(private_mysql_database_t *this)
 	}
 	if (found == NULL)
 	{
-		found = malloc_thing(conn_t);
-		found->in_use = TRUE;
-		found->mysql = mysql_init(NULL);
+		INIT(found,
+			.in_use = TRUE,
+			.mysql = mysql_init(NULL),
+		);
 		if (!mysql_real_connect(found->mysql, this->host, this->username,
 								this->password, this->database, this->port,
 								NULL, 0))
@@ -332,6 +335,8 @@ static MYSQL_STMT* run(MYSQL *mysql, char *sql, va_list *args)
 typedef struct {
 	/** implements enumerator_t */
 	enumerator_t public;
+	/** mysql database */
+	private_mysql_database_t *db;
 	/** associated MySQL statement */
 	MYSQL_STMT *stmt;
 	/** result bindings */
@@ -373,7 +378,7 @@ static void mysql_enumerator_destroy(mysql_enumerator_t *this)
 		}
 	}
 	mysql_stmt_close(this->stmt);
-	conn_release(this->conn);
+	conn_release(this->db, this->conn);
 	free(this->bind);
 	free(this->val.p_void);
 	free(this->length);
@@ -496,11 +501,16 @@ METHOD(database_t, query, enumerator_t*,
 	{
 		int columns, i;
 
-		enumerator = malloc_thing(mysql_enumerator_t);
-		enumerator->public.enumerate = (void*)mysql_enumerator_enumerate;
-		enumerator->public.destroy = (void*)mysql_enumerator_destroy;
-		enumerator->stmt = stmt;
-		enumerator->conn = conn;
+		INIT(enumerator,
+			.public = {
+				.enumerate = (void*)mysql_enumerator_enumerate,
+				.destroy = (void*)mysql_enumerator_destroy,
+
+			},
+			.db = this,
+			.stmt = stmt,
+			.conn = conn,
+		);
 		columns = mysql_stmt_field_count(stmt);
 		enumerator->bind = calloc(columns, sizeof(MYSQL_BIND));
 		enumerator->length = calloc(columns, sizeof(unsigned long));
@@ -557,7 +567,7 @@ METHOD(database_t, query, enumerator_t*,
 	}
 	else
 	{
-		conn_release(conn);
+		conn_release(this, conn);
 	}
 	va_end(args);
 	return (enumerator_t*)enumerator;
@@ -588,7 +598,7 @@ METHOD(database_t, execute, int,
 		mysql_stmt_close(stmt);
 	}
 	va_end(args);
-	conn_release(conn);
+	conn_release(this, conn);
 	return affected;
 }
 
@@ -697,7 +707,6 @@ mysql_database_t *mysql_database_create(char *uri)
 		destroy(this);
 		return NULL;
 	}
-	conn_release(conn);
+	conn_release(this, conn);
 	return &this->public;
 }
-

From 947b76cda8b28f8b07e30977ca1c902ed8736dbe Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 6 Sep 2013 08:16:39 +0200
Subject: [PATCH 02/11] database: Add interface to handle transactions

---
 src/libstrongswan/database/database.h         | 33 ++++++++++++++++++-
 .../plugins/mysql/mysql_database.c            | 22 +++++++++++++
 .../plugins/sqlite/sqlite_database.c          | 22 +++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/src/libstrongswan/database/database.h b/src/libstrongswan/database/database.h
index d46fc3d34..77d4da144 100644
--- a/src/libstrongswan/database/database.h
+++ b/src/libstrongswan/database/database.h
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2013 Tobias Brunner
  * Copyright (C) 2008 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -102,7 +103,7 @@ struct database_t {
 	enumerator_t* (*query)(database_t *this, char *sql, ...);
 
 	/**
-	 * Execute a query which dows not return rows, such as INSERT.
+	 * Execute a query which does not return rows, such as INSERT.
 	 *
 	 * @param rowid		pointer to write inserted AUTO_INCREMENT row ID, or NULL
 	 * @param sql		sql string, containing '?' placeholders
@@ -111,6 +112,36 @@ struct database_t {
 	 */
 	int (*execute)(database_t *this, int *rowid, char *sql, ...);
 
+	/**
+	 * Start a transaction.
+	 *
+	 * @note Either commit() or rollback() has to be called to end the
+	 * transaction.
+	 * @note Transactions are thread-specific. So commit()/rollbak() has to be
+	 * called from the same thread.
+	 * @note While this method can be called multiple times (commit/rollback
+	 * have to be called an equal number of times) real nested transactions are
+	 * not supported.  So if any if the "inner" transactions are rolled back
+	 * the outer most transaction is rolled back.
+	 *
+	 * @return			TRUE on success
+	 */
+	bool (*transaction)(database_t *this);
+
+	/**
+	 * Commit all changes made during the current transaction.
+	 *
+	 * @return			TRUE on success
+	 */
+	bool (*commit)(database_t *this);
+
+	/**
+	 * Rollback/revert all changes made during the current transaction.
+	 *
+	 * @return			TRUE on success
+	 */
+	bool (*rollback)(database_t *this);
+
 	/**
 	 * Get the database implementation type.
 	 *
diff --git a/src/libstrongswan/plugins/mysql/mysql_database.c b/src/libstrongswan/plugins/mysql/mysql_database.c
index fdb6fd684..0757c2a32 100644
--- a/src/libstrongswan/plugins/mysql/mysql_database.c
+++ b/src/libstrongswan/plugins/mysql/mysql_database.c
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2013 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -602,6 +603,24 @@ METHOD(database_t, execute, int,
 	return affected;
 }
 
+METHOD(database_t, transaction, bool,
+	private_mysql_database_t *this)
+{
+	return FALSE;
+}
+
+METHOD(database_t, commit, bool,
+	private_mysql_database_t *this)
+{
+	return FALSE;
+}
+
+METHOD(database_t, rollback, bool,
+	private_mysql_database_t *this)
+{
+	return FALSE;
+}
+
 METHOD(database_t, get_driver,db_driver_t,
 	private_mysql_database_t *this)
 {
@@ -686,6 +705,9 @@ mysql_database_t *mysql_database_create(char *uri)
 			.db = {
 				.query = _query,
 				.execute = _execute,
+				.transaction = _transaction,
+				.commit = _commit,
+				.rollback = _rollback,
 				.get_driver = _get_driver,
 				.destroy = _destroy,
 			},
diff --git a/src/libstrongswan/plugins/sqlite/sqlite_database.c b/src/libstrongswan/plugins/sqlite/sqlite_database.c
index 41d45dee7..6c8b48324 100644
--- a/src/libstrongswan/plugins/sqlite/sqlite_database.c
+++ b/src/libstrongswan/plugins/sqlite/sqlite_database.c
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2013 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -280,6 +281,24 @@ METHOD(database_t, execute, int,
 	return affected;
 }
 
+METHOD(database_t, transaction, bool,
+	private_sqlite_database_t *this)
+{
+	return FALSE;
+}
+
+METHOD(database_t, commit, bool,
+	private_sqlite_database_t *this)
+{
+	return FALSE;
+}
+
+METHOD(database_t, rollback, bool,
+	private_sqlite_database_t *this)
+{
+	return FALSE;
+}
+
 METHOD(database_t, get_driver, db_driver_t,
 	private_sqlite_database_t *this)
 {
@@ -330,6 +349,9 @@ sqlite_database_t *sqlite_database_create(char *uri)
 			.db = {
 				.query = _query,
 				.execute = _execute,
+				.transaction = _transaction,
+				.commit = _commit,
+				.rollback = _rollback,
 				.get_driver = _get_driver,
 				.destroy = _destroy,
 			},

From f3cb889c9b4539ef0669fd080122fb129a5eea67 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 5 Sep 2013 16:46:24 +0200
Subject: [PATCH 03/11] mysql: Implement transaction handling

---
 .../plugins/mysql/mysql_database.c            | 126 +++++++++++++++++-
 1 file changed, 119 insertions(+), 7 deletions(-)

diff --git a/src/libstrongswan/plugins/mysql/mysql_database.c b/src/libstrongswan/plugins/mysql/mysql_database.c
index 0757c2a32..7a612ff8b 100644
--- a/src/libstrongswan/plugins/mysql/mysql_database.c
+++ b/src/libstrongswan/plugins/mysql/mysql_database.c
@@ -50,6 +50,11 @@ struct private_mysql_database_t {
 	 */
 	linked_list_t *pool;
 
+	/**
+	 * thread-specific transaction, as transaction_t
+	 */
+	thread_value_t *transaction;
+
 	/**
 	 * mutex to lock pool
 	 */
@@ -99,6 +104,28 @@ struct conn_t {
 	bool in_use;
 };
 
+/**
+ * database transaction
+ */
+typedef struct {
+
+	/**
+	 * Reference to the specific connection we started the transaction on
+	 */
+	conn_t *conn;
+
+	/**
+	 * Refcounter if transaction() is called multiple times
+	 */
+	refcount_t refs;
+
+	/**
+	 * TRUE if transaction was rolled back
+	 */
+	bool rollback;
+
+} transaction_t;
+
 /**
  * Release a mysql connection
  */
@@ -109,6 +136,16 @@ static void conn_release(private_mysql_database_t *this, conn_t *conn)
 	this->mutex->unlock(this->mutex);
 }
 
+/**
+ * Destroy a transaction and release the connection
+ */
+static void transaction_destroy(private_mysql_database_t *this,
+								transaction_t *trans)
+{
+	conn_release(this, trans->conn);
+	free(trans);
+}
+
 /**
  * thread specific initialization flag
  */
@@ -161,13 +198,24 @@ static void conn_destroy(conn_t *this)
 /**
  * Acquire/Reuse a mysql connection
  */
-static conn_t *conn_get(private_mysql_database_t *this)
+static conn_t *conn_get(private_mysql_database_t *this, transaction_t **trans)
 {
 	conn_t *current, *found = NULL;
 	enumerator_t *enumerator;
+	transaction_t *transaction;
 
 	thread_initialize();
 
+	transaction = this->transaction->get(this->transaction);
+	if (transaction)
+	{
+		if (trans)
+		{
+			*trans = transaction;
+		}
+		return transaction->conn;
+	}
+
 	while (TRUE)
 	{
 		this->mutex->lock(this->mutex);
@@ -490,7 +538,7 @@ METHOD(database_t, query, enumerator_t*,
 	mysql_enumerator_t *enumerator = NULL;
 	conn_t *conn;
 
-	conn = conn_get(this);
+	conn = conn_get(this, NULL);
 	if (!conn)
 	{
 		return NULL;
@@ -582,7 +630,7 @@ METHOD(database_t, execute, int,
 	conn_t *conn;
 	int affected = -1;
 
-	conn = conn_get(this);
+	conn = conn_get(this, NULL);
 	if (!conn)
 	{
 		return -1;
@@ -606,19 +654,81 @@ METHOD(database_t, execute, int,
 METHOD(database_t, transaction, bool,
 	private_mysql_database_t *this)
 {
-	return FALSE;
+	transaction_t *trans = NULL;
+	conn_t *conn;
+
+	conn = conn_get(this, &trans);
+	if (!conn)
+	{
+		return FALSE;
+	}
+	else if (trans)
+	{
+		ref_get(&trans->refs);
+		return TRUE;
+	}
+	/* these statements are not supported in prepared statements that are used
+	 * by the execute() method */
+	if (mysql_query(conn->mysql, "START TRANSACTION") != 0)
+	{
+		DBG1(DBG_LIB, "starting transaction failed: %s",
+			 mysql_error(conn->mysql));
+		conn_release(this, conn);
+		return FALSE;
+	}
+	INIT(trans,
+		.conn = conn,
+		.refs = 1,
+	);
+	this->transaction->set(this->transaction, trans);
+	return TRUE;
+}
+
+/**
+ * Finalize a transaction depending on the reference count and if it should be
+ * rolled back.
+ */
+static bool finalize_transaction(private_mysql_database_t *this,
+								 bool rollback)
+{
+	transaction_t *trans;
+	char *command = "COMMIT";
+	bool success;
+
+	trans = this->transaction->get(this->transaction);
+	if (!trans)
+	{
+		DBG1(DBG_LIB, "no database transaction found");
+		return FALSE;
+	}
+	/* set flag, can't be unset */
+	trans->rollback |= rollback;
+
+	if (ref_put(&trans->refs))
+	{
+		if (trans->rollback)
+		{
+			command = "ROLLBACK";
+		}
+		success = mysql_query(trans->conn->mysql, command) == 0;
+
+		this->transaction->set(this->transaction, NULL);
+		transaction_destroy(this, trans);
+		return success;
+	}
+	return TRUE;
 }
 
 METHOD(database_t, commit, bool,
 	private_mysql_database_t *this)
 {
-	return FALSE;
+	return finalize_transaction(this, FALSE);
 }
 
 METHOD(database_t, rollback, bool,
 	private_mysql_database_t *this)
 {
-	return FALSE;
+	return finalize_transaction(this, TRUE);
 }
 
 METHOD(database_t, get_driver,db_driver_t,
@@ -630,6 +740,7 @@ METHOD(database_t, get_driver,db_driver_t,
 METHOD(database_t, destroy, void,
 	private_mysql_database_t *this)
 {
+	this->transaction->destroy(this->transaction);
 	this->pool->destroy_function(this->pool, (void*)conn_destroy);
 	this->mutex->destroy(this->mutex);
 	free(this->host);
@@ -721,9 +832,10 @@ mysql_database_t *mysql_database_create(char *uri)
 	}
 	this->mutex = mutex_create(MUTEX_TYPE_DEFAULT);
 	this->pool = linked_list_create();
+	this->transaction = thread_value_create(NULL);
 
 	/* check connectivity */
-	conn = conn_get(this);
+	conn = conn_get(this, NULL);
 	if (!conn)
 	{
 		destroy(this);

From fad11d602dc6f969b9d2207a3d3c736fad418812 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 5 Sep 2013 16:50:23 +0200
Subject: [PATCH 04/11] sqlite: Implement transaction handling

---
 .../plugins/sqlite/sqlite_database.c          | 89 +++++++++++++++++--
 1 file changed, 83 insertions(+), 6 deletions(-)

diff --git a/src/libstrongswan/plugins/sqlite/sqlite_database.c b/src/libstrongswan/plugins/sqlite/sqlite_database.c
index 6c8b48324..b5ed7eed2 100644
--- a/src/libstrongswan/plugins/sqlite/sqlite_database.c
+++ b/src/libstrongswan/plugins/sqlite/sqlite_database.c
@@ -21,6 +21,7 @@
 #include <library.h>
 #include <utils/debug.h>
 #include <threading/mutex.h>
+#include <threading/thread_value.h>
 
 typedef struct private_sqlite_database_t private_sqlite_database_t;
 
@@ -40,11 +41,33 @@ struct private_sqlite_database_t {
 	sqlite3 *db;
 
 	/**
-	 * mutex used to lock execute()
+	 * thread-specific transaction, as transaction_t
+	 */
+	thread_value_t *transaction;
+
+	/**
+	 * mutex used to lock execute(), if necessary
 	 */
 	mutex_t *mutex;
 };
 
+/**
+ * Database transaction
+ */
+typedef struct {
+
+	/**
+	 * Refcounter if transaction() is called multiple times
+	 */
+	refcount_t refs;
+
+	/**
+	 * TRUE if transaction was rolled back
+	 */
+	bool rollback;
+
+} transaction_t;
+
 /**
  * Create and run a sqlite stmt using a sql string and args
  */
@@ -284,19 +307,72 @@ METHOD(database_t, execute, int,
 METHOD(database_t, transaction, bool,
 	private_sqlite_database_t *this)
 {
-	return FALSE;
+	transaction_t *trans;
+
+	trans = this->transaction->get(this->transaction);
+	if (trans)
+	{
+		ref_get(&trans->refs);
+		return TRUE;
+	}
+	if (execute(this, NULL, "BEGIN EXCLUSIVE TRANSACTION") == -1)
+	{
+		return FALSE;
+	}
+	INIT(trans,
+		.refs = 1,
+	);
+	this->transaction->set(this->transaction, trans);
+	return TRUE;
+}
+
+/**
+ * Finalize a transaction depending on the reference count and if it should be
+ * rolled back.
+ */
+static bool finalize_transaction(private_sqlite_database_t *this,
+								 bool rollback)
+{
+	transaction_t *trans;
+	char *command = "COMMIT TRANSACTION";
+	bool success;
+
+	trans = this->transaction->get(this->transaction);
+	if (!trans)
+	{
+		DBG1(DBG_LIB, "no database transaction found");
+		return FALSE;
+	}
+
+	if (ref_put(&trans->refs))
+	{
+		if (trans->rollback)
+		{
+			command = "ROLLBACK TRANSACTION";
+		}
+		success = execute(this, NULL, command) != -1;
+
+		this->transaction->set(this->transaction, NULL);
+		free(trans);
+		return success;
+	}
+	else
+	{	/* set flag, can't be unset */
+		trans->rollback |= rollback;
+	}
+	return TRUE;
 }
 
 METHOD(database_t, commit, bool,
 	private_sqlite_database_t *this)
 {
-	return FALSE;
+	return finalize_transaction(this, FALSE);
 }
 
 METHOD(database_t, rollback, bool,
 	private_sqlite_database_t *this)
 {
-	return FALSE;
+	return finalize_transaction(this, TRUE);
 }
 
 METHOD(database_t, get_driver, db_driver_t,
@@ -323,6 +399,7 @@ METHOD(database_t, destroy, void,
 	{
 		DBG1(DBG_LIB, "sqlite close failed because database is busy");
 	}
+	this->transaction->destroy(this->transaction);
 	this->mutex->destroy(this->mutex);
 	free(this);
 }
@@ -357,13 +434,14 @@ sqlite_database_t *sqlite_database_create(char *uri)
 			},
 		},
 		.mutex = mutex_create(MUTEX_TYPE_RECURSIVE),
+		.transaction = thread_value_create(NULL),
 	);
 
 	if (sqlite3_open(file, &this->db) != SQLITE_OK)
 	{
 		DBG1(DBG_LIB, "opening SQLite database '%s' failed: %s",
 			 file, sqlite3_errmsg(this->db));
-		_destroy(this);
+		destroy(this);
 		return NULL;
 	}
 
@@ -371,4 +449,3 @@ sqlite_database_t *sqlite_database_create(char *uri)
 
 	return &this->public;
 }
-

From 4b8b1354cea559844d7cbf2e460e90dab3655fad Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 5 Sep 2013 17:03:11 +0200
Subject: [PATCH 05/11] attr-sql: Don't use database transactions in
 create_attribute_enumerator

There could, of course, be race conditions when enumerating the attributes,
but those probably don't matter (e.g. missing an attribute that was
concurrently added).

Transactions are more intended to revert multiple changes if anything
fails in the process.
---
 src/libhydra/plugins/attr_sql/sql_attribute.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/libhydra/plugins/attr_sql/sql_attribute.c b/src/libhydra/plugins/attr_sql/sql_attribute.c
index e91e1ed15..cad5bfae3 100644
--- a/src/libhydra/plugins/attr_sql/sql_attribute.c
+++ b/src/libhydra/plugins/attr_sql/sql_attribute.c
@@ -346,8 +346,6 @@ METHOD(attribute_provider_t, create_attribute_enumerator, enumerator_t*,
 		u_int count;
 		char *name;
 
-		this->db->execute(this->db, NULL, "BEGIN EXCLUSIVE TRANSACTION");
-
 		/* in a first step check for attributes that match name and id */
 		if (id)
 		{
@@ -418,8 +416,6 @@ METHOD(attribute_provider_t, create_attribute_enumerator, enumerator_t*,
 			pool_enumerator->destroy(pool_enumerator);
 		}
 
-		this->db->execute(this->db, NULL, "END TRANSACTION");
-
 		/* lastly try to find global attributes */
 		if (!attr_enumerator)
 		{
@@ -474,4 +470,3 @@ sql_attribute_t *sql_attribute_create(database_t *db)
 					  DB_UINT, now);
 	return &this->public;
 }
-

From 5abe3c52d396339f3e1ac5f5a63cb8a4dfa0209d Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 13 Sep 2013 13:25:49 +0200
Subject: [PATCH 06/11] attr-sql: Handle concurrent insertion of identities

If the same identity is added concurrently by two threads (or by the
pool utility) INSERT might fail even though the SELECT was unsuccessful
before.

We are currently not able to lock the identities table in a portable way
(something like SELECT ... FOR UPDATE on MySQL).
---
 src/libhydra/plugins/attr_sql/sql_attribute.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/libhydra/plugins/attr_sql/sql_attribute.c b/src/libhydra/plugins/attr_sql/sql_attribute.c
index cad5bfae3..20c606ef3 100644
--- a/src/libhydra/plugins/attr_sql/sql_attribute.c
+++ b/src/libhydra/plugins/attr_sql/sql_attribute.c
@@ -50,19 +50,24 @@ static u_int get_identity(private_sql_attribute_t *this, identification_t *id)
 {
 	enumerator_t *e;
 	u_int row;
+	int try = 0;
 
+retry:
 	/* look for peer identity in the identities table */
 	e = this->db->query(this->db,
 						"SELECT id FROM identities WHERE type = ? AND data = ?",
 						DB_INT, id->get_type(id), DB_BLOB, id->get_encoding(id),
 						DB_UINT);
-
 	if (e && e->enumerate(e, &row))
 	{
 		e->destroy(e);
 		return row;
 	}
 	DESTROY_IF(e);
+	if (try > 0)
+	{
+		return 0;
+	}
 	/* not found, insert new one */
 	if (this->db->execute(this->db, &row,
 				  "INSERT INTO identities (type, data) VALUES (?, ?)",
@@ -70,7 +75,12 @@ static u_int get_identity(private_sql_attribute_t *this, identification_t *id)
 	{
 		return row;
 	}
-	return 0;
+	/* the INSERT could fail due to the UNIQUE constraint, if the identity was
+	 * added concurrently by another thread or the pool utility,
+	 * therefore try finding it again. a nicer fix would be to use locking
+	 * on the database, but our API currently not supports that */
+	try++;
+	goto retry;
 }
 
 /**

From ec6ad6b08616b9d3e1a3b3dffc06d56f65179a06 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 5 Sep 2013 18:00:48 +0200
Subject: [PATCH 07/11] pool: Move the pool utility to its own directory in src

---
 configure.ac                                     |  1 +
 src/Makefile.am                                  |  4 ++++
 src/libhydra/plugins/attr_sql/Makefile.am        | 10 +---------
 .../plugins/attr_sql => pool}/.gitignore         |  0
 src/pool/Makefile.am                             | 16 ++++++++++++++++
 src/{libhydra/plugins/attr_sql => pool}/pool.c   |  1 -
 .../plugins/attr_sql => pool}/pool_attributes.c  | 13 ++++++-------
 .../plugins/attr_sql => pool}/pool_attributes.h  |  2 --
 .../plugins/attr_sql => pool}/pool_usage.c       |  1 -
 .../plugins/attr_sql => pool}/pool_usage.h       |  1 -
 10 files changed, 28 insertions(+), 21 deletions(-)
 rename src/{libhydra/plugins/attr_sql => pool}/.gitignore (100%)
 create mode 100644 src/pool/Makefile.am
 rename src/{libhydra/plugins/attr_sql => pool}/pool.c (99%)
 rename src/{libhydra/plugins/attr_sql => pool}/pool_attributes.c (98%)
 rename src/{libhydra/plugins/attr_sql => pool}/pool_attributes.h (99%)
 rename src/{libhydra/plugins/attr_sql => pool}/pool_usage.c (99%)
 rename src/{libhydra/plugins/attr_sql => pool}/pool_usage.h (99%)

diff --git a/configure.ac b/configure.ac
index 0f558161e..66cbfa987 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1478,6 +1478,7 @@ AC_CONFIG_FILES([
 	src/scepclient/Makefile
 	src/pki/Makefile
 	src/pki/man/Makefile
+	src/pool/Makefile
 	src/dumm/Makefile
 	src/dumm/ext/extconf.rb
 	src/libfast/Makefile
diff --git a/src/Makefile.am b/src/Makefile.am
index 8ed45acc9..218c9434c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -100,6 +100,10 @@ if USE_INTEGRITY_TEST
   SUBDIRS += checksum
 endif
 
+if USE_ATTR_SQL
+  SUBDIRS += pool
+endif
+
 if USE_TKM
   SUBDIRS += charon-tkm
 endif
diff --git a/src/libhydra/plugins/attr_sql/Makefile.am b/src/libhydra/plugins/attr_sql/Makefile.am
index 4c369a2bd..d126bb035 100644
--- a/src/libhydra/plugins/attr_sql/Makefile.am
+++ b/src/libhydra/plugins/attr_sql/Makefile.am
@@ -1,7 +1,6 @@
 AM_CPPFLAGS = \
 	-I$(top_srcdir)/src/libstrongswan \
-	-I$(top_srcdir)/src/libhydra \
-	-DPLUGINS=\""${pool_plugins}\""
+	-I$(top_srcdir)/src/libhydra
 
 AM_CFLAGS = \
 	-rdynamic
@@ -17,10 +16,3 @@ libstrongswan_attr_sql_la_SOURCES = \
 	sql_attribute.h sql_attribute.c
 
 libstrongswan_attr_sql_la_LDFLAGS = -module -avoid-version
-
-ipsec_PROGRAMS = pool
-pool_SOURCES = pool.c pool_attributes.c pool_attributes.h \
-			   pool_usage.h pool_usage.c
-pool_LDADD = $(top_builddir)/src/libstrongswan/libstrongswan.la \
-			 $(top_builddir)/src/libhydra/libhydra.la
-pool.o :	$(top_builddir)/config.status
diff --git a/src/libhydra/plugins/attr_sql/.gitignore b/src/pool/.gitignore
similarity index 100%
rename from src/libhydra/plugins/attr_sql/.gitignore
rename to src/pool/.gitignore
diff --git a/src/pool/Makefile.am b/src/pool/Makefile.am
new file mode 100644
index 000000000..8b429a4ba
--- /dev/null
+++ b/src/pool/Makefile.am
@@ -0,0 +1,16 @@
+ipsec_PROGRAMS = pool
+
+pool_SOURCES = \
+	pool.c pool_attributes.c pool_attributes.h \
+	pool_usage.h pool_usage.c
+
+pool.o :	$(top_builddir)/config.status
+
+AM_CPPFLAGS = \
+	-I$(top_srcdir)/src/libstrongswan \
+	-I$(top_srcdir)/src/libhydra \
+	-DPLUGINS=\""${pool_plugins}\""
+
+pool_LDADD = \
+	$(top_builddir)/src/libstrongswan/libstrongswan.la \
+	$(top_builddir)/src/libhydra/libhydra.la
diff --git a/src/libhydra/plugins/attr_sql/pool.c b/src/pool/pool.c
similarity index 99%
rename from src/libhydra/plugins/attr_sql/pool.c
rename to src/pool/pool.c
index 4e7c48e23..831b3c439 100644
--- a/src/libhydra/plugins/attr_sql/pool.c
+++ b/src/pool/pool.c
@@ -1284,4 +1284,3 @@ int main(int argc, char *argv[])
 
 	exit(EXIT_SUCCESS);
 }
-
diff --git a/src/libhydra/plugins/attr_sql/pool_attributes.c b/src/pool/pool_attributes.c
similarity index 98%
rename from src/libhydra/plugins/attr_sql/pool_attributes.c
rename to src/pool/pool_attributes.c
index 1d1ba8f58..72af4f494 100644
--- a/src/libhydra/plugins/attr_sql/pool_attributes.c
+++ b/src/pool/pool_attributes.c
@@ -49,7 +49,7 @@ static const attr_info_t attr_info[] = {
 	{ "internal_ip4_netmask", VALUE_ADDR,   INTERNAL_IP4_NETMASK, 0 },
 	{ "internal_ip6_netmask", VALUE_ADDR,   INTERNAL_IP6_NETMASK, 0 },
 	{ "netmask",              VALUE_ADDR,   INTERNAL_IP4_NETMASK,
-							 				INTERNAL_IP6_NETMASK    },
+											INTERNAL_IP6_NETMASK    },
 	{ "internal_ip4_dns",     VALUE_ADDR,   INTERNAL_IP4_DNS,     0 },
 	{ "internal_ip6_dns",     VALUE_ADDR,   INTERNAL_IP6_DNS,     0 },
 	{ "dns",                  VALUE_ADDR,   INTERNAL_IP4_DNS,
@@ -57,7 +57,7 @@ static const attr_info_t attr_info[] = {
 	{ "internal_ip4_nbns",    VALUE_ADDR,   INTERNAL_IP4_NBNS,    0 },
 	{ "internal_ip6_nbns",    VALUE_ADDR,   INTERNAL_IP6_NBNS,    0 },
 	{ "nbns",                 VALUE_ADDR,   INTERNAL_IP4_NBNS,
-							 				INTERNAL_IP6_NBNS       },
+											INTERNAL_IP6_NBNS       },
 	{ "wins",                 VALUE_ADDR,   INTERNAL_IP4_NBNS,
 											INTERNAL_IP6_NBNS       },
 	{ "internal_ip4_dhcp",    VALUE_ADDR,   INTERNAL_IP4_DHCP,    0 },
@@ -214,7 +214,7 @@ static bool parse_attributes(char *name, char *value, value_type_t *value_type,
 			if (*value_type == VALUE_ADDR)
 			{
 				*type = (addr->get_family(addr) == AF_INET) ?
-						 	attr_info[i].type : attr_info[i].type_ip6;
+							attr_info[i].type : attr_info[i].type_ip6;
 				addr->destroy(addr);
 			}
 			else if (*value_type == VALUE_HEX)
@@ -493,14 +493,14 @@ void del_attr(char *name, char *pool, char *identity,
 			else if (value_type == VALUE_STRING)
 			{
 				fprintf(stderr, "deleting %s attribute (%N) with value '%.*s'%s failed.\n",
-						 		name, configuration_attribute_type_names, type,
+								name, configuration_attribute_type_names, type,
 								(int)blob_db.len, blob_db.ptr, id_pool_str);
 			}
 
 			else
 			{
 				fprintf(stderr, "deleting %s attribute (%N) with value %#B%s failed.\n",
-						 		name, configuration_attribute_type_names, type,
+								name, configuration_attribute_type_names, type,
 								&blob_db, id_pool_str);
 			}
 			query->destroy(query);
@@ -529,7 +529,7 @@ void del_attr(char *name, char *pool, char *identity,
 
 	if (!found)
 	{
-	 	if (blob.len == 0)
+		if (blob.len == 0)
 		{
 			if (type_ip6 == 0)
 			{
@@ -714,4 +714,3 @@ void show_attr(void)
 		}
 	}
 }
-
diff --git a/src/libhydra/plugins/attr_sql/pool_attributes.h b/src/pool/pool_attributes.h
similarity index 99%
rename from src/libhydra/plugins/attr_sql/pool_attributes.h
rename to src/pool/pool_attributes.h
index a42291f57..6a5af3349 100644
--- a/src/libhydra/plugins/attr_sql/pool_attributes.h
+++ b/src/pool/pool_attributes.h
@@ -61,5 +61,3 @@ void status_attr(bool hexout);
 void show_attr(void);
 
 #endif /* POOL_ATTRIBUTES_H_ */
-
-
diff --git a/src/libhydra/plugins/attr_sql/pool_usage.c b/src/pool/pool_usage.c
similarity index 99%
rename from src/libhydra/plugins/attr_sql/pool_usage.c
rename to src/pool/pool_usage.c
index 985bc3ae8..7622cfa86 100644
--- a/src/libhydra/plugins/attr_sql/pool_usage.c
+++ b/src/pool/pool_usage.c
@@ -124,4 +124,3 @@ Usage:\n\
                lines are ignored. The file may not contain a --batch command.\n\
   \n");
 }
-
diff --git a/src/libhydra/plugins/attr_sql/pool_usage.h b/src/pool/pool_usage.h
similarity index 99%
rename from src/libhydra/plugins/attr_sql/pool_usage.h
rename to src/pool/pool_usage.h
index a98b0d680..0082ef6f2 100644
--- a/src/libhydra/plugins/attr_sql/pool_usage.h
+++ b/src/pool/pool_usage.h
@@ -22,5 +22,4 @@
  */
 void usage(void);
 
-
 #endif /* POOL_USAGE_H_ */

From 03c801cb2b026935bf75bb5211854bde9fa3e145 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 6 Sep 2013 11:29:17 +0200
Subject: [PATCH 08/11] pool: Change transaction handling

---
 src/pool/pool.c | 54 ++++++++-----------------------------------------
 1 file changed, 8 insertions(+), 46 deletions(-)

diff --git a/src/pool/pool.c b/src/pool/pool.c
index 831b3c439..c7cbcfc16 100644
--- a/src/pool/pool.c
+++ b/src/pool/pool.c
@@ -51,41 +51,6 @@ bool replace_pool = FALSE;
 static void del(char *name);
 static void do_args(int argc, char *argv[]);
 
-/**
- * nesting counter for database transaction functions
- */
-int nested_transaction = 0;
-
-/**
- * start a database transaction
- */
-static void begin_transaction()
-{
-	if (db->get_driver(db) == DB_SQLITE)
-	{
-		if (!nested_transaction)
-		{
-			db->execute(db, NULL, "BEGIN EXCLUSIVE TRANSACTION");
-		}
-		++nested_transaction;
-	}
-}
-
-/**
- * commit a database transaction
- */
-static void commit_transaction()
-{
-	if (db->get_driver(db) == DB_SQLITE)
-	{
-		--nested_transaction;
-		if (!nested_transaction)
-		{
-			db->execute(db, NULL, "END TRANSACTION");
-		}
-	}
-}
-
 /**
  * Create or replace a pool by name
  */
@@ -370,8 +335,7 @@ static void add(char *name, host_t *start, host_t *end, int timeout)
 	id = create_pool(name, start_addr, end_addr, timeout);
 	printf("allocating %d addresses... ", count);
 	fflush(stdout);
-	/* run population in a transaction for sqlite */
-	begin_transaction();
+	db->transaction(db);
 	while (TRUE)
 	{
 		db->execute(db, NULL,
@@ -384,7 +348,7 @@ static void add(char *name, host_t *start, host_t *end, int timeout)
 		}
 		chunk_increment(cur_addr);
 	}
-	commit_transaction();
+	db->commit(db);
 	printf("done.\n");
 }
 
@@ -449,8 +413,7 @@ static void add_addresses(char *pool, char *path, int timeout)
 	host_t *addr;
 	FILE *file;
 
-	/* run population in a transaction for sqlite */
-	begin_transaction();
+	db->transaction(db);
 
 	addr = host_create_from_string("%any", 0);
 	pool_id = create_pool(pool, addr->get_address(addr),
@@ -510,7 +473,7 @@ static void add_addresses(char *pool, char *path, int timeout)
 		addr->destroy(addr);
 	}
 
-	commit_transaction();
+	db->commit(db);
 
 	printf("%d addresses done.\n", count);
 }
@@ -596,6 +559,7 @@ static void resize(char *name, host_t *end)
 	}
 	DESTROY_IF(old_end);
 
+	db->transaction(db);
 	if (db->execute(db, NULL,
 			"UPDATE pools SET end = ? WHERE name = ?",
 			DB_BLOB, new_addr, DB_TEXT, name) <= 0)
@@ -606,8 +570,6 @@ static void resize(char *name, host_t *end)
 
 	printf("allocating %d new addresses... ", count);
 	fflush(stdout);
-	/* run population in a transaction for sqlite */
-	begin_transaction();
 	while (count-- > 0)
 	{
 		chunk_increment(cur_addr);
@@ -616,7 +578,7 @@ static void resize(char *name, host_t *end)
 			"VALUES (?, ?, ?, ?, ?)",
 			DB_UINT, id, DB_BLOB, cur_addr,	DB_UINT, 0, DB_UINT, 0, DB_UINT, 1);
 	}
-	commit_transaction();
+	db->commit(db);
 	printf("done.\n");
 
 }
@@ -900,7 +862,7 @@ static void batch(char *argv0, char *name)
 		exit(EXIT_FAILURE);
 	}
 
-	begin_transaction();
+	db->transaction(db);
 	while (fgets(command, sizeof(command), file))
 	{
 		char *argv[ARGV_SIZE], *start;
@@ -939,7 +901,7 @@ static void batch(char *argv0, char *name)
 
 		do_args(argc, argv);
 	}
-	commit_transaction();
+	db->commit(db);
 
 	if (file != stdin)
 	{

From e745f5f69ff524cae7a2e34199c01f1dd1e4294e Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 6 Sep 2013 14:09:32 +0200
Subject: [PATCH 09/11] sql: Don't use MyISAM engine and set collation/charset
 for all tables

The MyISAM engine doesn't support transactions.
---
 src/libcharon/plugins/sql/mysql.sql | 51 ++++++++++++++---------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/src/libcharon/plugins/sql/mysql.sql b/src/libcharon/plugins/sql/mysql.sql
index e5d878689..0d1468176 100644
--- a/src/libcharon/plugins/sql/mysql.sql
+++ b/src/libcharon/plugins/sql/mysql.sql
@@ -1,5 +1,4 @@
 
-
 DROP TABLE IF EXISTS `identities`;
 CREATE TABLE `identities` (
   `id` int(10) unsigned NOT NULL auto_increment,
@@ -7,7 +6,7 @@ CREATE TABLE `identities` (
   `data` varbinary(64) NOT NULL,
   PRIMARY KEY (`id`),
   UNIQUE (`type`, `data`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `child_configs`;
@@ -27,7 +26,7 @@ CREATE TABLE `child_configs` (
   `reqid` mediumint(8) unsigned NOT NULL default '0',
   PRIMARY KEY (`id`),
   INDEX (`name`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `child_config_traffic_selector`;
@@ -36,7 +35,7 @@ CREATE TABLE `child_config_traffic_selector` (
   `traffic_selector` int(10) unsigned NOT NULL,
   `kind` tinyint(3) unsigned NOT NULL,
   INDEX (`child_cfg`, `traffic_selector`)
-) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `proposals`;
@@ -44,7 +43,7 @@ CREATE TABLE `proposals` (
   `id` int(10) unsigned NOT NULL auto_increment,
   `proposal` varchar(128) NOT NULL,
   PRIMARY KEY (`id`)
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `child_config_proposal`;
@@ -52,7 +51,7 @@ CREATE TABLE `child_config_proposal` (
   `child_cfg` int(10) unsigned NOT NULL,
   `prio` smallint(5) unsigned NOT NULL,
   `prop` int(10) unsigned NOT NULL
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `ike_configs`;
@@ -63,7 +62,7 @@ CREATE TABLE `ike_configs` (
   `local` varchar(128) collate utf8_unicode_ci NOT NULL,
   `remote` varchar(128) collate utf8_unicode_ci NOT NULL,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `ike_config_proposal`;
@@ -71,7 +70,7 @@ CREATE TABLE `ike_config_proposal` (
   `ike_cfg` int(10) unsigned NOT NULL,
   `prio` smallint(5) unsigned NOT NULL,
   `prop` int(10) unsigned NOT NULL
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `peer_configs`;
@@ -101,7 +100,7 @@ CREATE TABLE `peer_configs` (
   `peer_id` int(10) unsigned NOT NULL default '0',
   PRIMARY KEY (`id`),
   INDEX (`name`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `peer_config_child_config`;
@@ -109,7 +108,7 @@ CREATE TABLE `peer_config_child_config` (
   `peer_cfg` int(10) unsigned NOT NULL,
   `child_cfg` int(10) unsigned NOT NULL,
   PRIMARY KEY (`peer_cfg`, `child_cfg`)
-) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS `traffic_selectors`;
@@ -122,7 +121,7 @@ CREATE TABLE `traffic_selectors` (
   `start_port` smallint(5) unsigned NOT NULL default '0',
   `end_port` smallint(5) unsigned NOT NULL default '65535',
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS certificates;
@@ -132,7 +131,7 @@ CREATE TABLE certificates (
   `keytype` tinyint(3) unsigned NOT NULL,
   `data` BLOB NOT NULL,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS certificate_identity;
@@ -140,7 +139,7 @@ CREATE TABLE certificate_identity (
   `certificate` int(10) unsigned NOT NULL,
   `identity` int(10) unsigned NOT NULL,
   PRIMARY KEY (`certificate`, `identity`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS private_keys;
@@ -149,7 +148,7 @@ CREATE TABLE private_keys (
   `type` tinyint(3) unsigned NOT NULL,
   `data` BLOB NOT NULL,
   PRIMARY KEY  (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS private_key_identity;
@@ -157,7 +156,7 @@ CREATE TABLE private_key_identity (
   `private_key` int(10) unsigned NOT NULL,
   `identity` int(10) unsigned NOT NULL,
   PRIMARY KEY (`private_key`, `identity`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS shared_secrets;
@@ -166,7 +165,7 @@ CREATE TABLE shared_secrets (
   `type` tinyint(3) unsigned NOT NULL,
   `data` varbinary(256) NOT NULL,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS shared_secret_identity;
@@ -174,7 +173,7 @@ CREATE TABLE shared_secret_identity (
   `shared_secret` int(10) unsigned NOT NULL,
   `identity` int(10) unsigned NOT NULL,
   PRIMARY KEY (`shared_secret`, `identity`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS certificate_authorities;
@@ -182,7 +181,7 @@ CREATE TABLE certificate_authorities (
   `id` int(10) unsigned NOT NULL auto_increment,
   `certificate` int(10) unsigned NOT NULL,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS certificate_distribution_points;
@@ -192,7 +191,7 @@ CREATE TABLE certificate_distribution_points (
   `type` tinyint(3) unsigned NOT NULL,
   `uri` varchar(256) NOT NULL,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS pools;
@@ -204,7 +203,7 @@ CREATE TABLE pools (
   `timeout` int(10) unsigned NOT NULL,
   PRIMARY KEY (`id`),
   UNIQUE (`name`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS addresses;
@@ -219,7 +218,7 @@ CREATE TABLE addresses (
   INDEX (`pool`),
   INDEX (`identity`),
   INDEX (`address`)
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 DROP TABLE IF EXISTS leases;
 CREATE TABLE leases (
@@ -229,14 +228,14 @@ CREATE TABLE leases (
   `acquired` int(10) unsigned NOT NULL,
   `released` int(10) unsigned DEFAULT NULL,
   PRIMARY KEY (`id`)
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 DROP TABLE IF EXISTS attribute_pools;
 CREATE TABLE attribute_pools (
   `id` int(10) unsigned NOT NULL auto_increment,
   `name` varchar(32) NOT NULL,
   PRIMARY KEY (`id`)
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 DROP TABLE IF EXISTS attributes;
 CREATE TABLE attributes (
@@ -248,7 +247,7 @@ CREATE TABLE attributes (
   PRIMARY KEY (`id`),
   INDEX (`identity`),
   INDEX (`pool`)
-);
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 DROP TABLE IF EXISTS ike_sas;
 CREATE TABLE ike_sas (
@@ -265,7 +264,7 @@ CREATE TABLE ike_sas (
   `remote_host_data` varbinary(16) NOT NULL,
   `lastuse` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
   PRIMARY KEY (`local_spi`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 
 DROP TABLE IF EXISTS logs;
@@ -277,6 +276,6 @@ CREATE TABLE logs (
   `msg` varchar(256) NOT NULL,
   `time` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
   PRIMARY KEY (`id`)
-) ENGINE=MyISAM  DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 
 

From b283a6e9efd2ff16ec5b189435604b0d82d714cd Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 10 Oct 2013 10:58:40 +0200
Subject: [PATCH 10/11] database: Add support for serializable transactions

---
 src/libstrongswan/database/database.h              |  9 +++++++--
 src/libstrongswan/plugins/mysql/mysql_database.c   | 13 ++++++++++++-
 src/libstrongswan/plugins/sqlite/sqlite_database.c |  6 ++++--
 src/pool/pool.c                                    |  8 ++++----
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/libstrongswan/database/database.h b/src/libstrongswan/database/database.h
index 77d4da144..ad5ccf95e 100644
--- a/src/libstrongswan/database/database.h
+++ b/src/libstrongswan/database/database.h
@@ -115,6 +115,10 @@ struct database_t {
 	/**
 	 * Start a transaction.
 	 *
+	 * A serializable transaction forces a strict separation between other
+	 * transactions.  Due to the performance overhead they should only be used
+	 * in certain situations (e.g. SELECT->INSERT|UPDATE).
+	 *
 	 * @note Either commit() or rollback() has to be called to end the
 	 * transaction.
 	 * @note Transactions are thread-specific. So commit()/rollbak() has to be
@@ -124,9 +128,10 @@ struct database_t {
 	 * not supported.  So if any if the "inner" transactions are rolled back
 	 * the outer most transaction is rolled back.
 	 *
-	 * @return			TRUE on success
+	 * @param serializable	TRUE to create a serializable transaction
+	 * @return				TRUE on success
 	 */
-	bool (*transaction)(database_t *this);
+	bool (*transaction)(database_t *this, bool serializable);
 
 	/**
 	 * Commit all changes made during the current transaction.
diff --git a/src/libstrongswan/plugins/mysql/mysql_database.c b/src/libstrongswan/plugins/mysql/mysql_database.c
index 7a612ff8b..373e9dc7c 100644
--- a/src/libstrongswan/plugins/mysql/mysql_database.c
+++ b/src/libstrongswan/plugins/mysql/mysql_database.c
@@ -652,7 +652,7 @@ METHOD(database_t, execute, int,
 }
 
 METHOD(database_t, transaction, bool,
-	private_mysql_database_t *this)
+	private_mysql_database_t *this, bool serializable)
 {
 	transaction_t *trans = NULL;
 	conn_t *conn;
@@ -669,6 +669,17 @@ METHOD(database_t, transaction, bool,
 	}
 	/* these statements are not supported in prepared statements that are used
 	 * by the execute() method */
+	if (serializable)
+	{
+		if (mysql_query(conn->mysql,
+						"SET TRANSACTION ISOLATION LEVEL SERIALIZABLE") != 0)
+		{
+			DBG1(DBG_LIB, "starting transaction failed: %s",
+				 mysql_error(conn->mysql));
+			conn_release(this, conn);
+			return FALSE;
+		}
+	}
 	if (mysql_query(conn->mysql, "START TRANSACTION") != 0)
 	{
 		DBG1(DBG_LIB, "starting transaction failed: %s",
diff --git a/src/libstrongswan/plugins/sqlite/sqlite_database.c b/src/libstrongswan/plugins/sqlite/sqlite_database.c
index b5ed7eed2..7b4767855 100644
--- a/src/libstrongswan/plugins/sqlite/sqlite_database.c
+++ b/src/libstrongswan/plugins/sqlite/sqlite_database.c
@@ -305,9 +305,11 @@ METHOD(database_t, execute, int,
 }
 
 METHOD(database_t, transaction, bool,
-	private_sqlite_database_t *this)
+	private_sqlite_database_t *this, bool serializable)
 {
 	transaction_t *trans;
+	char *cmd = serializable ? "BEGIN EXCLUSIVE TRANSACTION"
+							 : "BEGIN TRANSACTION";
 
 	trans = this->transaction->get(this->transaction);
 	if (trans)
@@ -315,7 +317,7 @@ METHOD(database_t, transaction, bool,
 		ref_get(&trans->refs);
 		return TRUE;
 	}
-	if (execute(this, NULL, "BEGIN EXCLUSIVE TRANSACTION") == -1)
+	if (execute(this, NULL, cmd) == -1)
 	{
 		return FALSE;
 	}
diff --git a/src/pool/pool.c b/src/pool/pool.c
index c7cbcfc16..05043cd8c 100644
--- a/src/pool/pool.c
+++ b/src/pool/pool.c
@@ -335,7 +335,7 @@ static void add(char *name, host_t *start, host_t *end, int timeout)
 	id = create_pool(name, start_addr, end_addr, timeout);
 	printf("allocating %d addresses... ", count);
 	fflush(stdout);
-	db->transaction(db);
+	db->transaction(db, FALSE);
 	while (TRUE)
 	{
 		db->execute(db, NULL,
@@ -413,7 +413,7 @@ static void add_addresses(char *pool, char *path, int timeout)
 	host_t *addr;
 	FILE *file;
 
-	db->transaction(db);
+	db->transaction(db, FALSE);
 
 	addr = host_create_from_string("%any", 0);
 	pool_id = create_pool(pool, addr->get_address(addr),
@@ -559,7 +559,7 @@ static void resize(char *name, host_t *end)
 	}
 	DESTROY_IF(old_end);
 
-	db->transaction(db);
+	db->transaction(db, FALSE);
 	if (db->execute(db, NULL,
 			"UPDATE pools SET end = ? WHERE name = ?",
 			DB_BLOB, new_addr, DB_TEXT, name) <= 0)
@@ -862,7 +862,7 @@ static void batch(char *argv0, char *name)
 		exit(EXIT_FAILURE);
 	}
 
-	db->transaction(db);
+	db->transaction(db, FALSE);
 	while (fgets(command, sizeof(command), file))
 	{
 		char *argv[ARGV_SIZE], *start;

From bd085dd978b2dc7891c0d8486dd883e76e15e9a3 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 10 Oct 2013 11:02:16 +0200
Subject: [PATCH 11/11] attr-sql: Use a serializable transaction when inserting
 identities

---
 src/libhydra/plugins/attr_sql/sql_attribute.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/libhydra/plugins/attr_sql/sql_attribute.c b/src/libhydra/plugins/attr_sql/sql_attribute.c
index 20c606ef3..0a06c419f 100644
--- a/src/libhydra/plugins/attr_sql/sql_attribute.c
+++ b/src/libhydra/plugins/attr_sql/sql_attribute.c
@@ -50,9 +50,8 @@ static u_int get_identity(private_sql_attribute_t *this, identification_t *id)
 {
 	enumerator_t *e;
 	u_int row;
-	int try = 0;
 
-retry:
+	this->db->transaction(this->db, TRUE);
 	/* look for peer identity in the identities table */
 	e = this->db->query(this->db,
 						"SELECT id FROM identities WHERE type = ? AND data = ?",
@@ -61,26 +60,20 @@ retry:
 	if (e && e->enumerate(e, &row))
 	{
 		e->destroy(e);
+		this->db->commit(this->db);
 		return row;
 	}
 	DESTROY_IF(e);
-	if (try > 0)
-	{
-		return 0;
-	}
 	/* not found, insert new one */
 	if (this->db->execute(this->db, &row,
 				  "INSERT INTO identities (type, data) VALUES (?, ?)",
 				  DB_INT, id->get_type(id), DB_BLOB, id->get_encoding(id)) == 1)
 	{
+		this->db->commit(this->db);
 		return row;
 	}
-	/* the INSERT could fail due to the UNIQUE constraint, if the identity was
-	 * added concurrently by another thread or the pool utility,
-	 * therefore try finding it again. a nicer fix would be to use locking
-	 * on the database, but our API currently not supports that */
-	try++;
-	goto retry;
+	this->db->rollback(this->db);
+	return 0;
 }
 
 /**