Re: [PATCH 3/9] xfs_copy: actually do directio writes to block devices

Darrick J. Wong
2023-11-27T18:14:58Z

On Wed, Nov 22, 2023 at 10:40:32PM -0800, Christoph Hellwig wrote:
> > diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
> > index 79f65946709..26de6b2ee1c 100644
> > --- a/copy/xfs_copy.c
> > +++ b/copy/xfs_copy.c
> > @@ -854,6 +854,8 @@ main(int argc, char **argv)
> > progname, target[i].name, progname);
> > exit(1);
> > }
> > + if (!buffered_output)
> > + open_flags |= O_DIRECT;
> > }
>
> I'd just move this outside of the if/else if chain and do the
> assignment once.

Fixed.

> > @@ -887,20 +889,22 @@ main(int argc, char **argv)
> > }
> > }
> > } else {
> > - char *lb[XFS_MAX_SECTORSIZE] = { NULL };
> > + char *lb = memalign(wbuf_align, XFS_MAX_SECTORSIZE);
> > off64_t off;
> >
> > /* ensure device files are sufficiently large */
> > + memset(lb, 0, XFS_MAX_SECTORSIZE);
> >
> > off = mp->m_sb.sb_dblocks * source_blocksize;
> > + off -= XFS_MAX_SECTORSIZE;
> > + if (pwrite(target[i].fd, lb, XFS_MAX_SECTORSIZE, off) < 0) {
>
> We should probably check for a short write as well?
> Also this line is a bit long.

Ok, I'll check for short writes and split the error messaging so that it
no longer says "Is target too small?" on EIO.

--D

>
>
>