----- 原始邮件 -----
> 发件人: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
> 收件人: "Jianhong.Yin" <yin-jianhong@xxxxxxx>
> 抄送: linux-xfs@xxxxxxxxxxxxxxx, jiyin@xxxxxxxxxx
> 发送时间: 星期四, 2019年 9 月 05日 上午 1:27:36
> 主题: Re: [PATCH] xfsprogs: copy_range don't truncate dstfile if same with srcfile
>
> On Wed, Sep 04, 2019 at 02:32:22PM +0800, Jianhong.Yin wrote:
> > now if we do copy_range in same file without any extra option
> > will truncate the file, and not any document indicate this default
> > action. that's risky to users.
> >
> > '''
> > $ LANG=C ll testfile
> > -rw-rw-r--. 1 yjh yjh 4054 Sep 4 14:22 testfile
> > $ ./xfs_io -c 'copy_range testfile' testfile
> > $ LANG=C ll testfile
> > -rw-rw-r--. 1 yjh yjh 4054 Sep 4 14:23 testfile
> > '''
> >
> > Signed-off-by: Jianhong Yin <yin-jianhong@xxxxxxx>
> > ---
> > io/copy_file_range.c | 23 +++++++++++++++++++----
> > 1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/io/copy_file_range.c b/io/copy_file_range.c
> > index b7b9fd88..487041c0 100644
> > --- a/io/copy_file_range.c
> > +++ b/io/copy_file_range.c
> > @@ -75,6 +75,19 @@ copy_dst_truncate(void)
> > return ret;
> > }
> >
> > +int is_same_file(int fd1, int fd2) {
> > + struct stat stat1, stat2;
> > + if (fstat(fd1, &stat1) < 0) {
> > + perror("fstat");
> > + return -1;
> > + }
> > + if (fstat(fd2, &stat2) < 0) {
> > + perror("fstat");
> > + return -1;
> > + }
> > + return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino);
> > +}
> > +
> > static int
> > copy_range_f(int argc, char **argv)
> > {
> > @@ -147,10 +160,12 @@ copy_range_f(int argc, char **argv)
> > }
> > len = sz;
> >
> > - ret = copy_dst_truncate();
> > - if (ret < 0) {
> > - ret = 1;
> > - goto out;
> > + if (!is_same_file(fd, file->fd)) {
>
> Uggggh, why does xfs_io copy_range have this weird behavior? It should
> be a simple wrapper for copy_file_range (the syscall) and nothing else.
>
> The code patch looks fine for solving this edge case, but we really
> shouldn't have this "extra" functionality in a debugging tool that
> should be athin wrapper around the syscall for xfstests purposes.
right, agree.
>
> --D
>
> > + ret = copy_dst_truncate();
> > + if (ret < 0) {
> > + ret = 1;
> > + goto out;
> > + }
> > }
> > }
> >
> > --
> > 2.17.2
> >
>
