From b491197c587e16dcacc2af41a2d35da6a2d98330 Mon Sep 17 00:00:00 2001 From: Daniele Sciascia Date: Tue, 1 Jul 2025 13:58:32 +0200 Subject: [PATCH] Change provider options callback signature to return error code The callback now returns an error code and takes a reference to provider options string. --- include/wsrep/provider.hpp | 12 ++++++------ include/wsrep/server_state.hpp | 17 +++++++++++------ src/provider.cpp | 14 +++++++++----- src/server_state.cpp | 2 +- src/wsrep_provider_v26.cpp | 16 +++++++++++----- src/wsrep_provider_v26.hpp | 7 ++++--- test/mock_server_state.hpp | 3 ++- 7 files changed, 44 insertions(+), 27 deletions(-) diff --git a/include/wsrep/provider.hpp b/include/wsrep/provider.hpp index c45a621..dc46843 100644 --- a/include/wsrep/provider.hpp +++ b/include/wsrep/provider.hpp @@ -526,12 +526,12 @@ namespace wsrep * @param provider_options_cb Callback to get initial provider options * @param thread_service Optional thread service implementation. */ - static std::unique_ptr - make_provider(wsrep::server_state&, const std::string& provider_spec, - const std::function& - provider_options_cb, - const wsrep::provider::services& services - = wsrep::provider::services()); + static std::unique_ptr make_provider( + wsrep::server_state&, const std::string& provider_spec, + const std::function& + provider_options_cb, + const wsrep::provider::services& services + = wsrep::provider::services()); protected: wsrep::server_state& server_state_; diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp index 9d55397..ce72bb4 100644 --- a/include/wsrep/server_state.hpp +++ b/include/wsrep/server_state.hpp @@ -296,11 +296,12 @@ namespace wsrep * * @return Zero on success, non-zero on error. */ - int load_provider(const std::string& provider, - const std::function& provider_options_cb, - const wsrep::provider::services& services - = wsrep::provider::services()); + int load_provider( + const std::string& provider, + const std::function& + provider_options_cb, + const wsrep::provider::services& services + = wsrep::provider::services()); /** * Load WSRep provider. @@ -321,7 +322,11 @@ namespace wsrep = wsrep::provider::services()) { return load_provider( - provider, [options](const provider_options&) { return options; }, + provider, + [options](const provider_options&, std::string& option_string) { + option_string.append(options); + return 0; + }, services); } diff --git a/src/provider.cpp b/src/provider.cpp index 05cc03a..aa82e00 100644 --- a/src/provider.cpp +++ b/src/provider.cpp @@ -29,7 +29,7 @@ std::unique_ptr wsrep::provider::make_provider( wsrep::server_state& server_state, const std::string& provider_spec, - const std::function& + const std::function& provider_options_cb, const wsrep::provider::services& services) { @@ -41,19 +41,23 @@ std::unique_ptr wsrep::provider::make_provider( } catch (const wsrep::runtime_error& e) { - provider_options opts; + provider_options options; + std::string options_string; wsrep::log_error() << "Failed to create a new provider '" << provider_spec << "'" - << " with options '" << provider_options_cb(opts) + << " with options '" + << provider_options_cb(options, options_string) << "': " << e.what(); } catch (...) { - provider_options opts; + provider_options options; + std::string options_string; wsrep::log_error() << "Caught unknown exception when trying to " << "create a new provider '" << provider_spec << "'" - << " with options '" << provider_options_cb(opts); + << " with options '" + << provider_options_cb(options, options_string); } return 0; } diff --git a/src/server_state.cpp b/src/server_state.cpp index 8fa8f78..420929f 100644 --- a/src/server_state.cpp +++ b/src/server_state.cpp @@ -500,7 +500,7 @@ static int apply_toi(wsrep::provider& provider, int wsrep::server_state::load_provider( const std::string& provider_spec, - const std::function& + const std::function& provider_options_cb, const wsrep::provider::services& services) { diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp index 199ed18..4498bb2 100644 --- a/src/wsrep_provider_v26.cpp +++ b/src/wsrep_provider_v26.cpp @@ -778,7 +778,7 @@ void wsrep::wsrep_provider_v26::deinit_services() wsrep::wsrep_provider_v26::wsrep_provider_v26( wsrep::server_state& server_state, const std::string& provider_spec, - const std::function& provider_options_cb, + const std::function& provider_options_cb, const wsrep::provider::services& services) : provider(server_state) , wsrep_() @@ -798,8 +798,6 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26( } init_services(services); - provider_options options; - config_service_v2_fetch(wsrep_, &options); struct wsrep_init_args init_args; memset(&init_args, 0, sizeof(init_args)); @@ -808,8 +806,6 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26( init_args.node_address = server_state_.address().c_str(); init_args.node_incoming = server_state_.incoming_address().c_str(); init_args.data_dir = server_state_.working_dir().c_str(); - const auto& provider_options = provider_options_cb(options); - init_args.options = provider_options.c_str(); init_args.proto_ver = server_state.max_protocol_version(); init_args.state_id = &state_id; init_args.state = 0; @@ -823,6 +819,16 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26( init_args.sst_donate_cb = &sst_donate_cb; init_args.synced_cb = &synced_cb; + provider_options options; + config_service_v2_fetch(wsrep_, &options); + + std::string provider_options; + if (provider_options_cb(options, provider_options)) + { + throw wsrep::runtime_error("Failed to initialize wsrep provider options"); + } + init_args.options = provider_options.c_str(); + if (wsrep_->init(wsrep_, &init_args) != WSREP_OK) { throw wsrep::runtime_error("Failed to initialize wsrep provider"); diff --git a/src/wsrep_provider_v26.hpp b/src/wsrep_provider_v26.hpp index 2fcf769..767aca4 100644 --- a/src/wsrep_provider_v26.hpp +++ b/src/wsrep_provider_v26.hpp @@ -33,9 +33,10 @@ namespace wsrep public: void init_services(const wsrep::provider::services& services); void deinit_services(); - wsrep_provider_v26(wsrep::server_state&, const std::string&, - const std::function&, - const wsrep::provider::services& services); + wsrep_provider_v26( + wsrep::server_state&, const std::string&, + const std::function&, + const wsrep::provider::services& services); ~wsrep_provider_v26() WSREP_OVERRIDE; enum wsrep::provider::status connect(const std::string&, const std::string&, const std::string&, diff --git a/test/mock_server_state.hpp b/test/mock_server_state.hpp index 9e7dec7..cec7af2 100644 --- a/test/mock_server_state.hpp +++ b/test/mock_server_state.hpp @@ -263,7 +263,8 @@ namespace wsrep { set_provider_factory( [&](wsrep::server_state&, const std::string&, - const std::function&, + const std::function&, const wsrep::provider::services&) { // The provider object is destroyed upon server state