linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] xfsprogs: copy_range don't truncate dstfile
@ 2019-09-04 23:36 Jianhong.Yin
  2019-09-05  0:14 ` Darrick J. Wong
  0 siblings, 1 reply; 3+ messages in thread
From: Jianhong.Yin @ 2019-09-04 23:36 UTC (permalink / raw)
  To: linux-xfs; +Cc: jiyin, darrick.wong, Jianhong.Yin

now if we do copy_range from srcfile to dstfile without any option
will truncate the dstfile, and not any document indicate this default
action. that's unexpected and confuse people.

'''
$ ./xfs_io -f -c 'copy_range copy_file_range.c'  testfile
$ ll testfile
-rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:15 testfile
$ ./xfs_io -c 'copy_range testfile'  testfile
$ ll testfile
-rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:16 testfile
$ ./xfs_io -c 'copy_range testfile -l 3534 -d 3534' testfile
$ ll testfile
-rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:17 testfile
$ ./xfs_io -c 'copy_range copy_file_range.c'  testfile
$ ll testfile
-rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:18 testfile
$ cmp -n 3534 copy_file_range.c testfile
$ cmp -i 0:3534 copy_file_range.c testfile
'''

Signed-off-by: Jianhong Yin <yin-jianhong@163.com>
---
 io/copy_file_range.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/io/copy_file_range.c b/io/copy_file_range.c
index b7b9fd88..283f5094 100644
--- a/io/copy_file_range.c
+++ b/io/copy_file_range.c
@@ -66,15 +66,6 @@ copy_src_filesize(int fd)
 	return st.st_size;
 }
 
-static int
-copy_dst_truncate(void)
-{
-	int ret = ftruncate(file->fd, 0);
-	if (ret < 0)
-		perror("ftruncate");
-	return ret;
-}
-
 static int
 copy_range_f(int argc, char **argv)
 {
@@ -146,12 +137,6 @@ copy_range_f(int argc, char **argv)
 			goto out;
 		}
 		len = sz;
-
-		ret = copy_dst_truncate();
-		if (ret < 0) {
-			ret = 1;
-			goto out;
-		}
 	}
 
 	ret = copy_file_range_cmd(fd, &src, &dst, len);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] xfsprogs: copy_range don't truncate dstfile
  2019-09-04 23:36 [PATCH v2] xfsprogs: copy_range don't truncate dstfile Jianhong.Yin
@ 2019-09-05  0:14 ` Darrick J. Wong
  2019-09-05  1:08   ` Jianhong Yin
  0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2019-09-05  0:14 UTC (permalink / raw)
  To: Jianhong.Yin; +Cc: linux-xfs, jiyin

On Thu, Sep 05, 2019 at 07:36:34AM +0800, Jianhong.Yin wrote:
> now if we do copy_range from srcfile to dstfile without any option
> will truncate the dstfile, and not any document indicate this default
> action. that's unexpected and confuse people.

Needs manpage update.

Also, did you check that xfstests doesn't somehow use this?  There are
several cfr tests now...

generic/430 generic/431 generic/432 generic/433 generic/434 generic/553
generic/554 generic/564 generic/565 generic/716

--D

> '''
> $ ./xfs_io -f -c 'copy_range copy_file_range.c'  testfile
> $ ll testfile
> -rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:15 testfile
> $ ./xfs_io -c 'copy_range testfile'  testfile
> $ ll testfile
> -rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:16 testfile
> $ ./xfs_io -c 'copy_range testfile -l 3534 -d 3534' testfile
> $ ll testfile
> -rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:17 testfile
> $ ./xfs_io -c 'copy_range copy_file_range.c'  testfile
> $ ll testfile
> -rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:18 testfile
> $ cmp -n 3534 copy_file_range.c testfile
> $ cmp -i 0:3534 copy_file_range.c testfile
> '''
> 
> Signed-off-by: Jianhong Yin <yin-jianhong@163.com>
> ---
>  io/copy_file_range.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/io/copy_file_range.c b/io/copy_file_range.c
> index b7b9fd88..283f5094 100644
> --- a/io/copy_file_range.c
> +++ b/io/copy_file_range.c
> @@ -66,15 +66,6 @@ copy_src_filesize(int fd)
>  	return st.st_size;
>  }
>  
> -static int
> -copy_dst_truncate(void)
> -{
> -	int ret = ftruncate(file->fd, 0);
> -	if (ret < 0)
> -		perror("ftruncate");
> -	return ret;
> -}
> -
>  static int
>  copy_range_f(int argc, char **argv)
>  {
> @@ -146,12 +137,6 @@ copy_range_f(int argc, char **argv)
>  			goto out;
>  		}
>  		len = sz;
> -
> -		ret = copy_dst_truncate();
> -		if (ret < 0) {
> -			ret = 1;
> -			goto out;
> -		}
>  	}
>  
>  	ret = copy_file_range_cmd(fd, &src, &dst, len);
> -- 
> 2.21.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] xfsprogs: copy_range don't truncate dstfile
  2019-09-05  0:14 ` Darrick J. Wong
@ 2019-09-05  1:08   ` Jianhong Yin
  0 siblings, 0 replies; 3+ messages in thread
From: Jianhong Yin @ 2019-09-05  1:08 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Jianhong.Yin, linux-xfs



----- 原始邮件 -----
> 发件人: "Darrick J. Wong" <darrick.wong@oracle.com>
> 收件人: "Jianhong.Yin" <yin-jianhong@163.com>
> 抄送: linux-xfs@vger.kernel.org, jiyin@redhat.com
> 发送时间: 星期四, 2019年 9 月 05日 上午 8:14:57
> 主题: Re: [PATCH v2] xfsprogs: copy_range don't truncate dstfile
> 
> On Thu, Sep 05, 2019 at 07:36:34AM +0800, Jianhong.Yin wrote:
> > now if we do copy_range from srcfile to dstfile without any option
> > will truncate the dstfile, and not any document indicate this default
> > action. that's unexpected and confuse people.
> 
> Needs manpage update.
> 
> Also, did you check that xfstests doesn't somehow use this?  There are
> several cfr tests now...
> 
> generic/430 generic/431 generic/432 generic/433 generic/434 generic/553
> generic/554 generic/564 generic/565 generic/716
I've checked all of them, there are 5 line might use the feature(truncate dstfile)

'''
./430:$XFS_IO_PROG -f -c "copy_range $testdir/file" "$testdir/copy"
./430:$XFS_IO_PROG -f -c "copy_range $testdir/file" "$testdir/copy"
./433:$XFS_IO_PROG -f -c "copy_range $testdir/file" "$testdir/copy"
./432:$XFS_IO_PROG -f -c "copy_range $testdir/file" "$testdir/copy"
./431:$XFS_IO_PROG -f -c "copy_range $testdir/file" "$testdir/copy"
'''

if there's already a $testdir/copy and it's size > $testdir/file, will break the test

should add -t option to fix them:
  $XFS_IO_PROG -t -f -c "copy_range $testdir/file" "$testdir/copy

I will send patch to xfstests-dev, later


> 
> --D
> 
> > '''
> > $ ./xfs_io -f -c 'copy_range copy_file_range.c'  testfile
> > $ ll testfile
> > -rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:15 testfile
> > $ ./xfs_io -c 'copy_range testfile'  testfile
> > $ ll testfile
> > -rw-rw-r--. 1 yjh yjh 3534 Sep  5 07:16 testfile
> > $ ./xfs_io -c 'copy_range testfile -l 3534 -d 3534' testfile
> > $ ll testfile
> > -rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:17 testfile
> > $ ./xfs_io -c 'copy_range copy_file_range.c'  testfile
> > $ ll testfile
> > -rw-rw-r--. 1 yjh yjh 7068 Sep  5 07:18 testfile
> > $ cmp -n 3534 copy_file_range.c testfile
> > $ cmp -i 0:3534 copy_file_range.c testfile
> > '''
> > 
> > Signed-off-by: Jianhong Yin <yin-jianhong@163.com>
> > ---
> >  io/copy_file_range.c | 15 ---------------
> >  1 file changed, 15 deletions(-)
> > 
> > diff --git a/io/copy_file_range.c b/io/copy_file_range.c
> > index b7b9fd88..283f5094 100644
> > --- a/io/copy_file_range.c
> > +++ b/io/copy_file_range.c
> > @@ -66,15 +66,6 @@ copy_src_filesize(int fd)
> >  	return st.st_size;
> >  }
> >  
> > -static int
> > -copy_dst_truncate(void)
> > -{
> > -	int ret = ftruncate(file->fd, 0);
> > -	if (ret < 0)
> > -		perror("ftruncate");
> > -	return ret;
> > -}
> > -
> >  static int
> >  copy_range_f(int argc, char **argv)
> >  {
> > @@ -146,12 +137,6 @@ copy_range_f(int argc, char **argv)
> >  			goto out;
> >  		}
> >  		len = sz;
> > -
> > -		ret = copy_dst_truncate();
> > -		if (ret < 0) {
> > -			ret = 1;
> > -			goto out;
> > -		}
> >  	}
> >  
> >  	ret = copy_file_range_cmd(fd, &src, &dst, len);
> > --
> > 2.21.0
> > 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-05  1:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04 23:36 [PATCH v2] xfsprogs: copy_range don't truncate dstfile Jianhong.Yin
2019-09-05  0:14 ` Darrick J. Wong
2019-09-05  1:08   ` Jianhong Yin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).