* [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names
@ 2019-06-26 19:04 Scott Mayhew
2019-06-26 19:04 ` [nfs-utils PATCH 2/2] sqlite.c: Add a simple database health check Scott Mayhew
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Scott Mayhew @ 2019-06-26 19:04 UTC (permalink / raw)
To: steved; +Cc: linux-nfs
Commit a8133e1fd1742 removed the zero-padding from the table names and
broke grace period handling. Running nfsdcld with verbose logging shows
messages similar to the following:
nfsdcld: cld_gracestart: sending client records to the kernel
nfsdcld: sqlite_iterate_recovery: select statement prepare failed: no such table: rec-1b
nfsdcld: Doing downcall with status -121
nfsdcld: cld_inotify_cb: called for EV_READ
nfsdcld: cld_pipe_open: opening upcall pipe /var/lib/nfs/rpc_pipefs/nfsd/cld
nfsdcld: cld_gracedone: grace done.
nfsdcld: Unable to drop table for recovery epoch: no such table: rec-1b
nfsdcld: Doing downcall with status -121
Fixes: a8133e1fd1742 ("sqlite.c: Use PRIx64 macro to print 64-bit integers")
Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
utils/nfsdcld/sqlite.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index cd658ef..5d78d24 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -536,7 +536,7 @@ sqlite_copy_cltrack_records(int *num_rec)
xlog(L_ERROR, "Unable to begin transaction: %s", err);
goto rollback;
}
- ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
+ ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
current_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -551,7 +551,7 @@ sqlite_copy_cltrack_records(int *num_rec)
xlog(L_ERROR, "Unable to clear records from current epoch: %s", err);
goto rollback;
}
- ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%" PRIx64 "\" "
+ ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%016" PRIx64 "\" "
"SELECT id FROM attached.clients;",
current_epoch);
if (ret < 0) {
@@ -704,7 +704,7 @@ sqlite_insert_client(const unsigned char *clname, const size_t namelen)
int ret;
sqlite3_stmt *stmt = NULL;
- ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%" PRIx64 "\" "
+ ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%016" PRIx64 "\" "
"VALUES (?);", current_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -749,7 +749,7 @@ sqlite_remove_client(const unsigned char *clname, const size_t namelen)
int ret;
sqlite3_stmt *stmt = NULL;
- ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\" "
+ ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\" "
"WHERE id==?;", current_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -799,7 +799,7 @@ sqlite_check_client(const unsigned char *clname, const size_t namelen)
int ret;
sqlite3_stmt *stmt = NULL;
- ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%" PRIx64 "\" "
+ ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%016" PRIx64 "\" "
"WHERE id==?;", recovery_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -892,7 +892,7 @@ sqlite_grace_start(void)
goto rollback;
}
- ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%" PRIx64 "\" "
+ ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%016" PRIx64 "\" "
"(id BLOB PRIMARY KEY);",
tcur);
if (ret < 0) {
@@ -916,7 +916,7 @@ sqlite_grace_start(void)
* values in the grace table, just clear out the records for
* the current reboot epoch.
*/
- ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
+ ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
tcur);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -977,7 +977,7 @@ sqlite_grace_done(void)
goto rollback;
}
- ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%" PRIx64 "\";",
+ ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%016" PRIx64 "\";",
recovery_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
@@ -1028,7 +1028,7 @@ sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_client *c
return -EINVAL;
}
- ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%" PRIx64 "\";",
+ ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%016" PRIx64 "\";",
recovery_epoch);
if (ret < 0) {
xlog(L_ERROR, "sprintf failed!");
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [nfs-utils PATCH 2/2] sqlite.c: Add a simple database health check
2019-06-26 19:04 [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
@ 2019-06-26 19:04 ` Scott Mayhew
2019-07-22 11:39 ` [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Scott Mayhew @ 2019-06-26 19:04 UTC (permalink / raw)
To: steved; +Cc: linux-nfs
Commit a8133e1fd1742 removed the zero-padding from the table names and
broke grace period handling. Add a simple health check for the database
to try to detect and fix these. Other checks could be added in the
future if the database schema changes.
Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
utils/nfsdcld/sqlite.c | 131 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 5d78d24..fa81df8 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -67,6 +67,7 @@
#include "cld-internal.h"
#include "conffile.h"
#include "legacy.h"
+#include "nfslib.h"
#define CLD_SQLITE_LATEST_SCHEMA_VERSION 3
#define CLTRACK_DEFAULT_STORAGEDIR NFS_STATEDIR "/nfsdcltrack"
@@ -448,6 +449,129 @@ out:
return ret;
}
+/*
+ * Helper for renaming a recovery table to fix the padding.
+ */
+static int
+sqlite_fix_table_name(const char *name)
+{
+ int ret;
+ uint64_t val;
+ char *err;
+
+ if (sscanf(name, "rec-%" PRIx64, &val) != 1)
+ return -EINVAL;
+ ret = snprintf(buf, sizeof(buf), "ALTER TABLE \"%s\" "
+ "RENAME TO \"rec-%016" PRIx64 "\";",
+ name, val);
+ if (ret < 0) {
+ xlog(L_ERROR, "sprintf failed!");
+ return -EINVAL;
+ } else if ((size_t)ret >= sizeof(buf)) {
+ xlog(L_ERROR, "sprintf output too long! (%d chars)", ret);
+ return -EINVAL;
+ }
+ ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
+ if (ret != SQLITE_OK) {
+ xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
+ val, err);
+ goto out;
+ }
+ xlog(D_GENERAL, "Renamed table %s to rec-%016" PRIx64, name, val);
+out:
+ sqlite3_free(err);
+ return ret;
+}
+
+/*
+ * Callback for the sqlite_exec statement in sqlite_check_table_names.
+ * If the epoch encoded in the table name matches either the current
+ * epoch or the recovery epoch, then try to fix the padding. Otherwise,
+ * we bail.
+ */
+static int
+sqlite_check_table_names_cb(void *UNUSED(arg), int ncols, char **cols,
+ char **UNUSED(colnames))
+{
+ int ret = SQLITE_OK;
+ uint64_t val;
+
+ if (ncols > 1)
+ return -EINVAL;
+ if (sscanf(cols[0], "rec-%" PRIx64, &val) != 1)
+ return -EINVAL;
+ if (val == current_epoch || val == recovery_epoch) {
+ xlog(D_GENERAL, "found invalid table name %s for %s epoch",
+ cols[0], val == current_epoch ? "current" : "recovery");
+ ret = sqlite_fix_table_name(cols[0]);
+ } else {
+ xlog(L_ERROR, "found invalid table name %s for unknown epoch %"
+ PRId64, cols[0], val);
+ return -EINVAL;
+ }
+ return ret;
+}
+
+/*
+ * Look for recovery table names where the epoch isn't zero-padded
+ */
+static int
+sqlite_check_table_names(void)
+{
+ int ret;
+ char *err;
+
+ ret = sqlite3_exec(dbh, "SELECT name FROM sqlite_master "
+ "WHERE type=\"table\" AND name LIKE \"%rec-%\" "
+ "AND length(name) < 20;",
+ sqlite_check_table_names_cb, NULL, &err);
+ if (ret != SQLITE_OK) {
+ xlog(L_ERROR, "Table names check failed: %s", err);
+ }
+ sqlite3_free(err);
+ return ret;
+}
+
+/*
+ * Simple db health check. For now we're just making sure that the recovery
+ * table names are of the format "rec-CCCCCCCCCCCCCCCC" (where C is the hex
+ * representation of the epoch value) and that epoch value matches either
+ * the current epoch or the recovery epoch.
+ */
+static int
+sqlite_check_db_health(void)
+{
+ int ret, ret2;
+ char *err;
+
+ ret = sqlite3_exec(dbh, "BEGIN EXCLUSIVE TRANSACTION;", NULL, NULL,
+ &err);
+ if (ret != SQLITE_OK) {
+ xlog(L_ERROR, "Unable to begin transaction: %s", err);
+ goto rollback;
+ }
+
+ ret = sqlite_check_table_names();
+ if (ret != SQLITE_OK)
+ goto rollback;
+
+ ret = sqlite3_exec(dbh, "COMMIT TRANSACTION;", NULL, NULL, &err);
+ if (ret != SQLITE_OK) {
+ xlog(L_ERROR, "Unable to commit transaction: %s", err);
+ goto rollback;
+ }
+
+cleanup:
+ sqlite3_free(err);
+ xlog(D_GENERAL, "%s: returning %d", __func__, ret);
+ return ret;
+rollback:
+ ret2 = sqlite3_exec(dbh, "ROLLBACK TRANSACTION;", NULL, NULL, &err);
+ if (ret2 != SQLITE_OK)
+ xlog(L_ERROR, "Unable to rollback transaction: %s", err);
+ goto cleanup;
+}
+
static int
sqlite_attach_db(const char *path)
{
@@ -673,6 +797,13 @@ sqlite_prepare_dbh(const char *topdir)
if (ret)
goto out_close;
+ ret = sqlite_check_db_health();
+ if (ret) {
+ xlog(L_ERROR, "Database health check failed! "
+ "Database must be fixed manually.");
+ goto out_close;
+ }
+
/* one-time "upgrade" from older client tracking methods */
if (first_time) {
sqlite_copy_cltrack_records(&num_cltrack_records);
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names
2019-06-26 19:04 [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
2019-06-26 19:04 ` [nfs-utils PATCH 2/2] sqlite.c: Add a simple database health check Scott Mayhew
@ 2019-07-22 11:39 ` Scott Mayhew
2019-07-22 12:42 ` Steve Dickson
2019-07-22 12:42 ` Steve Dickson
3 siblings, 0 replies; 5+ messages in thread
From: Scott Mayhew @ 2019-07-22 11:39 UTC (permalink / raw)
To: steved; +Cc: linux-nfs
Ping.
-Scott
On Wed, 26 Jun 2019, Scott Mayhew wrote:
> Commit a8133e1fd1742 removed the zero-padding from the table names and
> broke grace period handling. Running nfsdcld with verbose logging shows
> messages similar to the following:
>
> nfsdcld: cld_gracestart: sending client records to the kernel
> nfsdcld: sqlite_iterate_recovery: select statement prepare failed: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
> nfsdcld: cld_inotify_cb: called for EV_READ
> nfsdcld: cld_pipe_open: opening upcall pipe /var/lib/nfs/rpc_pipefs/nfsd/cld
> nfsdcld: cld_gracedone: grace done.
> nfsdcld: Unable to drop table for recovery epoch: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
>
> Fixes: a8133e1fd1742 ("sqlite.c: Use PRIx64 macro to print 64-bit integers")
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
> utils/nfsdcld/sqlite.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
> index cd658ef..5d78d24 100644
> --- a/utils/nfsdcld/sqlite.c
> +++ b/utils/nfsdcld/sqlite.c
> @@ -536,7 +536,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to begin transaction: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -551,7 +551,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to clear records from current epoch: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%016" PRIx64 "\" "
> "SELECT id FROM attached.clients;",
> current_epoch);
> if (ret < 0) {
> @@ -704,7 +704,7 @@ sqlite_insert_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%016" PRIx64 "\" "
> "VALUES (?);", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -749,7 +749,7 @@ sqlite_remove_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -799,7 +799,7 @@ sqlite_check_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -892,7 +892,7 @@ sqlite_grace_start(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%016" PRIx64 "\" "
> "(id BLOB PRIMARY KEY);",
> tcur);
> if (ret < 0) {
> @@ -916,7 +916,7 @@ sqlite_grace_start(void)
> * values in the grace table, just clear out the records for
> * the current reboot epoch.
> */
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> tcur);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -977,7 +977,7 @@ sqlite_grace_done(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -1028,7 +1028,7 @@ sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_client *c
> return -EINVAL;
> }
>
> - ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> --
> 2.17.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names
2019-06-26 19:04 [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
2019-06-26 19:04 ` [nfs-utils PATCH 2/2] sqlite.c: Add a simple database health check Scott Mayhew
2019-07-22 11:39 ` [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
@ 2019-07-22 12:42 ` Steve Dickson
2019-07-22 12:42 ` Steve Dickson
3 siblings, 0 replies; 5+ messages in thread
From: Steve Dickson @ 2019-07-22 12:42 UTC (permalink / raw)
To: Scott Mayhew; +Cc: linux-nfs
On 6/26/19 3:04 PM, Scott Mayhew wrote:
> Commit a8133e1fd1742 removed the zero-padding from the table names and
> broke grace period handling. Running nfsdcld with verbose logging shows
> messages similar to the following:
>
> nfsdcld: cld_gracestart: sending client records to the kernel
> nfsdcld: sqlite_iterate_recovery: select statement prepare failed: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
> nfsdcld: cld_inotify_cb: called for EV_READ
> nfsdcld: cld_pipe_open: opening upcall pipe /var/lib/nfs/rpc_pipefs/nfsd/cld
> nfsdcld: cld_gracedone: grace done.
> nfsdcld: Unable to drop table for recovery epoch: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
>
> Fixes: a8133e1fd1742 ("sqlite.c: Use PRIx64 macro to print 64-bit integers")
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Committed...
steved.
> ---
> utils/nfsdcld/sqlite.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
> index cd658ef..5d78d24 100644
> --- a/utils/nfsdcld/sqlite.c
> +++ b/utils/nfsdcld/sqlite.c
> @@ -536,7 +536,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to begin transaction: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -551,7 +551,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to clear records from current epoch: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%016" PRIx64 "\" "
> "SELECT id FROM attached.clients;",
> current_epoch);
> if (ret < 0) {
> @@ -704,7 +704,7 @@ sqlite_insert_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%016" PRIx64 "\" "
> "VALUES (?);", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -749,7 +749,7 @@ sqlite_remove_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -799,7 +799,7 @@ sqlite_check_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -892,7 +892,7 @@ sqlite_grace_start(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%016" PRIx64 "\" "
> "(id BLOB PRIMARY KEY);",
> tcur);
> if (ret < 0) {
> @@ -916,7 +916,7 @@ sqlite_grace_start(void)
> * values in the grace table, just clear out the records for
> * the current reboot epoch.
> */
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> tcur);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -977,7 +977,7 @@ sqlite_grace_done(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -1028,7 +1028,7 @@ sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_client *c
> return -EINVAL;
> }
>
> - ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names
2019-06-26 19:04 [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
` (2 preceding siblings ...)
2019-07-22 12:42 ` Steve Dickson
@ 2019-07-22 12:42 ` Steve Dickson
3 siblings, 0 replies; 5+ messages in thread
From: Steve Dickson @ 2019-07-22 12:42 UTC (permalink / raw)
To: Scott Mayhew; +Cc: linux-nfs
On 6/26/19 3:04 PM, Scott Mayhew wrote:
> Commit a8133e1fd1742 removed the zero-padding from the table names and
> broke grace period handling. Running nfsdcld with verbose logging shows
> messages similar to the following:
>
> nfsdcld: cld_gracestart: sending client records to the kernel
> nfsdcld: sqlite_iterate_recovery: select statement prepare failed: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
> nfsdcld: cld_inotify_cb: called for EV_READ
> nfsdcld: cld_pipe_open: opening upcall pipe /var/lib/nfs/rpc_pipefs/nfsd/cld
> nfsdcld: cld_gracedone: grace done.
> nfsdcld: Unable to drop table for recovery epoch: no such table: rec-1b
> nfsdcld: Doing downcall with status -121
>
> Fixes: a8133e1fd1742 ("sqlite.c: Use PRIx64 macro to print 64-bit integers")
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Committed....
steved.
> ---
> utils/nfsdcld/sqlite.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
> index cd658ef..5d78d24 100644
> --- a/utils/nfsdcld/sqlite.c
> +++ b/utils/nfsdcld/sqlite.c
> @@ -536,7 +536,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to begin transaction: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -551,7 +551,7 @@ sqlite_copy_cltrack_records(int *num_rec)
> xlog(L_ERROR, "Unable to clear records from current epoch: %s", err);
> goto rollback;
> }
> - ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT INTO \"rec-%016" PRIx64 "\" "
> "SELECT id FROM attached.clients;",
> current_epoch);
> if (ret < 0) {
> @@ -704,7 +704,7 @@ sqlite_insert_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "INSERT OR REPLACE INTO \"rec-%016" PRIx64 "\" "
> "VALUES (?);", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -749,7 +749,7 @@ sqlite_remove_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", current_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -799,7 +799,7 @@ sqlite_check_client(const unsigned char *clname, const size_t namelen)
> int ret;
> sqlite3_stmt *stmt = NULL;
>
> - ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "SELECT count(*) FROM \"rec-%016" PRIx64 "\" "
> "WHERE id==?;", recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -892,7 +892,7 @@ sqlite_grace_start(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%" PRIx64 "\" "
> + ret = snprintf(buf, sizeof(buf), "CREATE TABLE \"rec-%016" PRIx64 "\" "
> "(id BLOB PRIMARY KEY);",
> tcur);
> if (ret < 0) {
> @@ -916,7 +916,7 @@ sqlite_grace_start(void)
> * values in the grace table, just clear out the records for
> * the current reboot epoch.
> */
> - ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DELETE FROM \"rec-%016" PRIx64 "\";",
> tcur);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -977,7 +977,7 @@ sqlite_grace_done(void)
> goto rollback;
> }
>
> - ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "DROP TABLE \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
> @@ -1028,7 +1028,7 @@ sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_client *c
> return -EINVAL;
> }
>
> - ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%" PRIx64 "\";",
> + ret = snprintf(buf, sizeof(buf), "SELECT * FROM \"rec-%016" PRIx64 "\";",
> recovery_epoch);
> if (ret < 0) {
> xlog(L_ERROR, "sprintf failed!");
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-22 12:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-26 19:04 [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
2019-06-26 19:04 ` [nfs-utils PATCH 2/2] sqlite.c: Add a simple database health check Scott Mayhew
2019-07-22 11:39 ` [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names Scott Mayhew
2019-07-22 12:42 ` Steve Dickson
2019-07-22 12:42 ` Steve Dickson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).