From 264b644c534418f3a27b7e6013afa4aee7bdf89c Mon Sep 17 00:00:00 2001 From: Ben Radford Date: Thu, 15 Jun 2023 13:22:17 +0100 Subject: [PATCH 1/8] More detail on why read-only mode disables locking. --- src/libutil/experimental-features.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index b758a0262..1fbc99b34 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -223,11 +223,14 @@ constexpr std::array xpFeatureDetails = {{ Set this parameter to `true` to allow stores with databases on read-only filesystems to be opened for querying; ordinarily Nix will refuse to do this. - Enabling this setting disables the locking required for safe concurrent - access, so you should be certain that the database will not be changed. - While the filesystem the database resides on might be read-only to this - process, consider whether another user, process, or system, might have - write access to it. + This is because SQLite requires write access to the database file to perform + the file locking operations necessary for safe concurrent access. When `read-only` + is set to `true`, the database will be opened in immutable mode. + + Under this mode, SQLite will not do any locking at all, so you should be certain + that the database will not be changed. While the filesystem the database resides + on might be read-only to this process, consider whether another user, process, + or system, might have write access to it. )", }, }}; From 7cdaa0b8a626004b19b87b1eec74d1adb5f20263 Mon Sep 17 00:00:00 2001 From: Ben Radford <104896700+benradf@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:25:15 +0100 Subject: [PATCH 2/8] Update tests/read-only-store.sh Co-authored-by: Valentin Gagarin --- tests/read-only-store.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/read-only-store.sh b/tests/read-only-store.sh index 2f89b849f..d63920c19 100644 --- a/tests/read-only-store.sh +++ b/tests/read-only-store.sh @@ -2,6 +2,8 @@ source common.sh enableFeatures "read-only-local-store" +needLocalStore "cannot open store read-only when daemon has already opened it writeable" + clearStore happy () { From 78e2f931d048f9c41e5432cd8e49bf6bc0c3baae Mon Sep 17 00:00:00 2001 From: Ben Radford <104896700+benradf@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:32:16 +0100 Subject: [PATCH 3/8] Update src/libstore/local-store.cc Co-authored-by: Valentin Gagarin --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 17385cdfb..364be5dd5 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -363,7 +363,7 @@ LocalStore::LocalStore(const Params & params) if (!readOnly) { migrateCASchema(state->db, dbDir + "/ca-schema", globalLock); } else { - throw Error("need to migrate to CA schema, but this cannot be done in read-only mode"); + throw Error("need to migrate to content-addressed schema, but this cannot be done in read-only mode"); } } From 984b01924a58dc80b0e5da7722ccfa108cbca036 Mon Sep 17 00:00:00 2001 From: Ben Radford <104896700+benradf@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:32:35 +0100 Subject: [PATCH 4/8] Update src/libstore/local-store.cc Co-authored-by: Valentin Gagarin --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 364be5dd5..e69460e6c 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -496,7 +496,7 @@ int LocalStore::getSchema() void LocalStore::openDB(State & state, bool create) { if (create && readOnly) { - throw Error("unable to create database while in read-only mode"); + throw Error("cannot create database while in read-only mode"); } if (access(dbDir.c_str(), R_OK | (readOnly ? 0 : W_OK))) From a7b1b92d817348669fc2cd34c085dae57ceed5b8 Mon Sep 17 00:00:00 2001 From: Ben Radford <104896700+benradf@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:32:56 +0100 Subject: [PATCH 5/8] Update src/libstore/local-store.hh Co-authored-by: Valentin Gagarin --- src/libstore/local-store.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 31a6f4fd9..35c4febef 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -50,7 +50,7 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig false, "read-only", R"( - Allow this store to be opened when its database is on a read-only filesystem. + Allow this store to be opened when its [database](@docroot@/glossary.md#gloss-nix-database) is on a read-only filesystem. Normally Nix will attempt to open the store database in read-write mode, even for querying (when write access is not needed). This causes it to fail if the From 4642b60afe2a79b00ae51ca1ec1fc35eb5a224c5 Mon Sep 17 00:00:00 2001 From: Ben Radford <104896700+benradf@users.noreply.github.com> Date: Thu, 15 Jun 2023 13:33:26 +0100 Subject: [PATCH 6/8] Update src/libstore/local-store.hh Co-authored-by: Valentin Gagarin --- src/libstore/local-store.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 35c4febef..a1ab3bb4d 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -57,7 +57,7 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig database is on a read-only filesystem. Enable read-only mode to disable locking and open the SQLite database with the - **immutable** parameter set. Do not use this unless the filesystem is read-only. + [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. Do not use this unless the filesystem is read-only. Using it when the filesystem is writable can cause incorrect query results or corruption errors if the database is changed by another process. )"}; From f2fe9822c14bc86e5da8f8cb97cb16a961ecc46f Mon Sep 17 00:00:00 2001 From: Ben Radford Date: Thu, 15 Jun 2023 13:31:37 +0100 Subject: [PATCH 7/8] Comment explaining what schema version 0 means. --- src/libstore/local-store.hh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index a1ab3bb4d..dd563348b 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -285,6 +285,10 @@ public: private: + /** + * Retrieve the current version of the database schema. + * If the database does not exist yet, the version returned will be 0. + */ int getSchema(); void openDB(State & state, bool create); From f5d83a80293426b2f869af206b93b5cdfeb34ec9 Mon Sep 17 00:00:00 2001 From: Ben Radford Date: Thu, 15 Jun 2023 13:36:28 +0100 Subject: [PATCH 8/8] One line per sentence in markdown docs. --- src/libstore/local-store.hh | 14 +++++++------- src/libutil/experimental-features.cc | 14 +++++--------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index dd563348b..0e7b1334f 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -52,14 +52,14 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig R"( Allow this store to be opened when its [database](@docroot@/glossary.md#gloss-nix-database) is on a read-only filesystem. - Normally Nix will attempt to open the store database in read-write mode, even - for querying (when write access is not needed). This causes it to fail if the - database is on a read-only filesystem. + Normally Nix will attempt to open the store database in read-write mode, even for querying (when write access is not needed). + This causes it to fail if the database is on a read-only filesystem. - Enable read-only mode to disable locking and open the SQLite database with the - [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. Do not use this unless the filesystem is read-only. - Using it when the filesystem is writable can cause incorrect query results or - corruption errors if the database is changed by another process. + Enable read-only mode to disable locking and open the SQLite database with the [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. + + **Warning** + Do not use this unless the filesystem is read-only. + Using it when the filesystem is writable can cause incorrect query results or corruption errors if the database is changed by another process. )"}; const std::string name() override { return "Local Store"; } diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 1fbc99b34..9705c5b38 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -220,17 +220,13 @@ constexpr std::array xpFeatureDetails = {{ .description = R"( Allow the use of the `read-only` parameter in local store URIs. - Set this parameter to `true` to allow stores with databases on read-only - filesystems to be opened for querying; ordinarily Nix will refuse to do this. + Set this parameter to `true` to allow stores with databases on read-only filesystems to be opened for querying; ordinarily Nix will refuse to do this. - This is because SQLite requires write access to the database file to perform - the file locking operations necessary for safe concurrent access. When `read-only` - is set to `true`, the database will be opened in immutable mode. + This is because SQLite requires write access to the database file to perform the file locking operations necessary for safe concurrent access. + When `read-only` is set to `true`, the database will be opened in immutable mode. - Under this mode, SQLite will not do any locking at all, so you should be certain - that the database will not be changed. While the filesystem the database resides - on might be read-only to this process, consider whether another user, process, - or system, might have write access to it. + Under this mode, SQLite will not do any locking at all, so you should be certain that the database will not be changed. + While the filesystem the database resides on might be read-only to this process, consider whether another user, process, or system, might have write access to it. )", }, }};