[ovs-dev] [PATCH] cfm: Log fault status changes more informatively.

[Ben Pfaff]

Thanks, I pushed this.

On Wed, Apr 25, 2012 at 06:22:21PM -0700, Ethan Jackson wrote:
> Looks good, thanks.
> 
> Ethan
> 
> On Fri, Apr 20, 2012 at 14:53, Ben Pfaff <blp at nicira.com> wrote:
> > Until now, fault status changes just log the new status.  This means that
> > the administrator has to find two consecutive status change messages to
> > see what actually changed.
> >
> > This commit changes the log message format to prefix new faults with '+'
> > and faults that disappeared with '-'.  Existing faults that are still
> > present are not prefixed.
> >
> > This also simplifies the code a little by making ds_put_cfm_fault()
> > put spaces before fault names instead of after.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/cfm.c |   30 +++++++++++++++---------------
> >  1 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/cfm.c b/lib/cfm.c
> > index d0ae55d..66bca75 100644
> > --- a/lib/cfm.c
> > +++ b/lib/cfm.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2010, 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -159,22 +159,21 @@ cfm_fault_reason_to_str(int reason) {
> >  }
> >
> >  static void
> > -ds_put_cfm_fault(struct ds *ds, int fault)
> > +ds_put_cfm_fault(struct ds *ds, int old_fault, int new_fault)
> >  {
> > -    size_t length = ds->length;
> >     int i;
> >
> >     for (i = 0; i < CFM_FAULT_N_REASONS; i++) {
> >         int reason = 1 << i;
> >
> > -        if (fault & reason) {
> > -            ds_put_format(ds, "%s ", cfm_fault_reason_to_str(reason));
> > +        if ((old_fault | new_fault) & reason) {
> > +            ds_put_format(ds, " %s%s",
> > +                          (!(old_fault & reason) ? "+"
> > +                           : !(new_fault & reason) ? "-"
> > +                           : ""),
> > +                          cfm_fault_reason_to_str(reason));
> >         }
> >     }
> > -
> > -    if (ds->length > length) {
> > -        ds_truncate(ds, ds->length - 1);
> > -    }
> >  }
> >
> >  static void
> > @@ -398,8 +397,8 @@ cfm_run(struct cfm *cfm)
> >         if (old_cfm_fault != cfm->fault) {
> >             struct ds ds = DS_EMPTY_INITIALIZER;
> >
> > -            ds_put_cfm_fault(&ds, cfm->fault);
> > -            VLOG_INFO_RL(&rl, "%s: CFM fault status changed: %s", cfm->name,
> > +            ds_put_cfm_fault(&ds, old_cfm_fault, cfm->fault);
> > +            VLOG_INFO_RL(&rl, "%s: CFM fault status changed:%s", cfm->name,
> >                          ds_cstr_ro(&ds));
> >             ds_destroy(&ds);
> >         }
> > @@ -702,16 +701,17 @@ static void
> >  cfm_print_details(struct ds *ds, const struct cfm *cfm)
> >  {
> >     struct remote_mp *rmp;
> > +    int fault;
> >
> >     ds_put_format(ds, "---- %s ----\n", cfm->name);
> >     ds_put_format(ds, "MPID %"PRIu64":%s%s\n", cfm->mpid,
> >                   cfm->extended ? " extended" : "",
> >                   cfm->fault_override >= 0 ? " fault_override" : "");
> >
> > -
> > -    if (cfm_get_fault(cfm)) {
> > -        ds_put_cstr(ds, "\tfault: ");
> > -        ds_put_cfm_fault(ds, cfm_get_fault(cfm));
> > +    fault = cfm_get_fault(cfm);
> > +    if (fault) {
> > +        ds_put_cstr(ds, "\tfault:");
> > +        ds_put_cfm_fault(ds, fault, fault);
> >         ds_put_cstr(ds, "\n");
> >     }
> >
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev