Skip to content

Commit 0c734cb

Browse files
jhendersonHDFjhendersonHDF
authored
Fix cache image testing issues and re-enable testing (#6311)
Fix issue where chunked datasets could get setup with an incorrect chunking index type in parallel HDF5 Fix issue where metadata cache images with an undefined address and size of 0 couldn't be properly decoded Fix issue where a flag in H5Cimage.c wasn't getting set correctly for release builds of the library, leading to incorrect error checking when reconstructing metadata cache entries
1 parent 994dbf6 commit 0c734cb

File tree

8 files changed

+75
-62
lines changed

8 files changed

+75
-62
lines changed

‎release_docs/CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,18 @@ We would like to thank the many HDF5 community members who contributed to this r
123123

124124
The direct I/O VFD attempts to determine data alignment requirements for a file on file open to try and avoid extra work when data alignment isn't required. Depending on the file access flags used when opening a file, the VFD could incorrectly determine these requirements for either writes or reads, eventually leading to a possible EINVAL return value on write or read. This has been fixed by separately determining the requirements for writes and reads and being more conservative about trying to avoid data alignment requirements.
125125

126+
### Fixed an issue with chunked datasets using the wrong index type with parallel HDF5
127+
128+
Fixed a bug in parallel HDF5 that would cause chunked datasets with fixed dimensions and without filters applied to use the "none" index type instead of the "fixed array" index type.
129+
130+
### Fixed an issue with decoding metadata cache image superblock extension messages
131+
132+
Fixed a bug where loading of a metadata cache image superblock extension message would fail when the image had an undefined address and size of 0.
133+
134+
### Fixed an issue with an incorrect file format validation check when decoding metadata cache entries
135+
136+
Fixed a bug where a flag in H5Cimage.c wasn't getting set correctly for release builds of HDF5, leading to incorrect error checking when reconstructing metadata cache entries.
137+
126138
## Java Library
127139

128140
## Configuration

‎src/H5Cimage.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2744,11 +2744,11 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, hsize_t *buf_size
27442744
flags = *p++;
27452745
if (flags & H5C__MDCI_ENTRY_DIRTY_FLAG)
27462746
is_dirty = true;
2747+
if (flags & H5C__MDCI_ENTRY_IS_FD_PARENT_FLAG)
2748+
is_fd_parent = true;
27472749
#ifndef NDEBUG /* only used in assertions */
27482750
if (flags & H5C__MDCI_ENTRY_IN_LRU_FLAG)
27492751
in_lru = true;
2750-
if (flags & H5C__MDCI_ENTRY_IS_FD_PARENT_FLAG)
2751-
is_fd_parent = true;
27522752
if (flags & H5C__MDCI_ENTRY_IS_FD_CHILD_FLAG)
27532753
is_fd_child = true;
27542754
#endif

‎src/H5Dint.c‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,19 +1326,6 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
13261326
if (H5O_fill_set_version(file, &new_dset->shared->dcpl_cache.fill) < 0)
13271327
HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, NULL, "can't set latest version of fill value");
13281328

1329-
/* Check if the file driver would like to force early space allocation */
1330-
if (H5F_HAS_FEATURE(file, H5FD_FEAT_ALLOCATE_EARLY))
1331-
new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
1332-
1333-
/*
1334-
* Check if this dataset is going into a parallel file and set space allocation time.
1335-
* If the dataset has filters applied to it, writes to the dataset must be collective,
1336-
* so we don't need to force early space allocation. Otherwise, we force early space
1337-
* allocation to facilitate independent raw data operations.
1338-
*/
1339-
if (H5F_HAS_FEATURE(file, H5FD_FEAT_HAS_MPI) && (new_dset->shared->dcpl_cache.pline.nused == 0))
1340-
new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
1341-
13421329
/* Set the dataset's I/O operations */
13431330
if (H5D__layout_set_io_ops(new_dset) < 0)
13441331
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "unable to initialize I/O operations");
@@ -1352,6 +1339,19 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id, hid_t
13521339
if (new_dset->shared->layout.version > H5O_layout_ver_bounds[H5F_HIGH_BOUND(file)])
13531340
HGOTO_ERROR(H5E_DATASET, H5E_BADRANGE, NULL, "layout version out of bounds");
13541341

1342+
/* Check if the file driver would like to force early space allocation */
1343+
if (H5F_HAS_FEATURE(file, H5FD_FEAT_ALLOCATE_EARLY))
1344+
new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
1345+
1346+
/*
1347+
* Check if this dataset is going into a parallel file and set space allocation time.
1348+
* If the dataset has filters applied to it, writes to the dataset must be collective,
1349+
* so we don't need to force early space allocation. Otherwise, we force early space
1350+
* allocation to facilitate independent raw data operations.
1351+
*/
1352+
if (H5F_HAS_FEATURE(file, H5FD_FEAT_HAS_MPI) && (new_dset->shared->dcpl_cache.pline.nused == 0))
1353+
new_dset->shared->dcpl_cache.fill.alloc_time = H5D_ALLOC_TIME_EARLY;
1354+
13551355
/* Update the dataset's object header info. */
13561356
if (H5D__update_oh_info(file, new_dset, new_dset->shared->dapl_id) < 0)
13571357
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "can't update the metadata cache");

‎src/H5Ocache_image.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,12 @@ H5O__mdci_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSE
116116
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
117117
H5F_DECODE_LENGTH(f, p, mesg->size);
118118

119-
if (mesg->addr >= (HADDR_UNDEF - mesg->size))
120-
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows");
121-
if (mesg->addr == HADDR_UNDEF)
122-
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address is undefined");
123-
if ((mesg->addr + mesg->size) > H5F_get_eoa(f, H5FD_MEM_SUPER))
124-
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size exceeds file eoa");
119+
if (H5_addr_defined(mesg->addr) && mesg->size) {
120+
if (mesg->addr >= (HADDR_UNDEF - mesg->size))
121+
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size overflows");
122+
if ((mesg->addr + mesg->size) > H5F_get_eoa(f, H5FD_MEM_SUPER))
123+
HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "address plus size exceeds file eoa");
124+
}
125125

126126
/* Set return value */
127127
ret_value = (void *)mesg;

‎test/CMakeTests.cmake‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -473,29 +473,27 @@ if (NOT CYGWIN)
473473
endif ()
474474
endif ()
475475

476-
if (TEST_CACHE_IMAGE)
477-
#-- Adding test for cache_image
478-
add_test (
479-
NAME H5TEST-cache_image-clear-objects
480-
COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
481-
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
482-
)
483-
set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image)
484-
add_test (
485-
NAME H5TEST-cache_image-clean-objects
486-
COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
487-
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
488-
)
489-
set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image)
490-
add_test (NAME H5TEST-cache_image COMMAND $<TARGET_FILE:cache_image>)
491-
set_tests_properties (H5TEST-cache_image PROPERTIES
492-
FIXTURES_REQUIRED clear_cache_image
493-
ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST"
494-
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
495-
)
496-
if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}")
497-
set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true)
498-
endif ()
476+
#-- Adding test for cache_image
477+
add_test (
478+
NAME H5TEST-cache_image-clear-objects
479+
COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
480+
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
481+
)
482+
set_tests_properties (H5TEST-cache_image-clear-objects PROPERTIES FIXTURES_SETUP clear_cache_image)
483+
add_test (
484+
NAME H5TEST-cache_image-clean-objects
485+
COMMAND ${CMAKE_COMMAND} -E remove cache_image_test.h5
486+
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
487+
)
488+
set_tests_properties (H5TEST-cache_image-clean-objects PROPERTIES FIXTURES_CLEANUP clear_cache_image)
489+
add_test (NAME H5TEST-cache_image COMMAND $<TARGET_FILE:cache_image>)
490+
set_tests_properties (H5TEST-cache_image PROPERTIES
491+
FIXTURES_REQUIRED clear_cache_image
492+
ENVIRONMENT "srcdir=${HDF5_TEST_BINARY_DIR}/H5TEST"
493+
WORKING_DIRECTORY ${HDF5_TEST_BINARY_DIR}/H5TEST
494+
)
495+
if ("H5TEST-cache_image" MATCHES "${HDF5_DISABLE_TESTS_REGEX}")
496+
set_tests_properties (H5TEST-cache_image PROPERTIES DISABLED true)
499497
endif ()
500498

501499
#-- Adding test for external_env

‎test/cache_image.c‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
529529
}
530530

531531
if (show_progress)
532-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
532+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
533533

534534
/* create a file access property list. */
535535
if (pass) {
@@ -544,7 +544,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
544544
}
545545

546546
if (show_progress)
547-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
547+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
548548

549549
/* call H5Pset_libver_bounds() on the fapl_id */
550550
if (pass) {
@@ -557,7 +557,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
557557
}
558558

559559
if (show_progress)
560-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
560+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
561561

562562
/* get metadata cache image config -- verify that it is the default */
563563
if (pass) {
@@ -581,7 +581,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
581581
}
582582

583583
if (show_progress)
584-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
584+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
585585

586586
/* set metadata cache image fapl entry if indicated */
587587
if ((pass) && (set_mdci_fapl)) {
@@ -601,7 +601,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
601601
}
602602

603603
if (show_progress)
604-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
604+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
605605

606606
/* setup the persistent free space manager if indicated */
607607
if ((pass) && (config_fsm)) {
@@ -623,7 +623,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
623623
}
624624

625625
if (show_progress)
626-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
626+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
627627

628628
/* set evict on close if indicated */
629629
if ((pass) && (set_eoc)) {
@@ -636,7 +636,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
636636
}
637637

638638
if (show_progress)
639-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
639+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
640640

641641
/* open the file */
642642
if (pass) {
@@ -686,7 +686,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
686686
}
687687

688688
if (show_progress)
689-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
689+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
690690

691691
/* get a pointer to the files internal data structure and then
692692
* to the cache structure
@@ -705,7 +705,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
705705
}
706706

707707
if (show_progress)
708-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
708+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
709709

710710
/* verify expected metadata cache status */
711711

@@ -724,7 +724,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
724724
}
725725

726726
if (show_progress)
727-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
727+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
728728

729729
if (pass) {
730730

@@ -767,7 +767,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
767767
}
768768

769769
if (show_progress)
770-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
770+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
771771

772772
if ((pass) && (set_mdci_fapl)) {
773773

@@ -781,7 +781,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
781781
}
782782

783783
if (show_progress)
784-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
784+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
785785

786786
if (pass) {
787787

@@ -821,7 +821,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
821821
}
822822

823823
if (show_progress)
824-
fprintf(stdout, "%s: cp = %d.\n", fcn_name, cp++);
824+
fprintf(stdout, "%s: cp = %d, pass = %d.\n", fcn_name, cp++, pass);
825825

826826
if (pass) {
827827

@@ -831,7 +831,7 @@ open_hdf5_file(bool create_file, bool mdci_sbem_expected, bool read_only, bool s
831831
}
832832

833833
if (show_progress)
834-
fprintf(stdout, "%s: cp = %d -- exiting.\n", fcn_name, cp++);
834+
fprintf(stdout, "%s: cp = %d, pass = %d -- exiting.\n", fcn_name, cp++, pass);
835835

836836
} /* open_hdf5_file() */
837837

@@ -6477,6 +6477,12 @@ cache_image_api_error_check_4(bool single_file_vfd)
64776477
pass = false;
64786478
failure_mssg = "h5_fileaccess() failed.\n";
64796479
}
6480+
6481+
if (H5Pset_libver_bounds(fapl_id, H5F_LIBVER_EARLIEST, H5F_LIBVER_V18) < 0) {
6482+
6483+
pass = false;
6484+
failure_mssg = "H5Pset_libver_bounds() failed.\n";
6485+
}
64806486
}
64816487

64826488
if (show_progress)

‎testpar/CMakeTests.cmake‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,6 @@ endforeach ()
8484
# list (REMOVE_ITEM H5P_TESTS t_shapesame)
8585
#endif ()
8686

87-
# do not test until new version is added
88-
list (REMOVE_ITEM H5P_TESTS t_cache_image)
89-
9087
set (test_par_CLEANFILES
9188
t_cache_image_00.h5
9289
t_cache_image_01.h5

‎testpar/t_cache_image.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3425,7 +3425,7 @@ smoke_check_1(MPI_Comm mpi_comm, MPI_Info mpi_info, int mpi_rank, int mpi_size)
34253425

34263426
/* 13) Get the size of the file. Verify that it is less
34273427
* than 20 KB. Without deletions and persistent free
3428-
* space managers, size size is about 30 MB, so this
3428+
* space managers, file size is about 30 MB, so this
34293429
* is sufficient to verify that the persistent free
34303430
* space managers are more or less doing their job.
34313431
*

0 commit comments

Comments
 (0)