From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Beekhof Date: Fri, 23 Oct 2009 15:05:53 +0200 Subject: [Cluster-devel] Re: [RFC PATCH] dlm: enhancing dlm_controld (pcmk) to be able to handle redundant rings In-Reply-To: <20091023103645.GA6438@linux-jjzhang> References: <20091023103645.GA6438@linux-jjzhang> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Fri, Oct 23, 2009 at 12:38 PM, jjzhang wrote: > , Lars Marowsky-Bree > 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 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 > --- > > ?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 ? ? ? plock ownership drop resources age (milliseconds)\n"); > ? ? ? ?printf(" ? ? ? ? ? ? ? ?Default is %u\n", DEFAULT_DROP_RESOURCES_AGE); > + ? ? ? printf(" ?-r ? ? ?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 > ?#include > > +#include > +#include > + > ?#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; > ?} > >