On Fri, Jan 12, 2018 at 04:56:00PM -0800, Justin Pettit wrote:
>
> > On Dec 31, 2017, at 9:16 PM, Ben Pfaff <[email protected]> wrote:
> >
> > +void
> > +ovsdb_file_destroy(struct ovsdb_file *file)
> > {
> > ovsdb_log_close(file->log);
> > free(file->file_name);
> > free(file);
> > }
>
> Generally, we handle the case where a null value is passed in. It
> might be nice to provide the same protection.
Thanks, fixed.
> > @@ -865,17 +866,16 @@ ovsdb_txn_commit_(struct ovsdb_txn *txn, bool durable)
> > }
> >
> > /* Send the commit to each replica. */
> > - LIST_FOR_EACH (replica, node, &txn->db->replicas) {
> > - error = (replica->class->commit)(replica, txn, durable);
> > + if (txn->db->file) {
> > + error = ovsdb_file_commit(txn->db->file, txn, durable);
> > if (error) {
> > ovsdb_txn_abort(txn);
> > return error;
> > }
> > }
> > + LIST_FOR_EACH (replica, node, &txn->db->replicas) {
> > + replica->class->commit(replica, txn, durable);
> > + }
>
> I think the comment about committing to each replica should be moved
> to this block, and a new one should be written about writing to the
> log.
I updated the comment, thanks.
> > diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
> > index 89bbfa2512fa..06cd3a72e49e 100644
> > --- a/ovsdb/ovsdb.h
> > +++ b/ovsdb/ovsdb.h
> > @@ -56,6 +56,7 @@ bool ovsdb_schema_equal(const struct ovsdb_schema *,
> > /* Database. */
> > struct ovsdb {
> > struct ovsdb_schema *schema;
> > + struct ovsdb_file *file; /* If nonnull, log for transactions. */
> > struct ovs_list replicas; /* Contains "struct ovsdb_replica"s. */
> > struct shash tables; /* Contains "struct ovsdb_table *"s. */
>
>
> The word "file" is kind of generic. What about something like
> "log_file" to make it clearer when it's referenced elsewhere?
Unfortunately we have a different lower-level entity called a "log", so
I doubt that this would clarify much.
I think that there is probably opportunity for better naming here, but I
think it needs to be carefully thought out.
> Acked-by: Justin Pettit <[email protected]>
Thanks!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
