[Freeipa-devel] [PATCHES][SSSD] Config monitoring patches

[Stephen Gallagher]

On 06/30/2009 02:53 PM, Simo Sorce wrote:
> On Wed, 2009-06-24 at 08:24 -0400, Stephen Gallagher wrote:
>> +        /* First, remove the old watch descriptor */
>> +        ret = inotify_rm_watch(file_ctx->fd, file_ctx->wd);
>> +        if (ret < 0) {
>> +            err = errno;
>> +            DEBUG(0, ("Could not remove watch descriptor [%s]",
>> +                      strerror(err)));
>> +        }
>> +
>> +        file_ctx->wd = inotify_add_watch(file_ctx->fd,
>> file_ctx->filename,
>> +                                         IN_MODIFY);
>> +        if (file_ctx->wd < 0) {
>> +            err = errno;
>> +            DEBUG(0, ("Could not add inotify watch for file [%s].
>> Error [%d:%s]\n",
>> +                      file_ctx->filename, err, strerror(err)));
>> +            close(file_ctx->fd);
>> +            kill(getpid(), SIGTERM);
>> +            return;
>> +        }
>>      }
> 
> Have to nack this.
> 
> As discussed on IRC the remove is unnecessary
> 
> But the problem is that here you give whatever editor (or human) just 1
> second to replace the file.
> Please allow more time maybe with a progressive delay scheme (wait
> 1,2,4,8,16 seconds) after X retries (I guess waiting for 30secs or so is
> fine) then give up and kill the daemon.
> 
> Haven't checked the rest yet, will do a review of other parts once this
> is fixed, unless you want me to review some spcific bits before you fix
> this issue.
> 
> Simo.
> 

Simo's review changes are incorporated into these new versions. Please
re-review.

I opted to just wait five seconds between retries for a total of 30
seconds. The progressive increase seemed like overkill, since I expect
it to be very rare that someone would be manually moving the config file
out of the way and then putting it back.

-- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Improvements-to-config-file-updates.patch
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090716/0ae875d6/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Monitor-resolv.conf-for-changes.patch
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090716/0ae875d6/attachment-0001.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0003-Implement-resInit-for-monitor-NSS-PAM-DP-and-the.patch
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090716/0ae875d6/attachment-0002.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090716/0ae875d6/attachment.sig>