Bug 784178

Summary:	lib_init_fn return codes are ignored
Product:	[Retired] Corosync Cluster Engine	Reporter:	Fabio Massimo Di Nitto <fdinitto>
Component:	unknown	Assignee:	Angus Salkeld <asalkeld>
Status:	CLOSED CURRENTRELEASE	QA Contact:
Severity:	high	Docs Contact:
Priority:	high
Version:	1.4	CC:	asalkeld, jfriesse, sdake
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	All
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-06-13 00:03:49 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

Description Fabio Massimo Di Nitto 2012-01-24 06:02:49 UTC

the prototype for lib_init_fn allows to return errors when a client -> lib -> IPC -> service connection fails/should be rejected, but at IPC level (within corosync exec/ipc_glue.c) the return code for lib_init_fn is ignored.

There is no warning or no failure propagation.

This removes completely the possibility for a service to report failures down the line when creating new connections to the service.

The client would always be able to connect, while in reality the service might have failed to allocate resources to serve those requests and there is no sign/track of it anywhere.

Comment 1 Angus Salkeld 2012-01-30 23:03:23 UTC

You say that but there is not a single init function that
does that (all do very basic init and return 0).

Any memory allocation for the connection should be contained in
the context (accessed via api->ipc_private_data_get (conn)) this
is refcounted and allocated/freed by the ipc system.

I would say a bigger problem is the possible failures in
cs_ipcs_connection_created(), as there is quite a bit
going on there.

Comment 2 Fabio Massimo Di Nitto 2012-01-31 04:59:49 UTC

(In reply to comment #1)
> You say that but there is not a single init function that
> does that (all do very basic init and return 0).

That's because I had to change votequorum and plugin loading service
to work around this problem and be able to move forward.
It is still incorrect to ignore the return code and future code might
require this to work as expected. Alternatively change lib_init_fn
prototype to void, at least it's clear that a plugin must be able
to handle an incoming connection (tho I think this is the wrong approach).

Comment 3 Steven Dake 2012-01-31 14:21:41 UTC

the return code should be handled
memory should not be allocated in lib_init_fn in general - this is what private data is for
There may be cases where a lib_init function calls an init that allocates memory - nothing we can do about that and need to handle these failures

While this is more like an rfe, it seems like the right thing to do.

Regards
-steve

Comment 4 Angus Salkeld 2012-02-03 02:07:09 UTC

Here is the patch that fixes the issue:

https://github.com/corosync/corosync/commit/45cb05f1add8d9983777f5e040232b05cc95d0c5

For best results it needs:
https://github.com/asalkeld/libqb/commit/7d9df9ef74b8f206f62ff260510f01fcb99208a1