Thanks for the comments! I made all of these changes and they will be in v6.
On Thu, Nov 12, 2020 at 09:30:03AM +0100, Dumitru Ceara wrote: > On 11/12/20 2:45 AM, Ben Pfaff wrote: > > From: Leonid Ryzhyk <[email protected]> > > > > Export `ddlog_warn` and `ddlog_err` functions that are just wrappers > > around `VLOG_WARN` and `VLOG_ERR`. This is not ideal because the > > functions are exported by `ovn_util.c` and the resulting log messages use > > `ovn_util` as module name. More importantly, these functions do not do > > log rate limiting. > > > > Should we add a TODO.rst item for this? I see we use warn() in quite a > few places in the ddlog implementation to report issues caused by user > misconfigs. > > > Signed-off-by: Leonid Ryzhyk <[email protected]> > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > lib/ovn-util.c | 17 +++++++++++++++++ > > lib/ovn-util.h | 6 ++++++ > > 2 files changed, 23 insertions(+) > > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index 18aac8da34ca..0416f44c79d0 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -713,3 +713,20 @@ ip_address_and_port_from_lb_key(const char *key, char > > **ip_address, > > *addr_family = ss.ss_family; > > return true; > > } > > + > > +#ifdef DDLOG > > + > > +/* Callbacks used by the ddlog northd code to print warnings and errors. > > + */ > > The '*/' fits fine on the previous line. > > > +void > > +ddlog_warn(const char *msg) > > +{ > > + VLOG_WARN("%s", msg); > > +} > > + > > +void > > +ddlog_err(const char *msg) > > +{ > > + VLOG_ERR("%s", msg); > > +} > > +#endif > > Nit: I'd either add a newline before #endif or remove the newline after > #ifdef DDLOG. > > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index 3496673b2641..835111872c07 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -221,4 +221,10 @@ char *str_tolower(const char *orig); > > bool ip_address_and_port_from_lb_key(const char *key, char **ip_address, > > uint16_t *port, int *addr_family); > > > > +#ifdef DDLOG > > +void ddlog_warn(const char *msg); > > +void ddlog_err(const char *msg); > > +#endif > > + > > + > > Nit: I don't think we need the second newline. > > Thanks, > Dumitru > > > #endif > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
