[ovs-dev] [PATCH 1/2] ovsdb: Warn about reaching max session limit

[Andy Zhou]

Thanks Russell and Ben for the quick review. I've sent out v2 that
removes this unintended logic change.

On Tue, Feb 24, 2015 at 8:34 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, Feb 24, 2015 at 01:36:20PM +0100, Russell Bryant wrote:
>> On 02/24/2015 08:43 AM, Andy Zhou wrote:
>> > Without the log message, it was not obvious why ovsdb-server no longer
>> > accepts new connections when the session limit was reached. This patch
>> > adds a log message to make it obvious.
>> >
>> > Signed-off-by: Andy Zhou <azhou at nicira.com>
>> > ---
>> >  ovsdb/jsonrpc-server.c | 9 ++++++++-
>> >  1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
>> > index caef515..a93ff62 100644
>> > --- a/ovsdb/jsonrpc-server.c
>> > +++ b/ovsdb/jsonrpc-server.c
>> > @@ -314,10 +314,17 @@ ovsdb_jsonrpc_server_run(struct ovsdb_jsonrpc_server *svr)
>> >      SHASH_FOR_EACH (node, &svr->remotes) {
>> >          struct ovsdb_jsonrpc_remote *remote = node->data;
>> >
>> > -        if (remote->listener && svr->n_sessions < svr->max_sessions) {
>> > +        if (remote->listener) {
>> >              struct stream *stream;
>> >              int error;
>> >
>> > +            if (svr->n_sessions >= svr->max_sessions) {
>> > +                VLOG_WARN_RL(&rl, "%s: number of connections exceeded maximum (%d) allowed",
>> > +                             pstream_get_name(remote->listener),
>> > +                             svr->max_sessions);
>> > +                continue;
>> > +            }
>> > +
>> >              error = pstream_accept(remote->listener, &stream);
>> >              if (!error) {
>> >                  struct jsonrpc_session *js;
>> >
>>
>> I have no idea if it matters, but there seems to be a slight change in
>> logic here.  Previously, without the continue, there would be a call to
>> ovsdb_jsonrpc_session_run_all(remote) in this case before the next
>> iteration of the loop.
>
> Thanks for noticing.
>
> Andy, I think that's an undesirable change.