On Fri, Jun 27, 2014 at 03:22:29PM -0700, Gurucharan Shetty wrote: > On Mon, Jun 23, 2014 at 11:44 AM, Ben Pfaff <b...@nicira.com> wrote: > > On Fri, Jun 13, 2014 at 07:40:01AM -0700, Gurucharan Shetty wrote: > >> After the change, both of them compile. test-netflow related > >> unit tests pass. > >> > >> test-sflow related tests do not pass because > >> of LOOPBACK_INTERFACE constraints for 'agent'. > >> (It should be revisited later.) > >> > >> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com> > > > > This patch kind of stands out to me when I look at it. It is awkward > > to have to create an extra object (wevent) on Windows but not use it > > anywhere except to pass to poll_fd_wait_event(). I didn't properly > > understand until now that this was necessary. > > > > Looking through the tree, I think that there are two or three distinct > > uses of wevents: > > > > 1. Some bits of code, like the ones added in this patch, don't > > care about the wevent directly at all. Instead, they > > really just want to wait on a socket. These pieces of code > > still have to create wevents and pass them around and > > eventually destroy them. This is the majority of use > > cases. > > > > 2. Other bits of code are oriented around wevents and do use > > them directly with calls to SetEvent(), ResetEvent, etc., > > and do not associate them with sockets. One example is > > daemon-windows.c. > > > > 3. fatal-signal.c appears to be a hybrid of these two cases, > > but I think that this could be replaced by use of a latch. > > > > Cases #1 and #2 are different enough that I wonder whether poll_loop > > should handle them differently. > > > > In case #1, the emphasis should be on convenience for the client. Can > > we make poll_loop automatically create a wevent when we poll a socket? > > (If this is expensive, then perhaps it could maintain a pool of > > wevents.) Since sockets are different from fds on Windows, maybe it > > would be best to have a function poll_loop_socket_wait() to make it > > clear that a socket is being polled. > I incorporated your suggestions with one difference. I did not rename > the function to poll_loop_socket_wait(). As that would create a total > of 3 functions > for waiting on file descriptors. > 1. Linux fds of pipes > 2. Linux and Windows sockets > 3. Windows events. > > If you still prefer a separate function, I will re-spin.
I'm happy with it as-is, please see my minor suggestions in the followup to your patch. Thank you! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
