While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
bye, Sumit
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ? Do you have any other comment ? The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
Simo.
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
bye, Sumit
Simo.
-- Simo Sorce * Red Hat, Inc * New York
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Simo.
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Simo.
-- Simo Sorce * Red Hat, Inc * New York
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
Wouldn't it be more consistent to mandate that all signals are sent to monitor? That's also how it's documented in the man pages.
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote:
While checking if our custom signal handlers properly handle errno, I stumbled on a few cleanups, they are attached.
turns out our few signal hanlders are errno safe, and tevent signal handling function is also fine.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 From: Simo Sorce simo@redhat.com Date: Sat, 9 Nov 2013 16:11:04 -0500 Subject: [PATCH 2/3] Signals: Remove empty sig_hup
SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
(on a tangent: Can we please always resend all patches?)
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote:
On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: > While checking if our custom signal handlers properly handle errno, I > stumbled on a few cleanups, they are attached. > > turns out our few signal hanlders are errno safe, and tevent signal > handling function is also fine. > > Simo. > > -- > Simo Sorce * Red Hat, Inc * New York
> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 > From: Simo Sorce simo@redhat.com > Date: Sat, 9 Nov 2013 16:11:04 -0500 > Subject: [PATCH 2/3] Signals: Remove empty sig_hup > > SIGHUP handling is simply not implemented, so just block the signal instead of
it is, see te_server_hup() and the usage is documented in the sssd man page.
> using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
(on a tangent: Can we please always resend all patches?) _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: > On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: > > While checking if our custom signal handlers properly handle errno, I > > stumbled on a few cleanups, they are attached. > > > > turns out our few signal hanlders are errno safe, and tevent signal > > handling function is also fine. > > > > Simo. > > > > -- > > Simo Sorce * Red Hat, Inc * New York > > > > From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 > > From: Simo Sorce simo@redhat.com > > Date: Sat, 9 Nov 2013 16:11:04 -0500 > > Subject: [PATCH 2/3] Signals: Remove empty sig_hup > > > > SIGHUP handling is simply not implemented, so just block the signal instead of > > it is, see te_server_hup() and the usage is documented in the sssd man > page. > > > using a fake handler.
Ok does it mean I need to remove the part where I block the signal ?
no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
Do you have any other comment ?
no, but I still need to test them.
The sig_hup(int sig) function *was* useless ? Or is it there as a place holder until te_server_hup() takes over ?
no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
OK, pushed those three to master.
On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote: > On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: >> On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: >>> While checking if our custom signal handlers properly handle errno, I >>> stumbled on a few cleanups, they are attached. >>> >>> turns out our few signal hanlders are errno safe, and tevent signal >>> handling function is also fine. >>> >>> Simo. >>> >>> -- >>> Simo Sorce * Red Hat, Inc * New York >> >>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 >>> From: Simo Sorce simo@redhat.com >>> Date: Sat, 9 Nov 2013 16:11:04 -0500 >>> Subject: [PATCH 2/3] Signals: Remove empty sig_hup >>> >>> SIGHUP handling is simply not implemented, so just block the signal instead of >> it is, see te_server_hup() and the usage is documented in the sssd man >> page. >> >>> using a fake handler. > Ok does it mean I need to remove the part where I block the signal ? no, but you have to unblock it in server_setup() and if it is harmless to unblock a signal twice I would unblock it in monitor_process_init() before calling tevent_add_signal() as well.
> Do you have any other comment ? no, but I still need to test them.
> The sig_hup(int sig) function *was* useless ? Or is it there as a place > holder until te_server_hup() takes over ? no, I think the common patter here is to block everything in setup_signals() and then unblock the signal when the related tevent_add_signal() is called.
Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
OK, pushed those three to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is installed. These problem seem to be solved when patch 'Signals: Remove unused functions' is applied. Can we push it to 1-11 branch?
Compilation errors:
In file included from ../src/providers/ad/ad_srv.c:27:0: ../src/util/util.h:230:6: error: conflicting types for 'CatchChild' void CatchChild(void); ^ In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_srv.c:24: /usr/include/samba-4.0/samba_util.h:110:8: note: previous declaration of 'CatchChild' was here void (*CatchChild(void))(int); ^ In file included from ../src/providers/ad/ad_srv.c:27:0: ../src/util/util.h:231:6: error: conflicting types for 'CatchChildLeaveStatus' void CatchChildLeaveStatus(void); ^ In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_srv.c:24: /usr/include/samba-4.0/samba_util.h:115:8: note: previous declaration of 'CatchChildLeaveStatus' was here void (*CatchChildLeaveStatus(void))(int); ^ make[2]: *** [src/providers/ad/libsss_ad_la-ad_srv.lo] Error 1 make[2]: *** Waiting for unfinished jobs.... In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_subdomains.c:32: /usr/include/samba-4.0/samba_util.h:110:8: error: conflicting types for 'CatchChild' void (*CatchChild(void))(int); ^ In file included from ../src/providers/data_provider.h:38:0, from ../src/providers/dp_backend.h:25, from ../src/providers/ldap/sdap_async.h:29, from ../src/providers/ad/ad_subdomains.c:25: ../src/util/util.h:230:6: note: previous declaration of 'CatchChild' was here void CatchChild(void); ^ In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_subdomains.c:32: /usr/include/samba-4.0/samba_util.h:115:8: error: conflicting types for 'CatchChildLeaveStatus' void (*CatchChildLeaveStatus(void))(int); ^ In file included from ../src/providers/data_provider.h:38:0, from ../src/providers/dp_backend.h:25, from ../src/providers/ldap/sdap_async.h:29, from ../src/providers/ad/ad_subdomains.c:25: ../src/util/util.h:231:6: note: previous declaration of 'CatchChildLeaveStatus' was here void CatchChildLeaveStatus(void); ^ make[2]: *** [src/providers/ad/libsss_ad_la-ad_subdomains.lo] Error 1 In file included from ../src/providers/data_provider.h:38:0, from ../src/providers/dp_backend.h:25, from ../src/providers/ldap/sdap.h:25, from ../src/providers/ad/ad_domain_info.c:31: ../src/util/util.h:230:6: error: conflicting types for 'CatchChild' void CatchChild(void); ^ In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_domain_info.c:28: /usr/include/samba-4.0/samba_util.h:110:8: note: previous declaration of 'CatchChild' was here void (*CatchChild(void))(int); ^ In file included from ../src/providers/data_provider.h:38:0, from ../src/providers/dp_backend.h:25, from ../src/providers/ldap/sdap.h:25, from ../src/providers/ad/ad_domain_info.c:31: ../src/util/util.h:231:6: error: conflicting types for 'CatchChildLeaveStatus' void CatchChildLeaveStatus(void); ^ In file included from /usr/include/samba-4.0/ndr.h:30:0, from ../src/providers/ad/ad_domain_info.c:28: /usr/include/samba-4.0/samba_util.h:115:8: note: previous declaration of 'CatchChildLeaveStatus' was here void (*CatchChildLeaveStatus(void))(int);
On Mon, Jan 05, 2015 at 06:48:29PM +0100, Pavel Reichl wrote:
On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote:
On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote: >On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote: >>On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: >>>On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: >>>>While checking if our custom signal handlers properly handle errno, I >>>>stumbled on a few cleanups, they are attached. >>>> >>>>turns out our few signal hanlders are errno safe, and tevent signal >>>>handling function is also fine. >>>> >>>>Simo. >>>> >>>>-- >>>>Simo Sorce * Red Hat, Inc * New York >>> >>>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 >>>>From: Simo Sorce simo@redhat.com >>>>Date: Sat, 9 Nov 2013 16:11:04 -0500 >>>>Subject: [PATCH 2/3] Signals: Remove empty sig_hup >>>> >>>>SIGHUP handling is simply not implemented, so just block the signal instead of >>>it is, see te_server_hup() and the usage is documented in the sssd man >>>page. >>> >>>>using a fake handler. >>Ok does it mean I need to remove the part where I block the signal ? >no, but you have to unblock it in server_setup() and if it is harmless >to unblock a signal twice I would unblock it in monitor_process_init() >before calling tevent_add_signal() as well. > >>Do you have any other comment ? >no, but I still need to test them. > >>The sig_hup(int sig) function *was* useless ? Or is it there as a place >>holder until te_server_hup() takes over ? >no, I think the common patter here is to block everything in >setup_signals() and then unblock the signal when the related >tevent_add_signal() is called. Ok after looking more carefully at how we setup the tevent handlers for SIGHUP I just reverted the part of the patch that would block the signal, it makes no sense to block it just to revert the action a few microseconds later.
New patch 0002 attached.
Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
OK, pushed those three to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is installed. These problem seem to be solved when patch 'Signals: Remove unused functions' is applied. Can we push it to 1-11 branch?
I don't have a problem with pushing a patch that removes unused static functions, although Samba really shouldn't export non-namespaced functions.
Is that another manifestation of: https://bugzilla.samba.org/show_bug.cgi?id=11033 ?
pushed to sssd-1-11 btw: fea2d8c6aef70f1ba6f7528c261606eac4fcea1c
On 01/08/2015 10:24 AM, Jakub Hrozek wrote:
On Mon, Jan 05, 2015 at 06:48:29PM +0100, Pavel Reichl wrote:
On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote: > On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote: >> On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote: >>> On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: >>>> On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: >>>>> While checking if our custom signal handlers properly handle errno, I >>>>> stumbled on a few cleanups, they are attached. >>>>> >>>>> turns out our few signal hanlders are errno safe, and tevent signal >>>>> handling function is also fine. >>>>> >>>>> Simo. >>>>> >>>>> -- >>>>> Simo Sorce * Red Hat, Inc * New York >>>>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 >>>>> From: Simo Sorce simo@redhat.com >>>>> Date: Sat, 9 Nov 2013 16:11:04 -0500 >>>>> Subject: [PATCH 2/3] Signals: Remove empty sig_hup >>>>> >>>>> SIGHUP handling is simply not implemented, so just block the signal instead of >>>> it is, see te_server_hup() and the usage is documented in the sssd man >>>> page. >>>> >>>>> using a fake handler. >>> Ok does it mean I need to remove the part where I block the signal ? >> no, but you have to unblock it in server_setup() and if it is harmless >> to unblock a signal twice I would unblock it in monitor_process_init() >> before calling tevent_add_signal() as well. >> >>> Do you have any other comment ? >> no, but I still need to test them. >> >>> The sig_hup(int sig) function *was* useless ? Or is it there as a place >>> holder until te_server_hup() takes over ? >> no, I think the common patter here is to block everything in >> setup_signals() and then unblock the signal when the related >> tevent_add_signal() is called. > Ok after looking more carefully at how we setup the tevent handlers for > SIGHUP I just reverted the part of the patch that would block the > signal, it makes no sense to block it just to revert the action a few > microseconds later. > > New patch 0002 attached. Thank you. With this patch SSSD receives and handles all documented signals.
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
OK, pushed those three to master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is installed. These problem seem to be solved when patch 'Signals: Remove unused functions' is applied. Can we push it to 1-11 branch?
I don't have a problem with pushing a patch that removes unused static functions, although Samba really shouldn't export non-namespaced functions.
Is that another manifestation of: https://bugzilla.samba.org/show_bug.cgi?id=11033 ?
I think the root of the problem is same - samba provides in its public headers (ndr.h includes samba_util.h) declarations of its functions (void (*CatchChild(void))(int); void (*CatchChildLeaveStatus(void))(int);) which conflicts with SSSD function's (void CatchChild(void); void CatchChildLeaveStatus(void);)
pushed to sssd-1-11 btw: fea2d8c6aef70f1ba6f7528c261606eac4fcea1c _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (08/01/15 11:10), Pavel Reichl wrote:
On 01/08/2015 10:24 AM, Jakub Hrozek wrote:
On Mon, Jan 05, 2015 at 06:48:29PM +0100, Pavel Reichl wrote:
On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote: >On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote: >>On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote: >>>On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote: >>>>On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: >>>>>On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: >>>>>>While checking if our custom signal handlers properly handle errno, I >>>>>>stumbled on a few cleanups, they are attached. >>>>>> >>>>>>turns out our few signal hanlders are errno safe, and tevent signal >>>>>>handling function is also fine. >>>>>> >>>>>>Simo. >>>>>> >>>>>>-- >>>>>>Simo Sorce * Red Hat, Inc * New York >>>>>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 >>>>>>From: Simo Sorce simo@redhat.com >>>>>>Date: Sat, 9 Nov 2013 16:11:04 -0500 >>>>>>Subject: [PATCH 2/3] Signals: Remove empty sig_hup >>>>>> >>>>>>SIGHUP handling is simply not implemented, so just block the signal instead of >>>>>it is, see te_server_hup() and the usage is documented in the sssd man >>>>>page. >>>>> >>>>>>using a fake handler. >>>>Ok does it mean I need to remove the part where I block the signal ? >>>no, but you have to unblock it in server_setup() and if it is harmless >>>to unblock a signal twice I would unblock it in monitor_process_init() >>>before calling tevent_add_signal() as well. >>> >>>>Do you have any other comment ? >>>no, but I still need to test them. >>> >>>>The sig_hup(int sig) function *was* useless ? Or is it there as a place >>>>holder until te_server_hup() takes over ? >>>no, I think the common patter here is to block everything in >>>setup_signals() and then unblock the signal when the related >>>tevent_add_signal() is called. >>Ok after looking more carefully at how we setup the tevent handlers for >>SIGHUP I just reverted the part of the patch that would block the >>signal, it makes no sense to block it just to revert the action a few >>microseconds later. >> >>New patch 0002 attached. >Thank you. With this patch SSSD receives and handles all documented >signals. > >ACK > >While testing I came across an oddness which we might want to fix. The >sssd_be processes can receive SIGUSR1 individually and go offline. But >the SIGUSR2 is not handled by sssd_be directly but only by the monitor >which then send a reset offline request to all backends. Shall sssd_be >handle SIGUSR2 as well? > >bye, >Sumit Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes
bye, Sumit
OK, pushed those three to master.
Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is installed. These problem seem to be solved when patch 'Signals: Remove unused functions' is applied. Can we push it to 1-11 branch?
I don't have a problem with pushing a patch that removes unused static functions, although Samba really shouldn't export non-namespaced functions.
Is that another manifestation of: https://bugzilla.samba.org/show_bug.cgi?id=11033 ?
I think the root of the problem is same - samba provides in its public headers (ndr.h includes samba_util.h) declarations of its functions (void (*CatchChild(void))(int); void (*CatchChildLeaveStatus(void))(int);) which conflicts with SSSD function's (void CatchChild(void); void CatchChildLeaveStatus(void);)
Actually, our file src/util/signal.c was copied from samba code. @see header of that file. So we should have used prefixed version as well.
LS
On Thu, Jan 08, 2015 at 11:18:23AM +0100, Lukas Slebodnik wrote:
On (08/01/15 11:10), Pavel Reichl wrote:
On 01/08/2015 10:24 AM, Jakub Hrozek wrote:
On Mon, Jan 05, 2015 at 06:48:29PM +0100, Pavel Reichl wrote:
On 11/12/2013 11:32 AM, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 11:15:17AM +0100, Sumit Bose wrote:
On Tue, Nov 12, 2013 at 10:45:48AM +0100, Jakub Hrozek wrote: >On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote: >>On Mon, Nov 11, 2013 at 02:37:39PM -0500, Simo Sorce wrote: >>>On Mon, 2013-11-11 at 17:33 +0100, Sumit Bose wrote: >>>>On Mon, Nov 11, 2013 at 08:56:12AM -0500, Simo Sorce wrote: >>>>>On Mon, 2013-11-11 at 09:20 +0100, Sumit Bose wrote: >>>>>>On Sat, Nov 09, 2013 at 04:26:36PM -0500, Simo Sorce wrote: >>>>>>>While checking if our custom signal handlers properly handle errno, I >>>>>>>stumbled on a few cleanups, they are attached. >>>>>>> >>>>>>>turns out our few signal hanlders are errno safe, and tevent signal >>>>>>>handling function is also fine. >>>>>>> >>>>>>>Simo. >>>>>>> >>>>>>>-- >>>>>>>Simo Sorce * Red Hat, Inc * New York >>>>>>> From 8244f7619c5042dc45751ce3bbff75f2dbc03e05 Mon Sep 17 00:00:00 2001 >>>>>>>From: Simo Sorce simo@redhat.com >>>>>>>Date: Sat, 9 Nov 2013 16:11:04 -0500 >>>>>>>Subject: [PATCH 2/3] Signals: Remove empty sig_hup >>>>>>> >>>>>>>SIGHUP handling is simply not implemented, so just block the signal instead of >>>>>>it is, see te_server_hup() and the usage is documented in the sssd man >>>>>>page. >>>>>> >>>>>>>using a fake handler. >>>>>Ok does it mean I need to remove the part where I block the signal ? >>>>no, but you have to unblock it in server_setup() and if it is harmless >>>>to unblock a signal twice I would unblock it in monitor_process_init() >>>>before calling tevent_add_signal() as well. >>>> >>>>>Do you have any other comment ? >>>>no, but I still need to test them. >>>> >>>>>The sig_hup(int sig) function *was* useless ? Or is it there as a place >>>>>holder until te_server_hup() takes over ? >>>>no, I think the common patter here is to block everything in >>>>setup_signals() and then unblock the signal when the related >>>>tevent_add_signal() is called. >>>Ok after looking more carefully at how we setup the tevent handlers for >>>SIGHUP I just reverted the part of the patch that would block the >>>signal, it makes no sense to block it just to revert the action a few >>>microseconds later. >>> >>>New patch 0002 attached. >>Thank you. With this patch SSSD receives and handles all documented >>signals. >> >>ACK >> >>While testing I came across an oddness which we might want to fix. The >>sssd_be processes can receive SIGUSR1 individually and go offline. But >>the SIGUSR2 is not handled by sssd_be directly but only by the monitor >>which then send a reset offline request to all backends. Shall sssd_be >>handle SIGUSR2 as well? >> >>bye, >>Sumit >Which patches should be pushed? #1 and #3 from the original mail and #2 >from the follow up? yes
bye, Sumit
OK, pushed those three to master.
Hello, I'm having some troubles compiling SSSD 1-11 when Samba 4.1.14-1 is installed. These problem seem to be solved when patch 'Signals: Remove unused functions' is applied. Can we push it to 1-11 branch?
I don't have a problem with pushing a patch that removes unused static functions, although Samba really shouldn't export non-namespaced functions.
Is that another manifestation of: https://bugzilla.samba.org/show_bug.cgi?id=11033 ?
I think the root of the problem is same - samba provides in its public headers (ndr.h includes samba_util.h) declarations of its functions (void (*CatchChild(void))(int); void (*CatchChildLeaveStatus(void))(int);) which conflicts with SSSD function's (void CatchChild(void); void CatchChildLeaveStatus(void);)
Actually, our file src/util/signal.c was copied from samba code. @see header of that file. So we should have used prefixed version as well.
I don't think static functions need to be prefixed.
The real problem was someone included an internal header in a public one, it seems.
On Mon, Jan 12, 2015 at 04:34:03PM +0100, Pavel Reichl wrote:
On 01/08/2015 10:24 AM, Jakub Hrozek wrote:
pushed to sssd-1-11 btw: fea2d8c6aef70f1ba6f7528c261606eac4fcea1c
I'm sorry, but I am a little confused here. The patch doesn't seem to be pushed in 1.11, if you pushed it where did you pushed it to? Thanks!
apparently I stopped at the dru-run earlier. Pushed now.
On Tue, 2013-11-12 at 10:45 +0100, Jakub Hrozek wrote:
On Tue, Nov 12, 2013 at 10:27:48AM +0100, Sumit Bose wrote:
ACK
While testing I came across an oddness which we might want to fix. The sssd_be processes can receive SIGUSR1 individually and go offline. But the SIGUSR2 is not handled by sssd_be directly but only by the monitor which then send a reset offline request to all backends. Shall sssd_be handle SIGUSR2 as well?
bye, Sumit
Which patches should be pushed? #1 and #3 from the original mail and #2 from the follow up?
yes.
(on a tangent: Can we please always resend all patches?)
Sorry about that, I thought of sending them all, and then ...
(on a tangent: Can we please always trim the quoting when replying to emails, we have some mails on this list that are thousands of line long just to comment on a single phrase all the way down at the end) SCNR :)
Simo.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Pavel Reichl -
Simo Sorce -
Sumit Bose