[Piglit] [PATCH] ext_image_dma_buf_import: update ownership_transfer test

[Pohjolainen, Topi]

On Fri, Aug 08, 2014 at 02:50:26PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> The EGL_EXT_image_dma_buf_import specification was revised (according to
> its revision history) on Dec 5th, 2013, for EGL to not take ownership of the
> file descriptors.
> 
> Update the ownership_transfer test to have the correct quote from the
> latest specification (version 6), and invert the final test on close().
> 
> Now the test verifies, that after a successful import into EGL, followed
> by guaranteed destruction of the EGLImage and the buffer reference
> inside EGL, the dma_buf file descriptor still stays valid in the caller.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> ---
> 
> I do not have commit access to piglit, and this is my first submission,
> IIRC.
> 
> This change causes Mesa to regress on this particular Piglit test. I am
> planning to fix Mesa, too.
> 
> I also didn't really understand the comment below the spec quote, so I
> left it as is. It seems this test does not test at all that the buffer
> stays valid in EGL after the creator side has been destroyed. This is
> only about (not) keeping the dmabuf file descriptor open.

This is correct, and your change is the right thing to do.

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

Same as in case of the mesa patch, lets wait for other people to comment
as well.

> 
> Thanks,
> pq
> ---
>  tests/spec/ext_image_dma_buf_import/ownership_transfer.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/spec/ext_image_dma_buf_import/ownership_transfer.c b/tests/spec/ext_image_dma_buf_import/ownership_transfer.c
> index ff51810..e3e34b6 100644
> --- a/tests/spec/ext_image_dma_buf_import/ownership_transfer.c
> +++ b/tests/spec/ext_image_dma_buf_import/ownership_transfer.c
> @@ -32,8 +32,9 @@
>   *
>   * "3. Does ownership of the file descriptor pass to the EGL library?
>   *
> - *   ANSWER: If eglCreateImageKHR is successful, EGL assumes ownership of the
> - *   file descriptors and is responsible for closing them."
> + *   ANSWER: No, EGL does not take ownership of the file descriptors. It is the
> + *   responsibility of the application to close the file descriptors on success
> + *   and failure.
>   *
>   *
>   * Here one checks that the creator of the buffer can drop its reference once
> @@ -99,8 +100,12 @@ test_create_and_destroy(unsigned w, unsigned h, void *buf, int fd,
>  		return PIGLIT_FAIL;
>  	}
>  
> -	/* EGL stack should have closed the importing file descriptor by now */
> -	return close(fd) && errno == EBADF ? PIGLIT_PASS : PIGLIT_FAIL;
> +	/*
> +	 * EGL stack should have closed the importing file descriptor by now,

Just drop this line here - "importing file descriptor" meant the very same
fd we gave to the EGL stack (and the one we are about to close here).

> +	 * but our own file descriptor must still be valid, and therefore
> +	 * closing it must succeed.
> +	 */
> +	return close(fd) == 0 ? PIGLIT_PASS : PIGLIT_FAIL;
>  }
>  
>  enum piglit_result
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit