Re: [pve-devel] [PATCH qemu-server 1/3] qmeventd: rework 'forced_cleanup' handling and set timeout to 60s

[Dominik Csapak <d.csapak@proxmox.com>]

[snip]
>> -/*
>> - * SIGALRM and cleanup handling
>> - *
>> - * terminate_client will set an alarm for 5 seconds and add its client's PID to
>> - * the forced_cleanups list - when the timer expires, we iterate the list and
>> - * attempt to issue SIGKILL to all processes which haven't yet stopped.
>> - */
>> -
>> -static void
>> -alarm_handler(__attribute__((unused)) int signum)
>> -{
>> -    alarm_triggered = 1;
>> -}
>> -
> 
> wasn't this intentionally decoupled like this?
> 
> alarm_handler just sets the flag
> actual force cleanup is conditionalized on the alarm having triggered,
> but the cleanup happens outside of the signal handler..
> 
> is there a reason from switching away from these scheme? we don't need
> to do the cleanup in the signal handler (timing is already plenty fuzzy
> anyway ;))

no real reason, i found the code somewhat cleaner, but you're right,
we probably want to keep that, and just trigger it regularly

> 
>>   static void
>>   sigkill(void *ptr, __attribute__((unused)) void *unused)
>>   {
>>       struct CleanupData data = *((struct CleanupData *)ptr);
>>       int err;
>>   
>> +    if (data.timeout > time(NULL)) {
> 
> nit: current time / cutoff could be passed in via the currently unused
> user_data parameter..
> 
make sense