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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9647d94
Fix for issue #4849 that settings in fapl libver bounds causes unexpe…
vchoi-hdfgroup Oct 8, 2024
fdcc97e
Committing clang-format changes
github-actions[bot] Oct 8, 2024
d58dbca
Fix c++ libver bound test failure.
vchoi-hdfgroup Oct 9, 2024
d49c906
Merge branch 'issue#4849_develop' of https://github.com/vchoi-hdfgrou…
vchoi-hdfgroup Oct 9, 2024
9e0af54
Correct error messages according review feedback.
vchoi-hdfgroup Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion c++/test/tfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ test_libver_bounds()

/* Run the tests */
test_libver_bounds_real(H5F_LIBVER_EARLIEST, H5O_VERSION_1, H5F_LIBVER_LATEST, H5O_VERSION_2);
test_libver_bounds_real(H5F_LIBVER_LATEST, H5O_VERSION_2, H5F_LIBVER_EARLIEST, H5O_VERSION_2);
test_libver_bounds_real(H5F_LIBVER_LATEST, H5O_VERSION_2, H5F_LIBVER_EARLIEST, H5O_VERSION_1);
PASSED();
} /* end test_libver_bounds() */

Expand Down
36 changes: 29 additions & 7 deletions src/H5Fint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3181,7 +3181,8 @@ H5F__set_libver_bounds(H5F_t *f, H5F_libver_t low, H5F_libver_t high)
assert(f->shared);

/* Set the bounds only if the existing setting is different from the inputs */
if (f->shared->low_bound != low || f->shared->high_bound != high) {
if ((f->shared->low_bound != low || f->shared->high_bound != high) &&
!(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)) {
/* Call the flush routine, for this file */
/* Note: This is done in case the binary format for representing a
* metadata entry class changes when the file format low / high
Expand Down Expand Up @@ -3704,6 +3705,7 @@ H5F_get_metadata_read_retry_info(H5F_t *file, H5F_retry_info_t *info)
* --only allow datasets and groups without attributes
* --disallow named datatype with/without attributes
* --disallow opened attributes attached to objects
* --disallow opened objects below 1.10
*
* NOTE: Currently, only opened groups and datasets are allowed
* when enabling SWMR via H5Fstart_swmr_write().
Expand Down Expand Up @@ -3746,7 +3748,7 @@ H5F__start_swmr_write(H5F_t *f)
if (f->shared->sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "file superblock version - should be at least 3");

/* Check for correct file format version */
/* Check for correct file format version to start SWMR writing */
if ((f->shared->low_bound < H5F_LIBVER_V110) || (f->shared->high_bound < H5F_LIBVER_V110))
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL,
"file format version does not support SWMR - needs to be 1.10 or greater");
Expand Down Expand Up @@ -3783,6 +3785,31 @@ H5F__start_swmr_write(H5F_t *f)
/* Allocate space for group and object locations */
if ((obj_ids = (hid_t *)H5MM_malloc(grp_dset_count * sizeof(hid_t))) == NULL)
HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t");

/* Get the list of opened object ids (groups & datasets) */
if (H5F_get_obj_ids(f, H5F_OBJ_GROUP | H5F_OBJ_DATASET, grp_dset_count, obj_ids, false,
&grp_dset_count) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5F_get_obj_ids failed");

/* Ensure that there's no old-style opened objects */
for (u = 0; u < grp_dset_count; u++) {
H5O_native_info_t ninfo;
H5O_loc_t *oloc;
uint8_t version;

if (NULL == (oloc = H5O_get_loc(obj_ids[u])))
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_loc() failed");

if (H5O_get_native_info(oloc, &ninfo, H5O_NATIVE_INFO_HDR) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_native_info() failed");

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.

HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "disallow opened objects below 1.10");
}

if ((obj_glocs = (H5G_loc_t *)H5MM_malloc(grp_dset_count * sizeof(H5G_loc_t))) == NULL)
HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for object group locations");
if ((obj_olocs = (H5O_loc_t *)H5MM_malloc(grp_dset_count * sizeof(H5O_loc_t))) == NULL)
Expand All @@ -3797,11 +3824,6 @@ H5F__start_swmr_write(H5F_t *f)
HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t");
assert(obj_apl_ids[0] == H5P_DEFAULT);

/* Get the list of opened object ids (groups & datasets) */
if (H5F_get_obj_ids(f, H5F_OBJ_GROUP | H5F_OBJ_DATASET, grp_dset_count, obj_ids, false,
&grp_dset_count) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5F_get_obj_ids failed");

/* Save the VOL connector and the object wrapping context for the refresh step */
if (grp_dset_count > 0) {
H5VL_object_t *vol_obj;
Expand Down
36 changes: 9 additions & 27 deletions src/H5Fsuper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,45 +432,27 @@ H5F__super_read(H5F_t *f, H5P_genplist_t *fa_plist, bool initial_read)
HGOTO_ERROR(H5E_FILE, H5E_CANTPROTECT, FAIL, "unable to load superblock");

/*
* When opening a file with SWMR-write access, the library will first
* check to ensure that superblock version 3 is used. Otherwise fail
* file open.
*
* Then the library will upgrade the file's low_bound depending on
* superblock version as follows:
* --version 0 or 1: no change to low_bound
* --version 2: upgrade low_bound to at least V18
* --version 3: upgrade low_bound to at least V110
* When opening a file with SWMR-write access, the library will check to
* ensure that:
* --superblock version 3 is used
* --superblock version does not exceed the version allowed by high bound
* --upgrade low_bound to at least V110
* Otherwise fail file open for SMWR-write access
*
* Upgrading low_bound will give the best format versions available for
* that superblock version. Due to the possible upgrade, the fapl returned
* from H5Fget_access_plist() might indicate a low_bound higher than what
* the user originally set.
*
* After upgrading low_bound, the library will check to ensure that the
* superblock version does not exceed the version allowed by high_bound.
* Otherwise fail file open.
*
* For details, please see RFC:Setting Bounds for Object Creation in HDF5 1.10.0.
*/

/* Check to ensure that superblock version 3 is used for SWMR-write access */
if (H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) {
if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version for SWMR is less than 3");
}

/* Upgrade low_bound to at least V18 when encountering version 2 superblock */
if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_2)
f->shared->low_bound = MAX(H5F_LIBVER_V18, f->shared->low_bound);

/* Upgrade low_bound to at least V110 when encountering version 3 superblock */
if (sblock->super_vers >= HDF5_SUPERBLOCK_VERSION_3)
if (sblock->super_vers > HDF5_superblock_ver_bounds[f->shared->high_bound])
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version exceeds high bound");
f->shared->low_bound = MAX(H5F_LIBVER_V110, f->shared->low_bound);

/* Version bounds check */
if (sblock->super_vers > HDF5_superblock_ver_bounds[f->shared->high_bound])
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version exceeds high bound");
}

/* Pin the superblock in the cache */
if (H5AC_pin_protected_entry(sblock) < 0)
Expand Down
17 changes: 17 additions & 0 deletions src/H5Oint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2959,3 +2959,20 @@ H5O_has_chksum(const H5O_t *oh)

FUNC_LEAVE_NOAPI(H5O_SIZEOF_CHKSUM_OH(oh) > 0)
} /* end H5O_has_chksum() */

/*-------------------------------------------------------------------------
* Function: H5O_get_version_bound
*
* Purpose: Retrieve the version for a given bound from object version array
*
*-------------------------------------------------------------------------
*/
herr_t
H5O_get_version_bound(H5F_libver_t bound, uint8_t *version)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR

*version = (uint8_t)H5O_obj_ver_bounds[bound];

FUNC_LEAVE_NOAPI(SUCCEED)
} /* end H5O_get_version_bound() */
1 change: 1 addition & 0 deletions src/H5Oprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,7 @@ H5_DLL uint8_t H5O_get_oh_version(const H5O_t *oh);
H5_DLL herr_t H5O_get_rc_and_type(const H5O_loc_t *oloc, unsigned *rc, H5O_type_t *otype);
H5_DLL H5AC_proxy_entry_t *H5O_get_proxy(const H5O_t *oh);
H5_DLL bool H5O_has_chksum(const H5O_t *oh);
H5_DLL herr_t H5O_get_version_bound(H5F_libver_t bound, uint8_t *version);

/* Object header message routines */
H5_DLL herr_t H5O_msg_create(const H5O_loc_t *loc, unsigned type_id, unsigned mesg_flags,
Expand Down
Loading
Loading