Implement the changes suggested in our PR code review

4 files changed, 134 insertions, 138 deletions

diff --git a/src/H5AC.c b/src/H5AC.c
index c5c4a76..4223158 100644
--- a/src/H5AC.c
+++ b/src/H5AC.c
@@ -661,7 +661,7 @@ H5AC_dest(H5F_t *f, hid_t dxpl_id)
 #ifdef H5_HAVE_PARALLEL
     if(aux_ptr != NULL) {
         if(aux_ptr->d_slist_ptr != NULL) {
-	    HDassert(H5SL_count(aux_ptr->d_slist_ptr) == 0);
+            HDassert(H5SL_count(aux_ptr->d_slist_ptr) == 0);
             H5SL_close(aux_ptr->d_slist_ptr);
         } /* end if */
         if(aux_ptr->c_slist_ptr != NULL) {
diff --git a/src/H5Dvirtual.c b/src/H5Dvirtual.c
index 8757cfb..ed393c9 100644
--- a/src/H5Dvirtual.c
+++ b/src/H5Dvirtual.c
@@ -719,8 +719,8 @@ H5D__virtual_insert_gh_obj(H5F_t *f, H5O_storage_virtual_t *storage)
         uint32_t chksum;
         size_t i;
 
-        // H5F_SET_LATEST_FLAGS(f, H5F_LATEST_ALL_FLAGS);
-        if(H5F_set_libver_bounds(f, H5F_LIBVER_V110, H5F_LIBVER_V110) < 0)
+        // if(H5F_set_libver_bounds(f, H5F_LIBVER_V110, H5F_LIBVER_V110) < 0)
+        if(H5F_set_libver_bounds(f, H5F_LOW_BOUND(f), H5F_HIGH_BOUND(f)) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "cannot set low/high bounds")
 
         /* Allocate array for caching results of strlen */
@@ -2597,12 +2597,6 @@ H5D__virtual_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
     storage = &io_info->dset->shared->layout.storage.u.virt;
     HDassert((storage->view == H5D_VDS_FIRST_MISSING) || (storage->view == H5D_VDS_LAST_AVAILABLE));
 
-#if defined(H5_HAVE_PARALLEL) && defined(H5_DISABLE_PARALLEL_VDS)
-    /* Parallel reads are not supported (yet) */
-    if(H5F_HAS_FEATURE(io_info->dset->oloc.file, H5FD_FEAT_HAS_MPI))
-        HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, FAIL, "parallel reads not supported on virtual datasets")
-#endif /* H5_HAVE_PARALLEL */
-
     /* Prepare for I/O operation */
     if(H5D__virtual_pre_io(io_info, storage, file_space, mem_space, &tot_nelmts) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTCLIP, FAIL, "unable to prepare for I/O operation")
@@ -2788,12 +2782,6 @@ H5D__virtual_write(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
     storage = &io_info->dset->shared->layout.storage.u.virt;
     HDassert((storage->view == H5D_VDS_FIRST_MISSING) || (storage->view == H5D_VDS_LAST_AVAILABLE));
 
-#if defined(H5_HAVE_PARALLEL) && defined(H5_DISABLE_PARALLEL_VDS)
-    /* Parallel writes are not supported (yet) */
-    if(H5F_HAS_FEATURE(io_info->dset->oloc.file, H5FD_FEAT_HAS_MPI))
-        HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, FAIL, "parallel writes not supported on virtual datasets")
-#endif /* H5_HAVE_PARALLEL */
-
     /* Prepare for I/O operation */
     if(H5D__virtual_pre_io(io_info, storage, file_space, mem_space, &tot_nelmts) < 0)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTCLIP, FAIL, "unable to prepare for I/O operation")
diff --git a/src/H5FDmpio.c b/src/H5FDmpio.c
index 9f04fcc..e5b4226 100644
--- a/src/H5FDmpio.c
+++ b/src/H5FDmpio.c
@@ -58,7 +58,24 @@ static char H5FD_mpi_native_g[] = "native";
  */
 typedef struct H5FD_mpio_t {
     H5FD_t	pub;		/*public stuff, must be first		*/
-    char        *filename;      /*Used for comparison  */
+
+/* For comparisons */
+#ifndef H5_HAVE_WIN32_API
+    /* On most systems the combination of device and i-node number uniquely
+     * identify a file.  Note that Cygwin, MinGW and other Windows POSIX
+     * environments have the stat function (which fakes inodes)
+     * and will use the 'device + inodes' scheme as opposed to the
+     * Windows code further below.
+     */
+    dev_t       device;     /* file device number   */
+    ino_t       inode;      /* file i-node number   */
+
+#else
+    DWORD       nFileIndexLow;
+    DWORD       nFileIndexHigh;
+    DWORD       dwVolumeSerialNumber;
+
+#endif
     MPI_File	f;		/*MPIO file handle			*/
     MPI_Comm	comm;		/*communicator				*/
     MPI_Info	info;		/*file information			*/
@@ -909,6 +926,70 @@ done:
     FUNC_LEAVE_NOAPI(ret_value)
 }
 
+
+/*
+ * Function:    gather_regular_file_info_
+ *
+ * Purpose:     Implements a normal file open for MPI rank 0.  This
+ *              essentially a copy of the H5FD_sec2_open, where we
+ *              open the file and cache a few key structures before
+ *              closing.  These cached structures are those which
+ *              are eventually utilized for file comparisons.
+ *
+ * Return:      NONE.  Any failures that occur here should ultimately
+ *              be reflected in the actual MPI_File_open call which
+ *              occurs immediately following completion of this
+ *              function.
+ */
+static void
+gather_regular_file_info_(const char *name, unsigned flags, H5FD_mpio_t *file)
+{
+    int             fd          = -1;       /* File descriptor          */
+    int             o_flags;                /* Flags for open() call    */
+    h5_stat_t       sb;
+
+#ifdef H5_HAVE_WIN32_API
+    struct _BY_HANDLE_FILE_INFORMATION fileinfo;
+    HANDLE          hFile;                  /* Native windows file handle */
+#endif
+    o_flags = (H5F_ACC_RDWR & flags) ? O_RDWR : O_RDONLY;
+
+    /* Open the file */
+    if((fd = HDopen(name, o_flags, H5_POSIX_CREATE_MODE_RW)) < 0) {
+#ifdef H5FDmpio_DEBUG
+if (H5FD_mpio_Debug[(int)'t'])
+fprintf(stdout, "gather_regular_file_info_: HDopen failed!\n");
+#endif
+        goto done;
+    }
+    if(HDfstat(fd, &sb) < 0) {
+#ifdef H5FDmpio_DEBUG
+if (H5FD_mpio_Debug[(int)'t'])
+fprintf(stdout, "gather_regular_file_info_: HDfstat failed!\n");
+#endif
+        goto done;
+    }
+#ifdef H5_HAVE_WIN32_API
+    hFile = (HANDLE)_get_osfhandle(fd);
+    if(INVALID_HANDLE_VALUE == hFile)
+        goto done;
+
+    if(!GetFileInformationByHandle((HANDLE)hFile, &fileinfo))
+        goto done;
+
+    file->nFileIndexHigh = fileinfo.nFileIndexHigh;
+    file->nFileIndexLow = fileinfo.nFileIndexLow;
+    file->dwVolumeSerialNumber = fileinfo.dwVolumeSerialNumber;
+#else /* H5_HAVE_WIN32_API */
+    file->device = sb.st_dev;
+    file->inode = sb.st_ino;
+#endif /* H5_HAVE_WIN32_API */
+
+done:
+    if(fd >= 0)
+        HDclose(fd);
+}
+
 
 /*-------------------------------------------------------------------------
  * Function:    H5FD_mpio_open
@@ -1015,7 +1096,6 @@ H5FD_mpio_open(const char *name, unsigned flags, hid_t fapl_id,
     if(NULL == (file = (H5FD_mpio_t *)H5MM_calloc(sizeof(H5FD_mpio_t))))
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
     file->f = fh;
-    file->filename = HDstrdup(name);
     file->comm = comm_dup;
     file->info = info_dup;
     file->mpi_rank = mpi_rank;
@@ -1025,6 +1105,7 @@ H5FD_mpio_open(const char *name, unsigned flags, hid_t fapl_id,
     if (mpi_rank == 0) {
         if (MPI_SUCCESS != (mpi_code=MPI_File_get_size(fh, &size)))
             HMPI_GOTO_ERROR(NULL, "MPI_File_get_size failed", mpi_code)
+
     } /* end if */
 
     /* Broadcast file size */
@@ -1051,6 +1132,10 @@ H5FD_mpio_open(const char *name, unsigned flags, hid_t fapl_id,
     /* Mark initial barriers in H5FD_mpio_truncate() as necessary */
     file->do_pre_trunc_barrier = TRUE;
 
+    if (mpi_rank == 0) {
+        /* Gather some file info for future comparisons */
+        gather_regular_file_info_( name, flags, file );
+    }
     /* Set return value */
     ret_value=(H5FD_t*)file;
 
@@ -1063,8 +1148,6 @@ done:
 	if (MPI_INFO_NULL != info_dup)
 	    MPI_Info_free(&info_dup);
 	if (file) {
-            if (file->filename)
-                HDfree(file->filename);
 	    H5MM_xfree(file);
         }
     } /* end if */
@@ -1076,12 +1159,6 @@ done:
     FUNC_LEAVE_NOAPI(ret_value)
 }
 
-#ifdef H5_HAVE_WIN32_API
-#define SLASH '\\'
-#else
-#define SLASH '/'
-#endif
-
 
 /*-------------------------------------------------------------------------
  * Function:    H5FD_mpio_cmp
@@ -1107,22 +1184,45 @@ H5FD_mpio_cmp(const H5FD_t *_f1, const H5FD_t *_f2)
 
     FUNC_ENTER_NOAPI_NOINIT_NOERR
 
-    char *filename1 = f1->filename;
-    char *filename2 = f2->filename;
-    char *fname1    = HDstrrchr(filename1,SLASH);
-    char *fname2    = HDstrrchr(filename2,SLASH);
-    /* the strrchr above points to a slash character.
-     * Increment these pointers prior use in strcmp()
-     */
-    if (fname1 == NULL)
-        if (fname2 == NULL)
-            ret_value = HDstrcmp(filename1, filename2);
-        else ret_value = HDstrcmp(filename1, ++fname2);
-    else if (fname2 == NULL)
-        ret_value = HDstrcmp(++fname1, filename2);
-    else ret_value = HDstrcmp(fname1, fname2);
+    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
+         * on MPI rank 0 prior to opening with the MPI-IO routines.
+         * The H5FD_mpio_t structure is utilized to cache the
+         * relevant comparison values which we use for comparisons
+         * below.
+         */
+        if (ret_value == 0) {
+#ifdef H5_HAVE_WIN32_API
+            if(f1->dwVolumeSerialNumber < f2->dwVolumeSerialNumber) HGOTO_DONE(-1)
+            if(f1->dwVolumeSerialNumber > f2->dwVolumeSerialNumber) HGOTO_DONE(1)
+
+            if(f1->nFileIndexHigh < f2->nFileIndexHigh) HGOTO_DONE(-1)
+            if(f1->nFileIndexHigh > f2->nFileIndexHigh) HGOTO_DONE(1)
+
+            if(f1->nFileIndexLow < f2->nFileIndexLow) HGOTO_DONE(-1)
+            if(f1->nFileIndexLow > f2->nFileIndexLow) HGOTO_DONE(1)
+#else  /* Not WIN32 */
+#ifdef H5_DEV_T_IS_SCALAR
+            if(f1->device < f2->device) HGOTO_DONE(-1)
+            if(f1->device > f2->device) HGOTO_DONE(1)
+#else /* H5_DEV_T_IS_SCALAR */
+            /* If dev_t isn't a scalar value on this system, just use memcmp to
+             * determine if the values are the same or not.  The actual return value
+             * shouldn't really matter...
+             */
+            if(HDmemcmp(&(f1->device),&(f2->device),sizeof(dev_t)) < 0) HGOTO_DONE(-1)
+            if(HDmemcmp(&(f1->device),&(f2->device),sizeof(dev_t)) > 0) HGOTO_DONE(1)
+#endif /* H5_DEV_T_IS_SCALAR */
+            if(f1->inode < f2->inode) HGOTO_DONE(-1)
+            if(f1->inode > f2->inode) HGOTO_DONE(1)
+#endif /* H5_HAVE_WIN32_API */
+        }
+    }
 
 done:
+
+    MPI_Bcast(&ret_value, 1, MPI_INT, 0, f1->comm);
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5FD_mpio_cmp() */
 
diff --git a/src/H5Olayout.c b/src/H5Olayout.c
index 870ebd9..99e8ba7 100644
--- a/src/H5Olayout.c
+++ b/src/H5Olayout.c
@@ -680,107 +680,15 @@ H5O__layout_encode(H5F_t *f, hbool_t H5_ATTR_UNUSED disable_shared, uint8_t *p,
             break;
 
         case H5D_VIRTUAL:
-            /* Create heap block if it has not been created yet */
-            /* Note that we assume here that the contents of the heap block
-             * cannot change!  If this ever stops being the case we must change
-             * this code to allow overwrites of the heap block.  -NAF */
-            
-            if((mesg->storage.u.virt.serial_list_hobjid.addr == HADDR_UNDEF)
-                    && (mesg->storage.u.virt.list_nused > 0)) {
-                uint8_t *heap_block_p;
-                size_t block_size;
-                hssize_t select_serial_size;
-                hsize_t tmp_hsize;
-                uint32_t chksum;
-                size_t i;
-
-                 if(H5F_set_libver_bounds(f, H5F_LIBVER_V110, H5F_LIBVER_V110) < 0)
-                    HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "cannot set low/high bounds")
-
-                /* Allocate array for caching results of strlen */
-                if(NULL == (str_size = (size_t *)H5MM_malloc(2 * mesg->storage.u.virt.list_nused *sizeof(size_t))))
-                    HGOTO_ERROR(H5E_OHDR, H5E_RESOURCE, FAIL, "unable to allocate string length array")
 
-                /*
-                 * Calculate heap block size
-                 */
-                /* Version and number of entries */
-                block_size = (size_t)1 + H5F_SIZEOF_SIZE(f);
-
-                /* Calculate size of each entry */
-                for(i = 0; i < mesg->storage.u.virt.list_nused; i++) {
-                    HDassert(mesg->storage.u.virt.list[i].source_file_name);
-                    HDassert(mesg->storage.u.virt.list[i].source_dset_name);
-                    HDassert(mesg->storage.u.virt.list[i].source_select);
-                    HDassert(mesg->storage.u.virt.list[i].source_dset.virtual_select);
-
-                    /* Source file name */
-                    str_size[2 * i] = HDstrlen(mesg->storage.u.virt.list[i].source_file_name) + (size_t)1;
-                    block_size += str_size[2 * i];
-
-                    /* Source dset name */
-                    str_size[(2 * i) + 1] = HDstrlen(mesg->storage.u.virt.list[i].source_dset_name) + (size_t)1;
-                    block_size += str_size[(2 * i) + 1];
-
-                    /* Source selection */
-                    if((select_serial_size = H5S_SELECT_SERIAL_SIZE(mesg->storage.u.virt.list[i].source_select, f)) < 0)
-                        HGOTO_ERROR(H5E_OHDR, H5E_CANTENCODE, FAIL, "unable to check dataspace selection size")
-                    block_size += (size_t)select_serial_size;
-
-                    /* Virtual dataset selection */
-                    if((select_serial_size = H5S_SELECT_SERIAL_SIZE(mesg->storage.u.virt.list[i].source_dset.virtual_select, f)) < 0)
-                        HGOTO_ERROR(H5E_OHDR, H5E_CANTENCODE, FAIL, "unable to check dataspace selection size")
-                    block_size += (size_t)select_serial_size;
-                } /* end for */
-
-                /* Checksum */
-                block_size += 4;
-
-                /* Allocate heap block */
-                if(NULL == (heap_block = (uint8_t *)H5MM_malloc(block_size)))
-                    HGOTO_ERROR(H5E_OHDR, H5E_RESOURCE, FAIL, "unable to allocate heap block")
-
-                /*
-                 * Encode heap block
-                 */
-                heap_block_p = heap_block;
-
-                /* Encode heap block encoding version */
-                *heap_block_p++ = (uint8_t)H5O_LAYOUT_VDS_GH_ENC_VERS;
-
-                /* Number of entries */
-                tmp_hsize = (hsize_t)mesg->storage.u.virt.list_nused;
-                H5F_ENCODE_LENGTH(f, heap_block_p, tmp_hsize)
-
-                /* Encode each entry */
-                for(i = 0; i < mesg->storage.u.virt.list_nused; i++) {
-                    /* Source file name */
-                    (void)HDmemcpy((char *)heap_block_p, mesg->storage.u.virt.list[i].source_file_name, str_size[2 * i]);
-                    heap_block_p += str_size[2 * i];
+            /* Heap block should have been created already! */
+            HDassert((mesg->storage.u.virt.serial_list_hobjid.addr != HADDR_UNDEF)
+		     || (mesg->storage.u.virt.list_nused == 0));
 
-                    /* Source dataset name */
-                    (void)HDmemcpy((char *)heap_block_p, mesg->storage.u.virt.list[i].source_dset_name, str_size[(2 * i) + 1]);
-                    heap_block_p += str_size[(2 * i) + 1];
-
-                    /* Source selection */
-                    if(H5S_SELECT_SERIALIZE(mesg->storage.u.virt.list[i].source_select, &heap_block_p, f) < 0)
-                        HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to serialize source selection")
-
-                    /* Virtual selection */
-                    if(H5S_SELECT_SERIALIZE(mesg->storage.u.virt.list[i].source_dset.virtual_select, &heap_block_p, f) < 0)
-                        HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to serialize virtual selection")
-                } /* end for */
-
-                /* Checksum */
-                chksum = H5_checksum_metadata(heap_block, block_size - (size_t)4, 0);
-                UINT32ENCODE(heap_block_p, chksum)
-
-                /* Insert block into global heap */
-                if(H5HG_insert(f, H5AC_ind_read_dxpl_id, block_size, heap_block, &((H5O_layout_t *)mesg)->storage.u.virt.serial_list_hobjid) < 0) /* Casting away const OK  --NAF */
-                    HGOTO_ERROR(H5E_OHDR, H5E_CANTINSERT, FAIL, "unable to insert virtual dataset heap block")
-            } /* end if */
-
-            /* Heap information */
+            /* Heap information
+             * During H5Fclose, the cache flushing can call H5O_msg_flush calls
+	     * which we need to handle here...
+             */
             H5F_addr_encode(f, &p, mesg->storage.u.virt.serial_list_hobjid.addr);
             UINT32ENCODE(p, mesg->storage.u.virt.serial_list_hobjid.idx);