[CRIU] [PATCH] service: rotate log by SIGHUP

[Ruslan Kuprieiev]

On 11.02.2014 12:11, Vladimir Davydov wrote:
> On 02/11/2014 11:49 AM, Ruslan Kuprieiev wrote:
>> On 11.02.2014 11:28, Vladimir Davydov wrote:
>>> On 02/11/2014 11:11 AM, Ruslan Kuprieiev wrote:
>>>> Because we wan't service to work non-stop.
>>>> And, as far as I know, by just using logrotate we will need to restart
>>>> service.
>>> Not necessarily, you can specify what should be done after rotate
>>> (`postrotate'), e.g. send a signal to your service so that it could
>>> reopen the logfile. However, I guess that `copytruncate' would be enough
>>> in your case.
>> But what if service will write something between the moment when
>> logrotate will copy log and the moment when it will send signal to
>> service?
> Oh, if your logs are so valuable that loosing a message or two once a
> day/week/month is a disaster, then logrotate is, of course, not an
> option for you, sorry.
=)
> However, from what I see you have the same race in your implementation:
> what if somebody prints to log while you're renaming it (after
> log_fini(), but before log_init())? Errors will go to stderr, which
> should be closed for a daemon?

In theory, no one, except service daemon, can write to that log.(In 
theory, because we are not shutting down log for children, but it needs 
to be done soon).
And as it rotate log in signal handler, service can't write to log, 
while signal handler is working.

>>>> On 11.02.2014 10:59, Vladimir Davydov wrote:
>>>>> Sorry for butting in, but why don't you use logrotate(8)?
>>>>>
>>>>> On 02/11/2014 10:41 AM, Ruslan Kuprieiev wrote:
>>>>>> Many existing daemons rotate logs by SIGHUP, so lets do the same
>>>>>> with our service.
>>>>>>
>>>>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>>>>> ---
>>>>>>     cr-service.c | 30 ++++++++++++++++++++++++++----
>>>>>>     1 file changed, 26 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/cr-service.c b/cr-service.c
>>>>>> index 26dd3e2..49454e7 100644
>>>>>> --- a/cr-service.c
>>>>>> +++ b/cr-service.c
>>>>>> @@ -489,12 +489,21 @@ static void reap_worker(int signo)
>>>>>>         }
>>>>>>     }
>>>>>>     -static int setup_sigchld_handler()
>>>>>> +static void rotate_log(int signo)
>>>>>> +{
>>>>>> +    pr_info("Rotating log.\n");
>>>>>> +
>>>>>> +    if (log_rotate())
>>>>>> +        exit(1);
>>>>>> +}
>>>>>> +
>>>>>> +static int setup_signal_handlers()
>>>>>>     {
>>>>>>         struct sigaction action;
>>>>>>           sigemptyset(&action.sa_mask);
>>>>>>         sigaddset(&action.sa_mask, SIGCHLD);
>>>>>> +    sigaddset(&action.sa_mask, SIGHUP);
>>>>>>         action.sa_handler    = reap_worker;
>>>>>>         action.sa_flags        = SA_RESTART;
>>>>>>     @@ -503,15 +512,23 @@ static int setup_sigchld_handler()
>>>>>>             return -1;
>>>>>>         }
>>>>>>     +    action.sa_handler    = rotate_log;
>>>>>> +
>>>>>> +    if (sigaction(SIGHUP, &action, NULL)) {
>>>>>> +        pr_perror("Can't setup SIGUSR1 handler");
>>>>>> +        return -1;
>>>>>> +    }
>>>>>> +
>>>>>>         return 0;
>>>>>>     }
>>>>>>     -static int restore_sigchld_handler()
>>>>>> +static int restore_signal_handlers()
>>>>>>     {
>>>>>>         struct sigaction action;
>>>>>>           sigemptyset(&action.sa_mask);
>>>>>>         sigaddset(&action.sa_mask, SIGCHLD);
>>>>>> +    sigaddset(&action.sa_mask, SIGHUP);
>>>>>>         action.sa_handler    = SIG_DFL;
>>>>>>         action.sa_flags        = SA_RESTART;
>>>>>>     @@ -520,6 +537,11 @@ static int restore_sigchld_handler()
>>>>>>             return -1;
>>>>>>         }
>>>>>>     +    if (sigaction(SIGHUP, &action, NULL)) {
>>>>>> +        pr_perror("Can't restore SIGHUP handler");
>>>>>> +        return -1;
>>>>>> +    }
>>>>>> +
>>>>>>         return 0;
>>>>>>     }
>>>>>>     @@ -596,7 +618,7 @@ int cr_service(bool daemon_mode)
>>>>>>             }
>>>>>>         }
>>>>>>     -    if (setup_sigchld_handler())
>>>>>> +    if (setup_signal_handlers())
>>>>>>             goto err;
>>>>>>           while (1) {
>>>>>> @@ -615,7 +637,7 @@ int cr_service(bool daemon_mode)
>>>>>>             if (child_pid == 0) {
>>>>>>                 int ret;
>>>>>>     -            if (restore_sigchld_handler())
>>>>>> +            if (restore_signal_handlers())
>>>>>>                     exit(1);
>>>>>>                   close(server_fd);