From a03d90abe1d3ac67bc5492db09e2c6e207288469 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 11 Mar 2009 15:26:51 +0100 Subject: try to make file writer restartable (experimental, untested) --- tools/omfile.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'tools/omfile.c') diff --git a/tools/omfile.c b/tools/omfile.c index 1539ae19..4bef5dbf 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -621,20 +621,21 @@ again: */ if((pData->fileType == eTypeTTY || pData->fileType == eTypeCONSOLE) #ifdef linux - && e == EIO) { + && e == EIO #else - && e == EBADF) { + && e == EBADF #endif + ) { pData->fd = open((char*) pData->f_fname, O_WRONLY|O_APPEND|O_NOCTTY); if (pData->fd < 0) { - iRet = RS_RET_DISABLE_ACTION; + iRet = RS_RET_SUSPENDED; errmsg.LogError(0, NO_ERRCODE, "%s", pData->f_fname); } else { untty(); goto again; } } else { - iRet = RS_RET_DISABLE_ACTION; + iRet = RS_RET_SUSPENDED; errno = e; errmsg.LogError(0, NO_ERRCODE, "%s", pData->f_fname); } -- cgit From f289422585ef36c39c5bc22ea20344d0c76b29ab Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 11 Mar 2009 19:54:30 +0100 Subject: changed one more status setting user feedback indicates it now looks like it is working ;) still some more work needed for a "good" solution --- tools/omfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/omfile.c') diff --git a/tools/omfile.c b/tools/omfile.c index 4bef5dbf..28bdcf2e 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -562,7 +562,7 @@ static rsRetVal writeFile(uchar **ppString, unsigned iMsgOpts, instanceData *pDa */ if(pData->bDynamicName) { if(prepareDynFile(pData, ppString[1], iMsgOpts) != 0) - ABORT_FINALIZE(RS_RET_ERR); + ABORT_FINALIZE(RS_RET_SUSPENDED); // TODO: different state? conditional based on what went wrong? 2009-03-11 } else if(pData->fd == -1) { prepareFile(pData, pData->f_fname); } -- cgit From 3762d2b7bc669e45d51a2157158e3cc925dd0152 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 17 Mar 2009 18:26:15 +0100 Subject: fixed some message-loss situations when file write failures happened --- tools/omfile.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'tools/omfile.c') diff --git a/tools/omfile.c b/tools/omfile.c index 28bdcf2e..8be815f2 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -600,20 +600,14 @@ again: } } - if (write(pData->fd, ppString[0], strlen((char*)ppString[0])) < 0) { + if(write(pData->fd, ppString[0], strlen((char*)ppString[0])) < 0) { int e = errno; +dbgprintf("++++++++++ log file writer error %d\n", e); /* If a named pipe is full, just ignore it for now - mrn 24 May 96 */ if (pData->fileType == eTypePIPE && e == EAGAIN) - ABORT_FINALIZE(RS_RET_OK); - - /* If the filesystem is filled up, just ignore - * it for now and continue writing when possible - * based on patch for sysklogd by Martin Schulze on 2007-05-24 - */ - if (pData->fileType == eTypeFILE && e == ENOSPC) - ABORT_FINALIZE(RS_RET_OK); + ABORT_FINALIZE(RS_RET_SUSPENDED); (void) close(pData->fd); /* Check for EBADF on TTY's due to vhangup() -- cgit From 27a639ed8875969574543f7738c7bcb14c6149d2 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 18 Mar 2009 17:55:15 +0100 Subject: omfile bugfixing - fixed a bug that caused action retries not to work correctly situation was only cleared by a restart - bugfix: closed dynafile was potentially never written until another dynafile name was generated - potential loss of messages --- tools/omfile.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) (limited to 'tools/omfile.c') diff --git a/tools/omfile.c b/tools/omfile.c index 8be815f2..bf84d1a7 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -457,6 +457,8 @@ static int prepareDynFile(instanceData *pData, uchar *newFileName, unsigned iMsg int i; int iFirstFree; dynaFileCacheEntry **pCache; + + BEGINfunc ASSERT(pData != NULL); ASSERT(newFileName != NULL); @@ -542,6 +544,8 @@ static int prepareDynFile(instanceData *pData, uchar *newFileName, unsigned iMsg pData->iCurrElt = iFirstFree; DBGPRINTF("Added new entry %d for file cache, file '%s'.\n", iFirstFree, newFileName); + ENDfunc + return 0; } @@ -553,6 +557,7 @@ static int prepareDynFile(instanceData *pData, uchar *newFileName, unsigned iMsg static rsRetVal writeFile(uchar **ppString, unsigned iMsgOpts, instanceData *pData) { off_t actualFileSize; + int iLenWritten; DEFiRet; ASSERT(pData != NULL); @@ -563,7 +568,9 @@ static rsRetVal writeFile(uchar **ppString, unsigned iMsgOpts, instanceData *pDa if(pData->bDynamicName) { if(prepareDynFile(pData, ppString[1], iMsgOpts) != 0) ABORT_FINALIZE(RS_RET_SUSPENDED); // TODO: different state? conditional based on what went wrong? 2009-03-11 - } else if(pData->fd == -1) { + } + + if(pData->fd == -1) { prepareFile(pData, pData->f_fname); } @@ -600,16 +607,21 @@ again: } } - if(write(pData->fd, ppString[0], strlen((char*)ppString[0])) < 0) { + iLenWritten = write(pData->fd, ppString[0], strlen((char*)ppString[0])); +//dbgprintf("lenwritten: %d\n", iLenWritten); + if(iLenWritten < 0) { int e = errno; -dbgprintf("++++++++++ log file writer error %d\n", e); + char errStr[1024]; + rs_strerror_r(errno, errStr, sizeof(errStr)); + DBGPRINTF("log file (%d) write error %d: %s\n", pData->fd, e, errStr); /* If a named pipe is full, just ignore it for now - mrn 24 May 96 */ - if (pData->fileType == eTypePIPE && e == EAGAIN) + if(pData->fileType == eTypePIPE && e == EAGAIN) ABORT_FINALIZE(RS_RET_SUSPENDED); (void) close(pData->fd); + pData->fd = -1; /* tell that fd is no longer open! */ /* Check for EBADF on TTY's due to vhangup() * Linux uses EIO instead (mrn 12 May 96) */ -- cgit From 508a9e0cef064b082cd9e13aecd9d6c0f1f51977 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 19 Mar 2009 12:01:43 +0100 Subject: omfile suspend handling for non-dynafiles, also bugfixes primarily bugs introduced by recent changes. We now also handle static file names correctly, that was not the case before. We now correctly reset the descriptor in the dynafile cache if somthing goes wrong. Keep in mind that reliablity of output is depending on the reliability of the file system driver (the cifs driver returns OK, but still loses data if it is disconnected for too-long). --- tools/omfile.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) (limited to 'tools/omfile.c') diff --git a/tools/omfile.c b/tools/omfile.c index bf84d1a7..ff4c4618 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -383,9 +383,12 @@ static void dynaFileFreeCache(instanceData *pData) * file access, which, among others, means the the file wil be opened * and any directories in between will be created (based on config, of * course). -- rgerhards, 2008-10-22 + * changed to iRet interface - 2009-03-19 */ -static void prepareFile(instanceData *pData, uchar *newFileName) +static rsRetVal +prepareFile(instanceData *pData, uchar *newFileName) { + DEFiRet; if(pData->fileType == eTypePIPE) { pData->fd = open((char*) pData->f_fname, O_RDWR|O_NONBLOCK); FINALIZE; /* we are done in this case */ @@ -399,14 +402,14 @@ static void prepareFile(instanceData *pData, uchar *newFileName) pData->fd = -1; /* file does not exist, create it (and eventually parent directories */ if(pData->bCreateDirs) { - /* we fist need to create parent dirs if they are missing + /* We first need to create parent dirs if they are missing. * We do not report any errors here ourselfs but let the code * fall through to error handler below. */ if(makeFileParentDirs(newFileName, strlen((char*)newFileName), pData->fDirCreateMode, pData->dirUID, pData->dirGID, pData->bFailOnChown) != 0) { - return; /* we give up */ + ABORT_FINALIZE(RS_RET_ERR); /* we give up */ } } /* no matter if we needed to create directories or not, we now try to create @@ -418,8 +421,7 @@ static void prepareFile(instanceData *pData, uchar *newFileName) /* check and set uid/gid */ if(pData->fileUID != (uid_t)-1 || pData->fileGID != (gid_t) -1) { /* we need to set owner/group */ - if(fchown(pData->fd, pData->fileUID, - pData->fileGID) != 0) { + if(fchown(pData->fd, pData->fileUID, pData->fileGID) != 0) { if(pData->bFailOnChown) { int eSave = errno; close(pData->fd); @@ -434,11 +436,17 @@ static void prepareFile(instanceData *pData, uchar *newFileName) } } finalize_it: - if((pData->fd) != 0 && isatty(pData->fd)) { + /* this was "pData->fd != 0", which I think was a bug. I guess 0 was intended to mean + * non-open file descriptor. Anyhow, I leave this comment for the time being to that if + * problems surface, one at least knows what happened. -- rgerhards, 2009-03-19 + */ + if(pData->fd != -1 && isatty(pData->fd)) { DBGPRINTF("file %d is a tty file\n", pData->fd); pData->fileType = eTypeTTY; untty(); } + + RETiRet; } @@ -520,7 +528,7 @@ static int prepareDynFile(instanceData *pData, uchar *newFileName, unsigned iMsg } /* Ok, we finally can open the file */ - prepareFile(pData, newFileName); + prepareFile(pData, newFileName); /* ignore exact error, we check fd below */ /* file is either open now or an error state set */ if(pData->fd == -1) { @@ -567,11 +575,14 @@ static rsRetVal writeFile(uchar **ppString, unsigned iMsgOpts, instanceData *pDa */ if(pData->bDynamicName) { if(prepareDynFile(pData, ppString[1], iMsgOpts) != 0) - ABORT_FINALIZE(RS_RET_SUSPENDED); // TODO: different state? conditional based on what went wrong? 2009-03-11 + ABORT_FINALIZE(RS_RET_SUSPENDED); /* whatever the failure was, we need to retry */ } if(pData->fd == -1) { - prepareFile(pData, pData->f_fname); + rsRetVal iRetLocal; + iRetLocal = prepareFile(pData, pData->f_fname); + if((iRetLocal != RS_RET_OK) || (pData->fd == -1)) + ABORT_FINALIZE(RS_RET_SUSPENDED); /* whatever the failure was, we need to retry */ } /* create the message based on format specified */ @@ -615,13 +626,19 @@ again: rs_strerror_r(errno, errStr, sizeof(errStr)); DBGPRINTF("log file (%d) write error %d: %s\n", pData->fd, e, errStr); - /* If a named pipe is full, just ignore it for now - - mrn 24 May 96 */ + /* If a named pipe is full, we suspend this action for a while */ if(pData->fileType == eTypePIPE && e == EAGAIN) ABORT_FINALIZE(RS_RET_SUSPENDED); - (void) close(pData->fd); + close(pData->fd); pData->fd = -1; /* tell that fd is no longer open! */ + if(pData->bDynamicName && pData->iCurrElt != -1) { + /* in this case, we need to invalidate the name in the cache, too + * otherwise, an invalid fd may show up if we had a file name change. + * rgerhards, 2009-03-19 + */ + pData->dynCache[pData->iCurrElt]->fd = -1; + } /* Check for EBADF on TTY's due to vhangup() * Linux uses EIO instead (mrn 12 May 96) */ @@ -793,6 +810,9 @@ CODESTARTparseSelectorAct pData->dirUID = dirUID; pData->dirGID = dirGID; + /* at this stage, we ignore the return value of prepareFile, this is taken + * care of in later steps. -- rgerhards, 2009-03-19 + */ prepareFile(pData, pData->f_fname); if(pData->fd < 0 ) { -- cgit