copy_file_range() support for install(1)

[mm]

Summary

This patch adds copy_file_range() support to install(1)

[asomers]

Unfortunately the need to compute a digest invalidates half of the benefit of copy_file_range. I think it would be ok to skip copy_file_range if the user requests a digest. Also, if you're going to use copy_file_range even in that case, I think you should add some ATF tests that cover digests.

usr.bin/xinstall/xinstall.c

1321

1333

err itself will supply the "Not enough memory" part. You should just do err(1, "malloc");

1341

Is p used uninitialized here?

[mm]

I have updated the patch to do copy_file_range() only if no digest is requested. That makes sense so we don't read the files twice.

[mm]

Update patch

[asomers]

Your latest diff is also missing context. Could you please regenerate it with diff -U 9999?

[asomers]

usr.bin/xinstall/xinstall.c

1325

You should also fallback in the case of ENOSYS. That is especially important for install(1), as people use it when upgrading.

[imp]

I'm kinda torn... I think we need a fallback, but I'm not 100% sure. though it is the most conservative approach and would allow people that installed a new world and then had to downgrade the kernel a chance to installworld back to an older place.

usr.bin/xinstall/xinstall.c

1325

I think you'll need to mask/ignore SIGSYS if we don't already do so. Otheriwse we'll get SIGSYS if the kernel is too old and doesn't have the copy_file_range system call..

[arichardson]

IIRC this is also built during bootstrap. Could you check that the GitHub actions CI is still happy before committing?

Linux should be fine but I think macos might need an ifdef?

[mm]

In D40898#930916, @imp wrote:

I'm kinda torn... I think we need a fallback, but I'm not 100% sure. though it is the most conservative approach and would allow people that installed a new world and then had to downgrade the kernel a chance to installworld back to an older place.

copy_file_range() was introduced on Jul 25, 2019 with __FreeBSD_version 1300038

[imp]

In D40898#930998, @mm wrote:

In D40898#930916, @imp wrote:

I'm kinda torn... I think we need a fallback, but I'm not 100% sure. though it is the most conservative approach and would allow people that installed a new world and then had to downgrade the kernel a chance to installworld back to an older place.

copy_file_range() was introduced on Jul 25, 2019 with __FreeBSD_version 1300038

Oh. I thought it more recent. Then no fallback needed

[rmacklem]

If ret is changed to ssize_t, it then looks fine
to me.

usr.bin/xinstall/xinstall.c

1301

ret should actually be ssize_t.

[mm]

Use ssize_t for copy_file_range() return

[imp]

still like it... I like it better

[arichardson]

This breaks the macos bootstrap jobs, could you add an ifdef around the new code?

> usr.bin/xinstall (obj,all,install)

/Users/runner/work/freebsd-src/freebsd-src/usr.bin/xinstall/xinstall.c:1318:10: error: implicit declaration of function 'copy_file_range' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

ret = copy_file_range(from_fd, NULL, to_fd, NULL,
      ^

1 error generated.