[ovs-dev] [PATCH] ovsdb-server: Drop 'txn' member from struct db.
Ben Pfaff
blp at ovn.org
Tue Dec 19 22:03:18 UTC 2017
Thank you for the review.
ovsdb_txn_commit() is at least meant to free its argument whether it
succeeds or not. Do you see a bug there?
I applied this to master.
On Tue, Dec 19, 2017 at 09:50:45AM -0800, Yifeng Sun wrote:
> It seems that txn is not freed when ovsdb_txn_commit returns an error.
> Other than that, this patch looks good to me.
>
> Thanks,
> Yifeng
>
> On Fri, Dec 15, 2017 at 11:01 AM, Ben Pfaff <blp at ovn.org> wrote:
>
> > This member was only used in one particular code path, so this commit
> > adds code to pass it around as a function parameter instead.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> > ovsdb/ovsdb-server.c | 40 ++++++++++++++++------------------------
> > 1 file changed, 16 insertions(+), 24 deletions(-)
> >
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> > index f9e4e529e32e..ffdcf6810ddf 100644
> > --- a/ovsdb/ovsdb-server.c
> > +++ b/ovsdb/ovsdb-server.c
> > @@ -62,13 +62,9 @@
> > VLOG_DEFINE_THIS_MODULE(ovsdb_server);
> >
> > struct db {
> > - /* Initialized in main(). */
> > char *filename;
> > struct ovsdb_file *file;
> > struct ovsdb *db;
> > -
> > - /* Only used by update_remote_status(). */
> > - struct ovsdb_txn *txn;
> > };
> >
> > /* SSL configuration. */
> > @@ -846,9 +842,10 @@ update_remote_row(const struct ovsdb_row *row, struct
> > ovsdb_txn *txn,
> > }
> >
> > static void
> > -update_remote_rows(const struct shash *all_dbs,
> > +update_remote_rows(const struct shash *all_dbs, const struct db *db_,
> > const char *remote_name,
> > - const struct ovsdb_jsonrpc_server *jsonrpc)
> > + const struct ovsdb_jsonrpc_server *jsonrpc,
> > + struct ovsdb_txn *txn)
> > {
> > const struct ovsdb_table *table, *ref_table;
> > const struct ovsdb_column *column;
> > @@ -866,7 +863,8 @@ update_remote_rows(const struct shash *all_dbs,
> > return;
> > }
> >
> > - if (column->type.key.type != OVSDB_TYPE_UUID
> > + if (db != db_
> > + || column->type.key.type != OVSDB_TYPE_UUID
> > || !column->type.key.u.uuid.refTable
> > || column->type.value.type != OVSDB_TYPE_VOID) {
> > return;
> > @@ -884,7 +882,7 @@ update_remote_rows(const struct shash *all_dbs,
> >
> > ref_row = ovsdb_table_get_row(ref_table,
> > &datum->keys[i].uuid);
> > if (ref_row) {
> > - update_remote_row(ref_row, db->txn, jsonrpc);
> > + update_remote_row(ref_row, txn, jsonrpc);
> > }
> > }
> > }
> > @@ -895,26 +893,20 @@ update_remote_status(const struct
> > ovsdb_jsonrpc_server *jsonrpc,
> > const struct sset *remotes,
> > struct shash *all_dbs)
> > {
> > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > - const char *remote;
> > - struct db *db;
> > struct shash_node *node;
> > + SHASH_FOR_EACH (node, all_dbs) {
> > + struct db *db = node->data;
> > + struct ovsdb_txn *txn = ovsdb_txn_create(db->db);
> >
> > - SHASH_FOR_EACH(node, all_dbs) {
> > - db = node->data;
> > - db->txn = ovsdb_txn_create(db->db);
> > - }
> > -
> > - /* Iterate over --remote arguments given on command line. */
> > - SSET_FOR_EACH (remote, remotes) {
> > - update_remote_rows(all_dbs, remote, jsonrpc);
> > - }
> > + /* Iterate over --remote arguments given on command line. */
> > + const char *remote;
> > + SSET_FOR_EACH (remote, remotes) {
> > + update_remote_rows(all_dbs, db, remote, jsonrpc, txn);
> > + }
> >
> > - SHASH_FOR_EACH(node, all_dbs) {
> > - struct ovsdb_error *error;
> > - db = node->data;
> > - error = ovsdb_txn_commit(db->txn, false);
> > + struct ovsdb_error *error = ovsdb_txn_commit(txn, false);
> > if (error) {
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > char *msg = ovsdb_error_to_string_free(error);
> > VLOG_ERR_RL(&rl, "Failed to update remote status: %s", msg);
> > free(msg);
> > --
> > 2.10.2
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
More information about the dev
mailing list