Change in osmo-hlr[master]: add database schema versioning to the HLR database

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Tue Dec 4 14:01:17 UTC 2018


Stefan Sperling has submitted this change and it was merged. ( https://gerrit.osmocom.org/11898 )

Change subject: add database schema versioning to the HLR database
......................................................................

add database schema versioning to the HLR database

Make use of pragma user_version to store our database schema version.
The present schema is now identitifed as 'version 0', which is also
the default value for databases on which we never ran the statement
'pragma user_version' before.

Only bootstrap the database if it hasn't been bootstrapped yet.
Previously, bootstrap SQL statements ran every time osmo-hlr
opened the database, and any errors were being ignored in SQL.
Instead, we now first run a query which checks whether tables
already exist, and only create them if necessary.
This change will allow future schema updates to work properly.

Prepare for future schema upgrades by adding a new command-line
option which enables upgrades. This option defaults to 'false'
in order to avoid accidental upgrades.

Change-Id: I8aeaa9a404b622657cbc7138106f38aa6ad8d01b
Related: OS#2838
---
M sql/hlr.sql
M src/db.c
M src/db.h
M src/hlr.c
M src/hlr_db_tool.c
M tests/db/db_test.c
6 files changed, 128 insertions(+), 25 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Neels Hofmeyr: Looks good to me, approved



diff --git a/sql/hlr.sql b/sql/hlr.sql
index 80eb3e5..3499109 100644
--- a/sql/hlr.sql
+++ b/sql/hlr.sql
@@ -1,4 +1,4 @@
-CREATE TABLE IF NOT EXISTS subscriber (
+CREATE TABLE subscriber (
 -- OsmoHLR's DB scheme is modelled roughly after TS 23.008 version 13.3.0
 	id		INTEGER PRIMARY KEY,
 	-- Chapter 2.1.1.1
@@ -39,24 +39,24 @@
 	ms_purged_ps	BOOLEAN NOT NULL DEFAULT 0
 );
 
-CREATE TABLE IF NOT EXISTS subscriber_apn (
+CREATE TABLE subscriber_apn (
 	subscriber_id	INTEGER,		-- subscriber.id
 	apn		VARCHAR(256) NOT NULL
 );
 
-CREATE TABLE IF NOT EXISTS subscriber_multi_msisdn (
+CREATE TABLE subscriber_multi_msisdn (
 -- Chapter 2.1.3
 	subscriber_id	INTEGER,		-- subscriber.id
 	msisdn		VARCHAR(15) NOT NULL
 );
 
-CREATE TABLE IF NOT EXISTS auc_2g (
+CREATE TABLE auc_2g (
 	subscriber_id	INTEGER PRIMARY KEY,	-- subscriber.id
 	algo_id_2g	INTEGER NOT NULL,	-- enum osmo_auth_algo value
 	ki		VARCHAR(32) NOT NULL	-- hex string: subscriber's secret key (128bit)
 );
 
-CREATE TABLE IF NOT EXISTS auc_3g (
+CREATE TABLE auc_3g (
 	subscriber_id	INTEGER PRIMARY KEY,	-- subscriber.id
 	algo_id_3g	INTEGER NOT NULL,	-- enum osmo_auth_algo value
 	k		VARCHAR(32) NOT NULL,	-- hex string: subscriber's secret key (128bit)
@@ -66,4 +66,7 @@
 	ind_bitlen	INTEGER NOT NULL DEFAULT 5	-- nr of index bits at lower SQN end
 );
 
-CREATE UNIQUE INDEX IF NOT EXISTS idx_subscr_imsi ON subscriber (imsi);
+CREATE UNIQUE INDEX idx_subscr_imsi ON subscriber (imsi);
+
+-- Set HLR database schema version number
+PRAGMA user_version = 0;
diff --git a/src/db.c b/src/db.c
index bcf83c6..df52f9b 100644
--- a/src/db.c
+++ b/src/db.c
@@ -27,6 +27,8 @@
 #include "db.h"
 #include "db_bootstrap.h"
 
+#define CURRENT_SCHEMA_VERSION	0
+
 #define SEL_COLUMNS \
 	"id," \
 	"imsi," \
@@ -197,36 +199,90 @@
 	for (i = 0; i < ARRAY_SIZE(stmt_bootstrap_sql); i++) {
 		int rc;
 		sqlite3_stmt *stmt;
-
-		rc = sqlite3_prepare_v2(dbc->db, stmt_bootstrap_sql[i], -1,
-					&stmt, NULL);
+		rc = sqlite3_prepare_v2(dbc->db, stmt_bootstrap_sql[i], -1, &stmt, NULL);
 		if (rc != SQLITE_OK) {
-			LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n",
-			     stmt_bootstrap_sql[i]);
+			LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", stmt_bootstrap_sql[i]);
 			return rc;
 		}
 
-		/* execute the statement */
 		rc = sqlite3_step(stmt);
 		db_remove_reset(stmt);
 		sqlite3_finalize(stmt);
 		if (rc != SQLITE_DONE) {
 			LOGP(DDB, LOGL_ERROR, "Cannot bootstrap database: SQL error: (%d) %s,"
 			     " during stmt '%s'",
-			     rc, sqlite3_errmsg(dbc->db),
-			     stmt_bootstrap_sql[i]);
+			     rc, sqlite3_errmsg(dbc->db), stmt_bootstrap_sql[i]);
 			return rc;
 		}
 	}
 	return SQLITE_OK;
 }
 
-struct db_context *db_open(void *ctx, const char *fname, bool enable_sqlite_logging)
+/* https://www.sqlite.org/fileformat2.html#storage_of_the_sql_database_schema */
+static bool db_table_exists(struct db_context *dbc, const char *table_name)
+{
+	const char *table_exists_sql = "SELECT name FROM sqlite_master WHERE type='table' AND name=?";
+	sqlite3_stmt *stmt;
+	int rc;
+
+	rc = sqlite3_prepare_v2(dbc->db, table_exists_sql, -1, &stmt, NULL);
+	if (rc != SQLITE_OK) {
+		LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", table_exists_sql);
+		return false;
+	}
+
+	if (!db_bind_text(stmt, NULL, table_name))
+		return false;
+
+	rc = sqlite3_step(stmt);
+	db_remove_reset(stmt);
+	sqlite3_finalize(stmt);
+	return (rc == SQLITE_ROW);
+}
+
+/* Indicate whether the database is initialized with tables for schema version 0.
+ * We only check for the 'subscriber' table here because Neels said so. */
+static bool db_is_bootstrapped_v0(struct db_context *dbc)
+{
+	if (!db_table_exists(dbc, "subscriber")) {
+		LOGP(DDB, LOGL_DEBUG, "Table 'subscriber' not found in database '%s'\n", dbc->fname);
+		return false;
+	}
+
+	return true;
+}
+
+static int db_get_user_version(struct db_context *dbc)
+{
+	const char *user_version_sql = "PRAGMA user_version";
+	sqlite3_stmt *stmt;
+	int version, rc;
+
+	rc = sqlite3_prepare_v2(dbc->db, user_version_sql, -1, &stmt, NULL);
+	if (rc != SQLITE_OK) {
+		LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", user_version_sql);
+		return -1;
+	}
+	rc = sqlite3_step(stmt);
+	if (rc == SQLITE_ROW) {
+		version = sqlite3_column_int(stmt, 0);
+	} else {
+		LOGP(DDB, LOGL_ERROR, "SQL statement '%s' failed: %d\n", user_version_sql, rc);
+		version = -1;
+	}
+
+	db_remove_reset(stmt);
+	sqlite3_finalize(stmt);
+	return version;
+}
+
+struct db_context *db_open(void *ctx, const char *fname, bool enable_sqlite_logging, bool allow_upgrade)
 {
 	struct db_context *dbc = talloc_zero(ctx, struct db_context);
 	unsigned int i;
 	int rc;
 	bool has_sqlite_config_sqllog = false;
+	int version;
 
 	LOGP(DDB, LOGL_NOTICE, "using database: %s\n", fname);
 	LOGP(DDB, LOGL_INFO, "Compiled against SQLite3 lib version %s\n", SQLITE_VERSION);
@@ -275,10 +331,40 @@
 		LOGP(DDB, LOGL_ERROR, "Unable to set Write-Ahead Logging: %s\n",
 			err_msg);
 
-	rc = db_bootstrap(dbc);
-	if (rc != SQLITE_OK) {
-		LOGP(DDB, LOGL_ERROR, "Failed to bootstrap DB: (rc=%d) %s\n",
-			rc, sqlite3_errmsg(dbc->db));
+	version = db_get_user_version(dbc);
+	if (version < 0) {
+		LOGP(DDB, LOGL_ERROR, "Unable to read user version number from database '%s'\n", dbc->fname);
+		goto out_free;
+	}
+
+	/* An empty database will always report version zero. */
+	if (version == 0 && !db_is_bootstrapped_v0(dbc)) {
+		LOGP(DDB, LOGL_NOTICE, "Missing database tables detected; Bootstrapping database '%s'\n", dbc->fname);
+		rc = db_bootstrap(dbc);
+		if (rc != SQLITE_OK) {
+			LOGP(DDB, LOGL_ERROR, "Failed to bootstrap DB: (rc=%d) %s\n",
+			     rc, sqlite3_errmsg(dbc->db));
+			goto out_free;
+		}
+	}
+
+	LOGP(DDB, LOGL_NOTICE, "Database '%s' has HLR DB schema version %d\n", dbc->fname, version);
+
+	if (version < CURRENT_SCHEMA_VERSION && allow_upgrade) {
+		/* Future version upgrades will happen here. */
+	}
+
+	if (version != CURRENT_SCHEMA_VERSION) {
+		if (version < CURRENT_SCHEMA_VERSION) {
+			LOGP(DDB, LOGL_NOTICE, "HLR DB schema version %d is outdated\n", version);
+			if (!allow_upgrade) {
+				LOGP(DDB, LOGL_ERROR, "Not upgrading HLR database to schema version %d; "
+				     "use the --db-upgrade option to allow HLR database upgrades\n",
+				     CURRENT_SCHEMA_VERSION);
+			}
+		} else
+			LOGP(DDB, LOGL_ERROR, "HLR DB schema version %d is unknown\n", version);
+
 		goto out_free;
 	}
 
diff --git a/src/db.h b/src/db.h
index 34582c8..66dfe57 100644
--- a/src/db.h
+++ b/src/db.h
@@ -39,7 +39,7 @@
 bool db_bind_int(sqlite3_stmt *stmt, const char *param_name, int nr);
 bool db_bind_int64(sqlite3_stmt *stmt, const char *param_name, int64_t nr);
 void db_close(struct db_context *dbc);
-struct db_context *db_open(void *ctx, const char *fname, bool enable_sqlite3_logging);
+struct db_context *db_open(void *ctx, const char *fname, bool enable_sqlite3_logging, bool allow_upgrades);
 
 #include <osmocom/crypt/auth.h>
 
diff --git a/src/hlr.c b/src/hlr.c
index 78d6c91..14945b6 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -483,6 +483,7 @@
 	printf("  -s --disable-color         Do not print ANSI colors in the log\n");
 	printf("  -T --timestamp             Prefix every log line with a timestamp.\n");
 	printf("  -e --log-level number      Set a global loglevel.\n");
+	printf("  -U --db-upgrade            Allow HLR database schema upgrades.\n");
 	printf("  -V --version               Print the version of OsmoHLR.\n");
 }
 
@@ -490,10 +491,12 @@
 	const char *config_file;
 	const char *db_file;
 	bool daemonize;
+	bool db_upgrade;
 } cmdline_opts = {
 	.config_file = "osmo-hlr.cfg",
 	.db_file = "hlr.db",
 	.daemonize = false,
+	.db_upgrade = false,
 };
 
 static void handle_options(int argc, char **argv)
@@ -509,11 +512,12 @@
 			{"disable-color", 0, 0, 's'},
 			{"log-level", 1, 0, 'e'},
 			{"timestamp", 0, 0, 'T'},
+			{"db-upgrade", 0, 0, 'U' },
 			{"version", 0, 0, 'V' },
 			{0, 0, 0, 0}
 		};
 
-		c = getopt_long(argc, argv, "hc:l:d:Dse:TV",
+		c = getopt_long(argc, argv, "hc:l:d:Dse:TUV",
 				long_options, &option_index);
 		if (c == -1)
 			break;
@@ -544,6 +548,9 @@
 		case 'T':
 			log_set_print_timestamp(osmo_stderr_target, 1);
 			break;
+		case 'U':
+			cmdline_opts.db_upgrade = true;
+			break;
 		case 'V':
 			print_version(1);
 			exit(0);
@@ -637,7 +644,7 @@
 		exit(1);
 	}
 
-	g_hlr->dbc = db_open(hlr_ctx, cmdline_opts.db_file, true);
+	g_hlr->dbc = db_open(hlr_ctx, cmdline_opts.db_file, true, cmdline_opts.db_upgrade);
 	if (!g_hlr->dbc) {
 		LOGP(DMAIN, LOGL_FATAL, "Error opening database\n");
 		exit(1);
diff --git a/src/hlr_db_tool.c b/src/hlr_db_tool.c
index e83b098..1a9c60c 100644
--- a/src/hlr_db_tool.c
+++ b/src/hlr_db_tool.c
@@ -44,8 +44,10 @@
 	const char *db_file;
 	bool bootstrap;
 	const char *import_nitb_db;
+	bool db_upgrade;
 } cmdline_opts = {
 	.db_file = "hlr.db",
+	.db_upgrade = false,
 };
 
 static void print_help()
@@ -59,6 +61,7 @@
 	printf("  -s --disable-color         Do not print ANSI colors in the log\n");
 	printf("  -T --timestamp             Prefix every log line with a timestamp.\n");
 	printf("  -e --log-level number      Set a global loglevel.\n");
+	printf("  -U --db-upgrade            Allow HLR database schema upgrades.\n");
 	printf("  -V --version               Print the version of OsmoHLR-db-tool.\n");
 	printf("\n");
 	printf("Commands:\n");
@@ -96,11 +99,12 @@
 			{"disable-color", 0, 0, 's'},
 			{"timestamp", 0, 0, 'T'},
 			{"log-level", 1, 0, 'e'},
+			{"db-upgrade", 0, 0, 'U' },
 			{"version", 0, 0, 'V' },
 			{0, 0, 0, 0}
 		};
 
-		c = getopt_long(argc, argv, "hl:d:sTe:V",
+		c = getopt_long(argc, argv, "hl:d:sTe:UV",
 				long_options, &option_index);
 		if (c == -1)
 			break;
@@ -124,6 +128,9 @@
 		case 'e':
 			log_set_log_level(osmo_stderr_target, atoi(optarg));
 			break;
+		case 'U':
+			cmdline_opts.db_upgrade = true;
+			break;
 		case 'V':
 			print_version(1);
 			exit(EXIT_SUCCESS);
@@ -409,7 +416,7 @@
 		exit(EXIT_FAILURE);
 	}
 
-	g_hlr_db_tool_ctx->dbc = db_open(g_hlr_db_tool_ctx, cmdline_opts.db_file, true);
+	g_hlr_db_tool_ctx->dbc = db_open(g_hlr_db_tool_ctx, cmdline_opts.db_file, true, cmdline_opts.db_upgrade);
 	if (!g_hlr_db_tool_ctx->dbc) {
 		LOGP(DMAIN, LOGL_FATAL, "Error opening database\n");
 		exit(EXIT_FAILURE);
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 058588b..c4ed6ed 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -850,7 +850,7 @@
 	log_set_log_level(osmo_stderr_target, LOGL_ERROR);
 	/* Disable SQLite logging so that we're not vulnerable on SQLite error messages changing across
 	 * library versions. */
-	dbc = db_open(ctx, "db_test.db", false);
+	dbc = db_open(ctx, "db_test.db", false, false);
 	log_set_log_level(osmo_stderr_target, 0);
 	OSMO_ASSERT(dbc);
 

-- 
To view, visit https://gerrit.osmocom.org/11898
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8aeaa9a404b622657cbc7138106f38aa6ad8d01b
Gerrit-Change-Number: 11898
Gerrit-PatchSet: 10
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181204/94c2579c/attachment.htm>


More information about the gerrit-log mailing list