From bb8fd5a2c4b91d8fd41e831887f053bb63b13ac5 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 17 Apr 2020 14:18:28 -0400 Subject: [PATCH 1/3] lib/commit: Add more error prefixing We think we're hitting an error in that function in the Fedora infra. Add some more error prefixing to help debugging. --- src/libostree/ostree-repo-commit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 3f2f2beb..6712b53e 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1943,6 +1943,9 @@ cleanup_txn_dir (OstreeRepo *self, GCancellable *cancellable, GError **error) { + const char *errprefix = glnx_strjoina ("Cleaning up txn dir ", path); + GLNX_AUTO_PREFIX_ERROR (errprefix, error); + g_auto(GLnxLockFile) lockfile = { 0, }; gboolean did_lock; From df065ad766c325bac2fa9a83ea1af7e1616abe16 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 17 Apr 2020 14:20:25 -0400 Subject: [PATCH 2/3] lib: Rename function for staging dir check Rename the function to more accurately reflect what it does, which is to check whether the filename has the `staging-` prefix. --- src/libostree/ostree-repo-commit.c | 2 +- src/libostree/ostree-repo-private.h | 2 +- src/libostree/ostree-repo.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 6712b53e..8c4bd3fe 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2011,7 +2011,7 @@ cleanup_tmpdir (OstreeRepo *self, continue; /* Handle transaction tmpdirs */ - if (_ostree_repo_is_locked_tmpdir (dent->d_name)) + if (_ostree_repo_has_staging_prefix (dent->d_name)) { if (!cleanup_txn_dir (self, dfd_iter.fd, dent->d_name, cancellable, error)) return FALSE; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index e52f9f0b..a1c7b7b4 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -247,7 +247,7 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, GError **error); gboolean -_ostree_repo_is_locked_tmpdir (const char *filename); +_ostree_repo_has_staging_prefix (const char *filename); gboolean _ostree_repo_try_lock_tmpdir (int tmpdir_dfd, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 97ce95b8..5fd92bb9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5959,7 +5959,7 @@ _ostree_repo_maybe_regenerate_summary (OstreeRepo *self, } gboolean -_ostree_repo_is_locked_tmpdir (const char *filename) +_ostree_repo_has_staging_prefix (const char *filename) { return g_str_has_prefix (filename, OSTREE_REPO_TMPDIR_STAGING); } @@ -6019,7 +6019,7 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (_ostree_repo_is_locked_tmpdir (tmpdir_prefix), FALSE); + g_return_val_if_fail (_ostree_repo_has_staging_prefix (tmpdir_prefix), FALSE); /* Look for existing tmpdir (with same prefix) to reuse */ g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; From 8ece36c28a3449de9dce9d43252aeb0ffc166687 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 17 Apr 2020 14:29:13 -0400 Subject: [PATCH 3/3] lib/commit: Check that dirent is a directory before cleaning I've only noticed this by inspection. But I think it's possible for `cleanup_txn_dir` to get called with the `staging-...-lock` file since it matches the prefix. Make the checking here stronger by verifying that it's a directory. If it's not a directory (lockfile), then follow the default pruning expiry logic so that we still cleanup stray lockfiles eventually. --- src/libostree/ostree-repo-commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 8c4bd3fe..7dd68e96 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2011,7 +2011,7 @@ cleanup_tmpdir (OstreeRepo *self, continue; /* Handle transaction tmpdirs */ - if (_ostree_repo_has_staging_prefix (dent->d_name)) + if (_ostree_repo_has_staging_prefix (dent->d_name) && S_ISDIR (stbuf.st_mode)) { if (!cleanup_txn_dir (self, dfd_iter.fd, dent->d_name, cancellable, error)) return FALSE;