[LTP] [PATCH 2/2] nfs05: rewrite the test, make use of new library

[Alexey Kodanev <alexey.kodanev@oracle.com>]

Hi,
On 06/22/2016 03:30 PM, Cyril Hrubis wrote:
> Hi!
>> --- a/testcases/network/nfs/nfs_stress/Makefile
>> +++ b/testcases/network/nfs/nfs_stress/Makefile
>> @@ -19,7 +19,7 @@ top_srcdir		?= ../../../..
>>   include $(top_srcdir)/include/mk/env_pre.mk
>>   
>>   nfs04_create_file: CPPFLAGS += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>> -nfs05_make_tree: LDLIBS += -lpthread
>> +nfs05_make_tree: LDLIBS += -I../../../kernel/include -lltp -lpthread
> Hmm, shouldn't we rather include testcase.mk to get the -lltp and
> the include path?
>
> And even if we don't the -I directive should go to CPPFLAGS instead.
Agree.

>>   INSTALL_TARGETS		:= nfs_lib.sh \
>>   			   nfs01 \
>> diff --git a/testcases/network/nfs/nfs_stress/nfs05 b/testcases/network/nfs/nfs_stress/nfs05
>> index 0e15c8c..26977ab 100755
>> --- a/testcases/network/nfs/nfs_stress/nfs05
>> +++ b/testcases/network/nfs/nfs_stress/nfs05
>> @@ -34,8 +34,12 @@ THREAD_NUM=${THREAD_NUM:-"8"}
>>   nfs_setup
> Shouldn't we check that make and gcc is installed at this point?

I'll add a check to make sure they are available.

>
>> ...
>> -			}
>> +static void *thread_fn(LTP_ATTRIBUTE_UNUSED void *args)
>> +{
>> +	const char prog_buf[] = "main()\n{\n\t printf(\"hello world\");\n}\n";
>> +	const size_t prog_buf_size = strlen(prog_buf);
>                                       ^
> 				     sizeof(prog_buf) - 1

Right, we can calculate the size at compile-time.

>> +	
>>   
>> -			dprt("pid[%d]: init_compile(): status = %d\n",
>> -			     gettid(), status);
>> -			dprt("we are here %d\n", __LINE__);
>> -			return status;
>> +			SAFE_WRITE(1, fd, prog_buf, prog_buf_size);
>> +			fsync(fd);
> Why do we fsync(fd) here? Isn't the whole point of this test to find a
> race conditions?

And given the fact that we have close(fd) thereafter, should make it 
useless...

Thanks,
Alexey