Looks good to me.

You never call ovsdb_session_destroy().  I would think you would want
to do that in ovsdb_jsonrpc_session_close() for completeness.

Ethan

On Thu, Jul 14, 2011 at 14:27, Ben Pfaff <[email protected]> wrote:
> An upcoming commit will need to expose the concept of a database session
> to the execution engine, to allow the execution engine to query the locks
> held by the session.  This commit prepares for that by making sessions a
> publicly visible data structure.
> ---
>  ovsdb/automake.mk      |    2 ++
>  ovsdb/jsonrpc-server.c |   38 +++++++++++++++++++-------------------
>  ovsdb/server.c         |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  ovsdb/server.h         |   43 +++++++++++++++++++++++++++++++++++++++++++
>  ovsdb/trigger.c        |   28 ++++++++++++++--------------
>  ovsdb/trigger.h        |   13 ++++++-------
>  tests/test-ovsdb.c     |   12 +++++++-----
>  7 files changed, 136 insertions(+), 45 deletions(-)
>  create mode 100644 ovsdb/server.c
>  create mode 100644 ovsdb/server.h
>
> diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
> index 1f53d20..5c9a8fb 100644
> --- a/ovsdb/automake.mk
> +++ b/ovsdb/automake.mk
> @@ -21,6 +21,8 @@ ovsdb_libovsdb_a_SOURCES = \
>        ovsdb/query.h \
>        ovsdb/row.c \
>        ovsdb/row.h \
> +       ovsdb/server.c \
> +       ovsdb/server.h \
>        ovsdb/table.c \
>        ovsdb/table.h \
>        ovsdb/trigger.c \
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index ba08e3b..991152a 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -30,6 +30,7 @@
>  #include "ovsdb.h"
>  #include "reconnect.h"
>  #include "row.h"
> +#include "server.h"
>  #include "stream.h"
>  #include "table.h"
>  #include "timeval.h"
> @@ -80,7 +81,7 @@ static void ovsdb_jsonrpc_monitor_remove_all(struct 
> ovsdb_jsonrpc_session *);
>  /* JSON-RPC database server. */
>
>  struct ovsdb_jsonrpc_server {
> -    struct ovsdb *db;
> +    struct ovsdb_server up;
>     unsigned int n_sessions, max_sessions;
>     struct shash remotes;      /* Contains "struct ovsdb_jsonrpc_remote *"s. 
> */
>  };
> @@ -102,7 +103,7 @@ struct ovsdb_jsonrpc_server *
>  ovsdb_jsonrpc_server_create(struct ovsdb *db)
>  {
>     struct ovsdb_jsonrpc_server *server = xzalloc(sizeof *server);
> -    server->db = db;
> +    ovsdb_server_init(&server->up, db);
>     server->max_sessions = 64;
>     shash_init(&server->remotes);
>     return server;
> @@ -117,6 +118,7 @@ ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server 
> *svr)
>         ovsdb_jsonrpc_server_del_remote(node);
>     }
>     shash_destroy(&svr->remotes);
> +    ovsdb_server_destroy(&svr->up);
>     free(svr);
>  }
>
> @@ -278,12 +280,12 @@ ovsdb_jsonrpc_server_wait(struct ovsdb_jsonrpc_server 
> *svr)
>  /* JSON-RPC database server session. */
>
>  struct ovsdb_jsonrpc_session {
> +    struct ovsdb_session up;
>     struct ovsdb_jsonrpc_remote *remote;
>     struct list node;           /* Element in remote's sessions list. */
>
>     /* Triggers. */
>     struct hmap triggers;       /* Hmap of "struct ovsdb_jsonrpc_trigger"s. */
> -    struct list completions;    /* Completed triggers. */
>
>     /* Monitors. */
>     struct hmap monitors;       /* Hmap of "struct ovsdb_jsonrpc_monitor"s. */
> @@ -310,11 +312,11 @@ ovsdb_jsonrpc_session_create(struct 
> ovsdb_jsonrpc_remote *remote,
>     struct ovsdb_jsonrpc_session *s;
>
>     s = xzalloc(sizeof *s);
> +    ovsdb_session_init(&s->up, remote->server->up.db);
>     s->remote = remote;
>     list_push_back(&remote->sessions, &s->node);
>     hmap_init(&s->triggers);
>     hmap_init(&s->monitors);
> -    list_init(&s->completions);
>     s->js = js;
>     s->js_seqno = jsonrpc_session_get_seqno(js);
>
> @@ -475,7 +477,7 @@ ovsdb_jsonrpc_session_get_status(const struct 
> ovsdb_jsonrpc_remote *remote,
>  static const char *
>  get_db_name(const struct ovsdb_jsonrpc_session *s)
>  {
> -    return s->remote->server->db->schema->name;
> +    return s->remote->server->up.db->schema->name;
>  }
>
>  static struct jsonrpc_msg *
> @@ -549,7 +551,7 @@ ovsdb_jsonrpc_session_got_request(struct 
> ovsdb_jsonrpc_session *s,
>         reply = ovsdb_jsonrpc_check_db_name(s, request);
>         if (!reply) {
>             reply = jsonrpc_create_reply(
> -                ovsdb_schema_to_json(s->remote->server->db->schema),
> +                ovsdb_schema_to_json(s->remote->server->up.db->schema),
>                 request->id);
>         }
>     } else if (!strcmp(request->method, "list_dbs")) {
> @@ -601,7 +603,6 @@ ovsdb_jsonrpc_session_got_notify(struct 
> ovsdb_jsonrpc_session *s,
>
>  struct ovsdb_jsonrpc_trigger {
>     struct ovsdb_trigger trigger;
> -    struct ovsdb_jsonrpc_session *session;
>     struct hmap_node hmap_node; /* In session's "triggers" hmap. */
>     struct json *id;
>  };
> @@ -629,10 +630,7 @@ ovsdb_jsonrpc_trigger_create(struct 
> ovsdb_jsonrpc_session *s,
>
>     /* Insert into trigger table. */
>     t = xmalloc(sizeof *t);
> -    ovsdb_trigger_init(s->remote->server->db,
> -                       &t->trigger, params, &s->completions,
> -                       time_msec());
> -    t->session = s;
> +    ovsdb_trigger_init(&s->up, &t->trigger, params, time_msec());
>     t->id = id;
>     hmap_insert(&s->triggers, &t->hmap_node, hash);
>
> @@ -660,7 +658,9 @@ ovsdb_jsonrpc_trigger_find(struct ovsdb_jsonrpc_session 
> *s,
>  static void
>  ovsdb_jsonrpc_trigger_complete(struct ovsdb_jsonrpc_trigger *t)
>  {
> -    struct ovsdb_jsonrpc_session *s = t->session;
> +    struct ovsdb_jsonrpc_session *s;
> +
> +    s = CONTAINER_OF(t->trigger.session, struct ovsdb_jsonrpc_session, up);
>
>     if (jsonrpc_session_is_connected(s->js)) {
>         struct jsonrpc_msg *reply;
> @@ -694,9 +694,9 @@ ovsdb_jsonrpc_trigger_complete_all(struct 
> ovsdb_jsonrpc_session *s)
>  static void
>  ovsdb_jsonrpc_trigger_complete_done(struct ovsdb_jsonrpc_session *s)
>  {
> -    while (!list_is_empty(&s->completions)) {
> +    while (!list_is_empty(&s->up.completions)) {
>         struct ovsdb_jsonrpc_trigger *t
> -            = CONTAINER_OF(s->completions.next,
> +            = CONTAINER_OF(s->up.completions.next,
>                            struct ovsdb_jsonrpc_trigger, trigger.node);
>         ovsdb_jsonrpc_trigger_complete(t);
>     }
> @@ -913,7 +913,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session 
> *s,
>
>     m = xzalloc(sizeof *m);
>     ovsdb_replica_init(&m->replica, &ovsdb_jsonrpc_replica_class);
> -    ovsdb_add_replica(s->remote->server->db, &m->replica);
> +    ovsdb_add_replica(s->remote->server->up.db, &m->replica);
>     m->session = s;
>     hmap_insert(&s->monitors, &m->node, json_hash(monitor_id, 0));
>     m->monitor_id = json_clone(monitor_id);
> @@ -926,7 +926,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session 
> *s,
>         const struct json *mr_value;
>         size_t i;
>
> -        table = ovsdb_get_table(s->remote->server->db, node->name);
> +        table = ovsdb_get_table(s->remote->server->up.db, node->name);
>         if (!table) {
>             error = ovsdb_syntax_error(NULL, NULL,
>                                        "no table named %s", node->name);
> @@ -975,7 +975,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session 
> *s,
>
>  error:
>     if (m) {
> -        ovsdb_remove_replica(s->remote->server->db, &m->replica);
> +        ovsdb_remove_replica(s->remote->server->up.db, &m->replica);
>     }
>
>     json = ovsdb_error_to_json(error);
> @@ -999,7 +999,7 @@ ovsdb_jsonrpc_monitor_cancel(struct ovsdb_jsonrpc_session 
> *s,
>             return jsonrpc_create_error(json_string_create("unknown monitor"),
>                                         request_id);
>         } else {
> -            ovsdb_remove_replica(s->remote->server->db, &m->replica);
> +            ovsdb_remove_replica(s->remote->server->up.db, &m->replica);
>             return jsonrpc_create_reply(json_object_create(), request_id);
>         }
>     }
> @@ -1011,7 +1011,7 @@ ovsdb_jsonrpc_monitor_remove_all(struct 
> ovsdb_jsonrpc_session *s)
>     struct ovsdb_jsonrpc_monitor *m, *next;
>
>     HMAP_FOR_EACH_SAFE (m, next, node, &s->monitors) {
> -        ovsdb_remove_replica(s->remote->server->db, &m->replica);
> +        ovsdb_remove_replica(s->remote->server->up.db, &m->replica);
>     }
>  }
>
> diff --git a/ovsdb/server.c b/ovsdb/server.c
> new file mode 100644
> index 0000000..ad9454d
> --- /dev/null
> +++ b/ovsdb/server.c
> @@ -0,0 +1,45 @@
> +/* Copyright (c) 2011 Nicira Networks
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "server.h"
> +
> +/* Initializes 'session' as a session that operates on 'db'. */
> +void
> +ovsdb_session_init(struct ovsdb_session *session, struct ovsdb *db)
> +{
> +    session->db = db;
> +    list_init(&session->completions);
> +}
> +
> +/* Destroys 'session'. */
> +void
> +ovsdb_session_destroy(struct ovsdb_session *session OVS_UNUSED)
> +{
> +}
> +
> +/* Initializes 'server' as a server that operates on 'db'. */
> +void
> +ovsdb_server_init(struct ovsdb_server *server, struct ovsdb *db)
> +{
> +    server->db = db;
> +}
> +
> +/* Destroys 'server'. */
> +void
> +ovsdb_server_destroy(struct ovsdb_server *server OVS_UNUSED)
> +{
> +}
> diff --git a/ovsdb/server.h b/ovsdb/server.h
> new file mode 100644
> index 0000000..ce19b8d
> --- /dev/null
> +++ b/ovsdb/server.h
> @@ -0,0 +1,43 @@
> +/* Copyright (c) 2011 Nicira Networks
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef SERVER_H
> +#define SERVER_H 1
> +
> +#include "hmap.h"
> +#include "list.h"
> +
> +/* Abstract representation of an OVSDB client connection, not tied to any
> + * particular network protocol.  Protocol implementations
> + * (e.g. jsonrpc-server.c) embed this in a larger data structure.  */
> +struct ovsdb_session {
> +    struct ovsdb *db;
> +    struct list completions;    /* Completed triggers. */
> +};
> +
> +void ovsdb_session_init(struct ovsdb_session *, struct ovsdb *);
> +void ovsdb_session_destroy(struct ovsdb_session *);
> +
> +/* Abstract representation of an OVSDB server not tied to any particular
> + * network protocol.  Protocol implementations (e.g. jsonrpc-server.c) embed
> + * this in a larger data structure.  */
> +struct ovsdb_server {
> +    struct ovsdb *db;
> +};
> +
> +void ovsdb_server_init(struct ovsdb_server *, struct ovsdb *);
> +void ovsdb_server_destroy(struct ovsdb_server *);
> +
> +#endif /* ovsdb/server.h */
> diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c
> index c222d89..b2eb011 100644
> --- a/ovsdb/trigger.c
> +++ b/ovsdb/trigger.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010 Nicira Networks
> +/* Copyright (c) 2009, 2010, 2011 Nicira Networks
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -24,23 +24,23 @@
>  #include "jsonrpc.h"
>  #include "ovsdb.h"
>  #include "poll-loop.h"
> +#include "server.h"
>
> -static bool ovsdb_trigger_try(struct ovsdb *db, struct ovsdb_trigger *,
> -                              long long int now);
> +static bool ovsdb_trigger_try(struct ovsdb_trigger *, long long int now);
>  static void ovsdb_trigger_complete(struct ovsdb_trigger *);
>
>  void
> -ovsdb_trigger_init(struct ovsdb *db, struct ovsdb_trigger *trigger,
> -                   struct json *request, struct list *completion,
> -                   long long int now)
> +ovsdb_trigger_init(struct ovsdb_session *session,
> +                   struct ovsdb_trigger *trigger,
> +                   struct json *request, long long int now)
>  {
> -    list_push_back(&db->triggers, &trigger->node);
> -    trigger->completion = completion;
> +    trigger->session = session;
> +    list_push_back(&trigger->session->db->triggers, &trigger->node);
>     trigger->request = request;
>     trigger->result = NULL;
>     trigger->created = now;
>     trigger->timeout_msec = LLONG_MAX;
> -    ovsdb_trigger_try(db, trigger, now);
> +    ovsdb_trigger_try(trigger, now);
>  }
>
>  void
> @@ -75,7 +75,7 @@ ovsdb_trigger_run(struct ovsdb *db, long long int now)
>     db->run_triggers = false;
>     LIST_FOR_EACH_SAFE (t, next, node, &db->triggers) {
>         if (run_triggers || now - t->created >= t->timeout_msec) {
> -            ovsdb_trigger_try(db, t, now);
> +            ovsdb_trigger_try(t, now);
>         }
>     }
>  }
> @@ -108,10 +108,10 @@ ovsdb_trigger_wait(struct ovsdb *db, long long int now)
>  }
>
>  static bool
> -ovsdb_trigger_try(struct ovsdb *db, struct ovsdb_trigger *t, long long int 
> now)
> +ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
>  {
> -    t->result = ovsdb_execute(db, t->request, now - t->created,
> -                              &t->timeout_msec);
> +    t->result = ovsdb_execute(t->session->db, t->request,
> +                              now - t->created, &t->timeout_msec);
>     if (t->result) {
>         ovsdb_trigger_complete(t);
>         return true;
> @@ -125,5 +125,5 @@ ovsdb_trigger_complete(struct ovsdb_trigger *t)
>  {
>     assert(t->result != NULL);
>     list_remove(&t->node);
> -    list_push_back(t->completion, &t->node);
> +    list_push_back(&t->session->completions, &t->node);
>  }
> diff --git a/ovsdb/trigger.h b/ovsdb/trigger.h
> index 521b150..78265e5 100644
> --- a/ovsdb/trigger.h
> +++ b/ovsdb/trigger.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009 Nicira Networks
> +/* Copyright (c) 2009, 2011 Nicira Networks
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -21,18 +21,17 @@
>  struct ovsdb;
>
>  struct ovsdb_trigger {
> -    struct list node;           /* !result: in struct ovsdb "triggers" list;
> -                                 * result: in completion list. */
> -    struct list *completion;    /* Completion list. */
> +    struct ovsdb_session *session; /* Session that owns this trigger. */
> +    struct list node;           /* !result: in session->db->triggers;
> +                                 * result: in session->completions. */
>     struct json *request;       /* Database request. */
>     struct json *result;        /* Result (null if none yet). */
>     long long int created;      /* Time created. */
>     long long int timeout_msec; /* Max wait duration. */
>  };
>
> -void ovsdb_trigger_init(struct ovsdb *, struct ovsdb_trigger *,
> -                        struct json *request, struct list *completion,
> -                        long long int now);
> +void ovsdb_trigger_init(struct ovsdb_session *, struct ovsdb_trigger *,
> +                        struct json *request, long long int now);
>  void ovsdb_trigger_destroy(struct ovsdb_trigger *);
>
>  bool ovsdb_trigger_is_complete(const struct ovsdb_trigger *);
> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> index e8d87b6..e151dd8 100644
> --- a/tests/test-ovsdb.c
> +++ b/tests/test-ovsdb.c
> @@ -39,6 +39,7 @@
>  #include "ovsdb/ovsdb.h"
>  #include "ovsdb/query.h"
>  #include "ovsdb/row.h"
> +#include "ovsdb/server.h"
>  #include "ovsdb/table.h"
>  #include "ovsdb/transaction.h"
>  #include "ovsdb/trigger.h"
> @@ -1292,7 +1293,7 @@ static void
>  do_trigger(int argc OVS_UNUSED, char *argv[])
>  {
>     struct ovsdb_schema *schema;
> -    struct list completions;
> +    struct ovsdb_session session;
>     struct json *json;
>     struct ovsdb *db;
>     long long int now;
> @@ -1305,7 +1306,8 @@ do_trigger(int argc OVS_UNUSED, char *argv[])
>     json_destroy(json);
>     db = ovsdb_create(schema);
>
> -    list_init(&completions);
> +    ovsdb_session_init(&session, db);
> +
>     now = 0;
>     number = 0;
>     for (i = 2; i < argc; i++) {
> @@ -1319,7 +1321,7 @@ do_trigger(int argc OVS_UNUSED, char *argv[])
>             json_destroy(params);
>         } else {
>             struct test_trigger *t = xmalloc(sizeof *t);
> -            ovsdb_trigger_init(db, &t->trigger, params, &completions, now);
> +            ovsdb_trigger_init(&session, &t->trigger, params, now);
>             t->number = number++;
>             if (ovsdb_trigger_is_complete(&t->trigger)) {
>                 do_trigger_dump(t, now, "immediate");
> @@ -1329,8 +1331,8 @@ do_trigger(int argc OVS_UNUSED, char *argv[])
>         }
>
>         ovsdb_trigger_run(db, now);
> -        while (!list_is_empty(&completions)) {
> -            do_trigger_dump(CONTAINER_OF(list_pop_front(&completions),
> +        while (!list_is_empty(&session.completions)) {
> +            
> do_trigger_dump(CONTAINER_OF(list_pop_front(&session.completions),
>                                          struct test_trigger, trigger.node),
>                             now, "delayed");
>         }
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to