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

Christoph Hellwig
2023-11-23T06:40:32Z

> 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.

> @@ -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.