[Cluster-devel] Re: [RFC PATCH] dlm: enhancing dlm_controld (pcmk) to be able to handle redundant rings

[Andrew Beekhof <andrew@beekhof.net>]

On Fri, Oct 23, 2009 at 12:38 PM, jjzhang <jjzhang.linux@gmail.com> wrote:
> <andrew@beekhof.net>, Lars Marowsky-Bree <lmb@suse.de>
> Cc: cluster-devel at redhat.com
> Bcc:
> Subject: Re: [Cluster-devel] Re: [RFC PATCH] dlm: enhancing dlm_controld
> ? ? ? ?(pcmk) to be able to handle redundant rings
> Reply-To:
> In-Reply-To: <20091016101722.GZ15975@suse.de>
>
> On Fri, Oct 16, 2009 at 12:17:22PM +0200, Lars Marowsky-Bree wrote:
>> On 2009-10-15T09:51:08, David Teigland <teigland@redhat.com> wrote:
>>
>> > On Thu, Oct 15, 2009 at 09:43:56AM +0200, Andrew Beekhof wrote:
>> > > Looks good to me :-)
>> > > Although it might be nice to have the automated detection in place too.
>> > >
>> > > Less things for the admin to get wrong. ?What do you think Dave?
>> >
>> > I don't mind, now that I've taken a moment to think how it should work...
>> > There should be three protocol options, "tcp", "sctp", "detect". ?Detect
>> > should work by reading the totem/rrp_mode value in setup_ccs().
>>
>> Can we then also agree on "detect" being the default?
>>
>> (Experience shows that admins will misconfigure just about anything if
>> they have to deviate from the defaults.)
>
> Hello,
>
> Below is the improved patch. I have set the "default" as the same logic as
> "detect", but I'm not sure if this is OK.

I consider that to be the ideal default.

>
> Thanks a lot for your kindly review and comments :)

Looks good to me.
Unless anyone objects I'd like to apply this.

 (Can you resend as an attachment, inline patches have a habit of
getting mangled)

> Signed-off-by: Jiaju Zhang <jjzhang.linux@gmail.com>
> ---
>
> ?configure.ac ? ? ? ? ? ? ? ? ? | ? ?1 +
> ?group/dlm_controld/Makefile.am | ? ?4 ++-
> ?group/dlm_controld/main.c ? ? ?| ? ?9 ++++++-
> ?group/dlm_controld/pacemaker.c | ? 48 ++++++++++++++++++++++++++++++++++++++++
> ?4 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index d50de87..e0a1a0d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -131,6 +131,7 @@ if test "x${enable_pacemaker}" = xyes; then
> ? ? ? ?PKG_CHECK_MODULES([totempg],[libtotem_pg])
> ? ? ? ?PKG_CHECK_MODULES([xml],[libxml-2.0])
> ? ? ? ?PKG_CHECK_MODULES([glib],[glib-2.0])
> + ? ? ? PKG_CHECK_MODULES([cfg],[libcfg])
> ? ? ? ?check_lib_no_libs cib cib_new
> ? ? ? ?check_lib_no_libs crmcluster crm_set_status_callback
> ? ? ? ?check_lib_no_libs crmcommon init_server_ipc_comms
> diff --git a/group/dlm_controld/Makefile.am b/group/dlm_controld/Makefile.am
> index c14ab89..ce6e587 100644
> --- a/group/dlm_controld/Makefile.am
> +++ b/group/dlm_controld/Makefile.am
> @@ -62,12 +62,14 @@ dlm_controld_pcmk_CPPFLAGS= $(shared_CPPFLAGS) \
> ?dlm_controld_pcmk_CFLAGS ?= $(shared_CFLAGS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?$(glib_CFLAGS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?$(xml_CFLAGS) \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? $(totempg_CFLAGS)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? $(totempg_CFLAGS) \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? $(cfg_CFLAGS)
>
> ?dlm_controld_pcmk_LDFLAGS = $(shared_LIBS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?$(glib_LIBS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?$(xml_LIBS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?$(totempg_LIBS) \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? $(cfg_LIBS) \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?-lcib -lcrmcommon -lcrmcluster
>
> ?dlm_controld_pcmk_LDADD ? ? ? ? ?= $(shared_LDADD)
> diff --git a/group/dlm_controld/main.c b/group/dlm_controld/main.c
> index f90cd21..8949d11 100644
> --- a/group/dlm_controld/main.c
> +++ b/group/dlm_controld/main.c
> @@ -1058,11 +1058,13 @@ static void print_usage(void)
> ? ? ? ?printf(" ? ? ? ? ? ? ? ?Default is %u\n", DEFAULT_DROP_RESOURCES_COUNT);
> ? ? ? ?printf(" ?-a <ms> ? ? ? plock ownership drop resources age (milliseconds)\n");
> ? ? ? ?printf(" ? ? ? ? ? ? ? ?Default is %u\n", DEFAULT_DROP_RESOURCES_AGE);
> + ? ? ? printf(" ?-r <num> ? ? ?DLM in-kernel communication protocols: TCP(0), SCTP(1), DETECT(2)\n");
> + ? ? ? printf(" ? ? ? ? ? ? ? ?Default is auto-detect (2)");
> ? ? ? ?printf(" ?-h ? ? ? ? ? ?Print this help, then exit\n");
> ? ? ? ?printf(" ?-V ? ? ? ? ? ?Print program version information, then exit\n");
> ?}
>
> -#define OPTION_STRING "LDKf:q:d:p:Pl:o:t:c:a:hV"
> +#define OPTION_STRING "LDKf:q:d:p:Pl:o:t:c:a:r:hV"
>
> ?static void read_arguments(int argc, char **argv)
> ?{
> @@ -1142,6 +1144,11 @@ static void read_arguments(int argc, char **argv)
> ? ? ? ? ? ? ? ? ? ? ? ?cfgd_drop_resources_age = atoi(optarg);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> + ? ? ? ? ? ? ? case 'r':
> + ? ? ? ? ? ? ? ? ? ? ? optk_protocol = 1;
> + ? ? ? ? ? ? ? ? ? ? ? cfgk_protocol = atoi(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> ? ? ? ? ? ? ? ?case 'h':
> ? ? ? ? ? ? ? ? ? ? ? ?print_usage();
> ? ? ? ? ? ? ? ? ? ? ? ?exit(EXIT_SUCCESS);
> diff --git a/group/dlm_controld/pacemaker.c b/group/dlm_controld/pacemaker.c
> index 810c644..b3b147a 100644
> --- a/group/dlm_controld/pacemaker.c
> +++ b/group/dlm_controld/pacemaker.c
> @@ -22,7 +22,42 @@
> ?#include <pacemaker/crm/msg_xml.h>
> ?#include <pacemaker/crm/cib.h>
>
> +#include <corosync/corotypes.h>
> +#include <corosync/cfg.h>
> +
> ?#define COMMS_DIR ? ? "/sys/kernel/config/dlm/cluster/comms"
> +#define PROTO_TCP ? ? 0
> +#define PROTO_SCTP ? ?1
> +#define PROTO_DETECT ?2
> +
> +static int get_ring_count(void)
> +{
> + ? ?cs_error_t result;
> + ? ?corosync_cfg_handle_t handle;
> + ? ?unsigned int interface_count;
> + ? ?char **interface_names;
> + ? ?char **interface_status;
> +
> + ? ?result = corosync_cfg_initialize(&handle, NULL);
> + ? ?if (result != CS_OK) {
> + ? ? ? log_error("Failed to initialize corosync configuration API (error=%d)", result);
> + ? ? ? (void)corosync_cfg_finalize(handle);
> + ? ? ? return -1;
> + ? ?}
> +
> + ? ?result = corosync_cfg_ring_status_get(handle,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &interface_names,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &interface_status,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &interface_count);
> + ? ?if (result != CS_OK) {
> + ? ? ? log_error("Failed to get the ring status (error=%d)", result);
> + ? ? ? (void)corosync_cfg_finalize(handle);
> + ? ? ? return -1;
> + ? ?}
> +
> + ? ?(void)corosync_cfg_finalize(handle);
> + ? ?return interface_count;
> +}
>
> ?int setup_ccs(void)
> ?{
> @@ -30,6 +65,19 @@ int setup_ccs(void)
> ? ? ?* only allow configuration from the command-line until CoroSync is stable
> ? ? ?* enough to be used with Pacemaker
> ? ? ?*/
> + ? ?if (cfgk_protocol != -1 && cfgk_protocol != PROTO_TCP &&
> + ? ? ? cfgk_protocol != PROTO_SCTP && cfgk_protocol != PROTO_DETECT) {
> + ? ? ? log_error("ignore invalid value %d for dlm protocol", cfgk_protocol);
> + ? ? ? cfgk_protocol = -1;
> + ? ?}
> +
> + ? ?if (cfgk_protocol == PROTO_DETECT || cfgk_protocol == -1) {
> + ? ? ? if (get_ring_count() > 1)
> + ? ? ? ? ? cfgk_protocol = PROTO_SCTP;
> + ? ? ? else
> + ? ? ? ? ? cfgk_protocol = PROTO_TCP;
> + ? ?}
> +
> ? ? return 0;
> ?}
>
>