2019-06-26 19:06:23

by Scott Mayhew

Subject: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names

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 <[email protected]>
---
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

2019-07-22 12:46:42

by Scott Mayhew

Subject: Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names

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 <[email protected]>
> ---
> 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
>

2019-07-22 13:11:04

by Steve Dickson

Subject: Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names

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 <[email protected]>
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!");
>

2019-07-22 13:12:05

by Steve Dickson

Subject: Re: [nfs-utils PATCH 1/2] sqlite.c: restore zero-padding to the recovery table names

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 <[email protected]>
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!");
>