On Fri, Apr 16, 2010 at 01:26:17PM +0200, Lars Marowsky-Bree wrote:
> On 2010-04-16T11:48:35, Lars Ellenberg <[email protected]> wrote:
> 
> > We may add the functionality back, in case anyone actually needs it.
> 
> We need it for ACL support in pacemaker.
> 
> > But certainly not breaking backwards ABI compatibility, if not
> > absolutely necessary. Which it is not for this thing.
> 
> Well, yes, breaking ABI compatibility while keeping the soname was
> definitely a bug.
> 
> > Question is: is this functionality actually going to be used,
> > or was it an experimental proof of concept but the CIB ACLs
> > will be implemented without them anyways?
> 
> It will be used. And for internal reasons, we have a very strong need to
> get the ABI sorted now, not in a few weeks.
> 
> What is the alternative to incrementing the soname? It's a really
> trivial change, is it not? 
> 
> For a libglue1 update to take effect, the admin needs to restart
> pacemaker et al anyway, so there's no real operational penalty to having
> to install consistent build packages for these too; the node is
> restarting already.
> 
> I understand that you seem to favor a new connection type value; what
> would that back-end implementation look like?

This is my proposal for now.
It keeps backwards ABI compatibility.

Old code works with old and new glue.
New code works with old and new glue.

New code depending on the new struct members get a define to check for
at compile time, and a runtime check just as well.

New code depending on the new struct members SHOULD request IPC_UDS_CRED
instead of IPC_DOMAIN_SOCKET or IPC_ANYTYPE, even though with new glue
they get the same struct back.

This is so they will notice cleanly when they accidentally are running
against old glue, and not notice by segfaulting uncontrollably somewhen
later.

This is still all hackish, but should work well enough.

Makes sense so far?


diff -r 848689af0fc9 include/clplumbing/ipc.h
--- a/include/clplumbing/ipc.h  Wed Apr 14 15:15:09 2010 +0200
+++ b/include/clplumbing/ipc.h  Thu Apr 15 15:28:08 2010 +0200
@@ -180,6 +178,17 @@
        int             conntype;
        
        char            failreason[MAXFAILREASON];
+
+       /* New members to support Multi-level ACLs for the CIB.
+        * As older libraries do not have these, they are
+        * added at the end of the struct to maintain ABI compatibility.
+        *
+        * Newer code MUST NOT assume these are there at runtime,
+        * unless IPC_UDS_CRED is used as ch_type to create this channel,
+        * which will fail with older runtime. */
+       /* FIXME we need proper library versioning and feature flags. */
+       uid_t           farside_uid;    /* far side uid */
+       gid_t           farside_gid;    /* far side gid */
 };
 
 struct IPC_QUEUE{
@@ -614,8 +623,12 @@
  *    the pointer to a new waiting connection or NULL if the connection
  *                     can't be created.
  * Note:
- *    current implementation only supports unix domain socket 
- *    whose type is IPC_DOMAIN_SOCKET 
+ *    current implementation supports
+ *    IPC_ANYTYPE:       alias for IPC_DOMAIN_SOCKET
+ *    IPC_DOMAIN_SOCKET: unix domain sockets
+ *    IPC_UDS_CRED:      unix domain sockets with support for
+ *                       farside uid + gid credential,
+ *                       not available with older libraries.
  *
  */
 extern IPC_WaitConnection * ipc_wait_conn_constructor(const char * ch_type
@@ -639,8 +652,12 @@
  *     or NULL if the channel can't be created.
  *
  * Note:
- *   current implementation only support unix domain socket 
- *   whose type is IPC_DOMAIN_SOCKET 
+ *    current implementation supports
+ *    IPC_ANYTYPE:       alias for IPC_DOMAIN_SOCKET
+ *    IPC_DOMAIN_SOCKET: unix domain sockets
+ *    IPC_UDS_CRED:      unix domain sockets with support for
+ *                       farside uid + gid credential,
+ *                       not available with older libraries.
  *
  */
 extern IPC_Channel  * ipc_channel_constructor(const char * ch_type
@@ -753,6 +770,10 @@
 
 #define        IPC_PATH_ATTR           "path"          /* pathname attribute */
 #define        IPC_DOMAIN_SOCKET       "uds"           /* Unix domain socket */
+/* Unix domain socket with farside uid + gid credentials.
+ * Pending a better way to do "feature flags",
+ * we just add an ascii tag for now. */
+#define        IPC_UDS_CRED            "uds_c"
 #define IPC_MODE_ATTR           "sockmode"      /* socket mode attribute */
 
 #ifdef IPC_DOMAIN_SOCKET
diff -r 848689af0fc9 lib/clplumbing/ocf_ipc.c
--- a/lib/clplumbing/ocf_ipc.c  Wed Apr 14 15:15:09 2010 +0200
+++ b/lib/clplumbing/ocf_ipc.c  Thu Apr 15 15:28:08 2010 +0200
@@ -62,6 +62,7 @@
 ipc_wait_conn_constructor(const char * ch_type, GHashTable* ch_attrs)
 {
   if (strcmp(ch_type, "domain_socket") == 0
+  ||   strcmp(ch_type, IPC_UDS_CRED) == 0
   ||   strcmp(ch_type, IPC_ANYTYPE) == 0
   ||   strcmp(ch_type, IPC_DOMAIN_SOCKET) == 0) {
     return socket_wait_conn_new(ch_attrs);
@@ -73,6 +74,7 @@
 ipc_channel_constructor(const char * ch_type, GHashTable* ch_attrs)
 {
   if   (strcmp(ch_type, "domain_socket") == 0
+  ||   strcmp(ch_type, IPC_UDS_CRED) == 0
   ||   strcmp(ch_type, IPC_ANYTYPE) == 0
   ||   strcmp(ch_type, IPC_DOMAIN_SOCKET) == 0) {
 

-- 
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to