Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

romio: clean up file system detection #5671

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

roblatham00
Copy link
Contributor

@roblatham00 roblatham00 commented Nov 12, 2021

ROMIO file system checking had grown out of hand:

  • use AC_CHECK_MEMBERS instead of our ancient home-grown stat member
    checking logic.
  • abstract the "stat/statfs/statvfs" part from the rest of the code so
    we don't duplicate detection logic in three places

Pull Request Description

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@roblatham00 roblatham00 added the romio anything having to do with our MPI-IO implementation label Nov 12, 2021
@roblatham00
Copy link
Contributor Author

test:mpich/ch4/ofi
test:mpich/ch3/tcp

@roblatham00
Copy link
Contributor Author

huge big concern about portability -- this code represents 20+ years of porting across some platforms that don't even exist any longer.

@roblatham00
Copy link
Contributor Author

test:mpich/ch3/tcp

@roblatham00
Copy link
Contributor Author

test:mpich/ch3/tcp

1 similar comment
@roblatham00
Copy link
Contributor Author

test:mpich/ch3/tcp

@roblatham00 roblatham00 force-pushed the romio-stat-rework branch 2 times, most recently from 947db3b to 6020f3e Compare November 22, 2021 19:22
@roblatham00
Copy link
Contributor Author

test:mpich/ch3/tcp

@roblatham00
Copy link
Contributor Author

I was able to (eventually) build on OpenBSD, fixing a bug in the st_fstype and f_fstypename path.

Because this code is so platform-specific and we have so few of the old platforms left to test on, I'd appreciate a second set of eyes from @wkliao (or anyone else) just to make sure I didn't miss anything obvious.

@roblatham00
Copy link
Contributor Author

I still want to test this on a system with GPFS or Lustre to make sure we really do find GPFS or Lustre and I also want to test on OS X (it's BSD like but different enough).

@wkliao
Copy link
Contributor

wkliao commented Dec 3, 2021

This looks fine with.
I built it on Cori with option --with-file-system="ufs lustre nfs" without a problem.

@roblatham00 roblatham00 force-pushed the romio-stat-rework branch 2 times, most recently from c7fe9ee to 0c242f1 Compare January 6, 2022 21:13
@roblatham00
Copy link
Contributor Author

Tested and fixed on OSX (which is indeed different enough -- it has both 'f_fstypename' and a (reserved) fstype field.

@hzhou
Copy link
Contributor

hzhou commented Jan 6, 2022

test:mpich/ch3/most

ROMIO file system checking had grown out of hand:
- use AC_CHECK_MEMBERS instead of our ancient home-grown stat member
checking logic.
- abstract the "stat/statfs/statvfs" part from the rest of the code so
  we don't duplicate detection logic in three places
- add a 'magic number' to the 'fstypes' struct so we can use that
  table multiple ways.

check for more file systems
@roblatham00 roblatham00 merged commit 791b36a into pmodels:main Jan 7, 2022
@roblatham00 roblatham00 deleted the romio-stat-rework branch March 7, 2022 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
romio anything having to do with our MPI-IO implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants