On Thu, Mar 24, 2016 at 10:50:47AM -0500, Ryan Moats wrote: > From: RYAN D. MOATS <rmo...@us.ibm.com> > > Allows for auto detection and reconnect if the ovn-remote needs > to change. ovn-controller test case updated to include testing > this code > > Signed-off-by: RYAN D. MOATS <rmo...@us.ibm.com>
Thanks for the patch! Ordinarily we don't add code at the end of a program's main loop, following the call to poll_block(). I'd recommend putting this code at the top of the main loop instead. I'd prefer to put this new code in a function instead of inline. This approach to changing the remote blocks the entire ovn-controller main loop while it retrieves a snapshot from the new remote. That is not necessary and it prevents other work from getting done. (Also, if the retrieval stalls, e.g. because the new remote does not point to an ovsdb-server, and the remote gets changed back to a correct database server, then ovn-controller will never recover.) Thus, I suggest instead adding an ovsdb_idl_*() function to set a new remote and letting the IDL retrieve a new snapshot asynchronously; it already knows how to do the latter. Thanks for writing a test. I don't think the "sleep" calls are needed because check_patches will already wait until the change takes effect. In the test, please use on_exit to ensure that the new ovsdb-server is killed if the test is interrupted or fails. Also, please use OVS_APP_EXIT_AND_WAIT to cause the new ovsdb-server to exit gracefully. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
