libcib and based refactors: Simplify cib_perform_op() further, drop args from cib__op_fn_t

daemons/based/based_callbacks.c

@@ -29,7 +29,6 @@
 #include <crm/cib/internal.h>       // cib__*
 #include <crm/cluster.h>            // pcmk_cluster_disconnect
 #include <crm/cluster/internal.h>   // pcmk__cluster_send_message
-#include <crm/common/cib.h>         // pcmk_find_cib_element
 #include <crm/common/internal.h>    // pcmk__s, pcmk__str_eq
 #include <crm/common/ipc.h>         // crm_ipc_*, pcmk_ipc_*
 #include <crm/common/logging.h>     // CRM_LOG_ASSERT, CRM_CHECK
@@ -248,37 +247,32 @@ process_ping_reply(xmlNode *reply)
         if (!pcmk__str_eq(ping_digest, digest, pcmk__str_casei)) {
             xmlNode *wrapper = pcmk__xe_first_child(pong, PCMK__XE_CIB_CALLDATA,
                                                     NULL, NULL);
-            xmlNode *remote_cib = pcmk__xe_first_child(wrapper, NULL, NULL, NULL);
+            xmlNode *remote_versions = pcmk__xe_first_child(wrapper, NULL, NULL,
+                                                            NULL);
 
             const char *admin_epoch_s = NULL;
             const char *epoch_s = NULL;
             const char *num_updates_s = NULL;
 
-            if (remote_cib != NULL) {
-                admin_epoch_s = pcmk__xe_get(remote_cib, PCMK_XA_ADMIN_EPOCH);
-                epoch_s = pcmk__xe_get(remote_cib, PCMK_XA_EPOCH);
-                num_updates_s = pcmk__xe_get(remote_cib, PCMK_XA_NUM_UPDATES);
+            if (remote_versions != NULL) {
+                admin_epoch_s = pcmk__xe_get(remote_versions,
+                                             PCMK_XA_ADMIN_EPOCH);
+                epoch_s = pcmk__xe_get(remote_versions,
+                                       PCMK_XA_EPOCH);
+                num_updates_s = pcmk__xe_get(remote_versions,
+                                             PCMK_XA_NUM_UPDATES);
             }
 
-            pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s "
-                         "%p",
+            pcmk__notice("Local CIB %s.%s.%s.%s differs from %s: %s.%s.%s.%s",
                          pcmk__xe_get(the_cib, PCMK_XA_ADMIN_EPOCH),
                          pcmk__xe_get(the_cib, PCMK_XA_EPOCH),
                          pcmk__xe_get(the_cib, PCMK_XA_NUM_UPDATES),
                          ping_digest, host,
                          pcmk__s(admin_epoch_s, "_"),
                          pcmk__s(epoch_s, "_"),
-                         pcmk__s(num_updates_s, "_"),
-                         digest, remote_cib);
-
-            if(remote_cib && remote_cib->children) {
-                // Additional debug
-                pcmk__xml_mark_changes(the_cib, remote_cib);
-                pcmk__log_xml_changes(LOG_INFO, remote_cib);
-                pcmk__trace("End of differences");
-            }
+                         pcmk__s(num_updates_s, "_"), digest);
 
-            pcmk__xml_free(remote_cib);
+            pcmk__xml_free(remote_versions);
             sync_our_cib(reply, false);
         }
     }
@@ -517,54 +511,17 @@ forward_request(xmlNode *request)
     pcmk__xe_remove_attr(request, PCMK__XA_CIB_DELEGATED_FROM);
 }
 
-/*!
- * \internal
- * \brief Get a CIB operation's input from the request XML
- *
- * \param[in]  request  CIB request XML
- * \param[in]  type     CIB operation type
- * \param[out] section  Where to store CIB section name
- *
- * \return Input XML for CIB operation
- *
- * \note If not \c NULL, the return value is a non-const pointer to part of
- *       \p request. The caller should not free it directly.
- */
-static xmlNode *
-prepare_input(const xmlNode *request, enum cib__op_type type,
-              const char **section)
-{
-    xmlNode *wrapper = pcmk__xe_first_child(request, PCMK__XE_CIB_CALLDATA,
-                                            NULL, NULL);
-    xmlNode *input = pcmk__xe_first_child(wrapper, NULL, NULL, NULL);
-
-    if (type == cib__op_apply_patch) {
-        *section = NULL;
-    } else {
-        *section = pcmk__xe_get(request, PCMK__XA_CIB_SECTION);
-    }
-
-    // Grab the specified section
-    if ((*section != NULL) && pcmk__xe_is(input, PCMK_XE_CIB)) {
-        input = pcmk_find_cib_element(input, *section);
-    }
-
-    return input;
-}
-
 static int
 cib_process_command(xmlNode *request, const cib__operation_t *operation,
                     cib__op_fn_t op_function, xmlNode **reply, bool privileged)
 {
     xmlNode *cib_diff = NULL;
-    xmlNode *input = NULL;
     xmlNode *output = NULL;
     xmlNode *result_cib = NULL;
 
     uint32_t call_options = cib_none;
 
     const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP);
-    const char *section = NULL;
     const char *call_id = pcmk__xe_get(request, PCMK__XA_CIB_CALLID);
     const char *client_id = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTID);
     const char *client_name = pcmk__xe_get(request, PCMK__XA_CIB_CLIENTNAME);
@@ -573,7 +530,6 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation,
     int rc = pcmk_rc_ok;
 
     bool config_changed = false;
-    bool manage_counters = true;
 
     static mainloop_timer_t *digest_timer = NULL;
 
@@ -600,36 +556,21 @@ cib_process_command(xmlNode *request, const cib__operation_t *operation,
         goto done;
     }
 
-    input = prepare_input(request, operation->type, &section);
-
     if (!pcmk__is_set(operation->flags, cib__op_attr_modifies)) {
-        rc = cib__perform_query(op, call_options, op_function, section, request,
-                                input, &the_cib, &output);
+        rc = cib__perform_query(op_function, request, &the_cib, &output);
         goto done;
     }
 
-    /* @COMPAT: Handle a valid write action (legacy)
-     *
-     * @TODO: Re-evaluate whether this is truly legacy. PCMK__XA_CIB_UPDATE may
-     * be set by a sync operation even in non-legacy mode, and manage_counters
-     * tells xml_create_patchset() whether to update version/epoch info.
-     */
-    if (pcmk__xe_attr_is_true(request, PCMK__XA_CIB_UPDATE)) {
-        manage_counters = false;
-        CRM_LOG_ASSERT(pcmk__str_eq(op, PCMK__CIB_REQUEST_REPLACE,
-                                    pcmk__str_none));
-    }
-
     ping_modified_since = true;
 
     /* result_cib must not be modified after cib_perform_op() returns.
      *
      * It's not important whether the client variant is cib_native or
      * cib_remote.
      */
-    rc = cib_perform_op(cib_undefined, op, call_options, op_function, section,
-                        request, input, manage_counters, &config_changed,
-                        &the_cib, &result_cib, &cib_diff, &output);
+    result_cib = the_cib;
+    rc = cib_perform_op(cib_undefined, op_function, request, &config_changed,
+                        &result_cib, &cib_diff, &output);
 
     /* Always write to disk for successful ops with the flag set. This also
      * negates the need to detect ordering changes.

daemons/based/based_messages.c

@@ -37,9 +37,6 @@ xmlNode *the_cib = NULL;
  * \internal
  * \brief Process a \c PCMK__CIB_REQUEST_ABS_DELETE
  *
- * \param[in] op       Ignored
- * \param[in] options  Ignored
- * \param[in] section  Ignored
  * \param[in] req      Ignored
  * \param[in] input    Ignored
  * \param[in] cib      Ignored
@@ -50,8 +47,7 @@ xmlNode *the_cib = NULL;
  * \note This is unimplemented and simply returns an error.
  */
 int
-based_process_abs_delete(const char *op, int options, const char *section,
-                         xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_abs_delete(xmlNode *req, xmlNode *input, xmlNode **cib,
                          xmlNode **answer)
 {
     /* @COMPAT Remove when PCMK__CIB_REQUEST_ABS_DELETE is removed. Note that
@@ -61,8 +57,7 @@ based_process_abs_delete(const char *op, int options, const char *section,
 }
 
 int
-based_process_commit_transact(const char *op, int options, const char *section,
-                              xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib,
                               xmlNode **answer)
 {
     /* On success, our caller will activate *cib locally, trigger a replace
@@ -87,8 +82,7 @@ based_process_commit_transact(const char *op, int options, const char *section,
 }
 
 int
-based_process_is_primary(const char *op, int options, const char *section,
-                         xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_is_primary(xmlNode *req, xmlNode *input, xmlNode **cib,
                          xmlNode **answer)
 {
     // @COMPAT Pacemaker Remote clients <3.0.0 may send this
@@ -97,17 +91,15 @@ based_process_is_primary(const char *op, int options, const char *section,
 
 // @COMPAT: Remove when PCMK__CIB_REQUEST_NOOP is removed
 int
-based_process_noop(const char *op, int options, const char *section,
-                   xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_noop(xmlNode *req, xmlNode *input, xmlNode **cib,
                    xmlNode **answer)
 {
     *answer = NULL;
     return pcmk_rc_ok;
 }
 
 int
-based_process_ping(const char *op, int options, const char *section,
-                   xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib,
                    xmlNode **answer)
 {
     /* existing_cib and *cib should be identical. In the absence of ACL
@@ -133,21 +125,10 @@ based_process_ping(const char *op, int options, const char *section,
 
     if (*cib != NULL) {
         // Use *cib so that ACL filtering is applied to the answer
-        pcmk__if_tracing(
-            {
-                /* Append additional detail so the receiver can log the
-                 * differences
-                 */
-                pcmk__xml_copy(wrapper, *cib);
-            },
-            {
-                // Always include at least the version details
-                const char *name = (const char *) (*cib)->name;
-                xmlNode *shallow = pcmk__xe_create(wrapper, name);
-
-                pcmk__xe_copy_attrs(shallow, *cib, pcmk__xaf_none);
-            }
-        );
+        xmlNode *shallow = pcmk__xe_create(wrapper,
+                                           (const char *) (*cib)->name);
+
+        pcmk__xe_copy_attrs(shallow, *cib, pcmk__xaf_none);
     }
 
     pcmk__info("Reporting our current digest to %s: %s for %s.%s.%s",
@@ -162,8 +143,7 @@ based_process_ping(const char *op, int options, const char *section,
 }
 
 int
-based_process_primary(const char *op, int options, const char *section,
-                      xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_primary(xmlNode *req, xmlNode *input, xmlNode **cib,
                       xmlNode **answer)
 {
     if (!based_is_primary) {
@@ -178,8 +158,7 @@ based_process_primary(const char *op, int options, const char *section,
 }
 
 int
-based_process_schemas(const char *op, int options, const char *section,
-                      xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib,
                       xmlNode **answer)
 {
     xmlNode *wrapper = NULL;
@@ -224,8 +203,7 @@ based_process_schemas(const char *op, int options, const char *section,
 }
 
 int
-based_process_secondary(const char *op, int options, const char *section,
-                        xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_secondary(xmlNode *req, xmlNode *input, xmlNode **cib,
                         xmlNode **answer)
 {
     if (based_is_primary) {
@@ -240,8 +218,7 @@ based_process_secondary(const char *op, int options, const char *section,
 }
 
 int
-based_process_shutdown(const char *op, int options, const char *section,
-                       xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_shutdown(xmlNode *req, xmlNode *input, xmlNode **cib,
                        xmlNode **answer)
 {
     const char *host = pcmk__xe_get(req, PCMK__XA_SRC);
@@ -264,16 +241,14 @@ based_process_shutdown(const char *op, int options, const char *section,
 }
 
 int
-based_process_sync(const char *op, int options, const char *section,
-                   xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_sync(xmlNode *req, xmlNode *input, xmlNode **cib,
                    xmlNode **answer)
 {
     return sync_our_cib(req, true);
 }
 
 int
-based_process_upgrade(const char *op, int options, const char *section,
-                      xmlNode *req, xmlNode *input, xmlNode **cib,
+based_process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib,
                       xmlNode **answer)
 {
     int rc = pcmk_rc_ok;
@@ -286,8 +261,7 @@ based_process_upgrade(const char *op, int options, const char *section,
          * re-broadcasts the request with PCMK__XA_CIB_SCHEMA_MAX, and each node
          * performs the upgrade (and notifies its local clients) here.
          */
-        return cib__process_upgrade(op, options, section, req, input, cib,
-                                    answer);
+        return cib__process_upgrade(req, input, cib, answer);
 
     } else {
         xmlNode *scratch = pcmk__xml_copy(NULL, *cib);

daemons/based/based_messages.h

@@ -17,53 +17,40 @@
 extern bool based_is_primary;
 extern xmlNode *the_cib;
 
-int based_process_abs_delete(const char *op, int options, const char *section,
-                             xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_abs_delete(xmlNode *req, xmlNode *input, xmlNode **cib,
                              xmlNode **answer);
 
-int based_process_apply_patch(const char *op, int options, const char *section,
-                              xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_apply_patch(xmlNode *req, xmlNode *input, xmlNode **cib,
                               xmlNode **answer);
 
-int based_process_commit_transact(const char *op, int options,
-                                  const char *section, xmlNode *req,
-                                  xmlNode *input, xmlNode **cib,
+int based_process_commit_transact(xmlNode *req, xmlNode *input, xmlNode **cib,
                                   xmlNode **answer);
 
-int based_process_is_primary(const char *op, int options, const char *section,
-                             xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_is_primary(xmlNode *req, xmlNode *input, xmlNode **cib,
                              xmlNode **answer);
 
-int based_process_noop(const char *op, int options, const char *section,
-                       xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_noop(xmlNode *req, xmlNode *input, xmlNode **cib,
                        xmlNode **answer);
 
-int based_process_ping(const char *op, int options, const char *section,
-                       xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_ping(xmlNode *req, xmlNode *input, xmlNode **cib,
                        xmlNode **answer);
 
-int based_process_primary(const char *op, int options, const char *section,
-                          xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_primary(xmlNode *req, xmlNode *input, xmlNode **cib,
                           xmlNode **answer);
 
-int based_process_schemas(const char *op, int options, const char *section,
-                          xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_schemas(xmlNode *req, xmlNode *input, xmlNode **cib,
                           xmlNode **answer);
 
-int based_process_secondary(const char *op, int options, const char *section,
-                            xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_secondary(xmlNode *req, xmlNode *input, xmlNode **cib,
                             xmlNode **answer);
 
-int based_process_shutdown(const char *op, int options, const char *section,
-                           xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_shutdown(xmlNode *req, xmlNode *input, xmlNode **cib,
                            xmlNode **answer);
 
-int based_process_sync(const char *op, int options, const char *section,
-                       xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_sync(xmlNode *req, xmlNode *input, xmlNode **cib,
                        xmlNode **answer);
 
-int based_process_upgrade(const char *op, int options, const char *section,
-                          xmlNode *req, xmlNode *input, xmlNode **cib,
+int based_process_upgrade(xmlNode *req, xmlNode *input, xmlNode **cib,
                           xmlNode **answer);
 
 int sync_our_cib(xmlNode *request, bool all);

daemons/based/based_transaction.c

@@ -137,10 +137,8 @@ based_commit_transaction(xmlNode *transaction, const pcmk__client_t *client,
 
     /* *result_cib should be a copy of the_cib (created by cib_perform_op()). If
      * not, make a copy now. Change tracking isn't strictly required here
-     * because:
-     * * Each request in the transaction will have changes tracked and ACLs
-     *   checked if appropriate.
-     * * cib_perform_op() will infer changes for the commit request at the end.
+     * because each request in the transaction will have changes tracked and
+     * ACLs checked if appropriate.
      */
     CRM_CHECK((*result_cib != NULL) && (*result_cib != the_cib),
               *result_cib = pcmk__xml_copy(NULL, the_cib));