[PATCH] tevent test suite fixes for Solaris.

[Michael Adam]

Hi Jeremy,

sorry for the delay!

I am currently reviewing and testing the patchset.
Will push later today.

Cheers - Michael

On 2013-03-08 at 14:11 -0800, Jeremy Allison wrote:
> On Fri, Mar 08, 2013 at 10:23:19AM -0800, Jeremy Allison wrote:
> > So I finally got my Solaris/Nexenta/Illumos
> > virtual machine that Gordon Ross gave me a while
> > ago up and running to do some tests on.
> > 
> > Turns out this fix is needed for the tevent
> > testsuite as by default Solaris creates bi-directional
> > pipes from the pipe() system call.
> > 
> > Please review and push if you're happy.
> 
> Ok, here's a follow-up I've tested on my Solaris/Nexenta/Illumos
> box. It allows :
> 
> bin/smbtorture //127.0.0.1/tmp local.event
> 
> to pass successfully on the Solaris build.
> 
> There were 2 issues.
> 
> 1). Solaris creates bi-directional pipes from the pipe() system call.
> 2). Once a pipe is full even when 1 byte is read Solaris doesn't
> flag the pipe as writable again until 4096 bytes have been read.
> 
> This screams of PIPE_MAX/PIPE_BUF atomicity to me, but thankfully
> it's simple to change the test_event_fd2_sock_handler() code to
> cope with this - without using any magic numbers (other than
> PIPE_BUF, which is defined in limits.h anyway).
> 
> This also passes on Linux of course :-). Metze, Ira, please
> review and push if you're happy. It should go into 4.0.next
> also as it allows the local.events test to pass in the 4.0.next
> tree when being run on Solaris/Nexenta.
> 
> Note that both of these are only changes to the test suite
> to cope with the idiosyncrasies of the Solaris implementation,
> no changes to the core tevent libraries are needed.
> 
> Cheers,
> 
> 	Jeremy.

> From 2dab691ee8f36d7c4b690ff4bd0c93386a2e31f4 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Fri, 8 Mar 2013 10:09:01 -0800
> Subject: [PATCH 1/2] Solaris/Illumos/Nexenta creates pipes that are
>  bi-directional by default.
> 
> Ensure the test code will pass against such a system (allow writes/reads
> going both ways).
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  lib/tevent/testsuite.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
> index 8e3f4af..1fcfa1c 100644
> --- a/lib/tevent/testsuite.c
> +++ b/lib/tevent/testsuite.c
> @@ -60,22 +60,28 @@ static void fde_handler_write(struct tevent_context *ev_ctx, struct tevent_fd *f
>  }
>  
>  
> -/* These should never fire... */
> +/* This will only fire if the fd's returned from pipe() are bi-directional. */
>  static void fde_handler_read_1(struct tevent_context *ev_ctx, struct tevent_fd *f,
>  			uint16_t flags, void *private_data)
>  {
> -	struct torture_context *test = (struct torture_context *)private_data;
> -	torture_comment(test, "fde_handler_read_1 should never fire !\n");
> -	abort();
> +	int *fd = (int *)private_data;
> +	char c;
> +#ifdef SA_SIGINFO
> +	kill(getpid(), SIGUSR1);
> +#endif
> +	kill(getpid(), SIGALRM);
> +
> +	read(fd[1], &c, 1);
> +	fde_count++;
>  }
>  
> -/* These should never fire... */
> +/* This will only fire if the fd's returned from pipe() are bi-directional. */
>  static void fde_handler_write_1(struct tevent_context *ev_ctx, struct tevent_fd *f,
>  			uint16_t flags, void *private_data)
>  {
> -	struct torture_context *test = (struct torture_context *)private_data;
> -	torture_comment(test, "fde_handler_write_1 should never fire !\n");
> -	abort();
> +	int *fd = (int *)private_data;
> +	char c = 0;
> +	write(fd[0], &c, 1);
>  }
>  
>  static void finished_handler(struct tevent_context *ev_ctx, struct tevent_timer *te,
> @@ -133,12 +139,12 @@ static bool test_event_context(struct torture_context *test,
>  	fde_read = tevent_add_fd(ev_ctx, ev_ctx, fd[0], TEVENT_FD_READ,
>  			    fde_handler_read, fd);
>  	fde_write_1 = tevent_add_fd(ev_ctx, ev_ctx, fd[0], TEVENT_FD_WRITE,
> -			    fde_handler_write_1, test);
> +			    fde_handler_write_1, fd);
>  
>  	fde_write = tevent_add_fd(ev_ctx, ev_ctx, fd[1], TEVENT_FD_WRITE,
>  			    fde_handler_write, fd);
>  	fde_read_1 = tevent_add_fd(ev_ctx, ev_ctx, fd[1], TEVENT_FD_READ,
> -			    fde_handler_read_1, test);
> +			    fde_handler_read_1, fd);
>  
>  	tevent_fd_set_auto_close(fde_read);
>  	tevent_fd_set_auto_close(fde_write);
> -- 
> 1.8.1.3
> 
> 
> From 6360ae475533af035c6f50922cc5ca01670ac505 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Fri, 8 Mar 2013 13:44:08 -0800
> Subject: [PATCH 2/2] Fix tevent testsuite issue on Solaris.
> 
> On Solaris/Nexenta/Illumos once a pipe is full it will not be reported
> as writable until PIPE_BUF (actually on Solaris 4096, which is less than
> PIPE_BUF) bytes have been read from it.
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  lib/tevent/testsuite.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
> index 1fcfa1c..7851c6c 100644
> --- a/lib/tevent/testsuite.c
> +++ b/lib/tevent/testsuite.c
> @@ -482,8 +482,13 @@ static void test_event_fd2_sock_handler(struct tevent_context *ev_ctx,
>  		return;
>  	}
>  
> -	if (oth_sock->num_read > 0) {
> +	if (oth_sock->num_read >= PIPE_BUF) {
>  		/*
> +		 * On Linux we become writable once we've read
> +		 * one byte. On Solaris we only become writable
> +		 * again once we've read 4096 bytes. PIPE_BUF
> +		 * is probably a safe bet to test against.
> +		 *
>  		 * There should be room to write a byte again
>  		 */
>  		if (!(flags & TEVENT_FD_WRITE)) {
> -- 
> 1.8.1.3
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130322/76b88d34/attachment.pgp>