Re: [patch] add an aio test which closes the fd before destroying the ioctx

[Dave Chinner <david@fromorbit.com>]

On Wed, Jun 18, 2014 at 02:17:01PM -0400, Jeff Moyer wrote:
> Dave Chinner <david@fromorbit.com> writes:
> >> +  CPU_ZERO(&cpuset);
> >> +  CPU_SET(mycpu, &cpuset);
> >> +  if (sched_setaffinity(mytid, sizeof(cpuset), &cpuset)) {
> >> +    printf("FAILED to set thread %d to run on cpu %ld\n", mytid, mycpu);
> >> +  }
> >
> > Any reason why process layout like this is necessary? Comments are
> > nice, even in test code...
> 
> I honestly don't remember the specifics, other than it gave a better
> probability for reproducing the failure.  I can at least make a comment
> stating that.

Sounds good.

> >> +#-----------------------------------------------------------------------
> >> +# Copyright (c) 2014 Jeff Moyer.  All Rights Reserved.
> >
> > Sent from a Red Hat email address. Can you please clarify who owns
> > the copyright?
> 
> The script is mine, the code is Matt's.  There are copyright headers in
> both, and they attribute the copyright to the proper individual.  What
> isn't clear about that?

I have a patch that is:


From: Jeff Moyer <jmoyer@redhat.com>

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

with code you wrote but doesn't have a Red Hat copyright on it. I
don't know when you wrote it or what context it was written in, but normally
the copyright of work done on behalf of an employer is owned by the
employer. Hence the ownership for the portions of the patch you wrote
is not clear to me from the information presented and so I've asked
for clarification.

FWIW, most people use their personal email addresses for code they
write that is not owned by their employer to avoid this confusion.

> >> +
> >> +_run_323() {
> >> +	local testtemp=$TEST_DIR/aio-testfile
> >
> > testfile?
> 
> Are you suggesting that I rename aio-tesfile to testfile?  Or rename
> testtempt to testfile?  Something else?

testtemp is a terrible variable name. It's a test file.

> >> +	return $status
> >> +}
> >> +
> >> +_run_323
> >
> > Like I said, no need for a function for such a simple test... 
> 
> I'm no bash guru.  I created a function in order to avoid creating a
> global testtemp variable.  If that doesn't matter, I'm happen to flatten
> things out.

For xfstests, library functions and internal variables are preceeded
by a "_" and global variables are in upper case so you don't have to
worry about namespace pollution your the test script.

Cheers,

Dave.


-- 
Dave Chinner
david@fromorbit.com