From afdb7903c644ef8eab44a66248b85ce9bed60db8 Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Wed, 17 Feb 2021 06:37:23 -0600 Subject: Fix for HDFFV-11109 - Copy MPI comm and info object into output FAPL from H5F_get_access_plist (#342) * Avoid freeing MPI_COMM_WORLD in H5_mpi_comm_free * Copy MPI Comm and Info to new FAPL in H5F_get_access_plist --- release_docs/RELEASE.txt | 9 +++ src/H5Fint.c | 28 +++++-- src/H5mpi.c | 2 +- testpar/t_prop.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++ testpar/testphdf5.c | 2 + testpar/testphdf5.h | 1 + 6 files changed, 224 insertions(+), 7 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index c844b82..228f771 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -696,6 +696,15 @@ Bug Fixes since HDF5-1.12.0 release ================================== Library ------- + - Fixed issue with MPI communicator and info object not being + copied into new FAPL retrieved from H5F_get_access_plist + + Added logic to copy the MPI communicator and info object into + the output FAPL. MPI communicator is retrieved from the VFD, while + the MPI info object is retrieved from the file's original FAPL. + + (JTH - 2021/02/15, HDFFV-11109) + - Fixed problems with vlens and refs inside compound using H5VLget_file_type() diff --git a/src/H5Fint.c b/src/H5Fint.c index 67245d5..272f060 100644 --- a/src/H5Fint.c +++ b/src/H5Fint.c @@ -413,24 +413,40 @@ H5F_get_access_plist(H5F_t *f, hbool_t app_ref) if (f->shared->efc) efc_size = H5F__efc_max_nfiles(f->shared->efc); if (H5P_set(new_plist, H5F_ACS_EFC_SIZE_NAME, &efc_size) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't set elink file cache size") + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set elink file cache size") if (f->shared->page_buf != NULL) { if (H5P_set(new_plist, H5F_ACS_PAGE_BUFFER_SIZE_NAME, &(f->shared->page_buf->max_size)) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't set page buffer size") + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set page buffer size") if (H5P_set(new_plist, H5F_ACS_PAGE_BUFFER_MIN_META_PERC_NAME, &(f->shared->page_buf->min_meta_perc)) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set minimum metadata fraction of page buffer") if (H5P_set(new_plist, H5F_ACS_PAGE_BUFFER_MIN_RAW_PERC_NAME, &(f->shared->page_buf->min_raw_perc)) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set minimum raw data fraction of page buffer") } /* end if */ #ifdef H5_HAVE_PARALLEL if (H5P_set(new_plist, H5_COLL_MD_READ_FLAG_NAME, &(f->shared->coll_md_read)) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't set collective metadata read flag") + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set collective metadata read flag") if (H5P_set(new_plist, H5F_ACS_COLL_MD_WRITE_FLAG_NAME, &(f->shared->coll_md_write)) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't set collective metadata read flag") + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set collective metadata read flag") + if (H5F_HAS_FEATURE(f, H5FD_FEAT_HAS_MPI)) { + MPI_Comm mpi_comm; + MPI_Info mpi_info; + + /* Retrieve and set MPI communicator */ + if (MPI_COMM_NULL == (mpi_comm = H5F_mpi_get_comm(f))) + HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't get MPI communicator") + if (H5P_set(new_plist, H5F_ACS_MPI_PARAMS_COMM_NAME, &mpi_comm) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set MPI communicator") + + /* Retrieve and set MPI info object */ + if (H5P_get(old_plist, H5F_ACS_MPI_PARAMS_INFO_NAME, &mpi_info) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTGET, H5I_INVALID_HID, "can't get MPI info object") + if (H5P_set(new_plist, H5F_ACS_MPI_PARAMS_INFO_NAME, &mpi_info) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTSET, H5I_INVALID_HID, "can't set MPI info object") + } #endif /* H5_HAVE_PARALLEL */ if (H5P_set(new_plist, H5F_ACS_META_CACHE_INIT_IMAGE_CONFIG_NAME, &(f->shared->mdc_initCacheImageCfg)) < 0) diff --git a/src/H5mpi.c b/src/H5mpi.c index 613a4bf..1c2d3b7 100644 --- a/src/H5mpi.c +++ b/src/H5mpi.c @@ -210,7 +210,7 @@ H5_mpi_comm_free(MPI_Comm *comm) HGOTO_ERROR(H5E_INTERNAL, H5E_BADVALUE, FAIL, "comm pointer cannot be NULL") /* Free the communicator */ - if (MPI_COMM_NULL != *comm) + if (MPI_COMM_WORLD != *comm && MPI_COMM_NULL != *comm) MPI_Comm_free(comm); *comm = MPI_COMM_NULL; diff --git a/testpar/t_prop.c b/testpar/t_prop.c index e84a942..a7e297e 100644 --- a/testpar/t_prop.c +++ b/testpar/t_prop.c @@ -441,3 +441,192 @@ test_plist_ed(void) ret = H5Pclose(acpl); VRFY((ret >= 0), "H5Pclose succeeded"); } + +void +external_links(void) +{ + hid_t lcpl = H5I_INVALID_HID; /* link create prop. list */ + hid_t lapl = H5I_INVALID_HID; /* link access prop. list */ + hid_t fapl = H5I_INVALID_HID; /* file access prop. list */ + hid_t gapl = H5I_INVALID_HID; /* group access prop. list */ + hid_t fid = H5I_INVALID_HID; /* file id */ + hid_t group = H5I_INVALID_HID; /* group id */ + int mpi_size, mpi_rank; + + MPI_Comm comm; + int doIO; + int i, mrc; + + herr_t ret; /* Generic return value */ + htri_t tri_status; /* tri return value */ + + const char *filename = "HDF5test.h5"; + const char *filename_ext = "HDF5test_ext.h5"; + const char *group_path = "/Base/Block/Step"; + const char *link_name = "link"; /* external link */ + char link_path[50]; + + if (VERBOSE_MED) + HDprintf("Check external links\n"); + + /* set up MPI parameters */ + MPI_Comm_size(MPI_COMM_WORLD, &mpi_size); + MPI_Comm_rank(MPI_COMM_WORLD, &mpi_rank); + + /* Check MPI communicator access properties are passed to + linked external files */ + + if (mpi_rank == 0) { + + lcpl = H5Pcreate(H5P_LINK_CREATE); + VRFY((lcpl >= 0), "H5Pcreate succeeded"); + + ret = H5Pset_create_intermediate_group(lcpl, 1); + VRFY((ret >= 0), "H5Pset_create_intermediate_group succeeded"); + + /* Create file to serve as target for external link.*/ + fid = H5Fcreate(filename_ext, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + VRFY((fid >= 0), "H5Fcreate succeeded"); + + group = H5Gcreate2(fid, group_path, lcpl, H5P_DEFAULT, H5P_DEFAULT); + VRFY((group >= 0), "H5Gcreate succeeded"); + + ret = H5Gclose(group); + VRFY((ret >= 0), "H5Gclose succeeded"); + + ret = H5Fclose(fid); + VRFY((ret >= 0), "H5Fclose succeeded"); + + fapl = H5Pcreate(H5P_FILE_ACCESS); + VRFY((fapl >= 0), "H5Pcreate succeeded"); + + /* Create a new file using the file access property list. */ + fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl); + VRFY((fid >= 0), "H5Fcreate succeeded"); + + ret = H5Pclose(fapl); + VRFY((ret >= 0), "H5Pclose succeeded"); + + group = H5Gcreate2(fid, group_path, lcpl, H5P_DEFAULT, H5P_DEFAULT); + VRFY((group >= 0), "H5Gcreate succeeded"); + + /* Create external links to the target files. */ + ret = H5Lcreate_external(filename_ext, group_path, group, link_name, H5P_DEFAULT, H5P_DEFAULT); + VRFY((ret >= 0), "H5Lcreate_external succeeded"); + + /* Close and release resources. */ + ret = H5Pclose(lcpl); + VRFY((ret >= 0), "H5Pclose succeeded"); + ret = H5Gclose(group); + VRFY((ret >= 0), "H5Gclose succeeded"); + ret = H5Fclose(fid); + VRFY((ret >= 0), "H5Fclose succeeded"); + } + + MPI_Barrier(MPI_COMM_WORLD); + + /* + * For the first case, use all the processes. For the second case + * use a sub-communicator to verify the correct communicator is + * being used for the externally linked files. + * There is no way to determine if MPI info is being used for the + * externally linked files. + */ + + for (i = 0; i < 2; i++) { + + if (i == 0) { + doIO = 1; + comm = MPI_COMM_WORLD; + } + else { + doIO = mpi_rank % 2; + mrc = MPI_Comm_split(MPI_COMM_WORLD, doIO, mpi_rank, &comm); + VRFY((mrc == MPI_SUCCESS), ""); + } + + if (doIO) { + fapl = H5Pcreate(H5P_FILE_ACCESS); + VRFY((fapl >= 0), "H5Pcreate succeeded"); + ret = H5Pset_fapl_mpio(fapl, comm, MPI_INFO_NULL); + VRFY((fapl >= 0), "H5Pset_fapl_mpio succeeded"); + + fid = H5Fopen(filename, H5F_ACC_RDWR, fapl); + VRFY((fid >= 0), "H5Fopen succeeded"); + + /* test opening a group that is to an external link, the external linked + file should inherit the source file's access properties */ + HDsprintf(link_path, "%s%s%s", group_path, "/", link_name); + group = H5Gopen2(fid, link_path, H5P_DEFAULT); + VRFY((group >= 0), "H5Gopen succeeded"); + ret = H5Gclose(group); + VRFY((ret >= 0), "H5Gclose succeeded"); + + /* test opening a group that is external link by setting group + creation property */ + gapl = H5Pcreate(H5P_GROUP_ACCESS); + VRFY((gapl >= 0), "H5Pcreate succeeded"); + + ret = H5Pset_elink_fapl(gapl, fapl); + VRFY((ret >= 0), "H5Pset_elink_fapl succeeded"); + + group = H5Gopen2(fid, link_path, gapl); + VRFY((group >= 0), "H5Gopen succeeded"); + + ret = H5Gclose(group); + VRFY((ret >= 0), "H5Gclose succeeded"); + + ret = H5Pclose(gapl); + VRFY((ret >= 0), "H5Pclose succeeded"); + + /* test link APIs */ + lapl = H5Pcreate(H5P_LINK_ACCESS); + VRFY((lapl >= 0), "H5Pcreate succeeded"); + + ret = H5Pset_elink_fapl(lapl, fapl); + VRFY((ret >= 0), "H5Pset_elink_fapl succeeded"); + + tri_status = H5Lexists(fid, link_path, H5P_DEFAULT); + VRFY((tri_status == TRUE), "H5Lexists succeeded"); + + tri_status = H5Lexists(fid, link_path, lapl); + VRFY((tri_status == TRUE), "H5Lexists succeeded"); + + group = H5Oopen(fid, link_path, H5P_DEFAULT); + VRFY((group >= 0), "H5Oopen succeeded"); + + ret = H5Oclose(group); + VRFY((ret >= 0), "H5Oclose succeeded"); + + group = H5Oopen(fid, link_path, lapl); + VRFY((group >= 0), "H5Oopen succeeded"); + + ret = H5Oclose(group); + VRFY((ret >= 0), "H5Oclose succeeded"); + + ret = H5Pclose(lapl); + VRFY((ret >= 0), "H5Pclose succeeded"); + + /* close the remaining resources */ + + ret = H5Pclose(fapl); + VRFY((ret >= 0), "H5Pclose succeeded"); + + ret = H5Fclose(fid); + VRFY((ret >= 0), "H5Fclose succeeded"); + + if (i == 1) { + mrc = MPI_Comm_free(&comm); + VRFY((mrc == MPI_SUCCESS), "MPI_Comm_free succeeded"); + } + } + } + + MPI_Barrier(MPI_COMM_WORLD); + + /* delete the test files */ + if (mpi_rank == 0) { + MPI_File_delete(filename, MPI_INFO_NULL); + MPI_File_delete(filename_ext, MPI_INFO_NULL); + } +} diff --git a/testpar/testphdf5.c b/testpar/testphdf5.c index ca38623..c1851bf 100644 --- a/testpar/testphdf5.c +++ b/testpar/testphdf5.c @@ -467,6 +467,8 @@ main(int argc, char **argv) AddTest("edpl", test_plist_ed, NULL, "encode/decode Property Lists", NULL); + AddTest("extlink", external_links, NULL, "test external links", NULL); + if ((mpi_size < 2) && MAINPROCESS) { HDprintf("File Image Ops daisy chain test needs at least 2 processes.\n"); HDprintf("File Image Ops daisy chain test will be skipped \n"); diff --git a/testpar/testphdf5.h b/testpar/testphdf5.h index a821e6f..ac73301 100644 --- a/testpar/testphdf5.h +++ b/testpar/testphdf5.h @@ -235,6 +235,7 @@ extern int dxfer_coll_type; /* Test program prototypes */ void test_plist_ed(void); +void external_links(void); void zero_dim_dset(void); void test_file_properties(void); void multiple_dset_write(void); -- cgit v0.12