Fix for issue #4849 that settings in fapl libver bounds causes unexpe… (

#4939) * Fix for issue #4849 that settings in fapl libver bounds causes unexpected H5Fopen failures. File with non-SWMR-write access can now be opened without regard for superblock version. Due to the fix, H5Fstart_swmr_write() also needs to be modified as well as the tests for libver bounds. The "RFC: Setting Bounds for Object Creation in HDF5 1.10.0" is also updated to reflect the changes. * Fix c++ libver bound test failure.
HDFGroup · Oct 11, 2024 · 9df4c0d · 9df4c0d
1 parent 6e8c7a9
commit 9df4c0d
Show file tree

Hide file tree

Showing 8 changed files with 261 additions and 273 deletions.
diff --git a/c++/test/tfile.cpp b/c++/test/tfile.cpp
@@ -759,7 +759,7 @@ test_libver_bounds()
 
     /* Run the tests */
     test_libver_bounds_real(H5F_LIBVER_EARLIEST, H5O_VERSION_1, H5F_LIBVER_LATEST, H5O_VERSION_2);
-    test_libver_bounds_real(H5F_LIBVER_LATEST, H5O_VERSION_2, H5F_LIBVER_EARLIEST, H5O_VERSION_2);
+    test_libver_bounds_real(H5F_LIBVER_LATEST, H5O_VERSION_2, H5F_LIBVER_EARLIEST, H5O_VERSION_1);
     PASSED();
 } /* end test_libver_bounds() */
 

diff --git a/src/H5Fint.c b/src/H5Fint.c
@@ -3181,7 +3181,8 @@ H5F__set_libver_bounds(H5F_t *f, H5F_libver_t low, H5F_libver_t high)
     assert(f->shared);
 
     /* Set the bounds only if the existing setting is different from the inputs */
-    if (f->shared->low_bound != low || f->shared->high_bound != high) {
+    if ((f->shared->low_bound != low || f->shared->high_bound != high) &&
+        !(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)) {
         /* Call the flush routine, for this file */
         /* Note: This is done in case the binary format for representing a
          *      metadata entry class changes when the file format low / high
@@ -3704,6 +3705,7 @@ H5F_get_metadata_read_retry_info(H5F_t *file, H5F_retry_info_t *info)
  *                  --only allow datasets and groups without attributes
  *                  --disallow named datatype with/without attributes
  *                  --disallow opened attributes attached to objects
+ *                  --disallow opened objects below 1.10
  *
  * NOTE:        Currently, only opened groups and datasets are allowed
  *              when enabling SWMR via H5Fstart_swmr_write().
@@ -3746,7 +3748,7 @@ H5F__start_swmr_write(H5F_t *f)
     if (f->shared->sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "file superblock version - should be at least 3");
 
-    /* Check for correct file format version */
+    /* Check for correct file format version to start SWMR writing */
     if ((f->shared->low_bound < H5F_LIBVER_V110) || (f->shared->high_bound < H5F_LIBVER_V110))
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL,
                     "file format version does not support SWMR - needs to be 1.10 or greater");
@@ -3783,6 +3785,31 @@ H5F__start_swmr_write(H5F_t *f)
         /* Allocate space for group and object locations */
         if ((obj_ids = (hid_t *)H5MM_malloc(grp_dset_count * sizeof(hid_t))) == NULL)
             HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t");
+
+        /* Get the list of opened object ids (groups & datasets) */
+        if (H5F_get_obj_ids(f, H5F_OBJ_GROUP | H5F_OBJ_DATASET, grp_dset_count, obj_ids, false,
+                            &grp_dset_count) < 0)
+            HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5F_get_obj_ids failed");
+
+        /* Ensure that there's no old-style opened objects */
+        for (u = 0; u < grp_dset_count; u++) {
+            H5O_native_info_t ninfo;
+            H5O_loc_t        *oloc;
+            uint8_t           version;
+
+            if (NULL == (oloc = H5O_get_loc(obj_ids[u])))
+                HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_loc() failed");
+
+            if (H5O_get_native_info(oloc, &ninfo, H5O_NATIVE_INFO_HDR) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_native_info() failed");
+
+            if (H5O_get_version_bound(f->shared->low_bound, &version) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5O_get_version_bound() failed");
+
+            if (ninfo.hdr.version < version)
+                HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "disallow opened objects below 1.10");
+        }
+
         if ((obj_glocs = (H5G_loc_t *)H5MM_malloc(grp_dset_count * sizeof(H5G_loc_t))) == NULL)
             HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for object group locations");
         if ((obj_olocs = (H5O_loc_t *)H5MM_malloc(grp_dset_count * sizeof(H5O_loc_t))) == NULL)
@@ -3797,11 +3824,6 @@ H5F__start_swmr_write(H5F_t *f)
             HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t");
         assert(obj_apl_ids[0] == H5P_DEFAULT);
 
-        /* Get the list of opened object ids (groups & datasets) */
-        if (H5F_get_obj_ids(f, H5F_OBJ_GROUP | H5F_OBJ_DATASET, grp_dset_count, obj_ids, false,
-                            &grp_dset_count) < 0)
-            HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "H5F_get_obj_ids failed");
-
         /* Save the VOL connector and the object wrapping context for the refresh step */
         if (grp_dset_count > 0) {
             H5VL_object_t *vol_obj;

diff --git a/src/H5Fsuper.c b/src/H5Fsuper.c
@@ -432,45 +432,27 @@ H5F__super_read(H5F_t *f, H5P_genplist_t *fa_plist, bool initial_read)
         HGOTO_ERROR(H5E_FILE, H5E_CANTPROTECT, FAIL, "unable to load superblock");
 
     /*
-     * When opening a file with SWMR-write access, the library will first
-     * check to ensure that superblock version 3 is used.  Otherwise fail
-     * file open.
-     *
-     * Then the library will upgrade the file's low_bound depending on
-     * superblock version as follows:
-     *      --version 0 or 1: no change to low_bound
-     *      --version 2: upgrade low_bound to at least V18
-     *      --version 3: upgrade low_bound to at least V110
+     * When opening a file with SWMR-write access, the library will check to
+     * ensure that:
+     *      --superblock version 3 is used
+     *      --superblock version does not exceed the version allowed by high bound
+     *      --upgrade low_bound to at least V110
+     * Otherwise fail file open for SMWR-write access
      *
      * Upgrading low_bound will give the best format versions available for
      * that superblock version.  Due to the possible upgrade, the fapl returned
      * from H5Fget_access_plist() might indicate a low_bound higher than what
      * the user originally set.
      *
-     * After upgrading low_bound, the library will check to ensure that the
-     * superblock version does not exceed the version allowed by high_bound.
-     * Otherwise fail file open.
-     *
      * For details, please see RFC:Setting Bounds for Object Creation in HDF5 1.10.0.
      */
-
-    /* Check to ensure that superblock version 3 is used for SWMR-write access */
     if (H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) {
         if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version for SWMR is less than 3");
-    }
-
-    /* Upgrade low_bound to at least V18 when encountering version 2 superblock */
-    if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_2)
-        f->shared->low_bound = MAX(H5F_LIBVER_V18, f->shared->low_bound);
-
-    /* Upgrade low_bound to at least V110 when encountering version 3 superblock */
-    if (sblock->super_vers >= HDF5_SUPERBLOCK_VERSION_3)
+        if (sblock->super_vers > HDF5_superblock_ver_bounds[f->shared->high_bound])
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version exceeds high bound");
         f->shared->low_bound = MAX(H5F_LIBVER_V110, f->shared->low_bound);
-
-    /* Version bounds check */
-    if (sblock->super_vers > HDF5_superblock_ver_bounds[f->shared->high_bound])
-        HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "superblock version exceeds high bound");
+    }
 
     /* Pin the superblock in the cache */
     if (H5AC_pin_protected_entry(sblock) < 0)

diff --git a/src/H5Oint.c b/src/H5Oint.c
@@ -2959,3 +2959,20 @@ H5O_has_chksum(const H5O_t *oh)
 
     FUNC_LEAVE_NOAPI(H5O_SIZEOF_CHKSUM_OH(oh) > 0)
 } /* end H5O_has_chksum() */
+
+/*-------------------------------------------------------------------------
+ * Function:    H5O_get_version_bound
+ *
+ * Purpose:     Retrieve the version for a given bound from object version array
+ *
+ *-------------------------------------------------------------------------
+ */
+herr_t
+H5O_get_version_bound(H5F_libver_t bound, uint8_t *version)
+{
+    FUNC_ENTER_NOAPI_NOINIT_NOERR
+
+    *version = (uint8_t)H5O_obj_ver_bounds[bound];
+
+    FUNC_LEAVE_NOAPI(SUCCEED)
+} /* end H5O_get_version_bound() */
diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h
@@ -933,6 +933,7 @@ H5_DLL uint8_t H5O_get_oh_version(const H5O_t *oh);
 H5_DLL herr_t  H5O_get_rc_and_type(const H5O_loc_t *oloc, unsigned *rc, H5O_type_t *otype);
 H5_DLL H5AC_proxy_entry_t *H5O_get_proxy(const H5O_t *oh);
 H5_DLL bool                H5O_has_chksum(const H5O_t *oh);
+H5_DLL herr_t              H5O_get_version_bound(H5F_libver_t bound, uint8_t *version);
 
 /* Object header message routines */
 H5_DLL herr_t H5O_msg_create(const H5O_loc_t *loc, unsigned type_id, unsigned mesg_flags,