[Pacemaker] [libqb]Unlink of files bound to sockets

Grüninger, Andreas (LGL Extern) Andreas.Grueninger at lgl.bwl.de
Tue Dec 23 09:29:20 UTC 2014


Thanks for the explanations.
I will change the pull request as supposed.

-----Ursprüngliche Nachricht-----
Von: David Vossel [mailto:dvossel at redhat.com] 
Gesendet: Montag, 22. Dezember 2014 22:25
An: The Pacemaker cluster resource manager
Betreff: Re: [Pacemaker] [libqb]Unlink of files bound to sockets



----- Original Message -----
> E.g. this 5 files are created for the sockets.
> srwxr-x--x 1 hacluster haclient       0 Dec 19 20:27
> qb-cib_ro-4444-13852-26-event
> srwxr-x--x 1 hacluster haclient       0 Dec 19 20:27
> qb-cib_ro-4444-13852-26-response
> -rw-rw---- 1 hacluster haclient      24 Dec 19 20:27
> qb-cib_ro-control-4444-13852-26
> srwxr-x--x 1 hacluster haclient       0 Dec 19 20:27
> qb-cib_ro-4444-13852-26-event-tx
> srwxr-x--x 1 hacluster haclient       0 Dec 19 20:27
> qb-cib_ro-4444-13852-26-request
> 
> How much should I find?
> 
> Where to find this trick?

I got side tracked. I'm sorry. This trick is for the shared memory we use for ipc flow control. This is not related to what you're seeing.
Now that I see your file list, the problem is much more obvious. Thanks for being patient with me.

> I will try hard to understand it.

You are experiencing a bug in libqb for only exists for non-linux systems.

I'll explain.

Linux has this non-portable concept of abstract sockets. This lets us create sockets without mapping directly to something on the filesystem. Unfortunately solaris doesn't have this feature. Somewhere along the line we introduced a file leak for solaris.

Your original patch is actually very close to being correct.

Take a look at this function.

https://github.com/davidvossel/libqb/blob/master/lib/ipc_socket.c#L53

That's where we set the sun_path.

for the '#if defined(QB_LINUX) || defined(QB_CYGWIN)' sun_path is actually being set to ='\0',  which indicates we want to use that abstract socket feature. I'm unsure why we continue with what appears to be a useless snprintf to sun_path+1.

For the #else we set the sun_path for systems like solaris. This maps the socket to a location on the filesystem like we'd expect, and an unlink will be required.

So, for your patch. A more complete solution would be to replace your #if QB_SOLARIS with #if !(defined(QB_LINUX) || defined(QB_CYGWIN)) 

Would you be interested in submitting that as a pull request to the libqb upstream code on github? I don't have access to a solaris machine at the moment to test with. Please run 'make check' to verify the change.

Thanks for the contribution.

-- Vossel


> -----Ursprüngliche Nachricht-----
> Von: David Vossel [mailto:dvossel at redhat.com]
> Gesendet: Freitag, 19. Dezember 2014 20:19
> An: The Pacemaker cluster resource manager
> Betreff: Re: [Pacemaker] [libqb]Unlink of files bound to sockets
> 
> 
> 
> ----- Original Message -----
> > I used the current trunk.
> > I could not find the unlink calls.
> > 
> > If domain sockets are used this two methods are used.
> > 
> > In ./lib/ipc_socket.c
> > static void
> > qb_ipcc_us_disconnect(struct qb_ipcc_connection *c) {
> >   munmap(c->request.u.us.shared_data, SHM_CONTROL_SIZE);
> >   unlink(c->request.u.us.shared_file_name);
> 
> right, so here we're doing the unlinking of the shared file.
> There's some trick we're using to only have a single file created for 
> all 3 sockets.
> 
> Is this not working for solaris?
> 
> 
> >   qb_ipcc_us_sock_close(c->event.u.us.sock);
> >   qb_ipcc_us_sock_close(c->request.u.us.sock);
> >   qb_ipcc_us_sock_close(c->setup.u.us.sock);
> > }
> > 
> > In ./lib/ipc_setup.c
> > void
> > qb_ipcc_us_sock_close(int32_t sock)
> > {
> > shutdown(sock, SHUT_RDWR);
> > close(sock);
> > }
> > 
> > I added in the latter the unlink calls.
> > 
> > -----Ursprüngliche Nachricht-----
> > Von: David Vossel [mailto:dvossel at redhat.com]
> > Gesendet: Donnerstag, 18. Dezember 2014 18:13
> > An: The Pacemaker cluster resource manager
> > Betreff: Re: [Pacemaker] [libqb]Unlink of files bound to sockets
> > 
> > 
> > 
> > ----- Original Message -----
> > > 
> > > 
> > > I sent yesterday this email to the mailing list of libq 
> > > 'quarterback-devel at lists.fedorahosted.org'.
> > > 
> > > But there is nearly no activity since august.
> > 
> > i saw the email. i flagged it so it would get a response.
> > 
> > > 
> > > I use the current trunk of libqb.
> > > 
> > > In qb_ipcc_us_sock_close nd qb_ipcs_us_withdraw of lib/ipc_setup.c 
> > > sockets are closed.
> > > 
> > > Is there a reason why the files bound to the sockets are not 
> > > deleted with unlink?
> > > 
> > > Is unlinking not necessary with Linux?
> > 
> > Unlinking is required for linux.
> > 
> > For client/server connections.
> > 
> > qb_ipcc_us_disconnect unlinks on the client side.
> > qb_ipcs_us_disconnect unlinks on the server side.
> > 
> > 
> > > I found thousands of files in statedir=/var/corosync/run after a while.
> > 
> > What version of corosync are you using? There were some reference 
> > leaks for ipc connections in the corosync code we fixed a year or so 
> > ago that should have fixed this.
> > 
> > -- David
> > 
> > > 
> > > 
> > > I tried this and it seems to work without errors.
> > > 
> > > 
> > > 
> > > e.g.
> > > 
> > > void
> > > 
> > > qb_ipcc_us_sock_close(int32_t sock)
> > > 
> > > {
> > > 
> > > #ifdef QB_SOLARIS
> > > 
> > > struct sockaddr_un un_addr;
> > > 
> > > socklen_t un_addr_len = sizeof(struct sockaddr_un);
> > > 
> > > #endif
> > > 
> > > shutdown(sock, SHUT_RDWR);
> > > 
> > > #ifdef QB_SOLARIS
> > > 
> > > if (getsockname(sock, (struct sockaddr *)&un_addr, &un_addr_len) 
> > > ==
> > > 0) {
> > > 
> > > if(strstr(un_addr.sun_path,"-") != NULL) {
> > > 
> > > qb_util_log(LOG_DEBUG, "un_addr.sun_path=%s", un_addr.sun_path);
> > > 
> > > unlink(un_addr.sun_path);
> > > 
> > > }
> > > 
> > > } else {
> > > 
> > > qb_util_log(LOG_DEBUG, "getsockname returned errno=%d", errno);
> > > 
> > > }
> > > 
> > > #endif
> > > 
> > > close(sock);
> > > 
> > > }
> > > 
> > > 
> > > 
> > > Regards
> > > 
> > > 
> > > 
> > > Andreas
> > > 
> > > _______________________________________________
> > > Pacemaker mailing list: Pacemaker at oss.clusterlabs.org 
> > > http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> > > 
> > > Project Home: http://www.clusterlabs.org Getting started:
> > > http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> > > Bugs: http://bugs.clusterlabs.org
> > > 
> > 
> > _______________________________________________
> > Pacemaker mailing list: Pacemaker at oss.clusterlabs.org 
> > http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> > 
> > Project Home: http://www.clusterlabs.org Getting started:
> > http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> > Bugs: http://bugs.clusterlabs.org
> > 
> > _______________________________________________
> > Pacemaker mailing list: Pacemaker at oss.clusterlabs.org 
> > http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> > 
> > Project Home: http://www.clusterlabs.org Getting started:
> > http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> > Bugs: http://bugs.clusterlabs.org
> > 
> 
> _______________________________________________
> Pacemaker mailing list: Pacemaker at oss.clusterlabs.org 
> http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> 
> Project Home: http://www.clusterlabs.org Getting started:
> http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> Bugs: http://bugs.clusterlabs.org
> _______________________________________________
> Pacemaker mailing list: Pacemaker at oss.clusterlabs.org 
> http://oss.clusterlabs.org/mailman/listinfo/pacemaker
> 
> Project Home: http://www.clusterlabs.org Getting started: 
> http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
> Bugs: http://bugs.clusterlabs.org
>

_______________________________________________
Pacemaker mailing list: Pacemaker at oss.clusterlabs.org http://oss.clusterlabs.org/mailman/listinfo/pacemaker

Project Home: http://www.clusterlabs.org Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf
Bugs: http://bugs.clusterlabs.org


More information about the Pacemaker mailing list