From ef711f968ba143b12fb0654b6d75045a04f83d37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 9 Feb 2017 22:30:17 +0100
Subject: [PATCH 1/6] Refactor: lib/services: unify/add include guards for *.h
(wild guess has it that MH stood for long-dead Matahari project)
---
lib/services/pcmk-dbus.h | 8 ++++----
lib/services/services_private.h | 6 +++---
lib/services/systemd.h | 5 +++++
lib/services/upstart.h | 7 +++----
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/lib/services/pcmk-dbus.h b/lib/services/pcmk-dbus.h
index c575dda..31bcf0e 100644
--- a/lib/services/pcmk-dbus.h
+++ b/lib/services/pcmk-dbus.h
@@ -8,9 +8,9 @@
#ifndef PCMK_DBUS__H
# define PCMK_DBUS__H
-#ifndef DBUS_TIMEOUT_USE_DEFAULT
-# define DBUS_TIMEOUT_USE_DEFAULT -1
-#endif
+# ifndef DBUS_TIMEOUT_USE_DEFAULT
+# define DBUS_TIMEOUT_USE_DEFAULT -1
+# endif
DBusConnection *pcmk_dbus_connect(void);
void pcmk_dbus_connection_setup_with_select(DBusConnection *c);
@@ -28,4 +28,4 @@ char *pcmk_dbus_get_property(
bool pcmk_dbus_find_error(DBusPendingCall *pending, DBusMessage *reply,
DBusError *error);
-#endif /* PCMK_DBUS__H */
+#endif /* PCMK_DBUS__H */
diff --git a/lib/services/services_private.h b/lib/services/services_private.h
index 43aedc6..0e12d18 100644
--- a/lib/services/services_private.h
+++ b/lib/services/services_private.h
@@ -16,8 +16,8 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#ifndef __MH_SERVICES_PRIVATE_H__
-# define __MH_SERVICES_PRIVATE_H__
+#ifndef SERVICES_PRIVATE__H
+# define SERVICES_PRIVATE__H
#if SUPPORT_DBUS
# include <dbus/dbus.h>
@@ -69,4 +69,4 @@ gboolean is_op_blocked(const char *rsc);
void services_set_op_pending(svc_action_t *op, DBusPendingCall *pending);
#endif
-#endif /* __MH_SERVICES_PRIVATE_H__ */
+#endif /* SERVICES_PRIVATE__H */
diff --git a/lib/services/systemd.h b/lib/services/systemd.h
index acd832e..17e654e 100644
--- a/lib/services/systemd.h
+++ b/lib/services/systemd.h
@@ -16,8 +16,13 @@
* Copyright (C) 2012 Andrew Beekhof <andrew@beekhof.net>
*/
+#ifndef SYSTEMD__H
+# define SYSTEMD__H
+
G_GNUC_INTERNAL GList *systemd_unit_listall(void);
G_GNUC_INTERNAL int systemd_unit_exec(svc_action_t * op);
G_GNUC_INTERNAL gboolean systemd_unit_exists(const gchar * name);
G_GNUC_INTERNAL gboolean systemd_unit_running(const gchar * name);
G_GNUC_INTERNAL void systemd_cleanup(void);
+
+#endif /* SYSTEMD__H */
diff --git a/lib/services/upstart.h b/lib/services/upstart.h
index a837226..523d208 100644
--- a/lib/services/upstart.h
+++ b/lib/services/upstart.h
@@ -13,12 +13,11 @@
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*
- * File: upstart-dbus.c
* Copyright (C) 2010 Senko Rasic <senko.rasic@dobarkod.hr>
* Copyright (c) 2010 Ante Karamatic <ivoks@init.hr>
*/
-#ifndef _UPSTART_DBUS_H_
-# define _UPSTART_DBUS_H_
+#ifndef UPSTART__H
+# define UPSTART__H
# include <glib.h>
# include "crm/services.h"
@@ -29,4 +28,4 @@ G_GNUC_INTERNAL gboolean upstart_job_exists(const gchar * name);
G_GNUC_INTERNAL gboolean upstart_job_running(const gchar * name);
G_GNUC_INTERNAL void upstart_cleanup(void);
-#endif /* _UPSTART_DBUS_H_ */
+#endif /* UPSTART__H */
--
1.8.3.1
From 5dc4937b9588b3fb1d4297eedd7c433aafd8cc81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 9 Feb 2017 22:40:39 +0100
Subject: [PATCH 2/6] Refactor: lib/services: make *.h self-sufficient wrt.
other includes
Also make function declaration follow the actual definitions.
---
lib/services/pcmk-dbus.h | 2 ++
lib/services/services_private.h | 3 +++
lib/services/systemd.h | 5 ++++-
lib/services/upstart.h | 2 +-
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/services/pcmk-dbus.h b/lib/services/pcmk-dbus.h
index 31bcf0e..e071349 100644
--- a/lib/services/pcmk-dbus.h
+++ b/lib/services/pcmk-dbus.h
@@ -8,6 +8,8 @@
#ifndef PCMK_DBUS__H
# define PCMK_DBUS__H
+# include <dbus/dbus.h>
+
# ifndef DBUS_TIMEOUT_USE_DEFAULT
# define DBUS_TIMEOUT_USE_DEFAULT -1
# endif
diff --git a/lib/services/services_private.h b/lib/services/services_private.h
index 0e12d18..60b1b14 100644
--- a/lib/services/services_private.h
+++ b/lib/services/services_private.h
@@ -19,6 +19,9 @@
#ifndef SERVICES_PRIVATE__H
# define SERVICES_PRIVATE__H
+# include <glib.h>
+# include "crm/services.h"
+
#if SUPPORT_DBUS
# include <dbus/dbus.h>
#endif
diff --git a/lib/services/systemd.h b/lib/services/systemd.h
index 17e654e..98150dd 100644
--- a/lib/services/systemd.h
+++ b/lib/services/systemd.h
@@ -19,8 +19,11 @@
#ifndef SYSTEMD__H
# define SYSTEMD__H
+# include <glib.h>
+# include "crm/services.h"
+
G_GNUC_INTERNAL GList *systemd_unit_listall(void);
-G_GNUC_INTERNAL int systemd_unit_exec(svc_action_t * op);
+G_GNUC_INTERNAL gboolean systemd_unit_exec(svc_action_t * op);
G_GNUC_INTERNAL gboolean systemd_unit_exists(const gchar * name);
G_GNUC_INTERNAL gboolean systemd_unit_running(const gchar * name);
G_GNUC_INTERNAL void systemd_cleanup(void);
diff --git a/lib/services/upstart.h b/lib/services/upstart.h
index 523d208..8d41c11 100644
--- a/lib/services/upstart.h
+++ b/lib/services/upstart.h
@@ -23,7 +23,7 @@
# include "crm/services.h"
G_GNUC_INTERNAL GList *upstart_job_listall(void);
-G_GNUC_INTERNAL int upstart_job_exec(svc_action_t * op, gboolean synchronous);
+G_GNUC_INTERNAL gboolean upstart_job_exec(svc_action_t * op, gboolean synchronous);
G_GNUC_INTERNAL gboolean upstart_job_exists(const gchar * name);
G_GNUC_INTERNAL gboolean upstart_job_running(const gchar * name);
G_GNUC_INTERNAL void upstart_cleanup(void);
--
1.8.3.1
From 6b12e0d1284cac96e66780aa4aeb7c32619089fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 9 Feb 2017 22:46:42 +0100
Subject: [PATCH 3/6] Refactor: lib/services: ensure *.h declare externally
hidden functions
...following the precedent of {upstart,systemd}.h. Note also that
since hardening-related commit 658fff944 [$LD -z now], it's even more
desirable to limit "symbol smog" as much as possible and symbols only
used within the library like these are trivially expendable (assuming
suitable toolchain).
As we use G_GNU_C_INTERNAL macro from glib2, we should care to require
at least that version that introduced it, i.e., 2.6 per documentation.
---
lib/services/pcmk-dbus.h | 13 +++++++++++++
lib/services/services_private.h | 14 ++++++++++++++
pacemaker.spec.in | 4 +++-
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/lib/services/pcmk-dbus.h b/lib/services/pcmk-dbus.h
index e071349..3f3547e 100644
--- a/lib/services/pcmk-dbus.h
+++ b/lib/services/pcmk-dbus.h
@@ -14,19 +14,32 @@
# define DBUS_TIMEOUT_USE_DEFAULT -1
# endif
+G_GNUC_INTERNAL
DBusConnection *pcmk_dbus_connect(void);
+
+G_GNUC_INTERNAL
void pcmk_dbus_connection_setup_with_select(DBusConnection *c);
+
+G_GNUC_INTERNAL
void pcmk_dbus_disconnect(DBusConnection *connection);
+G_GNUC_INTERNAL
DBusPendingCall *pcmk_dbus_send(DBusMessage *msg, DBusConnection *connection,
void(*done)(DBusPendingCall *pending, void *user_data), void *user_data, int timeout);
+
+G_GNUC_INTERNAL
DBusMessage *pcmk_dbus_send_recv(DBusMessage *msg, DBusConnection *connection, DBusError *error, int timeout);
+
+G_GNUC_INTERNAL
bool pcmk_dbus_type_check(DBusMessage *msg, DBusMessageIter *field, int expected, const char *function, int line);
+
+G_GNUC_INTERNAL
char *pcmk_dbus_get_property(
DBusConnection *connection, const char *target, const char *obj, const gchar * iface, const char *name,
void (*callback)(const char *name, const char *value, void *userdata), void *userdata,
DBusPendingCall **pending, int timeout);
+G_GNUC_INTERNAL
bool pcmk_dbus_find_error(DBusPendingCall *pending, DBusMessage *reply,
DBusError *error);
diff --git a/lib/services/services_private.h b/lib/services/services_private.h
index 60b1b14..41895a9 100644
--- a/lib/services/services_private.h
+++ b/lib/services/services_private.h
@@ -45,30 +45,44 @@ struct svc_action_private_s {
#endif
};
+G_GNUC_INTERNAL
GList *services_os_get_directory_list(const char *root, gboolean files, gboolean executable);
+G_GNUC_INTERNAL
gboolean services_os_action_execute(svc_action_t * op, gboolean synchronous);
+G_GNUC_INTERNAL
GList *resources_os_list_lsb_agents(void);
+G_GNUC_INTERNAL
GList *resources_os_list_ocf_providers(void);
+G_GNUC_INTERNAL
GList *resources_os_list_ocf_agents(const char *provider);
+G_GNUC_INTERNAL
GList *resources_os_list_nagios_agents(void);
+G_GNUC_INTERNAL
gboolean cancel_recurring_action(svc_action_t * op);
+G_GNUC_INTERNAL
gboolean recurring_action_timer(gpointer data);
+
+G_GNUC_INTERNAL
gboolean operation_finalize(svc_action_t * op);
+G_GNUC_INTERNAL
void services_add_inflight_op(svc_action_t *op);
+G_GNUC_INTERNAL
void handle_blocked_ops(void);
+G_GNUC_INTERNAL
gboolean is_op_blocked(const char *rsc);
#if SUPPORT_DBUS
+G_GNUC_INTERNAL
void services_set_op_pending(svc_action_t *op, DBusPendingCall *pending);
#endif
diff --git a/pacemaker.spec.in b/pacemaker.spec.in
index 34a1076..6385350 100644
--- a/pacemaker.spec.in
+++ b/pacemaker.spec.in
@@ -190,7 +190,9 @@ BuildRequires: coreutils findutils grep sed
# Required for core functionality
BuildRequires: automake autoconf libtool pkgconfig libtool-ltdl-devel
-BuildRequires: pkgconfig(glib-2.0) libxml2-devel libxslt-devel libuuid-devel
+## version lower bound for: G_GNUC_INTERNAL
+BuildRequires: pkgconfig(glib-2.0) >= 2.6
+BuildRequires: libxml2-devel libxslt-devel libuuid-devel
BuildRequires: bzip2-devel pam-devel
# Required for agent_config.h which specifies the correct scratch directory
--
1.8.3.1
From 31adceb587c1beb90a0d61bb3bf1b634e5516d54 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Fri, 10 Feb 2017 10:50:59 +0100
Subject: [PATCH 4/6] Refactor: lib/services: ensure some objects not exported
accidentally
They are not tracked by neither public nor private API anyway.
See also a note about "symbol smog" in the previous commit.
---
lib/services/services.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/services/services.c b/lib/services/services.c
index 4be425c..b1e3b26 100644
--- a/lib/services/services.c
+++ b/lib/services/services.c
@@ -37,11 +37,11 @@
/* TODO: Develop a rollover strategy */
static int operations = 0;
-GHashTable *recurring_actions = NULL;
+static GHashTable *recurring_actions = NULL;
/* ops waiting to run async because of conflicting active
- * pending ops*/
-GList *blocked_ops = NULL;
+ * pending ops */
+static GList *blocked_ops = NULL;
/* ops currently active (in-flight) */
GList *inflight_ops = NULL;
--
1.8.3.1
From eeeabd732b67d478bb50101e1bb30324ca2c3d9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 9 Feb 2017 23:06:36 +0100
Subject: [PATCH 5/6] Refactor: lib/services: unify on single source of
"synchronous" flag
Forerunner systemd.[ch] is followed by the rest of squad in decision
based merely on svc_action_t operand with suitably flipped member item
and no more on redundant extraneous parameter that in turn is dropped.
It was (for the former) and is possible because both a/synchronous paths
leading to such decision contains a priori setting of the flag as
expected (in services_action_async/services_action_sync, respectively).
---
lib/services/services.c | 8 ++++----
lib/services/services_linux.c | 16 ++++++++--------
lib/services/services_private.h | 2 +-
lib/services/upstart.c | 2 +-
lib/services/upstart.h | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/lib/services/services.c b/lib/services/services.c
index b1e3b26..decb2cb 100644
--- a/lib/services/services.c
+++ b/lib/services/services.c
@@ -573,14 +573,14 @@ action_async_helper(svc_action_t * op)
{
if (op->standard && strcasecmp(op->standard, "upstart") == 0) {
#if SUPPORT_UPSTART
- return upstart_job_exec(op, FALSE);
+ return upstart_job_exec(op);
#endif
} else if (op->standard && strcasecmp(op->standard, "systemd") == 0) {
#if SUPPORT_SYSTEMD
return systemd_unit_exec(op);
#endif
} else {
- return services_os_action_execute(op, FALSE);
+ return services_os_action_execute(op);
}
/* The 'op' has probably been freed if the execution functions return TRUE. */
/* Avoid using the 'op' in here. */
@@ -701,14 +701,14 @@ services_action_sync(svc_action_t * op)
op->synchronous = true;
if (op->standard && strcasecmp(op->standard, "upstart") == 0) {
#if SUPPORT_UPSTART
- rc = upstart_job_exec(op, TRUE);
+ rc = upstart_job_exec(op);
#endif
} else if (op->standard && strcasecmp(op->standard, "systemd") == 0) {
#if SUPPORT_SYSTEMD
rc = systemd_unit_exec(op);
#endif
} else {
- rc = services_os_action_execute(op, TRUE);
+ rc = services_os_action_execute(op);
}
crm_trace(" > %s_%s_%d: %s = %d", op->rsc, op->action, op->interval, op->opaque->exec, op->rc);
if (op->stdout_data) {
diff --git a/lib/services/services_linux.c b/lib/services/services_linux.c
index 16f25f3..adfefaa 100644
--- a/lib/services/services_linux.c
+++ b/lib/services/services_linux.c
@@ -606,7 +606,7 @@ action_synced_wait(svc_action_t * op, sigset_t *mask)
/* For an asynchronous 'op', returns FALSE if 'op' should be free'd by the caller */
/* For a synchronous 'op', returns FALSE if 'op' fails */
gboolean
-services_os_action_execute(svc_action_t * op, gboolean synchronous)
+services_os_action_execute(svc_action_t * op)
{
int stdout_fd[2];
int stderr_fd[2];
@@ -641,7 +641,7 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
int rc = errno;
crm_warn("Cannot execute '%s': %s (%d)", op->opaque->exec, pcmk_strerror(rc), rc);
services_handle_exec_error(op, rc);
- if (!synchronous) {
+ if (!op->synchronous) {
return operation_finalize(op);
}
return FALSE;
@@ -653,7 +653,7 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
crm_err("pipe(stdout_fd) failed. '%s': %s (%d)", op->opaque->exec, pcmk_strerror(rc), rc);
services_handle_exec_error(op, rc);
- if (!synchronous) {
+ if (!op->synchronous) {
return operation_finalize(op);
}
return FALSE;
@@ -668,13 +668,13 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
crm_err("pipe(stderr_fd) failed. '%s': %s (%d)", op->opaque->exec, pcmk_strerror(rc), rc);
services_handle_exec_error(op, rc);
- if (!synchronous) {
+ if (!op->synchronous) {
return operation_finalize(op);
}
return FALSE;
}
- if (synchronous) {
+ if (op->synchronous) {
#ifdef HAVE_SYS_SIGNALFD_H
sigemptyset(&mask);
sigaddset(&mask, SIGCHLD);
@@ -717,7 +717,7 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
crm_err("Could not execute '%s': %s (%d)", op->opaque->exec, pcmk_strerror(rc), rc);
services_handle_exec_error(op, rc);
- if (!synchronous) {
+ if (!op->synchronous) {
return operation_finalize(op);
}
@@ -740,7 +740,7 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
close(stderr_fd[1]);
}
- if (synchronous) {
+ if (op->synchronous) {
sigchld_cleanup();
}
@@ -758,7 +758,7 @@ services_os_action_execute(svc_action_t * op, gboolean synchronous)
op->opaque->stderr_fd = stderr_fd[0];
set_fd_opts(op->opaque->stderr_fd, O_NONBLOCK);
- if (synchronous) {
+ if (op->synchronous) {
action_synced_wait(op, pmask);
sigchld_cleanup();
} else {
diff --git a/lib/services/services_private.h b/lib/services/services_private.h
index 41895a9..ec9e1c4 100644
--- a/lib/services/services_private.h
+++ b/lib/services/services_private.h
@@ -49,7 +49,7 @@ G_GNUC_INTERNAL
GList *services_os_get_directory_list(const char *root, gboolean files, gboolean executable);
G_GNUC_INTERNAL
-gboolean services_os_action_execute(svc_action_t * op, gboolean synchronous);
+gboolean services_os_action_execute(svc_action_t * op);
G_GNUC_INTERNAL
GList *resources_os_list_lsb_agents(void);
diff --git a/lib/services/upstart.c b/lib/services/upstart.c
index 5651a14..a17c2bf 100644
--- a/lib/services/upstart.c
+++ b/lib/services/upstart.c
@@ -414,7 +414,7 @@ upstart_async_dispatch(DBusPendingCall *pending, void *user_data)
/* For an asynchronous 'op', returns FALSE if 'op' should be free'd by the caller */
/* For a synchronous 'op', returns FALSE if 'op' fails */
gboolean
-upstart_job_exec(svc_action_t * op, gboolean synchronous)
+upstart_job_exec(svc_action_t * op)
{
char *job = NULL;
int arg_wait = TRUE;
diff --git a/lib/services/upstart.h b/lib/services/upstart.h
index 8d41c11..6954943 100644
--- a/lib/services/upstart.h
+++ b/lib/services/upstart.h
@@ -23,7 +23,7 @@
# include "crm/services.h"
G_GNUC_INTERNAL GList *upstart_job_listall(void);
-G_GNUC_INTERNAL gboolean upstart_job_exec(svc_action_t * op, gboolean synchronous);
+G_GNUC_INTERNAL gboolean upstart_job_exec(svc_action_t * op);
G_GNUC_INTERNAL gboolean upstart_job_exists(const gchar * name);
G_GNUC_INTERNAL gboolean upstart_job_running(const gchar * name);
G_GNUC_INTERNAL void upstart_cleanup(void);
--
1.8.3.1
From 6e64b585ba628ef93d99fc17a6a880a76505e27d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 9 Feb 2017 23:28:24 +0100
Subject: [PATCH 6/6] Refactor: lib/services: async_helper -> versatile
exec_helper
Also mark it inline just in case the compiler won't get it that it's
just another form of up-to-singly-nested ternary operator making for
a tail call.
---
lib/services/services.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/lib/services/services.c b/lib/services/services.c
index decb2cb..0b535e6 100644
--- a/lib/services/services.c
+++ b/lib/services/services.c
@@ -568,9 +568,10 @@ handle_duplicate_recurring(svc_action_t * op, void (*action_callback) (svc_actio
return FALSE;
}
-static gboolean
-action_async_helper(svc_action_t * op)
+inline static gboolean
+action_exec_helper(svc_action_t * op)
{
+ /* Whether a/synchronous must be decided (op->synchronous) beforehand. */
if (op->standard && strcasecmp(op->standard, "upstart") == 0) {
#if SUPPORT_UPSTART
return upstart_job_exec(op);
@@ -582,7 +583,8 @@ action_async_helper(svc_action_t * op)
} else {
return services_os_action_execute(op);
}
- /* The 'op' has probably been freed if the execution functions return TRUE. */
+ /* The 'op' has probably been freed if the execution functions return TRUE
+ for the asynchronous 'op'. */
/* Avoid using the 'op' in here. */
return FALSE;
@@ -625,7 +627,7 @@ services_action_async(svc_action_t * op, void (*action_callback) (svc_action_t *
return TRUE;
}
- return action_async_helper(op);
+ return action_exec_helper(op);
}
@@ -670,7 +672,7 @@ handle_blocked_ops(void)
continue;
}
executed_ops = g_list_append(executed_ops, op);
- res = action_async_helper(op);
+ res = action_exec_helper(op);
if (res == FALSE) {
op->status = PCMK_LRM_OP_ERROR;
/* this can cause this function to be called recursively
@@ -699,17 +701,7 @@ services_action_sync(svc_action_t * op)
}
op->synchronous = true;
- if (op->standard && strcasecmp(op->standard, "upstart") == 0) {
-#if SUPPORT_UPSTART
- rc = upstart_job_exec(op);
-#endif
- } else if (op->standard && strcasecmp(op->standard, "systemd") == 0) {
-#if SUPPORT_SYSTEMD
- rc = systemd_unit_exec(op);
-#endif
- } else {
- rc = services_os_action_execute(op);
- }
+ rc = action_exec_helper(op);
crm_trace(" > %s_%s_%d: %s = %d", op->rsc, op->action, op->interval, op->opaque->exec, op->rc);
if (op->stdout_data) {
crm_trace(" > stdout: %s", op->stdout_data);
--
1.8.3.1