From 3965d839684190afde1ecdd3825414f7f06917f9 Mon Sep 17 00:00:00 2001
From: Daniele Sciascia <daniele.sciascia@galeracluster.com>
Date: Wed, 19 Feb 2025 13:31:52 +0200
Subject: [PATCH] Pass callback to get provider options to provider_v26
 constructor

Passing a callback allows constructing the options string using
config service after loading the provider but before initializing
the provider. This way it is possible get options which are given
at DBMS startup, in command line or config file.
---
 include/wsrep/provider.hpp         | 10 +++--
 include/wsrep/provider_options.hpp | 16 ++++---
 include/wsrep/server_state.hpp     | 39 +++++++++++++++--
 src/config_service_v1.cpp          | 70 +++++++++++++++++++++++++++++-
 src/config_service_v1.hpp          |  4 ++
 src/provider.cpp                   | 20 ++++++---
 src/provider_options.cpp           | 24 +++++-----
 src/server_state.cpp               |  8 ++--
 src/wsrep_provider_v26.cpp         | 26 ++++++++---
 src/wsrep_provider_v26.hpp         | 10 +++--
 test/mock_server_state.hpp         | 22 +++++-----
 wsrep-API/v26                      |  2 +-
 12 files changed, 195 insertions(+), 56 deletions(-)

diff --git a/include/wsrep/provider.hpp b/include/wsrep/provider.hpp
index 03709de..dc46843 100644
--- a/include/wsrep/provider.hpp
+++ b/include/wsrep/provider.hpp
@@ -29,6 +29,7 @@
 
 #include <cstring>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <vector>
@@ -50,6 +51,7 @@ namespace wsrep
     class event_service;
     class client_service;
     class connection_monitor_service;
+    class provider_options;
     class stid
     {
     public:
@@ -521,13 +523,13 @@ namespace wsrep
          * Create a new provider.
          *
          * @param provider_spec Provider specification
-         * @param provider_options Initial options to provider
+         * @param provider_options_cb Callback to get initial provider options
          * @param thread_service Optional thread service implementation.
          */
         static std::unique_ptr<provider> make_provider(
-            wsrep::server_state&,
-            const std::string& provider_spec,
-            const std::string& provider_options,
+            wsrep::server_state&, const std::string& provider_spec,
+            const std::function<int(const provider_options&, std::string&)>&
+                provider_options_cb,
             const wsrep::provider::services& services
             = wsrep::provider::services());
 
diff --git a/include/wsrep/provider_options.hpp b/include/wsrep/provider_options.hpp
index 022e159..7882a39 100644
--- a/include/wsrep/provider_options.hpp
+++ b/include/wsrep/provider_options.hpp
@@ -214,7 +214,7 @@ namespace wsrep
             int flags_;
         };
 
-        provider_options(wsrep::provider&);
+        provider_options();
         provider_options(const provider_options&) = delete;
         provider_options& operator=(const provider_options&) = delete;
 
@@ -225,7 +225,7 @@ namespace wsrep
          *
          * @return Provider status code.
          */
-        enum wsrep::provider::status initial_options();
+        enum wsrep::provider::status initial_options(wsrep::provider& provider);
 
         /**
          * Get the option with the given name
@@ -241,7 +241,8 @@ namespace wsrep
          * @return wsrep::provider::error_size_exceeded if memory could
          *         not be allocated for the new value.
          */
-        enum wsrep::provider::status set(const std::string& name,
+        enum wsrep::provider::status set(wsrep::provider& provider,
+                                         const std::string& name,
                                          std::unique_ptr<option_value> value);
 
         /**
@@ -252,10 +253,15 @@ namespace wsrep
                     std::unique_ptr<option_value> value,
                     std::unique_ptr<option_value> default_value, int flags);
 
-        void for_each(const std::function<void(option*)>& fn);
+        /**
+         * Invoke the given function with each provider option
+         * as argument.
+         *
+         * @param fn Function to call for each option
+         */
+        void for_each(const std::function<void(option*)>& fn) const;
 
     private:
-        provider& provider_;
         using options_map = std::map<std::string, std::unique_ptr<option>>;
         options_map options_;
     };
diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp
index 632d43b..ce72bb4 100644
--- a/include/wsrep/server_state.hpp
+++ b/include/wsrep/server_state.hpp
@@ -117,6 +117,7 @@ namespace wsrep
     class server_service;
     class client_service;
     class encryption_service;
+    class provider_options;
 
     /** @class Server Context
      *
@@ -287,17 +288,47 @@ namespace wsrep
          * Load WSRep provider.
          *
          * @param provider WSRep provider library to be loaded.
-         * @param provider_options Provider specific options string
-         *        to be passed for provider during initialization.
+         * @param provider_options_cb Callback to get provider options.
+         *                            The function to be called must be
+         *                            idempotent.
          * @param services Application defined services passed to
          *                 the provider.
          *
          * @return Zero on success, non-zero on error.
          */
+        int load_provider(
+            const std::string& provider,
+            const std::function<int(const provider_options&, std::string&)>&
+                provider_options_cb,
+            const wsrep::provider::services& services
+            = wsrep::provider::services());
+
+        /**
+         * Load WSRep provider.
+         *
+         * @param provider WSRep provider library to be loaded.
+         * @param options Provider specific options string
+         *        to be passed for provider during initialization.
+         * @param services Application defined services passed to
+         *                 the provider.
+         *
+         * @return Zero on success, non-zero on error.
+         *
+         * @note Provided for backward compatibility.
+         */
         int load_provider(const std::string& provider,
-                          const std::string& provider_options,
+                          const std::string& options,
                           const wsrep::provider::services& services
-                          = wsrep::provider::services());
+                          = wsrep::provider::services())
+        {
+            return load_provider(
+                provider,
+                [options](const provider_options&, std::string& option_string) {
+                  option_string.append(options);
+                  return 0;
+                },
+                services);
+        }
 
         using provider_factory_func =
             std::function<decltype(wsrep::provider::make_provider)>;
diff --git a/src/config_service_v1.cpp b/src/config_service_v1.cpp
index 5e5c9f0..2fe782b 100644
--- a/src/config_service_v1.cpp
+++ b/src/config_service_v1.cpp
@@ -19,6 +19,7 @@
 
 #include "config_service_v1.hpp"
 #include "service_helpers.hpp"
+#include "v26/wsrep_api.h"
 #include "v26/wsrep_config_service.h"
 #include "wsrep/logger.hpp"
 #include "wsrep/provider_options.hpp"
@@ -150,7 +151,7 @@ static void config_service_v1_deinit(void* dlh)
 int wsrep::config_service_v1_fetch(wsrep::provider& provider,
                                    wsrep::provider_options* options)
 {
-    struct wsrep_st* wsrep = (struct wsrep_st*)provider.native();
+    struct wsrep_st* wsrep = static_cast<struct wsrep_st*>(provider.native());
     if (wsrep == nullptr)
     {
         // Not a provider which was loaded via wsrep-API
@@ -179,3 +180,70 @@ int wsrep::config_service_v1_fetch(wsrep::provider& provider,
 
     return 0;
 }
+
+namespace wsrep_config_service_v2
+{
+    wsrep_config_service_v2_t service{ 0 };
+
+    wsrep_status_t service_callback(const wsrep_parameter* p, void* context)
+    {
+        return wsrep_config_service_v1::service_callback(p, context);
+    }
+} // namespace wsrep_config_service_v2
+
+static int config_service_v2_probe(void* dlh)
+{
+    typedef int (*init_fn)(wsrep_config_service_v2_t*);
+    typedef void (*deinit_fn)();
+    return wsrep_impl::service_probe<init_fn>(
+               dlh, WSREP_CONFIG_SERVICE_INIT_FUNC_V2, "config service v2")
+           || wsrep_impl::service_probe<deinit_fn>(
+               dlh, WSREP_CONFIG_SERVICE_DEINIT_FUNC_V2, "config service v2");
+}
+
+static int config_service_v2_init(void* dlh)
+{
+    typedef int (*init_fn)(wsrep_config_service_v2_t*);
+    return wsrep_impl::service_init<init_fn>(
+        dlh, WSREP_CONFIG_SERVICE_INIT_FUNC_V2,
+        &wsrep_config_service_v2::service, "config service v2");
+}
+
+static void config_service_v2_deinit(void* dlh)
+{
+    typedef int (*deinit_fn)();
+    wsrep_impl::service_deinit<deinit_fn>(
+        dlh, WSREP_CONFIG_SERVICE_DEINIT_FUNC_V2, "config service v2");
+}
+
+int wsrep::config_service_v2_fetch(struct wsrep_st* wsrep,
+                                   wsrep::provider_options* options)
+{
+    if (wsrep == nullptr)
+    {
+        // Not a provider which was loaded via wsrep-API
+        return 0;
+    }
+    if (config_service_v2_probe(wsrep->dlh))
+    {
+        wsrep::log_info() << "Provider does not support config service v2";
+        return 1;
+    }
+    if (config_service_v2_init(wsrep->dlh))
+    {
+        wsrep::log_warning() << "Failed to initialize config service v2";
+        return 1;
+    }
+
+    wsrep_status_t ret = wsrep_config_service_v2::service.get_parameters(
+        wsrep, &wsrep_config_service_v2::service_callback, options);
+
+    config_service_v2_deinit(wsrep->dlh);
+
+    if (ret != WSREP_OK)
+    {
+        return 1;
+    }
+
+    return 0;
+}
diff --git a/src/config_service_v1.hpp b/src/config_service_v1.hpp
index 49532cd..23485da 100644
--- a/src/config_service_v1.hpp
+++ b/src/config_service_v1.hpp
@@ -20,11 +20,15 @@
 #ifndef WSREP_CONFIG_SERVICE_V1_HPP
 #define WSREP_CONFIG_SERVICE_V1_HPP
 
+struct wsrep_st;
+
 namespace wsrep
 {
     class provider;
     class provider_options;
     int config_service_v1_fetch(provider& provider, provider_options* opts);
+    int config_service_v1_fetch(struct wsrep_st* wsrep, provider_options* opts);
+    int config_service_v2_fetch(struct wsrep_st* wsrep, provider_options* opts);
 } // namespace wsrep
 
 #endif // WSREP_CONFIG_SERVICE_V1_HPP
diff --git a/src/provider.cpp b/src/provider.cpp
index ce9f5da..aa82e00 100644
--- a/src/provider.cpp
+++ b/src/provider.cpp
@@ -19,6 +19,7 @@
 
 #include "wsrep/provider.hpp"
 #include "wsrep/logger.hpp"
+#include "wsrep/provider_options.hpp"
 
 #include "wsrep_provider_v26.hpp"
 
@@ -27,29 +28,36 @@
 #include <memory>
 
 std::unique_ptr<wsrep::provider> wsrep::provider::make_provider(
-    wsrep::server_state& server_state,
-    const std::string& provider_spec,
-    const std::string& provider_options,
+    wsrep::server_state& server_state, const std::string& provider_spec,
+    const std::function<int(const provider_options&, std::string&)>&
+        provider_options_cb,
     const wsrep::provider::services& services)
 {
     try
     {
         return std::unique_ptr<wsrep::provider>(new wsrep::wsrep_provider_v26(
-            server_state, provider_options, provider_spec, services));
+            server_state, provider_spec, provider_options_cb,
+            services));
     }
     catch (const wsrep::runtime_error& e)
     {
+        provider_options options;
+        std::string options_string;
         wsrep::log_error() << "Failed to create a new provider '"
                            << provider_spec << "'"
-                           << " with options '" << provider_options
+                           << " with options '"
+                           << provider_options_cb(options, options_string)
                            << "': " << e.what();
     }
     catch (...)
     {
+        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;
+                           << " with options '"
+                           << provider_options_cb(options, options_string);
     }
     return 0;
 }
diff --git a/src/provider_options.cpp b/src/provider_options.cpp
index fcd7a95..e22ba0d 100644
--- a/src/provider_options.cpp
+++ b/src/provider_options.cpp
@@ -18,6 +18,7 @@
  */
 
 #include "wsrep/provider_options.hpp"
+#include "wsrep/provider.hpp"
 #include "config_service_v1.hpp"
 #include "wsrep/logger.hpp"
 
@@ -92,16 +93,16 @@ void wsrep::provider_options::option::update_value(
 
 wsrep::provider_options::option::~option() {}
 
-wsrep::provider_options::provider_options(wsrep::provider& provider)
-    : provider_(provider)
-    , options_()
+wsrep::provider_options::provider_options()
+    : options_()
 {
 }
 
-enum wsrep::provider::status wsrep::provider_options::initial_options()
+enum wsrep::provider::status
+wsrep::provider_options::initial_options(wsrep::provider& provider)
 {
     options_.clear();
-    if (config_service_v1_fetch(provider_, this))
+    if (config_service_v1_fetch(provider, this))
     {
         return wsrep::provider::error_not_implemented;
     }
@@ -123,7 +124,7 @@ wsrep::provider_options::get_option(const std::string& name) const
 }
 
 enum wsrep::provider::status wsrep::provider_options::set(
-    const std::string& name,
+    wsrep::provider& provider, const std::string& name,
     std::unique_ptr<wsrep::provider_options::option_value> value)
 {
     auto option(options_.find(name));
@@ -132,7 +133,7 @@ enum wsrep::provider::status wsrep::provider_options::set(
         return not_found_error;
     }
     provider_options_sep sep;
-    auto ret(provider_.options(std::string(option->second->real_name())
+    auto ret(provider.options(std::string(option->second->real_name())
                                + sep.key_value + value->as_string()
                                + sep.param));
     if (ret == provider::success)
@@ -160,9 +161,10 @@ enum wsrep::provider::status wsrep::provider_options::set_default(
     return wsrep::provider::success;
 }
 
-void wsrep::provider_options::for_each(const std::function<void(option*)>& fn)
+void wsrep::provider_options::for_each(
+    const std::function<void(option*)>& fn) const
 {
-    std::for_each(
-        options_.begin(), options_.end(),
-        [&fn](const options_map::value_type& opt) { fn(opt.second.get()); });
+    std::for_each(options_.begin(), options_.end(),
+                  [&fn](const options_map::value_type& opt)
+                  { fn(opt.second.get()); });
 }
diff --git a/src/server_state.cpp b/src/server_state.cpp
index c727dba..420929f 100644
--- a/src/server_state.cpp
+++ b/src/server_state.cpp
@@ -499,13 +499,15 @@ static int apply_toi(wsrep::provider& provider,
 //////////////////////////////////////////////////////////////////////////////
 
 int wsrep::server_state::load_provider(
-    const std::string& provider_spec, const std::string& provider_options,
+    const std::string& provider_spec,
+    const std::function<int(const provider_options&, std::string&)>&
+        provider_options_cb,
     const wsrep::provider::services& services)
 {
     wsrep::log_info() << "Loading provider " << provider_spec
                       << " initial position: " << initial_position_;
-    provider_
-        = provider_factory_(*this, provider_spec, provider_options, services);
+    provider_ = provider_factory_(*this, provider_spec, provider_options_cb,
+                                  services);
     return (provider_ ? 0 : 1);
 }
 
diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp
index 953c615..4498bb2 100644
--- a/src/wsrep_provider_v26.cpp
+++ b/src/wsrep_provider_v26.cpp
@@ -29,12 +29,14 @@
 #include "wsrep/tls_service.hpp"
 #include "wsrep/allowlist_service.hpp"
 #include "wsrep/connection_monitor_service.hpp"
+#include "wsrep/provider_options.hpp"
 
 #include "service_helpers.hpp"
 #include "thread_service_v1.hpp"
 #include "tls_service_v1.hpp"
 #include "allowlist_service_v1.hpp"
 #include "event_service_v1.hpp"
+#include "config_service_v1.hpp"
 #include "v26/wsrep_api.h"
 #include "v26/wsrep_node_isolation.h"
 #include "connection_monitor_service_v1.hpp"
@@ -775,8 +777,8 @@ void wsrep::wsrep_provider_v26::deinit_services()
 
 wsrep::wsrep_provider_v26::wsrep_provider_v26(
     wsrep::server_state& server_state,
-    const std::string& provider_options,
     const std::string& provider_spec,
+    const std::function<int(provider_options&, std::string&)>& provider_options_cb,
     const wsrep::provider::services& services)
     : provider(server_state)
     , wsrep_()
@@ -789,6 +791,14 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26(
                 server_state.initial_position().id().data(),
                 sizeof(state_id.uuid.data));
     state_id.seqno = server_state.initial_position().seqno().get();
+
+    if (wsrep_load(provider_spec.c_str(), &wsrep_, logger_cb))
+    {
+        throw wsrep::runtime_error("Failed to load wsrep library");
+    }
+
+    init_services(services);
+
     struct wsrep_init_args init_args;
     memset(&init_args, 0, sizeof(init_args));
     init_args.app_ctx = &server_state;
@@ -796,7 +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();
-    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;
@@ -810,12 +819,15 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26(
     init_args.sst_donate_cb = &sst_donate_cb;
     init_args.synced_cb = &synced_cb;
 
-    if (wsrep_load(provider_spec.c_str(), &wsrep_, logger_cb))
-    {
-        throw wsrep::runtime_error("Failed to load wsrep library");
-    }
+    provider_options options;
+    config_service_v2_fetch(wsrep_, &options);
 
-    init_services(services);
+    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)
     {
diff --git a/src/wsrep_provider_v26.hpp b/src/wsrep_provider_v26.hpp
index 53edc20..767aca4 100644
--- a/src/wsrep_provider_v26.hpp
+++ b/src/wsrep_provider_v26.hpp
@@ -26,15 +26,17 @@ struct wsrep_st;
 
 namespace wsrep
 {
-    class thread_service;
+    class provider_options;
+
     class wsrep_provider_v26 : public wsrep::provider
     {
     public:
         void init_services(const wsrep::provider::services& services);
         void deinit_services();
-        wsrep_provider_v26(wsrep::server_state&, const std::string&,
-                           const std::string&,
-                           const wsrep::provider::services& services);
+        wsrep_provider_v26(
+            wsrep::server_state&, const std::string&,
+            const std::function<int(provider_options&, std::string&)>&,
+            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 c2334ef..cec7af2 100644
--- a/test/mock_server_state.hpp
+++ b/test/mock_server_state.hpp
@@ -22,6 +22,7 @@
 
 #include "wsrep/server_state.hpp"
 #include "wsrep/server_service.hpp"
+#include "wsrep/provider_options.hpp"
 #include "mock_client_state.hpp"
 #include "mock_high_priority_service.hpp"
 #include "mock_storage_service.hpp"
@@ -260,16 +261,17 @@ namespace wsrep
             , cond_()
             , provider_()
         {
-            set_provider_factory([&](wsrep::server_state&,
-                                     const std::string&,
-                                     const std::string&,
-                                     const wsrep::provider::services&)
-            {
-                // The provider object is destroyed upon server state
-                // destruction, so using a raw pointer is safe.
-                provider_ = new wsrep::mock_provider(*this);
-                return std::unique_ptr<wsrep::provider>(provider_);
-            });
+            set_provider_factory(
+                [&](wsrep::server_state&, const std::string&,
+                    const std::function<int(const wsrep::provider_options&,
+                                            std::string&)>&,
+                    const wsrep::provider::services&)
+                {
+                    // The provider object is destroyed upon server state
+                    // destruction, so using a raw pointer is safe.
+                    provider_ = new wsrep::mock_provider(*this);
+                    return std::unique_ptr<wsrep::provider>(provider_);
+                });
 
             const int ret WSREP_UNUSED = load_provider("mock", "");
             assert(ret == 0);
diff --git a/wsrep-API/v26 b/wsrep-API/v26
index 65608d3..e8313df 160000
--- a/wsrep-API/v26
+++ b/wsrep-API/v26
@@ -1 +1 @@
-Subproject commit 65608d3f503ba9f4c170fc4e01c539be9fafd46c
+Subproject commit e8313df29e9329521dced03a4cc558d0cf7d8032