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

Fix for issue #4849 that settings in fapl libver bounds causes unexpe… #4939

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

vchoi-hdfgroup
Copy link
Contributor

…cted H5Fopen failures.

File with non-SWMR-write access can now be opened without regard for superblock version. Due to the fix, H5Fstart_swmr_write() also needs to be modified as well as the tests for libver bounds. The "RFC: Setting Bounds for Object Creation in HDF5 1.10.0" is also updated to reflect the changes.

vchoi-hdfgroup and others added 2 commits October 8, 2024 18:49
…s unexpected H5Fopen failures.

File with non-SWMR-write access can now be opened without regard for superblock version.
Due to the fix, H5Fstart_swmr_write() also needs to be modified as well as the tests for libver bounds.
The "RFC: Setting Bounds for Object Creation in HDF5 1.10.0" is also updated to reflect the changes.
test/swmr.c Show resolved Hide resolved
if (H5O_get_version_bound(f->shared->low_bound, &version) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_version_bound failed");

if (ninfo.hdr.version < version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code and error message seem to be out-of-sync. Which is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the error message.

@bmribler bmribler added Priority - 1. High 🔼 These are important issues that should be resolved in the next release Component - C Library Core C library issues (usually in the src directory) Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub Branch - 2.0 PRs to the HDF5 2.x maintenance branch Merge - To 1.16 and removed Branch - 2.0 PRs to the HDF5 2.x maintenance branch labels Oct 10, 2024
@lrknox lrknox merged commit 9df4c0d into HDFGroup:develop Oct 11, 2024
60 checks passed
@markcmiller86
Copy link
Contributor

So, can I just confirm something about this fix. Does it mean that when running on 1.14, if a fapl that specifies a LATEST for both upper and lower bound is used in the H5Fcreate call, it will not wind up creating a file that cannot be opened by say v18?

I mean does the fix in this PR make 1.14's H5FCreate with LATEST in upper/lower has same effect as H5FCreate with DEFAULT fapl followed by H5Pset_libver_bounds with LATEST in upper/lower? That is kinda sorta the key question. In both cases, the fapl should (at least in my interpretation of this functionality) effect access to the objects in the file and not whether the file can actuall succesfully open or not.

@fortnern
Copy link
Member

fortnern commented Nov 5, 2024

So, can I just confirm something about this fix. Does it mean that when running on 1.14, if a fapl that specifies a LATEST for both upper and lower bound is used in the H5Fcreate call, it will not wind up creating a file that cannot be opened by say v18?

The library may create a file that cannot be opened by 1.8 if the upper bound is anything above V18. If the lower bound is not above V18 and you are careful not to use any features that require newer file formats then the file will be readable by 1.8 even if the upper bound is higher.

I mean does the fix in this PR make 1.14's H5FCreate with LATEST in upper/lower has same effect as H5FCreate with DEFAULT fapl followed by H5Pset_libver_bounds with LATEST in upper/lower? That is kinda sorta the key question. In both cases, the fapl should (at least in my interpretation of this functionality) effect access to the objects in the file and not whether the file can actuall succesfully open or not.

My understanding is that this fix eliminates a check for the superblock version that is not necessary in non-SWMR mode. In SWMR write mode, when opening the file, the library needs to write a message to the superblock, and that message is only supported in newer superblock versions, so the library needs to upgrade the superblock if it is on the older format. If it is prevented from upgrading the superblock by the libver bounds setting then it must issue an error (and it still does). The bug was it would check for this ability even if not in SWMR write mode. The libver bounds setting only affects newly created objects, it does not prevent reading previously created objects, no matter the format version, unless there's a bug like this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues (usually in the src directory) Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Bug / Bugfix Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants