On Tue, Nov 10, 2015 at 9:23 AM, Ben Pfaff <b...@ovn.org> wrote: > On Wed, Oct 21, 2015 at 09:45:27PM -0700, Andy Zhou wrote: >> Add functions that can generate "update2" notification for a >> "monitor2" session. "monitor2" and "update2" are RFC 7047 extensions >> deescribed by ovsdb-server(1) manpage. See the manpage changes >> for more details. >> >> Signed-off-by: Andy Zhou <az...@nicira.com> > > Thanks for implementing this! > > I think that there is only a one-character difference between > ovsdb_monitor_compose_update() and ovsdb_monitor_compose_update2(): the > first calls ovsdb_monitor_compose_row_update(), the latter calls > ovsdb_monitor_compose_row_update2(). If so, and if they won't diverge a > lot in later patches, then I'd suggest finding a way to avoid so much > code duplication. You could, for example, use a conditional test inside > the function, or a function pointer.
Thanks for pointing this out. I was planing for a bigger change than it turns out to be. Collapse both function into a single ovsdb_monitor_compose_update() function that takes a callback function that will generate row updates. > > There's a fair amount of code duplication from > ovsdb_monitor_compose_row_update() and > ovsdb_monitor_compose_row_update2(), also. Is there a way to cleanly > reduce that? Yes, there are some logic duplications at the beginning part of both functions, refactor out common logics into a separate function in V2. Hope it looks better now. > > Manpage > ------- > Thanks for working on this. Folded those changes into V2. > I have some suggestions for the manpage. I'm appending an incremental > diff. > > Thanks, > > Ben. > > diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in > index 1d68fd6..6c85729 100644 > --- a/ovsdb/ovsdb-server.1.in > +++ b/ovsdb/ovsdb-server.1.in > @@ -248,84 +248,87 @@ monitors with the same identifier. > .IP "4.1.12. Monitor2" > A new monitor method added in Open vSwitch version 2.5. Monitor2 allows > for more efficient update notifications (described below). > - > -Monitor method described in Section 4.1.5 also applies to monitor2, with > -the following exceptions. > - > +.IP > +The monitor method described in Section 4.1.5 also applies to > +monitor2, with the following exceptions. > +. > .RS > .IP \(bu > -RPC request method becomes "montior2" > +RPC request method becomes "monitor2". > .IP \(bu > Replay result follows <table-updates2>, described in Section 4.1.13. > .IP \(bu > Subsequent changes are sent to the client using the "update2" monitor > notification, described in Section 4.1.13 > .RE > - > +. > +.IP > Both monitor and monitor2 sessions can exist concurrently. However, > -monitor and monitor2 shares the smae <json-value> parameter space; it > -must be unique among all monitor and monitor2 seesion. > - > +monitor and monitor2 shares the same <json-value> parameter space; it > +must be unique among all monitor and monitor2 sessions. > +. > .IP "4.1.13. Update2 notification" > The "update2" notification is sent by the server to the client to report > changes in tables that are being monitored following a "monitor2" request > -as described above. The notifaction has the following members: > - > +as described above. The notification has the following members: > +. > .RS > .nf > "method": "update2" > "params": [<json-value>, <table-updates2>] > -"id": NULL > +"id": null > .fi > .RE > - > +. > .IP > The <json-value> in "params" is the same as the value passed as the > <json-value> in "params" for the corresponding "monitor" request. > <table-updates2> is an object that maps from a table name to a > <table-update2>. > -A <table-update2> is an object htat maps from row's UUID to a <row-update2> > object. A <row-update2> is an object with one of the following members: > - > +A <table-update2> is an object that maps from row's UUID to a <row-update2> > object. A <row-update2> is an object with one of the following members: > +. > .RS > -.nf > -"initial": <row> present for "initial' updates > -"insert": <row> present for "insert' updates > -"delete": <row> present for "delete' updates > -"modify": <row> present for "modify' updates > -.fi > +.IP "\(dqinitial\(dq: <row>" > +present for "initial" updates > +.IP "\(dqinsert\(dq: <row>" > +present for "insert" updates > +.IP "\(dqdelete\(dq: <row>" > +present for "delete" updates > +.IP "\(dqmodify\(dq: <row>" > +present for "modify" updates > .RE > - > +. > .IP > The format of <row> is described in Section 5.1. > - > +. > .IP > -<row> is always a NULL object with a "delete" object. In "initial" and > -"insert" objects, <row> omits columns whose values equal to default > +<row> is always a null object for a "delete" update. In "initial" and > +"insert" updates, <row> omits columns whose values equal the default > value of the column type. > - > +. > .IP > -In "modify" object, <row> contains only the columns that are modified. > +For a "modify" update, <row> contains only the columns that are modified. > <row> stores the difference between the old and new value for those columns, > as described below. > - > +. > .IP > For columns with single value, the difference is the value of the new > column. > - > +. > .IP > The difference between two sets are all elements that only belong > to one of the sets. > - > +. > .IP > -The difference between two maps are all key value pairs whose keys appears > -in only one of the maps. And elements with overlapping > -keys between the two maps, but are associated with different values. > -For those elements, <row> stores the key value pair from the new column. > - > +The difference between two maps are all key-value pairs whose keys > +appears in only one of the maps, plus the key-value pairs whose keys > +appear in both maps but with different values. For the latter > +elements, <row> includes the value from the new column. > +. > .IP > -Note that initial views of rows are not presented in update2 notifications, > +Initial views of rows are not presented in update2 notifications, > but in the response object to the monitor2 request. The formatting of the > <table-updates2> object, however, is the same in either case. > - > +. > .IP "5.1. Notation" > For <condition>, RFC 7047 only allows the use of \fB!=\fR, \fB==\fR, > \fBincludes\fR, and \fBexcludes\fR operators with set types. Open _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
