From ebba927ffdd15461eb9fd9df3d486482ea5c4b90 Mon Sep 17 00:00:00 2001 From: Richard Warren Date: Sat, 26 May 2018 15:43:01 -0400 Subject: Some fixes from Neil plus changes talked about in our last code review --- src/H5Dvirtual.c | 72 +++++++++++++++++++++++++++++++++----------------------- src/H5FDmpio.c | 30 +++++++++++++++++++++-- src/H5Oprivate.h | 3 ++- test/vds.c | 2 +- testpar/t_pvds.c | 10 ++++---- 5 files changed, 80 insertions(+), 37 deletions(-) diff --git a/src/H5Dvirtual.c b/src/H5Dvirtual.c index 66cf9d1..1bda4af 100644 --- a/src/H5Dvirtual.c +++ b/src/H5Dvirtual.c @@ -882,11 +882,8 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_srcdset_t *source_dset, hid_t dxpl_id) { H5F_t *src_file = NULL; /* Source file */ - hbool_t src_file_open = FALSE; /* Whether we have opened and need to close src_file */ - H5G_loc_t src_root_loc; /* Object location of source file root group */ herr_t ret_value = SUCCEED; /* Return value */ hid_t plist_id = -1; /* Property list pointer */ - unsigned intent; /* File access permissions */ FUNC_ENTER_STATIC @@ -898,54 +895,60 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, HDassert(source_dset->dset_name); /* Check if we need to open the source file */ - if(HDstrcmp(source_dset->file_name, ".")) { + if(!HDstrcmp(source_dset->file_name, ".")) + /* Source file is ".", use the virtual dataset's file */ + src_file = vdset->oloc.file; + else if(source_dset->file) + /* Use previously opened file */ + src_file = source_dset->file; + else { + unsigned intent; /* File access permissions */ + if((plist_id = H5D_get_access_plist((H5D_t *)vdset)) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "Can't get access plist") + + /* Get the virtual dataset's file open flags ("intent") */ intent = H5F_INTENT(vdset->oloc.file); - if(NULL == (src_file = H5F_prefix_open_file(plist_id, vdset->oloc.file, H5D_ACS_VDS_PREFIX_NAME, source_dset->file_name, intent, - vdset->shared->layout.storage.u.virt.source_fapl, dxpl_id))) - H5E_clear_stack(NULL); /* Quick hack until proper support for H5Fopen with missing file is implemented */ - else - src_file_open = TRUE; + + /* Try opening the file */ + src_file = H5F_prefix_open_file(plist_id, vdset->oloc.file, H5D_ACS_VDS_PREFIX_NAME, source_dset->file_name, intent, + vdset->shared->layout.storage.u.virt.source_fapl, dxpl_id); + /* Reset the error stack if we did not find the file */ + if(!src_file) + H5E_clear_stack(NULL); + + source_dset->file = src_file; + } /* end if */ - else - /* Source file is ".", use the virtual dataset's file */ - src_file = vdset->oloc.file; if(src_file) { + H5G_loc_t src_root_loc; /* Object location of source file root group */ + /* Set up the root group in the destination file */ if(NULL == (src_root_loc.oloc = H5G_oloc(H5G_rootof(src_file)))) HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "unable to get object location for root group") if(NULL == (src_root_loc.path = H5G_nameof(H5G_rootof(src_file)))) HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "unable to get path for root group") - /* Open the source dataset */ - if(NULL == (source_dset->dset = H5D__open_name(&src_root_loc, source_dset->dset_name, vdset->shared->layout.storage.u.virt.source_dapl, dxpl_id))) { - H5E_clear_stack(NULL); /* Quick hack until proper support for H5Dopen with missing file is implemented */ + /* Try opening the source dataset */ + source_dset->dset = H5D__open_name(&src_root_loc, source_dset->dset_name, vdset->shared->layout.storage.u.virt.source_dapl, dxpl_id); - /* Dataset does not exist */ - source_dset->dset_exists = FALSE; - } /* end if */ - else { - /* Dataset exists */ - source_dset->dset_exists = TRUE; + /* Dataset does not exist */ + if(NULL == source_dset->dset) + /* Reset the error stack */ + H5E_clear_stack(NULL); + else /* Patch the source selection if necessary */ if(virtual_ent->source_space_status != H5O_VIRTUAL_STATUS_CORRECT) { if(H5S_extent_copy(virtual_ent->source_select, source_dset->dset->shared->space) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTCOPY, FAIL, "can't copy source dataspace extent") virtual_ent->source_space_status = H5O_VIRTUAL_STATUS_CORRECT; } /* end if */ - } /* end else */ + } /* end if */ done: - if(plist_id >= 0) - H5Pclose(plist_id); - /* Close source file */ - if(src_file_open) - if(H5F_efc_close(vdset->oloc.file, src_file) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CANTCLOSEFILE, FAIL, "can't close source file") FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__virtual_open_source_dset() */ @@ -981,6 +984,14 @@ H5D__virtual_reset_source_dset(H5O_storage_virtual_ent_t *virtual_ent, source_dset->dset = NULL; } /* end if */ + /* Close file */ + if(source_dset->file) { + HDassert(virtual_ent->virtual_file); + if(H5F_efc_close(virtual_ent->virtual_file, source_dset->file) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CANTCLOSEFILE, FAIL, "can't close source file") + source_dset->file = NULL; + } /* end if */ + /* Free file name */ if(virtual_ent->parsed_source_file_name && (source_dset->file_name @@ -1587,7 +1598,7 @@ H5D__virtual_set_extent_unlim(const H5D_t *dset, hid_t dxpl_id) } /* end if */ /* Check if the dataset was already opened */ - if(storage->list[i].sub_dset[j].dset_exists) + if(storage->list[i].sub_dset[j].dset) first_missing = j + 1; else { /* Resolve file name */ @@ -2132,6 +2143,9 @@ H5D__virtual_init(H5F_t *f, hid_t H5_ATTR_UNUSED dxpl_id, const H5D_t *dset, HGOTO_ERROR(H5E_DATASET, H5E_BADSELECT, FAIL, "unable to normalize dataspace by offset") if(H5S_hyper_normalize_offset(storage->list[i].source_select, old_offset) < 0) HGOTO_ERROR(H5E_DATASET, H5E_BADSELECT, FAIL, "unable to normalize dataspace by offset") + + /* Save pointer to virtual dataset in entry */ + storage->list[i].virtual_file = (H5F_t *)dset->oloc.file; } /* end for */ /* Get dataset access property list */ diff --git a/src/H5FDmpio.c b/src/H5FDmpio.c index c7921b0..ed3331b 100644 --- a/src/H5FDmpio.c +++ b/src/H5FDmpio.c @@ -125,7 +125,7 @@ static const H5FD_class_mpi_t H5FD_mpio_g = { { /* Start of superclass information */ "mpio", /*name */ HADDR_MAX, /*maxaddr */ - H5F_CLOSE_WEAK, /* fc_degree */ + H5F_CLOSE_SEMI, /* fc_degree */ H5FD_mpio_term, /*terminate */ NULL, /*sb_size */ NULL, /*sb_encode */ @@ -991,7 +991,7 @@ done: if(fd >= 0) HDclose(fd); - FUNC_LEAVE_NOAPI(SUCCEED) + FUNC_LEAVE_NOAPI_VOID } @@ -1187,8 +1187,34 @@ H5FD_mpio_cmp(const H5FD_t *_f1, const H5FD_t *_f2) int ret_value = 0; int cmp_value = 0; int mpi_result; + MPI_Group f1_grp; + MPI_Group f2_grp; + FUNC_ENTER_NOAPI_NOINIT + if ((mpi_result = MPI_Comm_group(f1->comm, &f1_grp)) != MPI_SUCCESS) + HMPI_GOTO_ERROR(FAIL, "MPI_Comm_group(comm1) failed", mpi_result) + + if ((mpi_result = MPI_Comm_group(f1->comm, &f2_grp)) != MPI_SUCCESS) + HMPI_GOTO_ERROR(FAIL, "MPI_Comm_group(comm2) failed", mpi_result) + + if ((mpi_result = MPI_Group_compare(f1_grp,f2_grp,&cmp_value)) != MPI_SUCCESS) + HMPI_GOTO_ERROR(FAIL, "MPI_Group_compare failed", mpi_result) + + /* The group compare return values can be one of the following: + * MPI_IDENT(0) == two groups/communicators are identical + * MPI_CONGRUENT(1) == two groups/communicators are equal but + * are distinct communication domains + * ---------------- for comparison purposes, the above are ok + * ---------------- but those below can lead to unexpected + * ---------------- results, so we will FAIL the comparison. + * MPI_SIMILAR(2) == two groups have the same members but + * ordering may be different + * MPI_UNEQUAL(3) == self descriptive (unequal) + */ + if (cmp_value >= MPI_SIMILAR) + HMPI_GOTO_ERROR(FAIL, "MPI Comms are incompatable", cmp_value) + if (f1->mpi_rank == 0) { /* Because MPI file handles may NOT have any relation to * to actual file handle, we utilize a "regular" file open diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index a4cbc89..656f9eb 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -497,7 +497,7 @@ typedef struct H5O_storage_virtual_srcdset_t { struct H5S_t *clipped_source_select; /* Clipped version of source_select */ struct H5S_t *clipped_virtual_select; /* Clipped version of virtual_select */ struct H5D_t *dset; /* Source dataset */ - hbool_t dset_exists; /* Whether the dataset exists (was opened successfully) */ + struct H5F_t *file; /* Source file (if one was opened for this dataset) */ /* Temporary - only used during I/O operation, NULL at all other times */ struct H5S_t *projected_mem_space; /* Selection within mem_space for this mapping */ @@ -542,6 +542,7 @@ typedef struct H5O_storage_virtual_ent_t { hsize_t clip_size_source; /* Size selection would be clipped to in source selection when virtual extent == unlim_extent_virtual */ H5O_virtual_space_status_t source_space_status; /* Extent patching status of source_select */ H5O_virtual_space_status_t virtual_space_status; /* Extent patching status of virtual_select */ + H5F_t *virtual_file; /* Convenience pointer to file containing virtual dataset */ } H5O_storage_virtual_ent_t; typedef struct H5O_storage_virtual_t { diff --git a/test/vds.c b/test/vds.c index 7d55c37..a882698 100644 --- a/test/vds.c +++ b/test/vds.c @@ -618,7 +618,7 @@ test_api(test_api_config_t config, hid_t fapl) TEST_ERROR /* Get examination DCPL */ - if(test_api_get_ex_dcpl(config, fapl, dcpl, &ex_dcpl, vspace[0], filename, (hsize_t)174) < 0) + if(test_api_get_ex_dcpl(config, fapl, dcpl, &ex_dcpl, vspace[0], filename, (hsize_t)213) < 0) TEST_ERROR /* Test H5Pget_virtual_count */ diff --git a/testpar/t_pvds.c b/testpar/t_pvds.c index 0d0b197..1b81557 100644 --- a/testpar/t_pvds.c +++ b/testpar/t_pvds.c @@ -1849,10 +1849,6 @@ test_basic_io(unsigned config, hid_t fapl) TEST_ERROR } /* end if */ - /* Close copied virtual file */ - if(H5Fclose(vfile2) < 0) - TEST_ERROR - vfile2 = -1; } /* end if */ /* Close */ @@ -1871,6 +1867,12 @@ test_basic_io(unsigned config, hid_t fapl) if(H5Fclose(vfile) < 0) TEST_ERROR vfile = -1; + + /* Close copied virtual file */ + if((vfile2 != -1) && (H5Fclose(vfile2) < 0)) + TEST_ERROR + vfile2 = -1; + if(H5Sclose(srcspace[0]) < 0) TEST_ERROR srcspace[0] = -1; -- cgit v0.12