Re: [PATCH v2 2/2] xfstests: introduce 280 for SEEK_DATA/SEEK_HOLE copy check

[Jeff Liu <jeff.liu@oracle.com>]

Hi Mark,

On 02/07/2012 06:30 AM, Mark Tinguely wrote:

> On 02/06/12 08:30, Jeff Liu wrote:
>> Introduce 280 for SEEK_DATA/SEEK_HOLE copy check.
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>>
>> ---
> 
>>
>> diff --git a/src/seek_copy_tester.c b/src/seek_copy_tester.c
>> new file mode 100644
>> index 0000000..ddf683f
>> --- /dev/null
>> +++ b/src/seek_copy_tester.c
> 
>> +static size_t
>> +full_write(int fd, const void *buf, size_t count)
>> +{
>> +    size_t total = 0;
>> +    const char *ptr = (const char *) buf;
>> +
>> +    while (count>  0) {
>> +        ssize_t n = write(fd, ptr, count);
>> +        if (n<  0) {
>> +            if (errno == EINTR)
>> +                continue;
> 
> Wouldn't you want to stop the write loop if interrupted?

As this routine was called "full_write" which means it is expect to write as much as it can.
So I would keep retrying if the process was interrupted by EINTR.
Would you please give some opinions whether this approach is not suitable in this circumstance?

> 
>> +            error("full_write() failed as %s", strerror(errno));
>> +            break;
>> +        }
>> +
>> +        if (n == 0) {
>> +            error("full_write() ZERO bytes transferred");
> 
> You can tell them how many (total) bytes were written before the error.
> Same in full_pwrite()

This really a good idea!

> 
>> +create_data_and_holes(int fd, size_t nr_total_bytes, off_t start_offset,
>> +              uint64_t nr_skip_bytes, uint64_t nr_data_bytes,
>> +              int wrote_hole_at_eof)
>> +{
>> +    int ret = 0;
>> +    off_t total = nr_total_bytes;
>> +    off_t data_len = nr_data_bytes;
>> +    off_t off = start_offset;
>> +    char buf[BUF_SIZE];
>> +
>> +    memset(buf, 'A', sizeof(buf));
>> +


>> +    total -= start_offset;

>> +    while (total>  0) {
> 
> +    data_len = nr_data_bytes; /* see below */
> 
>> +        do {
>> +            size_t nr_write = MIN(total, BUF_SIZE);
> 
> There are 2 dependencies:
>  1) The last nr_data_bytes does not exceed total (outer loop)
>  2) The remaining data size (data_len) could be less than BUF_SIZE
>     (if data_len is not a multiple of BUF_SIZE for example).
> 
>> +            if (full_pwrite(fd, buf, nr_write, off) != nr_write) {
>> +                error("full_pwrite() failed as %s", strerror(errno));
>> +                ret = -1;
>> +                goto out;
>> +            }
>> +
>> +            off += nr_write;
>> +            data_len -= nr_write;
>> +        } while (data_len>  0);
> 
> data_len is <= 0 so the inner loop will execute only once if you have
> more than one chunk of data.

Thanks for the comments!

So I'll revise the code logic as following:
1) Ensure "total > start_offset" before go into this routine, i.e, start offset should less than the total file size.
2) The left total should larger than data_len(nr_data_bytes).
3) Calculate the nr_write based on above two points. 

Thanks,
-Jeff

>> +
>> +        off += (nr_skip_bytes + nr_data_bytes);
>> +        total -= off;
>> +    }
>> +
>> +    if (off<  nr_total_bytes) {
>> +        if (wrote_hole_at_eof) {
>> +            ret = ftruncate(fd, nr_total_bytes);
>> +            if (ret<  0) {
>> +                error("truncate source file to %lld bytes failed as %s",
>> +                       (long long)nr_total_bytes, strerror(errno));
>> +            }
>> +        }
>> +
>> +        ret = write_zeros(fd, nr_total_bytes - off);
>> +        if (ret<  0) {
>> +            error("write_zeros to end of file failed as %s",
>> +                   strerror(errno));
>> +        }
>> +    }
>> +
>> +out:
>> +    return ret;
>> +}
> 
> It is shaping up nicely.
> 
> --Mark
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs