Message ID | 20180518165537.32002-3-jkbs@redhat.com |
---|---|
State | RFC |
Headers | show |
Series | ovn: Observe and check for effects of incremental processing | expand |
On Fri, May 18, 2018 at 9:55 AM, Jakub Sitnicki <jkbs@redhat.com> wrote: > > Facilitate checking coverage counters from scripts and tests by > providing new "coverage/read-count" command that returns the value of a > given counter. > > Same could be achieved by scraping the output of "coverage/show" command > but the difficulties there are that output is in human readable format > and zero-value counters are not included. > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> > --- > lib/coverage.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/lib/coverage.c b/lib/coverage.c > index 6cef82614..4aa283be8 100644 > --- a/lib/coverage.c > +++ b/lib/coverage.c > @@ -44,6 +44,8 @@ static unsigned int idx_count = 0; > static void coverage_read(struct svec *); > static unsigned int coverage_array_sum(const unsigned int *arr, > const unsigned int len); > +static bool coverage_get_count(const char *counter_name, > + unsigned long long int *count); > > /* Registers a coverage counter with the coverage core */ > void > @@ -72,11 +74,32 @@ coverage_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, > svec_destroy(&lines); > } > > +static void > +coverage_unixctl_read_count(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[], void *aux OVS_UNUSED) > +{ > + unsigned long long count; > + char *reply; > + bool ok; > + > + ok = coverage_get_count(argv[1], &count); > + if (!ok) { > + unixctl_command_reply_error(conn, "No such counter"); > + return; > + } > + > + reply = xasprintf("%llu\n", count); > + unixctl_command_reply(conn, reply); > + free(reply); > +} > + > void > coverage_init(void) > { > unixctl_command_register("coverage/show", "", 0, 0, > coverage_unixctl_show, NULL); > + unixctl_command_register("coverage/read-count", "COUNTER", 1, 1, > + coverage_unixctl_read_count, NULL); > } > > /* Sorts coverage counters in descending order by total, within equal > @@ -372,3 +395,21 @@ coverage_array_sum(const unsigned int *arr, const unsigned int len) > ovs_mutex_unlock(&coverage_mutex); > return sum; > } > + > +static bool > +coverage_get_count(const char *counter_name, unsigned long long int *count) > +{ > + for (size_t i = 0; i < n_coverage_counters; i++) { > + struct coverage_counter *counter = coverage_counters[i]; > + > + if (!strcmp(counter->name, counter_name)) { > + ovs_mutex_lock(&coverage_mutex); This lock seems unnecessary, since it is protecting a single counter read. > + *count = counter->total; > + ovs_mutex_unlock(&coverage_mutex); > + > + return true; > + } > + } > + > + return false; > +} > -- > 2.14.3 Better to add some documentation, at least in lib/coverage-unixctl.man for this new command. (Note: not for this patch, but I just noticed that documentation is missing in ovn-controller/ovn-northd for coverage. ovsdb-server and ovs-vswitchd shares same documentation by including .so lib/coverage-unixctl.man, however, it doesn't apply to the xml based documentation for ovn daemons. This should be addressed separately) Thanks, Han
On Mon, 21 May 2018 16:22:07 -0700 Han Zhou <zhouhan@gmail.com> wrote: > On Fri, May 18, 2018 at 9:55 AM, Jakub Sitnicki <jkbs@redhat.com> wrote: > > > > Facilitate checking coverage counters from scripts and tests by > > providing new "coverage/read-count" command that returns the value of a > > given counter. > > > > Same could be achieved by scraping the output of "coverage/show" command > > but the difficulties there are that output is in human readable format > > and zero-value counters are not included. > > > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> > > --- > > lib/coverage.c | 41 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/lib/coverage.c b/lib/coverage.c > > index 6cef82614..4aa283be8 100644 > > --- a/lib/coverage.c > > +++ b/lib/coverage.c > > @@ -44,6 +44,8 @@ static unsigned int idx_count = 0; > > static void coverage_read(struct svec *); > > static unsigned int coverage_array_sum(const unsigned int *arr, > > const unsigned int len); > > +static bool coverage_get_count(const char *counter_name, > > + unsigned long long int *count); > > > > /* Registers a coverage counter with the coverage core */ > > void > > @@ -72,11 +74,32 @@ coverage_unixctl_show(struct unixctl_conn *conn, int > argc OVS_UNUSED, > > svec_destroy(&lines); > > } > > > > +static void > > +coverage_unixctl_read_count(struct unixctl_conn *conn, int argc > OVS_UNUSED, > > + const char *argv[], void *aux OVS_UNUSED) > > +{ > > + unsigned long long count; > > + char *reply; > > + bool ok; > > + > > + ok = coverage_get_count(argv[1], &count); > > + if (!ok) { > > + unixctl_command_reply_error(conn, "No such counter"); > > + return; > > + } > > + > > + reply = xasprintf("%llu\n", count); > > + unixctl_command_reply(conn, reply); > > + free(reply); > > +} > > + > > void > > coverage_init(void) > > { > > unixctl_command_register("coverage/show", "", 0, 0, > > coverage_unixctl_show, NULL); > > + unixctl_command_register("coverage/read-count", "COUNTER", 1, 1, > > + coverage_unixctl_read_count, NULL); > > } > > > > /* Sorts coverage counters in descending order by total, within equal > > @@ -372,3 +395,21 @@ coverage_array_sum(const unsigned int *arr, const > unsigned int len) > > ovs_mutex_unlock(&coverage_mutex); > > return sum; > > } > > + > > +static bool > > +coverage_get_count(const char *counter_name, unsigned long long int > *count) > > +{ > > + for (size_t i = 0; i < n_coverage_counters; i++) { > > + struct coverage_counter *counter = coverage_counters[i]; > > + > > + if (!strcmp(counter->name, counter_name)) { > > + ovs_mutex_lock(&coverage_mutex); > > This lock seems unnecessary, since it is protecting a single counter read. > > > + *count = counter->total; > > + ovs_mutex_unlock(&coverage_mutex); > > + > > + return true; > > + } > > + } > > + > > + return false; > > +} > > -- > > 2.14.3 > > Better to add some documentation, at least in lib/coverage-unixctl.man for > this new command. > > (Note: not for this patch, but I just noticed that documentation is missing > in ovn-controller/ovn-northd for coverage. ovsdb-server and ovs-vswitchd > shares same documentation by including .so lib/coverage-unixctl.man, > however, it doesn't apply to the xml based documentation for ovn daemons. > This should be addressed separately) Ack, I will make sure to add docs in v1. Thanks for taking a look at the RFC. -Jakub > > Thanks, > Han
diff --git a/lib/coverage.c b/lib/coverage.c index 6cef82614..4aa283be8 100644 --- a/lib/coverage.c +++ b/lib/coverage.c @@ -44,6 +44,8 @@ static unsigned int idx_count = 0; static void coverage_read(struct svec *); static unsigned int coverage_array_sum(const unsigned int *arr, const unsigned int len); +static bool coverage_get_count(const char *counter_name, + unsigned long long int *count); /* Registers a coverage counter with the coverage core */ void @@ -72,11 +74,32 @@ coverage_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, svec_destroy(&lines); } +static void +coverage_unixctl_read_count(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[], void *aux OVS_UNUSED) +{ + unsigned long long count; + char *reply; + bool ok; + + ok = coverage_get_count(argv[1], &count); + if (!ok) { + unixctl_command_reply_error(conn, "No such counter"); + return; + } + + reply = xasprintf("%llu\n", count); + unixctl_command_reply(conn, reply); + free(reply); +} + void coverage_init(void) { unixctl_command_register("coverage/show", "", 0, 0, coverage_unixctl_show, NULL); + unixctl_command_register("coverage/read-count", "COUNTER", 1, 1, + coverage_unixctl_read_count, NULL); } /* Sorts coverage counters in descending order by total, within equal @@ -372,3 +395,21 @@ coverage_array_sum(const unsigned int *arr, const unsigned int len) ovs_mutex_unlock(&coverage_mutex); return sum; } + +static bool +coverage_get_count(const char *counter_name, unsigned long long int *count) +{ + for (size_t i = 0; i < n_coverage_counters; i++) { + struct coverage_counter *counter = coverage_counters[i]; + + if (!strcmp(counter->name, counter_name)) { + ovs_mutex_lock(&coverage_mutex); + *count = counter->total; + ovs_mutex_unlock(&coverage_mutex); + + return true; + } + } + + return false; +}
Facilitate checking coverage counters from scripts and tests by providing new "coverage/read-count" command that returns the value of a given counter. Same could be achieved by scraping the output of "coverage/show" command but the difficulties there are that output is in human readable format and zero-value counters are not included. Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> --- lib/coverage.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) -- 2.14.3