| Message ID | 1503398837-2498-1-git-send-email-abalutoiu@cloudbasesolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu <abalutoiu@cloudbasesolutions.com> wrote: > The function poll from poller should return a list of tuples > containing the events and their types. > > On Windows the event type is not returned at the moment. > Instead of returning zero all the time, we check to see > the type of event and we set it accordingly before returning > the list. > > This is used only for debugging purposes inside the function > "__log_wakeup" later on. > > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > --- > python/ovs/poller.py | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/python/ovs/poller.py b/python/ovs/poller.py > index 809e512..2f3fcf9 100644 > --- a/python/ovs/poller.py > +++ b/python/ovs/poller.py > @@ -112,7 +112,14 @@ class _SelectSelect(object): > if retval == winutils.winerror.WAIT_TIMEOUT: > return [] > > - return [(events[retval], 0)] > + if events[retval] in self.rlist: > + revent = POLLIN > + elif events[retval] in self.wlist: > + revent = POLLOUT > + else: > + revent = POLLERR > + > + return [(events[retval], revent)] > else: > if timeout == -1: > # epoll uses -1 for infinite timeout, select uses None. Acked-by: Russell Bryant <russell@ovn.org> I tried looking up docs of WaitForMultipleObjects to look at possible return values. It looks like WAIT_FAILED could be returned and it would cause an exception when used as the index to events. If that's right, it was true before the patch as well, though.
Looking at the implementation of WaitForMultipleObjects in Python it looks that the call will raise an exception if WAIT_FAILED is returned. This case is treated by the try/except block around the WaitForMultipleObjects function. Thanks, Alin Balutoiu. > -----Original Message----- > From: Russell Bryant [mailto:russell@ovn.org] > Sent: Tuesday, August 22, 2017 2:22 PM > To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type returned > from poller > > On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu > <abalutoiu@cloudbasesolutions.com> wrote: > > The function poll from poller should return a list of tuples > > containing the events and their types. > > > > On Windows the event type is not returned at the moment. > > Instead of returning zero all the time, we check to see the type of > > event and we set it accordingly before returning the list. > > > > This is used only for debugging purposes inside the function > > "__log_wakeup" later on. > > > > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > > --- > > python/ovs/poller.py | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/python/ovs/poller.py b/python/ovs/poller.py index > > 809e512..2f3fcf9 100644 > > --- a/python/ovs/poller.py > > +++ b/python/ovs/poller.py > > @@ -112,7 +112,14 @@ class _SelectSelect(object): > > if retval == winutils.winerror.WAIT_TIMEOUT: > > return [] > > > > - return [(events[retval], 0)] > > + if events[retval] in self.rlist: > > + revent = POLLIN > > + elif events[retval] in self.wlist: > > + revent = POLLOUT > > + else: > > + revent = POLLERR > > + > > + return [(events[retval], revent)] > > else: > > if timeout == -1: > > # epoll uses -1 for infinite timeout, select uses None. > > Acked-by: Russell Bryant <russell@ovn.org> > > I tried looking up docs of WaitForMultipleObjects to look at possible return > values. It looks like WAIT_FAILED could be returned and it would cause an > exception when used as the index to events. If that's right, it was true > before the patch as well, though. > > -- > Russell Bryant
Ah, that makes sense for the Python version. The Python docs were sparse, so I wasn't sure. On Tue, Aug 22, 2017 at 8:35 AM, Alin Balutoiu <abalutoiu@cloudbasesolutions.com> wrote: > Looking at the implementation of WaitForMultipleObjects in Python it looks that > the call will raise an exception if WAIT_FAILED is returned. This case is treated by > the try/except block around the WaitForMultipleObjects function. > > Thanks, > Alin Balutoiu. > >> -----Original Message----- >> From: Russell Bryant [mailto:russell@ovn.org] >> Sent: Tuesday, August 22, 2017 2:22 PM >> To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> >> Cc: dev@openvswitch.org >> Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type returned >> from poller >> >> On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu >> <abalutoiu@cloudbasesolutions.com> wrote: >> > The function poll from poller should return a list of tuples >> > containing the events and their types. >> > >> > On Windows the event type is not returned at the moment. >> > Instead of returning zero all the time, we check to see the type of >> > event and we set it accordingly before returning the list. >> > >> > This is used only for debugging purposes inside the function >> > "__log_wakeup" later on. >> > >> > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> >> > --- >> > python/ovs/poller.py | 9 ++++++++- >> > 1 file changed, 8 insertions(+), 1 deletion(-) >> > >> > diff --git a/python/ovs/poller.py b/python/ovs/poller.py index >> > 809e512..2f3fcf9 100644 >> > --- a/python/ovs/poller.py >> > +++ b/python/ovs/poller.py >> > @@ -112,7 +112,14 @@ class _SelectSelect(object): >> > if retval == winutils.winerror.WAIT_TIMEOUT: >> > return [] >> > >> > - return [(events[retval], 0)] >> > + if events[retval] in self.rlist: >> > + revent = POLLIN >> > + elif events[retval] in self.wlist: >> > + revent = POLLOUT >> > + else: >> > + revent = POLLERR >> > + >> > + return [(events[retval], revent)] >> > else: >> > if timeout == -1: >> > # epoll uses -1 for infinite timeout, select uses None. >> >> Acked-by: Russell Bryant <russell@ovn.org> >> >> I tried looking up docs of WaitForMultipleObjects to look at possible return >> values. It looks like WAIT_FAILED could be returned and it would cause an >> exception when used as the index to events. If that's right, it was true >> before the patch as well, though. >> >> -- >> Russell Bryant
Does it make sense to use python ctypes instead of pypiwin32? It seems better to make our own calls instead of using wrappers which treat just certain cases. Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> > -----Original Message----- > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > bounces@openvswitch.org] On Behalf Of Russell Bryant > Sent: Tuesday, August 22, 2017 6:34 PM > To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type returned > from poller > > Ah, that makes sense for the Python version. > > The Python docs were sparse, so I wasn't sure. > > On Tue, Aug 22, 2017 at 8:35 AM, Alin Balutoiu > <abalutoiu@cloudbasesolutions.com> wrote: > > Looking at the implementation of WaitForMultipleObjects in Python it > > looks that the call will raise an exception if WAIT_FAILED is > > returned. This case is treated by the try/except block around the > WaitForMultipleObjects function. > > > > Thanks, > > Alin Balutoiu. > >
I also think that using ctypes instead of pypiwin32 would be a good idea. In this way we remove all the confusion caused by the implementation of wrappers from pypiwin32 and remove the dependency on the pypiwin32 module. Example for WaitForMultipleObjects where the wrapper from pypiwin32 raises an exception if the function returns WAIT_FAILED and returns the error code otherwise: https://github.com/pywin32/pypiwin32/blob/master/win32/src/win32event.i#L340-L343 For ReadFile, if we pass an overlapped structure and the ReadFile finished syncronously we will have to make an extra call to GetOverlappedResult to get the read number of bytes. This is due to the fact that the number of bytes read is not returned by the wrapper implementation from pypiwin32 and the buffer is not resized if an overlapped structure is passed. The implementation can be viewed here: https://github.com/pywin32/pypiwin32/blob/master/win32/src/win32file.i#L896-L987 > -----Original Message----- > From: aserdean@ovn.org [mailto:aserdean@ovn.org] > Sent: Wednesday, August 23, 2017 12:05 PM > To: 'Russell Bryant' <russell@ovn.org>; Alin Balutoiu > <abalutoiu@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subject: RE: [ovs-dev] [PATCH] windows, python: Fix event type returned > from poller > > Does it make sense to use python ctypes instead of pypiwin32? > > It seems better to make our own calls instead of using wrappers which treat > just certain cases. > > Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> > > > -----Original Message----- > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > > bounces@openvswitch.org] On Behalf Of Russell Bryant > > Sent: Tuesday, August 22, 2017 6:34 PM > > To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type > > returned from poller > > > > Ah, that makes sense for the Python version. > > > > The Python docs were sparse, so I wasn't sure. > > > > On Tue, Aug 22, 2017 at 8:35 AM, Alin Balutoiu > > <abalutoiu@cloudbasesolutions.com> wrote: > > > Looking at the implementation of WaitForMultipleObjects in Python it > > > looks that the call will raise an exception if WAIT_FAILED is > > > returned. This case is treated by the try/except block around the > > WaitForMultipleObjects function. > > > > > > Thanks, > > > Alin Balutoiu. > > >
Thanks Russel and Alin. I applied this on branch-2.7, branch-2.8 and master. > -----Original Message----- > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > bounces@openvswitch.org] On Behalf Of Russell Bryant > Sent: Tuesday, August 22, 2017 6:34 PM > To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type returned > from poller > > Ah, that makes sense for the Python version. > > The Python docs were sparse, so I wasn't sure. > > On Tue, Aug 22, 2017 at 8:35 AM, Alin Balutoiu > <abalutoiu@cloudbasesolutions.com> wrote: > > Looking at the implementation of WaitForMultipleObjects in Python it > > looks that the call will raise an exception if WAIT_FAILED is > > returned. This case is treated by the try/except block around the > WaitForMultipleObjects function. > > > > Thanks, > > Alin Balutoiu. > > > >> -----Original Message----- > >> From: Russell Bryant [mailto:russell@ovn.org] > >> Sent: Tuesday, August 22, 2017 2:22 PM > >> To: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > >> Cc: dev@openvswitch.org > >> Subject: Re: [ovs-dev] [PATCH] windows, python: Fix event type > >> returned from poller > >> > >> On Tue, Aug 22, 2017 at 6:47 AM, Alin Balutoiu > >> <abalutoiu@cloudbasesolutions.com> wrote: > >> > The function poll from poller should return a list of tuples > >> > containing the events and their types. > >> > > >> > On Windows the event type is not returned at the moment. > >> > Instead of returning zero all the time, we check to see the type of > >> > event and we set it accordingly before returning the list. > >> > > >> > This is used only for debugging purposes inside the function > >> > "__log_wakeup" later on. > >> > > >> > Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> > >> > --- > >> > python/ovs/poller.py | 9 ++++++++- > >> > 1 file changed, 8 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/python/ovs/poller.py b/python/ovs/poller.py index > >> > 809e512..2f3fcf9 100644 > >> > --- a/python/ovs/poller.py > >> > +++ b/python/ovs/poller.py > >> > @@ -112,7 +112,14 @@ class _SelectSelect(object): > >> > if retval == winutils.winerror.WAIT_TIMEOUT: > >> > return [] > >> > > >> > - return [(events[retval], 0)] > >> > + if events[retval] in self.rlist: > >> > + revent = POLLIN > >> > + elif events[retval] in self.wlist: > >> > + revent = POLLOUT > >> > + else: > >> > + revent = POLLERR > >> > + > >> > + return [(events[retval], revent)] > >> > else: > >> > if timeout == -1: > >> > # epoll uses -1 for infinite timeout, select uses None. > >> > >> Acked-by: Russell Bryant <russell@ovn.org> > >> > >> I tried looking up docs of WaitForMultipleObjects to look at possible > >> return values. It looks like WAIT_FAILED could be returned and it > >> would cause an exception when used as the index to events. If that's > >> right, it was true before the patch as well, though. > >> > >> -- > >> Russell Bryant > > > > -- > Russell Bryant > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/python/ovs/poller.py b/python/ovs/poller.py index 809e512..2f3fcf9 100644 --- a/python/ovs/poller.py +++ b/python/ovs/poller.py @@ -112,7 +112,14 @@ class _SelectSelect(object): if retval == winutils.winerror.WAIT_TIMEOUT: return [] - return [(events[retval], 0)] + if events[retval] in self.rlist: + revent = POLLIN + elif events[retval] in self.wlist: + revent = POLLOUT + else: + revent = POLLERR + + return [(events[retval], revent)] else: if timeout == -1: # epoll uses -1 for infinite timeout, select uses None.
The function poll from poller should return a list of tuples containing the events and their types. On Windows the event type is not returned at the moment. Instead of returning zero all the time, we check to see the type of event and we set it accordingly before returning the list. This is used only for debugging purposes inside the function "__log_wakeup" later on. Signed-off-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com> --- python/ovs/poller.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)