From cf6799e4b5323641502e6da9f546c9a6cee3cb21 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Wed, 31 Jan 2018 22:57:42 +0100 Subject: [PATCH] client: quit on failed remount during dentry invalidate test Fixes: http://tracker.ceph.com/issues/22269 Signed-off-by: Patrick Donnelly (cherry picked from commit 5b2b1d14468c290c56ee6c95ea557c99464e0098) Conflicts: PendingReleaseNotes: trivial conflict src/ceph_fuse.cc src/client/Client.cc: i chosed to pick code from 5b2b1d1 because, to fixthe issue, we need to call _do_remount which was introduced in 5b2b1d1. --- PendingReleaseNotes | 15 ++++++-- src/ceph_fuse.cc | 10 ++++-- src/client/Client.cc | 61 ++++++++++++++++++++------------- src/client/Client.h | 3 +- src/common/legacy_config_opts.h | 1 - src/common/options.cc | 9 +++-- 6 files changed, 66 insertions(+), 33 deletions(-) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 9094963840471..ac6b2c0c67a27 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -42,6 +42,15 @@ 12.2.3 ------ -* The RBD C API's rbd_discard method now enforces a maximum length of - 2GB to match the C++ API's Image::discard method. This restriction - prevents overflow of the result code. +- *RBD*: + + * The RBD C API's rbd_discard method now enforces a maximum length of + 2GB to match the C++ API's Image::discard method. This restriction + prevents overflow of the result code. + +- *CephFS*: + + * The CephFS client now catches failures to clear dentries during startup + and refuses to start as consistency and untrimmable cache issues may + develop. The new option client_die_on_failed_dentry_invalidate (default: + true) may be turned off to allow the client to proceed (dangerous!). diff --git a/src/ceph_fuse.cc b/src/ceph_fuse.cc index 4416e9d3c588d..851425af09701 100644 --- a/src/ceph_fuse.cc +++ b/src/ceph_fuse.cc @@ -158,10 +158,14 @@ int main(int argc, const char **argv, const char *envp[]) { #if defined(__linux__) int ver = get_linux_version(); assert(ver != 0); - bool can_invalidate_dentries = g_conf->client_try_dentry_invalidate && - ver < KERNEL_VERSION(3, 18, 0); + bool client_try_dentry_invalidate = g_conf->get_val( + "client_try_dentry_invalidate"); + bool can_invalidate_dentries = + client_try_dentry_invalidate && ver < KERNEL_VERSION(3, 18, 0); int tr = client->test_dentry_handling(can_invalidate_dentries); - if (tr != 0) { + bool client_die_on_failed_dentry_invalidate = g_conf->get_val( + "client_die_on_failed_dentry_invalidate"); + if (tr != 0 && client_die_on_failed_dentry_invalidate) { cerr << "ceph-fuse[" << getpid() << "]: fuse failed dentry invalidate/remount test with error " << cpp_strerror(tr) << ", stopping" << std::endl; diff --git a/src/client/Client.cc b/src/client/Client.cc index 1b08b3c3a2feb..97bba2607c9bc 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -239,7 +239,6 @@ Client::Client(Messenger *m, MonClient *mc, Objecter *objecter_) getgroups_cb(NULL), umask_cb(NULL), can_invalidate_dentries(false), - require_remount(false), async_ino_invalidator(m->cct), async_dentry_invalidator(m->cct), interrupt_finisher(m->cct), @@ -4024,22 +4023,40 @@ void Client::remove_session_caps(MetaSession *s) sync_cond.Signal(); } +int Client::_do_remount(void) +{ + errno = 0; + int r = remount_cb(callback_handle); + if (r != 0) { + int e = errno; + client_t whoami = get_nodeid(); + if (r == -1) { + lderr(cct) << + "failed to remount (to trim kernel dentries): " + "errno = " << e << " (" << strerror(e) << ")" << dendl; + } else { + lderr(cct) << + "failed to remount (to trim kernel dentries): " + "return code = " << r << dendl; + } + bool should_abort = cct->_conf->get_val("client_die_on_failed_remount") || + cct->_conf->get_val("client_die_on_failed_dentry_invalidate"); + if (should_abort && !unmounting) { + lderr(cct) << "failed to remount for kernel dentry trimming; quitting!" << dendl; + ceph_abort(); + } + } + return r; +} + class C_Client_Remount : public Context { private: Client *client; public: explicit C_Client_Remount(Client *c) : client(c) {} void finish(int r) override { - assert (r == 0); - r = client->remount_cb(client->callback_handle); - if (r != 0) { - client_t whoami = client->get_nodeid(); - lderr(client->cct) << "tried to remount (to trim kernel dentries) and got error " - << r << dendl; - if (client->require_remount && !client->unmounting) { - assert(0 == "failed to remount for kernel dentry trimming"); - } - } + assert(r == 0); + client->_do_remount(); } }; @@ -10066,21 +10083,19 @@ int Client::test_dentry_handling(bool can_invalidate) if (can_invalidate_dentries) { assert(dentry_invalidate_cb); ldout(cct, 1) << "using dentry_invalidate_cb" << dendl; + r = 0; } else if (remount_cb) { ldout(cct, 1) << "using remount_cb" << dendl; - int s = remount_cb(callback_handle); - if (s) { - lderr(cct) << "Failed to invoke remount, needed to ensure kernel dcache consistency" - << dendl; - } - if (cct->_conf->client_die_on_failed_remount) { - require_remount = true; - r = s; - } - } else { - lderr(cct) << "no method to invalidate kernel dentry cache; expect issues!" << dendl; - if (cct->_conf->client_die_on_failed_remount) + r = _do_remount(); + } + if (r) { + bool should_abort = cct->_conf->get_val("client_die_on_failed_dentry_invalidate"); + if (should_abort) { + lderr(cct) << "no method to invalidate kernel dentry cache; quitting!" << dendl; ceph_abort(); + } else { + lderr(cct) << "no method to invalidate kernel dentry cache; expect issues!" << dendl; + } } return r; } diff --git a/src/client/Client.h b/src/client/Client.h index 568ecdf1aa684..0d7cd787bad13 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -279,7 +279,6 @@ class Client : public Dispatcher, public md_config_obs_t { client_getgroups_callback_t getgroups_cb; client_umask_callback_t umask_cb; bool can_invalidate_dentries; - bool require_remount; Finisher async_ino_invalidator; Finisher async_dentry_invalidator; @@ -765,6 +764,8 @@ private: int _release_fh(Fh *fh); void _put_fh(Fh *fh); + int _do_remount(void); + friend class C_Client_Remount; struct C_Readahead : public Context { Client *client; diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h index 178284d4ff98f..5c54776374d76 100644 --- a/src/common/legacy_config_opts.h +++ b/src/common/legacy_config_opts.h @@ -395,7 +395,6 @@ OPTION(fuse_require_active_mds, OPT_BOOL) // if ceph_fuse requires active mds se OPTION(fuse_syncfs_on_mksnap, OPT_BOOL) OPTION(client_try_dentry_invalidate, OPT_BOOL) // the client should try to use dentry invaldation instead of remounting, on kernels it believes that will work for -OPTION(client_die_on_failed_remount, OPT_BOOL) OPTION(client_check_pool_perm, OPT_BOOL) OPTION(client_use_faked_inos, OPT_BOOL) OPTION(client_mds_namespace, OPT_STR) diff --git a/src/common/options.cc b/src/common/options.cc index 0469939ab737a..5ce2f0dd3cce0 100644 --- a/src/common/options.cc +++ b/src/common/options.cc @@ -6487,10 +6487,15 @@ std::vector