[Openais] [Corosync] [Patch] [FreeBSD] coroipcc_service_connect() returns 0 in some cases

[Jerome Flesch]

Hello,

I just had a weird case where SaClmInitialize() returned the value 0 to 
me. Since this value isn't valid for a enum SaAisErrorT, I investigated 
and I found out that corosync/coroipcc.c:coroipcc_service_connect() can 
return 0 in case Corosync was compiled with _POSIX_THREAD_PROCESS_SHARED 
<= 0 (ie on a FreeBSD system) when there is a problem with the IPC 
semaphores. This case is actually likely to happen on a FreeBSD system 
due to IPC default limits.

In the process, I also changed the way the variable 'res' was used: 
Since it's an enum, I've assumed it doesn't really make sense to use it 
to store return values of system calls, so I replaced it by sys_res.

Also, I was wondering if there could be a way to get more details when 
coroipcc_service_connect() fails. CS_ERR_LIBRARY and CS_ERR_TRY_AGAIN 
are not exactly what I would call meaningful error codes ... It's not 
very handy to have to try to guess if it failed because permissions on 
the Unix socket are wrong or because IPC limits have been reached or 
because an upper layer failed for some reason. I assume adding error 
codes like CS_ERR_SEM, CS_ERR_SHM, could be an option but letting such 
level of implementation details be visible in the error codes is not a 
valid option, right ?

Right now, when I have to diagnose this kind of issue, I'm running a 
patched version of Flatiron including the following macro (in lib/util.h):

+#ifdef DEBUG
+#define DPRINTX(...) do {                                            \
+     fprintf(stderr, "Corosync lib: %s:L%d: ", __FILE__, __LINE__);  \
+     fprintf(stderr, __VA_ARGS__);                                   \
+     fprintf(stderr, "\n");                                          \
+   } while (0)
+#else
+#define DPRINTX(...)
+#endif

Is there a more adequate way of getting debug information from the 
libraries themselves ?

I was thinking of a slightly more elegant way of doing it. I could add 
functions like for example: 'void coroipcc_set_lasterror(const char 
*str)', 'const char *coroipcc_get_lasterror(void)' ? Using 
pthread_setspecific(), it would be possible to make it thread-safe. Do 
you think it would be worth I spend some time on making a patch for that ?



-------------- next part --------------
A non-text attachment was scrubbed...
Name: coroipcc_service_connect-res.patch
Type: text/x-patch
Size: 1892 bytes
Desc: not available
Url : http://lists.linux-foundation.org/pipermail/openais/attachments/20100329/10c4f8c9/attachment-0001.bin