Jerome,

Thanks for the work. Few comments:

First, NAME_MAX was changed to FILENAME_MAX to match posix.  Also these
are defined in /usr/include/limits.h.  Doesn't bsd have this file? (I
recently fixed this problem for the Solaris port I did)  If it does,
then you shouldn't have to do any defining since this is now fixed in
current trunk.

I confirm, FILENAME_MAX is defined on FreeBSD (in stdio.h), so this part of my patch is actually not required. Do you want me to prepare a new one ?


I think the LDL part needs to be in some user defined flag in the
configure.ac.  The #if BSD with defining LDL is a bit hacky.  I'm not
sure where it should go however, so perhaps Jim or Fabio could comment.
If Jim or Fabio give that part the thumbs up, then this patch should be
good to go in.

the if defined semun wont work because unions are not defines.  That
entire code segment is wrong.  Can you try the attached patch on bsd for
me?

I think you misunderstood my problem: semun *is* already defined on FreeBSD, but the #if !defined(semun) doesn't work on my system (probably due to a GCC version difference ?) and the compiler ends up with a redefinition of semun.

Is there actually any system where semun is not already defined ?


I have merged the pf_local patch you provided.

I need a day to have a look at your refcounting patch for coroipc.

If it looks good to you, you may want to have a look at my patch for Whitetank. Both are actually the same :)


Thanks again!

Regards
-steve

On Tue, 2009-06-09 at 17:01 +0200, Jérôme Flesch wrote:
Hello,

Here are some patches that I made to fix BSD support in Corosync:

- patch-fix-bsd-compilation: Just reuse some flags used for Darwin, and make sure we don't compile with -ldl (included in the libc on FreeBSD)

- patch-fix-semun-def-on-local: "#if define(semun)" doesn't seem to work on my system (FreeBSD 7.2 / Gcc 4.2.1). However, I'm not sure if it's the best way to fix this problem :/

- patch-fix-refcount-for-bsd: It's an adaptation of my patch for Whitetank: The refcount isn't decremented correctly on FreeBSD at the disconnection of a client. On Whitetank, it created SHM leaks. On Corosync, it seems that it makes the CPU usage goes to 100%.

- patch-pf-local: I'm not sure if it really helps fixing the BSD support, but it makes it more compliant with the instructions from the BSD man pages.

These patchs make Corosync tests work on my workstation. However I'm not yet able to make it fully work with our programs, so it may not be the last patches for BSD.
plain text document attachment (patch-fix-bsd-compilation)
diff --git a/configure.ac b/configure.ac
index b87413e..7b73315 100644
--- configure.ac
+++ configure.ac
@@ -194,6 +194,7 @@ AC_ARG_WITH([socket-dir],
 # OS detection
 # THIS SECTION MUST DIE!
 CP=cp
+SYS_BSD="0"
 case "$host_os" in
        *linux*)
                AC_DEFINE_UNQUOTED([COROSYNC_LINUX], [1],
@@ -223,6 +224,12 @@ case "$host_os" in
                                   [Number of chars in a file name])
        ;;
        *bsd*)
+               AC_DEFINE_UNQUOTED([MAP_ANONYMOUS], [MAP_ANON],
+                                  [Shared memory define for Darwin platform])
+               AC_DEFINE_UNQUOTED([PATH_MAX], [4096],
+                                  [Number of chars in a path name including 
nul])
+               AC_DEFINE_UNQUOTED([NAME_MAX], [255],
+                                  [Number of chars in a file name])
                AC_DEFINE_UNQUOTED([COROSYNC_BSD], [1],
                                   [Compiling for BSD platform])
                OS_CFLAGS=""
@@ -230,6 +237,7 @@ case "$host_os" in
                OS_LDFLAGS="-L/usr/local/lib"
                OS_DYFLAGS="-export-dynamic"
                DARWIN_OPTS=""
+               SYS_BSD="1"
        ;;
        *solaris*)
                AC_DEFINE_UNQUOTED([COROSYNC_SOLARIS], [1],
@@ -360,6 +368,9 @@ AC_SUBST([NSS_LDFLAGS])
 AM_CONDITIONAL(BUILD_DARWIN, test -n "${DARWIN_OPTS}")
 AC_SUBST([DARWIN_OPTS])
+AM_CONDITIONAL(BUILD_BSD, test "${SYS_BSD}" -gt 0)
+
+
 AM_CONDITIONAL(BUILD_HTML_DOCS, test -n "${GROFF}")
AC_SUBST([LINT_FLAGS])
diff --git a/lib/Makefile.am b/lib/Makefile.am
index af3253d..9830d22 100644
--- lib/Makefile.am
+++ lib/Makefile.am
@@ -62,6 +62,12 @@ noinst_HEADERS               = sa-confdb.h util.h \
 ../lcr/lcr_ifact.o:
        $(MAKE) -C ../lcr lcr_ifact.o
+if BUILD_BSD
+       LDL = ""
+else
+       LDL = -ldl
+endif
+
 if BUILD_DARWIN
libcoroipcc.so.$(SONAME): coroipcc.o
@@ -71,7 +77,7 @@ libcoroipcc.so.$(SONAME): coroipcc.o
libconfdb.so.$(SONAME): confdb.o sa-confdb.o libcoroipcc.so.$(SONAME)
        $(CC) $(LDFLAGS) $(DARWIN_OPTS) coroipcc.o confdb.o \
-               sa-confdb.o ../lcr/lcr_ifact.o -o $@ -ldl $(AM_LDFLAGS)
+               sa-confdb.o ../lcr/lcr_ifact.o -o $@ $(LDL) $(AM_LDFLAGS)
        ln -sf libconfdb.so.$(SONAME) libconfdb.so
        ln -sf libconfdb.so.$(SONAME) libconfdb.so.$(SOMAJOR)
@@ -94,7 +100,7 @@ libconfdb.so.$(SONAME): confdb.o sa-confdb.o ../lcr/lcr_ifact.o libcoroipcc.so.$
        $(CC) -shared -o $@ \
                -Wl,-soname=libconfdb.so.$(SOMAJOR) \
                -Wl,-version-script=$(srcdir)/libconfdb.versions \
-               $^ $(LDFLAGS) $(OS_DYFLAGS) -ldl $(AM_LDFLAGS)
+               $^ $(LDFLAGS) $(OS_DYFLAGS) $(LDL) $(AM_LDFLAGS)
        ln -sf libconfdb.so.$(SONAME) libconfdb.so
        ln -sf libconfdb.so.$(SONAME) libconfdb.so.$(SOMAJOR)
plain text document attachment (patch-fix-refcount-for-bsd)
diff --git a/exec/coroipcs.c b/exec/coroipcs.c
index dcb4501..908e26d 100644
--- exec/coroipcs.c
+++ exec/coroipcs.c
@@ -1334,7 +1334,6 @@ int coroipcs_handler_dispatch (
                                res = 0;
                                break;
                        }
-                       coroipcs_refcount_dec (conn_info);
                }
 #if defined(COROSYNC_SOLARIS) || defined(COROSYNC_BSD) || 
defined(COROSYNC_DARWIN)
                /* On many OS poll never return POLLHUP or POLLERR.
@@ -1342,9 +1341,11 @@ int coroipcs_handler_dispatch (
                 */
                if (res == 0) {
                        ipc_disconnect (conn_info);
+                       coroipcs_refcount_dec (conn_info);
                        return (0);
                }
 #endif
+               coroipcs_refcount_dec (conn_info);
        }
coroipcs_refcount_inc (conn_info);
plain text document attachment (patch-fix-semun-def-on-bsd)
diff --git a/exec/coroipcs.c b/exec/coroipcs.c
index b50bd5a..1103f08 100644
--- exec/coroipcs.c
+++ exec/coroipcs.c
@@ -99,12 +99,14 @@ struct zcb_mapped {
        size_t size;
 };
+#if !defined(semun) && !defined(COROSYNC_BSD)
 union semun {
        int val;
        struct semid_ds *buf;
        unsigned short int *array;
        struct seminfo *__buf;
 };
+#endif
enum conn_state {
        CONN_STATE_THREAD_INACTIVE = 0,
diff --git a/lib/coroipcc.c b/lib/coroipcc.c
index 5f898dc..9bbcc52 100644
--- lib/coroipcc.c
+++ lib/coroipcc.c
@@ -251,7 +251,7 @@ priv_change_send (struct ipc_instance *ipc_instance)
        return (0);
 }
-#if !defined(semun)
+#if !defined(semun) && !defined(COROSYNC_BSD)
 union semun {
         int val;
         struct semid_ds *buf;
plain text document attachment (patch-pf-local)
diff --git a/exec/coroipcs.c b/exec/coroipcs.c
index 1103f08..3dca150 100644
--- exec/coroipcs.c
+++ exec/coroipcs.c
@@ -780,7 +780,7 @@ extern void coroipcs_ipc_init (
        /*
         * Create socket for IPC clients, name socket, listen for connections
         */
-       server_fd = socket (PF_UNIX, SOCK_STREAM, 0);
+       server_fd = socket (PF_LOCAL, SOCK_STREAM, 0);
        if (server_fd == -1) {
                api->log_printf ("Cannot create client connections socket.\n");
                api->fatal_error ("Can't create library listen socket");
@@ -795,7 +795,7 @@ extern void coroipcs_ipc_init (
        memset (&un_addr, 0, sizeof (struct sockaddr_un));
        un_addr.sun_family = AF_UNIX;
 #if defined(COROSYNC_BSD) || defined(COROSYNC_DARWIN)
-       un_addr.sun_len = sizeof(struct sockaddr_un);
+       un_addr.sun_len = SUN_LEN(&un_addr);
 #endif
#if defined(COROSYNC_LINUX)
@@ -807,7 +807,7 @@ extern void coroipcs_ipc_init (
res = bind (server_fd, (struct sockaddr *)&un_addr, COROSYNC_SUN_LEN(&un_addr));
        if (res) {
-               api->log_printf ("Could not bind AF_UNIX: %s.\n", strerror 
(errno));
+               api->log_printf ("Could not bind AF_UNIX (%s): %s.\n", 
un_addr.sun_path, strerror (errno));
                api->fatal_error ("Could not bind to AF_UNIX socket\n");
        }
        listen (server_fd, SERVER_BACKLOG);
diff --git a/lib/coroipcc.c b/lib/coroipcc.c
index 9bbcc52..10fb6e6 100644
--- lib/coroipcc.c
+++ lib/coroipcc.c
@@ -522,16 +522,16 @@ coroipcc_service_connect (
res_setup.error = CS_ERR_LIBRARY; - request_fd = socket (PF_UNIX, SOCK_STREAM, 0);
+       request_fd = socket (PF_LOCAL, SOCK_STREAM, 0);
        if (request_fd == -1) {
                return (CS_ERR_LIBRARY);
        }
memset (&address, 0, sizeof (struct sockaddr_un));
+       address.sun_family = AF_UNIX;
 #if defined(COROSYNC_BSD) || defined(COROSYNC_DARWIN)
-       address.sun_len = sizeof(struct sockaddr_un);
+       address.sun_len = SUN_LEN(&address);
 #endif
-       address.sun_family = PF_UNIX;
#if defined(COROSYNC_LINUX)
        sprintf (address.sun_path + 1, "%s", socket_name);
@@ -541,6 +541,9 @@ coroipcc_service_connect (
        sys_res = connect (request_fd, (struct sockaddr *)&address,
                COROSYNC_SUN_LEN(&address));
        if (sys_res == -1) {
+#ifdef DEBUG
+               fprintf(stderr, "Coroipcc: Can't connect: %d : %s\n", errno, 
strerror(errno));
+#endif
                close (request_fd);
                return (CS_ERR_TRY_AGAIN);
        }
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to