* [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling @ 2014-11-26 15:09 Andrew Cooper 2014-11-26 15:38 ` Zheng Li 0 siblings, 1 reply; 10+ messages in thread From: Andrew Cooper @ 2014-11-26 15:09 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, Dave Scott, Andrew Cooper, Ian Jackson, Zheng Li, Ian Campbell This makes fields 0 and 1 true more often than they should be, resulting problems when handling events. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Ian Campbell <Ian.Campbell@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Dave Scott <Dave.Scott@eu.citrix.com> CC: Zheng Li <zheng.li3@citrix.com> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- This was discovered with XenServers internal Coverity instance. I have yet to work out why the issue is not identified by the upstream coverity scanning. Konrad: This is a bug in the default event handling used by oxenstored in 4.5, as the default switched from select() to poll() in the 4.5 timeframe. It would appear that the negative side effects are limited to just logspam about certain clients attempting invalid actions, but I can't rule out anything more problematic. --- tools/ocaml/xenstored/select_stubs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c index 4a8edb5..af72b84 100644 --- a/tools/ocaml/xenstored/select_stubs.c +++ b/tools/ocaml/xenstored/select_stubs.c @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { events = Field(Field(fd_events, i), 1); if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 15:09 [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling Andrew Cooper @ 2014-11-26 15:38 ` Zheng Li 2014-11-26 18:24 ` Dave Scott 0 siblings, 1 reply; 10+ messages in thread From: Zheng Li @ 2014-11-26 15:38 UTC (permalink / raw) To: Andrew Cooper, Xen-devel Cc: Wei Liu, Ian Campbell, Ian Jackson, Zheng Li, Dave Scott On 26/11/2014 15:09, Andrew Cooper wrote: > This makes fields 0 and 1 true more often than they should be, resulting > problems when handling events. Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! Acked-by: Zheng Li <dev@zheng.li> Cheers, Zheng > --- > > This was discovered with XenServers internal Coverity instance. I have yet to > work out why the issue is not identified by the upstream coverity scanning. > > Konrad: This is a bug in the default event handling used by oxenstored in 4.5, > as the default switched from select() to poll() in the 4.5 timeframe. It > would appear that the negative side effects are limited to just logspam about > certain clients attempting invalid actions, but I can't rule out anything more > problematic. > --- > tools/ocaml/xenstored/select_stubs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c > index 4a8edb5..af72b84 100644 > --- a/tools/ocaml/xenstored/select_stubs.c > +++ b/tools/ocaml/xenstored/select_stubs.c > @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { > events = Field(Field(fd_events, i), 1); > > if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); > - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); > > } > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 15:38 ` Zheng Li @ 2014-11-26 18:24 ` Dave Scott 2014-11-26 18:41 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 10+ messages in thread From: Dave Scott @ 2014-11-26 18:24 UTC (permalink / raw) To: Zheng Li Cc: Dave Scott, Wei Liu, Ian Campbell, Andrew Cooper, Xen-devel, Zheng Li (3P), Ian Jackson > On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: > > On 26/11/2014 15:09, Andrew Cooper wrote: >> This makes fields 0 and 1 true more often than they should be, resulting >> problems when handling events. > > Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! > > Acked-by: Zheng Li <dev@zheng.li> This also looks fine to me Acked-by: David Scott <dave.scott@citrix.com> Cheers, Dave > > > Cheers, > Zheng > > >> --- >> >> This was discovered with XenServers internal Coverity instance. I have yet to >> work out why the issue is not identified by the upstream coverity scanning. >> >> Konrad: This is a bug in the default event handling used by oxenstored in 4.5, >> as the default switched from select() to poll() in the 4.5 timeframe. It >> would appear that the negative side effects are limited to just logspam about >> certain clients attempting invalid actions, but I can't rule out anything more >> problematic. >> --- >> tools/ocaml/xenstored/select_stubs.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c >> index 4a8edb5..af72b84 100644 >> --- a/tools/ocaml/xenstored/select_stubs.c >> +++ b/tools/ocaml/xenstored/select_stubs.c >> @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { >> events = Field(Field(fd_events, i), 1); >> >> if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); >> - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); >> - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); >> + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); >> + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); >> Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); >> >> } >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 18:24 ` Dave Scott @ 2014-11-26 18:41 ` Konrad Rzeszutek Wilk 2014-11-26 19:03 ` Andrew Cooper 2014-11-26 20:44 ` Dave Scott 0 siblings, 2 replies; 10+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-11-26 18:41 UTC (permalink / raw) To: Dave Scott Cc: Zheng Li, Wei Liu, Ian Campbell, Andrew Cooper, Xen-devel, Zheng Li (3P), Ian Jackson On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: > > > On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: > > > > On 26/11/2014 15:09, Andrew Cooper wrote: > >> This makes fields 0 and 1 true more often than they should be, resulting > >> problems when handling events. > > > > Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! > > > > Acked-by: Zheng Li <dev@zheng.li> > > This also looks fine to me > > Acked-by: David Scott <dave.scott@citrix.com> Would it be possible to get an Reviewed-by please? Thank you. > > Cheers, > Dave > > > > > > > Cheers, > > Zheng > > > > > >> --- > >> > >> This was discovered with XenServers internal Coverity instance. I have yet to > >> work out why the issue is not identified by the upstream coverity scanning. > >> > >> Konrad: This is a bug in the default event handling used by oxenstored in 4.5, > >> as the default switched from select() to poll() in the 4.5 timeframe. It > >> would appear that the negative side effects are limited to just logspam about > >> certain clients attempting invalid actions, but I can't rule out anything more > >> problematic. > >> --- > >> tools/ocaml/xenstored/select_stubs.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c > >> index 4a8edb5..af72b84 100644 > >> --- a/tools/ocaml/xenstored/select_stubs.c > >> +++ b/tools/ocaml/xenstored/select_stubs.c > >> @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { > >> events = Field(Field(fd_events, i), 1); > >> > >> if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); > >> - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > >> - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > >> + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > >> + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > >> Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); > >> > >> } > >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 18:41 ` Konrad Rzeszutek Wilk @ 2014-11-26 19:03 ` Andrew Cooper 2014-11-26 20:08 ` Zheng Li 2014-11-27 8:55 ` Ian Campbell 2014-11-26 20:44 ` Dave Scott 1 sibling, 2 replies; 10+ messages in thread From: Andrew Cooper @ 2014-11-26 19:03 UTC (permalink / raw) To: Konrad Rzeszutek Wilk, Dave Scott Cc: Zheng Li, Wei Liu, Ian Campbell, Xen-devel, Zheng Li (3P), Ian Jackson On 26/11/14 18:41, Konrad Rzeszutek Wilk wrote: > On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: >>> On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: >>> >>> On 26/11/2014 15:09, Andrew Cooper wrote: >>>> This makes fields 0 and 1 true more often than they should be, resulting >>>> problems when handling events. >>> Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! >>> >>> Acked-by: Zheng Li <dev@zheng.li> >> This also looks fine to me >> >> Acked-by: David Scott <dave.scott@citrix.com> > Would it be possible to get an Reviewed-by please? Strictly speaking Zheng, not being a maintainer, can't ack the patch, given what I believe to be Xens current rules for these things. However, as the author of the code and comment in this thread, his ack can reasonably be considered equivalent to a Reviewed-by: I guess this is just a matter of semantics. Furthermore, as we all share an office, I have already been through this process informally, and have confirmed the fix under my Xen-4.5 based XenServer branch. There appear to be 100% less "error -EINVAL" messages in the logs. ~Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 19:03 ` Andrew Cooper @ 2014-11-26 20:08 ` Zheng Li 2014-11-27 8:55 ` Ian Campbell 1 sibling, 0 replies; 10+ messages in thread From: Zheng Li @ 2014-11-26 20:08 UTC (permalink / raw) To: Andrew Cooper, Konrad Rzeszutek Wilk, Dave Scott Cc: Ian Jackson, Wei Liu, Ian Campbell, Xen-devel On 26/11/2014 19:03, Andrew Cooper wrote: > Strictly speaking Zheng, not being a maintainer, can't ack the patch, > given what I believe to be Xens current rules for these things. > However, as the author of the code and comment in this thread, his ack > can reasonably be considered equivalent to a Reviewed-by: I guess this > is just a matter of semantics. > > Furthermore, as we all share an office, I have already been through this > process informally, and have confirmed the fix under my Xen-4.5 based > XenServer branch. There appear to be 100% less "error -EINVAL" messages > in the logs. > Agreed. I was a bit confused by the "XXX-by" semantics. As Andy mentioned, he already went through the patch with me offline, so here is the line if more appropriate: Reviewed-by: Zheng Li <dev@zheng.li> Thanks, Zheng ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 19:03 ` Andrew Cooper 2014-11-26 20:08 ` Zheng Li @ 2014-11-27 8:55 ` Ian Campbell 1 sibling, 0 replies; 10+ messages in thread From: Ian Campbell @ 2014-11-27 8:55 UTC (permalink / raw) To: Andrew Cooper Cc: Dave Scott, Wei Liu, Zheng Li, Xen-devel, Zheng Li (3P), Ian Jackson On Wed, 2014-11-26 at 19:03 +0000, Andrew Cooper wrote: > On 26/11/14 18:41, Konrad Rzeszutek Wilk wrote: > > On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: > >>> On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: > >>> > >>> On 26/11/2014 15:09, Andrew Cooper wrote: > >>>> This makes fields 0 and 1 true more often than they should be, resulting > >>>> problems when handling events. > >>> Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! > >>> > >>> Acked-by: Zheng Li <dev@zheng.li> > >> This also looks fine to me > >> > >> Acked-by: David Scott <dave.scott@citrix.com> > > Would it be possible to get an Reviewed-by please? > > Strictly speaking Zheng, not being a maintainer, can't ack the patch, > given what I believe to be Xens current rules for these things. > However, as the author of the code and comment in this thread, his ack > can reasonably be considered equivalent to a Reviewed-by: I guess this > is just a matter of semantics. In theory/According to https://www.kernel.org/doc/Documentation/SubmittingPatches Reviewed-by "indicates that the patch has been reviewed and found acceptable according to the Reviewer's Statement: Reviewer's statement of oversight By offering my Reviewed-by: tag, I state that: (a) I have carried out a technical review of this patch to evaluate its appropriateness and readiness for inclusion into the mainline kernel. (b) Any problems, concerns, or questions relating to the patch have been communicated back to the submitter. I am satisfied with the submitter's response to my comments. (c) While there may be things that could be improved with this submission, I believe that it is, at this time, (1) a worthwhile modification to the kernel, and (2) free of known issues which would argue against its inclusion. (d) While I have reviewed the patch and believe it to be sound, I do not (unless explicitly stated elsewhere) make any warranties or guarantees that it will achieve its stated purpose or function properly in any given situation. Whereas Acked-by is just an indication of no-objections or fine-by-me from the maintainer or possibly a previous reviewer indicating that their previous concerns have been removed. That said when they come from someone relevant to the code at hand (as e.g. Zheng is here) personally I mostly treat them the same (and I pretty much always say Acked-by not Reviewed-by because my fingers just do that by default). I think there are others in the project who do treat them as distinct. All in all I think it's safe to say that the XenProject neither implements any distinction in a very strict way in practice nor has a very consistent view on the differences between them. Personally I don't think the distinction really matters a great deal and we have more than enough rules and process as it is without getting too worked up about Acked vs Reviewed by. Ian. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 18:41 ` Konrad Rzeszutek Wilk 2014-11-26 19:03 ` Andrew Cooper @ 2014-11-26 20:44 ` Dave Scott 2014-11-26 21:09 ` Konrad Rzeszutek Wilk 1 sibling, 1 reply; 10+ messages in thread From: Dave Scott @ 2014-11-26 20:44 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Zheng Li, Wei Liu, Ian Campbell, Andrew Cooper, Xen-devel, Zheng Li (3P), Ian Jackson > On 26 Nov 2014, at 18:41, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: >> >>> On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: >>> >>> On 26/11/2014 15:09, Andrew Cooper wrote: >>>> This makes fields 0 and 1 true more often than they should be, resulting >>>> problems when handling events. >>> >>> Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! >>> >>> Acked-by: Zheng Li <dev@zheng.li> >> >> This also looks fine to me >> >> Acked-by: David Scott <dave.scott@citrix.com> > > Would it be possible to get an Reviewed-by please? I’ll certainly offer Reviewed-by: David Scott <dave.scott@citrix.com> Cheers, Dave > > Thank you. >> >> Cheers, >> Dave >> >>> >>> >>> Cheers, >>> Zheng >>> >>> >>>> --- >>>> >>>> This was discovered with XenServers internal Coverity instance. I have yet to >>>> work out why the issue is not identified by the upstream coverity scanning. >>>> >>>> Konrad: This is a bug in the default event handling used by oxenstored in 4.5, >>>> as the default switched from select() to poll() in the 4.5 timeframe. It >>>> would appear that the negative side effects are limited to just logspam about >>>> certain clients attempting invalid actions, but I can't rule out anything more >>>> problematic. >>>> --- >>>> tools/ocaml/xenstored/select_stubs.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c >>>> index 4a8edb5..af72b84 100644 >>>> --- a/tools/ocaml/xenstored/select_stubs.c >>>> +++ b/tools/ocaml/xenstored/select_stubs.c >>>> @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { >>>> events = Field(Field(fd_events, i), 1); >>>> >>>> if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); >>>> - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); >>>> - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); >>>> + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); >>>> + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); >>>> Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); >>>> >>>> } >>>> >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 20:44 ` Dave Scott @ 2014-11-26 21:09 ` Konrad Rzeszutek Wilk 2014-11-28 12:11 ` Ian Campbell 0 siblings, 1 reply; 10+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-11-26 21:09 UTC (permalink / raw) To: Dave Scott Cc: Zheng Li, Wei Liu, Ian Campbell, Andrew Cooper, Xen-devel, Zheng Li (3P), Ian Jackson On Wed, Nov 26, 2014 at 08:44:41PM +0000, Dave Scott wrote: > > > On 26 Nov 2014, at 18:41, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > > > On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: > >> > >>> On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: > >>> > >>> On 26/11/2014 15:09, Andrew Cooper wrote: > >>>> This makes fields 0 and 1 true more often than they should be, resulting > >>>> problems when handling events. > >>> > >>> Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! > >>> > >>> Acked-by: Zheng Li <dev@zheng.li> > >> > >> This also looks fine to me > >> > >> Acked-by: David Scott <dave.scott@citrix.com> > > > > Would it be possible to get an Reviewed-by please? > > I’ll certainly offer > > Reviewed-by: David Scott <dave.scott@citrix.com> OK, Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thank you. > > Cheers, > Dave > > > > > Thank you. > >> > >> Cheers, > >> Dave > >> > >>> > >>> > >>> Cheers, > >>> Zheng > >>> > >>> > >>>> --- > >>>> > >>>> This was discovered with XenServers internal Coverity instance. I have yet to > >>>> work out why the issue is not identified by the upstream coverity scanning. > >>>> > >>>> Konrad: This is a bug in the default event handling used by oxenstored in 4.5, > >>>> as the default switched from select() to poll() in the 4.5 timeframe. It > >>>> would appear that the negative side effects are limited to just logspam about > >>>> certain clients attempting invalid actions, but I can't rule out anything more > >>>> problematic. > >>>> --- > >>>> tools/ocaml/xenstored/select_stubs.c | 4 ++-- > >>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/tools/ocaml/xenstored/select_stubs.c b/tools/ocaml/xenstored/select_stubs.c > >>>> index 4a8edb5..af72b84 100644 > >>>> --- a/tools/ocaml/xenstored/select_stubs.c > >>>> +++ b/tools/ocaml/xenstored/select_stubs.c > >>>> @@ -56,8 +56,8 @@ CAMLprim value stub_select_on_poll(value fd_events, value timeo) { > >>>> events = Field(Field(fd_events, i), 1); > >>>> > >>>> if (c_fds[i].revents & POLLNVAL) unix_error(EBADF, "select", Nothing); > >>>> - Field(events, 0) = Val_bool(c_fds[i].events | POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > >>>> - Field(events, 1) = Val_bool(c_fds[i].events | POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > >>>> + Field(events, 0) = Val_bool(c_fds[i].events & POLLIN && c_fds[i].revents & (POLLIN |POLLHUP|POLLERR)); > >>>> + Field(events, 1) = Val_bool(c_fds[i].events & POLLOUT && c_fds[i].revents & (POLLOUT|POLLHUP|POLLERR)); > >>>> Field(events, 2) = Val_bool(c_fds[i].revents & POLLPRI); > >>>> > >>>> } > >>>> > >> > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling 2014-11-26 21:09 ` Konrad Rzeszutek Wilk @ 2014-11-28 12:11 ` Ian Campbell 0 siblings, 0 replies; 10+ messages in thread From: Ian Campbell @ 2014-11-28 12:11 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Dave Scott, Wei Liu, Andrew Cooper, Zheng Li, Xen-devel, Zheng Li (3P), Ian Jackson On Wed, 2014-11-26 at 16:09 -0500, Konrad Rzeszutek Wilk wrote: > On Wed, Nov 26, 2014 at 08:44:41PM +0000, Dave Scott wrote: > > > > > On 26 Nov 2014, at 18:41, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > > > > > On Wed, Nov 26, 2014 at 06:24:11PM +0000, Dave Scott wrote: > > >> > > >>> On 26 Nov 2014, at 15:38, Zheng Li <dev@zheng.li> wrote: > > >>> > > >>> On 26/11/2014 15:09, Andrew Cooper wrote: > > >>>> This makes fields 0 and 1 true more often than they should be, resulting > > >>>> problems when handling events. > > >>> > > >>> Indeed, looks like a mistake I made when rewriting the logic terms lately. The result is POLLUP or POLLERR events being returned in more categories than we'd interest. Thanks for fixing this! > > >>> > > >>> Acked-by: Zheng Li <dev@zheng.li> > > >> > > >> This also looks fine to me > > >> > > >> Acked-by: David Scott <dave.scott@citrix.com> > > > > > > Would it be possible to get an Reviewed-by please? > > > > I’ll certainly offer > > > > Reviewed-by: David Scott <dave.scott@citrix.com> > > OK, Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Applied, thanks. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-11-28 12:11 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-26 15:09 [PATCH for-4.5] tools/oxenstored: Fix | vs & error in fd event handling Andrew Cooper 2014-11-26 15:38 ` Zheng Li 2014-11-26 18:24 ` Dave Scott 2014-11-26 18:41 ` Konrad Rzeszutek Wilk 2014-11-26 19:03 ` Andrew Cooper 2014-11-26 20:08 ` Zheng Li 2014-11-27 8:55 ` Ian Campbell 2014-11-26 20:44 ` Dave Scott 2014-11-26 21:09 ` Konrad Rzeszutek Wilk 2014-11-28 12:11 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.