Message ID | 154181075273.3727.12563703121689677996.stgit@magnolia (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
Series | xfsprogs-4.20: various fixes | expand |
Looks ok to me. You can add my review: Reviewed-by: Allison Henderson <allison.henderson@oracle.com> On 11/9/18 5:45 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Recently, it was pointed out that xfs_scrub{,_all} depend on components > and libraries (libicu, python) that live in /usr. /sbin binaries > shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin. > > Reported-by: xfs@tlinx.org > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > scrub/Makefile | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > > diff --git a/scrub/Makefile b/scrub/Makefile > index aebc7e43..c6473c12 100644 > --- a/scrub/Makefile > +++ b/scrub/Makefile > @@ -103,7 +103,7 @@ default: depend $(LTCOMMAND) $(XFS_SCRUB_ALL_PROG) $(OPTIONAL_TARGETS) > > xfs_scrub_all: xfs_scrub_all.in > @echo " [SED] $@" > - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \ > + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ > -e "s|@pkg_version@|$(PKG_VERSION)|g" \ > -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" < $< > $@ > $(Q)chmod a+x $@ > @@ -116,14 +116,14 @@ install: $(INSTALL_SCRUB) > > %.service: %.service.in > @echo " [SED] $@" > - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \ > + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ > -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" \ > -e "s|@pkg_lib_dir@|$(PKG_LIB_DIR)|g" \ > -e "s|@pkg_name@|$(PKG_NAME)|g" < $< > $@ > > %.cron: %.cron.in > @echo " [SED] $@" > - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" < $< > $@ > + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" < $< > $@ > > install-systemd: default $(SYSTEMD_SERVICES) > $(INSTALL) -m 755 -d $(SYSTEMD_SYSTEM_UNIT_DIR) > @@ -136,9 +136,9 @@ install-crond: default $(CRONTABS) > $(INSTALL) -m 644 $(CRONTABS) $(CROND_DIR) > > install-scrub: default > - $(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR) > - $(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR) > - $(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_ROOT_SBIN_DIR) > + $(INSTALL) -m 755 -d $(PKG_SBIN_DIR) > + $(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR) > + $(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_SBIN_DIR) > > install-dev: > >
On 11/9/2018 4:45 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Recently, it was pointed out that xfs_scrub{,_all} depend on components > and libraries (libicu, python) that live in /usr. /sbin binaries > shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin. ---- As stated in the original comment, only "xfs_scrub_all" depends on /usr, as it depends on /usr/bin/python. System binaries that might be used to repair problems should, IMO, reside on /sbin (or /bin) if they don't depend on /usr. xfs_scrub has no non-root dependencies on my version: > ldd /sbin/xfs_scrub linux-vdso.so.1 (0x00007ffe275d1000) libc.so.6 => /lib64/libc.so.6 (0x00007fd1c3561000) libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd1c3342000) /lib64/ld-linux-x86-64.so.2 (0x00007fd1c3b32000) Ideally the same should go for xfs_restore (and dump), xfs.mkfs, xfs_repair, xfs_admin, maybe xfs_db (hopefully rare), and xfs_irecover. Please don't _unnecessarily_ move binaries that might be needed for fs repair onto /usr if possible. I've found that using 'xfs' for a root file system eliminates my need for an indirect boot off of a ram disk. xfs has been far more stable and easy to use than indirect methods involving a separate ramdisk. Thanks, -linda
On Sat, Nov 10, 2018 at 12:43:58AM -0800, L A Walsh wrote: > > > On 11/9/2018 4:45 PM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Recently, it was pointed out that xfs_scrub{,_all} depend on components > > and libraries (libicu, python) that live in /usr. /sbin binaries > > shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin. > ---- > As stated in the original comment, only "xfs_scrub_all" > depends on /usr, as it depends on /usr/bin/python. > > System binaries that might be used to repair problems should, IMO, > reside on /sbin (or /bin) if they don't depend on /usr. > > > xfs_scrub has no non-root dependencies on my version: > > ldd /sbin/xfs_scrub > linux-vdso.so.1 (0x00007ffe275d1000) > libc.so.6 => /lib64/libc.so.6 (0x00007fd1c3561000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd1c3342000) > /lib64/ld-linux-x86-64.so.2 (0x00007fd1c3b32000) Ahh, but xfs_scrub depends on /usr on my system: $ ldd $(which xfs_scrub) linux-vdso.so.1 (0x00007ffd610d5000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd40f1e2000) libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fd40efc3000) libicui18n.so.60 => /usr/lib/x86_64-linux-gnu/libicui18n.so.60 (0x00007fd40eb22000) libicuuc.so.60 => /usr/lib/x86_64-linux-gnu/libicuuc.so.60 (0x00007fd40e76b000) /lib64/ld-linux-x86-64.so.2 (0x00007fd40f7ea000) libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fd40e3e2000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fd40e044000) libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fd40de2c000) libicudata.so.60 => /usr/lib/x86_64-linux-gnu/libicudata.so.60 (0x00007fd40c283000) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fd40c07f000) So that's why it moved too. (The libraries from /usr are used to notify the sysadmin about directories with file names that render in a deceptively similar fashion, if the system locale settings indicate UTF-8 mode.) > Ideally the same should go for xfs_restore (and dump), xfs.mkfs, > xfs_repair, xfs_admin, maybe xfs_db (hopefully rare), and xfs_irecover. mkfs and repair live in /sbin, at least on Debian. I've variously wondered if xfs_metadump (and therefore xfs_db) belong somewhere so that you can metadump or debug any filesystem possible, though so far that's only taken the form of an initramfs hook... > Please don't _unnecessarily_ move binaries that might be needed for fs repair > onto /usr if possible. I've found that using 'xfs' for a root file system > eliminates my need for an indirect boot off of a > ram disk. Please remember, scrub is the online fsck tool; if you can't get / and /usr online then the recovery procedure (xfs_repair) for them is the same as it always has been. > xfs has been far more stable and easy to use than indirect methods involving > a separate ramdisk. Yeah, I noticed that still mostly works even these days. :) (Granted that was the other day when I was test-booting a browser x86 vm off a simple xfs image just to see if we could do 'xfstests in your browser' but aside from the absurdly slow emulation it mostly worked. :P) --D > Thanks, > -linda >
diff --git a/scrub/Makefile b/scrub/Makefile index aebc7e43..c6473c12 100644 --- a/scrub/Makefile +++ b/scrub/Makefile @@ -103,7 +103,7 @@ default: depend $(LTCOMMAND) $(XFS_SCRUB_ALL_PROG) $(OPTIONAL_TARGETS) xfs_scrub_all: xfs_scrub_all.in @echo " [SED] $@" - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \ + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ -e "s|@pkg_version@|$(PKG_VERSION)|g" \ -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" < $< > $@ $(Q)chmod a+x $@ @@ -116,14 +116,14 @@ install: $(INSTALL_SCRUB) %.service: %.service.in @echo " [SED] $@" - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \ + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" \ -e "s|@pkg_lib_dir@|$(PKG_LIB_DIR)|g" \ -e "s|@pkg_name@|$(PKG_NAME)|g" < $< > $@ %.cron: %.cron.in @echo " [SED] $@" - $(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" < $< > $@ + $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" < $< > $@ install-systemd: default $(SYSTEMD_SERVICES) $(INSTALL) -m 755 -d $(SYSTEMD_SYSTEM_UNIT_DIR) @@ -136,9 +136,9 @@ install-crond: default $(CRONTABS) $(INSTALL) -m 644 $(CRONTABS) $(CROND_DIR) install-scrub: default - $(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR) - $(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR) - $(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_ROOT_SBIN_DIR) + $(INSTALL) -m 755 -d $(PKG_SBIN_DIR) + $(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR) + $(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_SBIN_DIR) install-dev: