On 2006-08-11T16:48:55, Huang Zhen <[EMAIL PROTECTED]> wrote:
> >> if (rc != EXECRA_OK || debug_level > 0) {
^^^^^^^^^^^^^^
Actually, all these logs are protected to only occur while debugging
already. Could you PLEASE settle on using either that abstraction _or_
lrmd_debug() calls? This is mightily confusing to review :-(
> >> if (signo != 0) {
> >>- lrmd_log(rc == EXECRA_OK ? LOG_DEBUG : LOG_WARNING
> >>+ lrmd_debug(rc == EXECRA_OK ? LOG_DEBUG : LOG_WARNING
> >> , "Resource Agent (%s): pid [%d] killed by"
> >> " signal %d", op_info(op), p->pid, signo);
> >> }else if (rc == exitcode) {
> >>- lrmd_log(rc == EXECRA_OK ? LOG_DEBUG : LOG_INFO
> >>+ lrmd_debug2(rc == EXECRA_OK ? LOG_DEBUG : LOG_INFO
> >> , "Resource Agent (%s): pid [%d] exited with"
> >> " return code %d", op_info(op), p->pid, rc);
> >> }else{
> >>- lrmd_log(rc == EXECRA_OK ? LOG_DEBUG : LOG_INFO
> >>+ lrmd_debug2(rc == EXECRA_OK ? LOG_DEBUG : LOG_INFO
> >> , "Resource Agent (%s): pid [%d] exited with"
> >> " return code %d (mapped from %d)"
> >> , op_info(op), p->pid, rc, exitcode);
> >> }
> >> if (rc != EXECRA_OK || debug_level > 1) {
> >>- lrmd_log(LOG_INFO, "Resource Agent output: [%s]"
> >>+ lrmd_debug2(LOG_INFO, "Resource Agent output: [%s]"
> >> , op->first_line_ra_stdout);
> >> }
> >> }
> >
> >
> >I don't think these are good candidates for debug logging. These are
> >important to trace the event from the LRM to the CRM (and make sure it
> >wasn't lost in the middle).
> In most case, rc!=EXECRA_OK does not mean something wrong.
Well, the first error leg handles child processes being killed by a
signal. That very clearly implies that something is wrong.
> Only in the case that the resource is running, the rc would be EXECRA_OK.
> So we got a lot of logs for the resource which is stopped.
> That's why I downgraded these logs.
Yes, instead of fixing them to be logged when appropriate (ie, when
something failed), you no longer log them at all. I don't think that is
the appropriate fix to attain useful logging.
> >Besides, the last one is protected with "debug_level > 1" already, so
> >lrmd_debug2() doesn't change anything.
> It's "||" instead of "&&" in the condition.
> So as the same reason above, it would log everytime that monitoring a
> stopped resource.
A cleanly stopped resource (monitor == 7) probably shouldn't log the
output, but all other non-zero exit codes ought to.
And, given that RAs shouldn't log to stdout/stderr _at all_, any data
present on stderr definitely should _always_ be logged.
Sincerely,
Lars Marowsky-Brée
--
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business -- Charles Darwin
"Ignorance more frequently begets confidence than does knowledge"
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
