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

["Darrick J. Wong" <djwong@kernel.org>]

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

> 
> 
>