Some fixes from Neil plus changes talked about in our last code reviewinactive/parallel_vds

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;