Persist dataset access properties when using H5Fstart_swmr_write - merge to 1.12 branch (#2031)

* Merge GitHub #1862 (HDFFV-11308) to 1.12 branch

Persist dataset access properties when using H5Fstart_swmr_write (#1862)

* Fix HDFFV-11308 - dataset access properties disappear when using
H5Fstart_swmr_write.

* Committing clang-format changes

* Add test for H5Fstart_swmr_write() persisting DAPL settings.  Fix bugs
in the library exposed by this test.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix bug in swmr.c introduced with recent merge (double_t) (#1913)

* Fix HDFFV-11308 - dataset access properties disappear when using
H5Fstart_swmr_write.

* Committing clang-format changes

* Add test for H5Fstart_swmr_write() persisting DAPL settings.  Fix bugs
in the library exposed by this test.

* Committing clang-format changes

* Replace accidental use of double_t in swmr.c with double.

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Add note to RELEASE.txt for HDFFV-11308 (#2029)

* Add note to RELEASE.txt for HDFFV-11308

* Fix formatting error in RELEASE.txt

* Committing clang-format changes

* Fix inappropriate use of HGOTO_ERROR (#2055)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

4 files changed, 121 insertions, 31 deletions

diff --git a/src/H5Dint.c b/src/H5Dint.c
index d38222a..ee4318a 100644
--- a/src/H5Dint.c
+++ b/src/H5Dint.c
@@ -3742,11 +3742,13 @@ done:
 hid_t
 H5D_get_access_plist(const H5D_t *dset)
 {
-    H5P_genplist_t    *old_plist;                   /* Stored DAPL from dset */
-    H5P_genplist_t    *new_plist;                   /* New DAPL */
-    H5P_genplist_t    *def_fapl;                    /* Default FAPL */
-    H5D_append_flush_t def_append_flush_info = {0}; /* Default append flush property */
-    H5D_rdcc_t         def_chunk_info;              /* Default chunk cache property */
+    H5P_genplist_t    *old_plist;                    /* Stored DAPL from dset */
+    H5P_genplist_t    *new_plist;                    /* New DAPL */
+    H5P_genplist_t    *def_dapl              = NULL; /* Default DAPL */
+    H5D_append_flush_t def_append_flush_info = {0};  /* Default append flush property */
+    H5D_rdcc_t         def_chunk_info;               /* Default chunk cache property */
+    H5D_vds_view_t     def_vds_view;                 /* Default virtual view property */
+    hsize_t            def_vds_gap;                  /* Default virtual printf gap property */
     hid_t              new_dapl_id = FAIL;
     hid_t              ret_value   = FAIL;
 
@@ -3775,23 +3777,23 @@ H5D_get_access_plist(const H5D_t *dset)
     }
     else {
         /* Get the default FAPL */
-        if (NULL == (def_fapl = (H5P_genplist_t *)H5I_object(H5P_LST_FILE_ACCESS_ID_g)))
+        if (NULL == (def_dapl = (H5P_genplist_t *)H5I_object(H5P_LST_DATASET_ACCESS_ID_g)))
             HGOTO_ERROR(H5E_DATASET, H5E_BADTYPE, FAIL, "not a property list")
 
         /* Set the data cache number of slots to the value of the default FAPL */
-        if (H5P_get(def_fapl, H5D_ACS_DATA_CACHE_NUM_SLOTS_NAME, &def_chunk_info.nslots) < 0)
+        if (H5P_get(def_dapl, H5D_ACS_DATA_CACHE_NUM_SLOTS_NAME, &def_chunk_info.nslots) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get data number of slots");
         if (H5P_set(new_plist, H5D_ACS_DATA_CACHE_NUM_SLOTS_NAME, &def_chunk_info.nslots) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set data cache number of slots")
 
         /* Set the data cache byte size to the value of the default FAPL */
-        if (H5P_get(def_fapl, H5D_ACS_DATA_CACHE_BYTE_SIZE_NAME, &def_chunk_info.nbytes_max) < 0)
+        if (H5P_get(def_dapl, H5D_ACS_DATA_CACHE_BYTE_SIZE_NAME, &def_chunk_info.nbytes_max) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get data cache byte size");
         if (H5P_set(new_plist, H5D_ACS_DATA_CACHE_BYTE_SIZE_NAME, &def_chunk_info.nbytes_max) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set data cache byte size")
 
         /* Set the preempt read chunks property to the value of the default FAPL */
-        if (H5P_get(def_fapl, H5D_ACS_PREEMPT_READ_CHUNKS_NAME, &def_chunk_info.w0) < 0)
+        if (H5P_get(def_dapl, H5D_ACS_PREEMPT_READ_CHUNKS_NAME, &def_chunk_info.w0) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get preempt read chunks");
         if (H5P_set(new_plist, H5D_ACS_PREEMPT_READ_CHUNKS_NAME, &def_chunk_info.w0) < 0)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set preempt read chunks")
@@ -3801,12 +3803,32 @@ H5D_get_access_plist(const H5D_t *dset)
             HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set append flush property")
     } /* end if-else */
 
-    /* Set the VDS view & printf gap options */
-    if (H5P_set(new_plist, H5D_ACS_VDS_VIEW_NAME, &(dset->shared->layout.storage.u.virt.view)) < 0)
-        HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set VDS view")
-    if (H5P_set(new_plist, H5D_ACS_VDS_PRINTF_GAP_NAME, &(dset->shared->layout.storage.u.virt.printf_gap)) <
-        0)
-        HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set VDS printf gap")
+    /* If the dataset is virtual then copy the VDS view & printf gap options.
+     * Otherwise, use the default values. */
+    if (dset->shared->layout.type == H5D_VIRTUAL) {
+        if (H5P_set(new_plist, H5D_ACS_VDS_VIEW_NAME, &(dset->shared->layout.storage.u.virt.view)) < 0)
+            HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set VDS view")
+        if (H5P_set(new_plist, H5D_ACS_VDS_PRINTF_GAP_NAME,
+                    &(dset->shared->layout.storage.u.virt.printf_gap)) < 0)
+            HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set VDS printf gap")
+    }
+    else {
+        /* Get the default FAPL if necessary */
+        if (!def_dapl && NULL == (def_dapl = (H5P_genplist_t *)H5I_object(H5P_LST_DATASET_ACCESS_ID_g)))
+            HGOTO_ERROR(H5E_DATASET, H5E_BADTYPE, FAIL, "not a property list")
+
+        /* Set the data cache number of slots to the value of the default FAPL */
+        if (H5P_get(def_dapl, H5D_ACS_VDS_VIEW_NAME, &def_vds_view) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get VDS view");
+        if (H5P_set(new_plist, H5D_ACS_VDS_VIEW_NAME, &def_vds_view) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set VDS view")
+
+        /* Set the data cache byte size to the value of the default FAPL */
+        if (H5P_get(def_dapl, H5D_ACS_VDS_PRINTF_GAP_NAME, &def_vds_gap) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get VDS printf gap");
+        if (H5P_set(new_plist, H5D_ACS_VDS_PRINTF_GAP_NAME, &def_vds_gap) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set VDS printf gap")
+    }
 
     /* Set the vds prefix option */
     if (H5P_set(new_plist, H5D_ACS_VDS_PREFIX_NAME, &(dset->shared->vds_prefix)) < 0)
diff --git a/src/H5Fint.c b/src/H5Fint.c
index 1da8c01..dc86390 100644
--- a/src/H5Fint.c
+++ b/src/H5Fint.c
@@ -3596,6 +3596,7 @@ H5F__start_swmr_write(H5F_t *f)
     size_t      grp_dset_count = 0;      /* # of open objects: groups & datasets */
     size_t      nt_attr_count  = 0;      /* # of opened named datatypes  + opened attributes */
     hid_t      *obj_ids        = NULL;   /* List of ids */
+    hid_t      *obj_apl_ids    = NULL;   /* List of access property lists */
     H5G_loc_t  *obj_glocs      = NULL;   /* Group location of the object */
     H5O_loc_t  *obj_olocs      = NULL;   /* Object location */
     H5G_name_t *obj_paths      = NULL;   /* Group hierarchy path */
@@ -3654,13 +3655,20 @@ H5F__start_swmr_write(H5F_t *f)
     if (grp_dset_count > 0) {
         /* 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_NOSPACE, FAIL, "can't allocate buffer for hid_t")
+            HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t")
         if ((obj_glocs = (H5G_loc_t *)H5MM_malloc(grp_dset_count * sizeof(H5G_loc_t))) == NULL)
-            HGOTO_ERROR(H5E_FILE, H5E_NOSPACE, FAIL, "can't allocate buffer for H5G_loc_t")
+            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)
-            HGOTO_ERROR(H5E_FILE, H5E_NOSPACE, FAIL, "can't allocate buffer for H5O_loc_t")
+            HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for object locations")
         if ((obj_paths = (H5G_name_t *)H5MM_malloc(grp_dset_count * sizeof(H5G_name_t))) == NULL)
-            HGOTO_ERROR(H5E_FILE, H5E_NOSPACE, FAIL, "can't allocate buffer for H5G_name_t")
+            HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for object paths")
+
+        /* Taking a shortcut here to use calloc to initialize obj_apl_ids to all H5P_DEFAULT.  If
+         * this changes in the future we'll need to either initialize this array to all H5P_DEFAULT
+         * or ensure 0 cannot be a valid value and check for 0 at cleanup. */
+        if ((obj_apl_ids = (hid_t *)H5MM_calloc(grp_dset_count * sizeof(hid_t))) == NULL)
+            HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, FAIL, "can't allocate buffer for hid_t")
+        HDassert(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,
@@ -3682,20 +3690,67 @@ H5F__start_swmr_write(H5F_t *f)
         /* Gather information about opened objects (groups, datasets) in the file */
         /* (For refresh later on) */
         for (u = 0; u < grp_dset_count; u++) {
-            H5O_loc_t *oloc; /* object location */
+            void      *obj = NULL; /* VOL object   */
+            H5I_type_t type;       /* Type of object for the ID */
             H5G_loc_t  tmp_loc;
 
+            /* Get object's type */
+            type = H5I_get_type(obj_ids[u]);
+
+            /* Get the object from the VOL */
+            if (NULL == (obj = H5VL_object(obj_ids[u])))
+                HGOTO_ERROR(H5E_DATASET, H5E_BADTYPE, FAIL, "invalid location identifier")
+
+            /* Get the object's access property list, if it is a dataset (access
+             * properties are not needed to reopen other object types currently)
+             */
+            switch (type) {
+                case H5I_GROUP:
+                    /* Access properties not needed currently */
+                    break;
+
+                case H5I_DATATYPE:
+                    /* Access properties not needed currently */
+                    break;
+
+                case H5I_DATASET:
+
+                    /* Get dataset access properties */
+                    if ((obj_apl_ids[u] = H5D_get_access_plist(obj)) < 0)
+                        HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL,
+                                    "unable to get dataset access property list")
+                    break;
+
+                case H5I_MAP:
+                    HGOTO_ERROR(H5E_FILE, H5E_BADTYPE, FAIL, "maps not supported in native VOL connector")
+
+                case H5I_UNINIT:
+                case H5I_BADID:
+                case H5I_FILE:
+                case H5I_DATASPACE:
+                case H5I_ATTR:
+                case H5I_VFL:
+                case H5I_VOL:
+                case H5I_GENPROP_CLS:
+                case H5I_GENPROP_LST:
+                case H5I_ERROR_CLASS:
+                case H5I_ERROR_MSG:
+                case H5I_ERROR_STACK:
+                case H5I_SPACE_SEL_ITER:
+                case H5I_NTYPES:
+                default:
+                    HGOTO_ERROR(H5E_FILE, H5E_BADTYPE, FAIL,
+                                "not a valid file object ID (dataset, group, or datatype)")
+                    break;
+            } /* end switch */
+
             /* Set up the id's group location */
             obj_glocs[u].oloc = &obj_olocs[u];
             obj_glocs[u].path = &obj_paths[u];
             H5G_loc_reset(&obj_glocs[u]);
 
-            /* get the id's object location */
-            if ((oloc = H5O_get_loc(obj_ids[u])) == NULL)
-                HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an object")
-
             /* Make deep local copy of object's location information */
-            H5G_loc(obj_ids[u], &tmp_loc);
+            H5G_loc_real(obj, type, &tmp_loc);
             H5G_loc_copy(&obj_glocs[u], &tmp_loc, H5_COPY_DEEP);
 
             /* Close the object */
@@ -3755,7 +3810,7 @@ H5F__start_swmr_write(H5F_t *f)
 
     /* Refresh (reopen) the objects (groups & datasets) in the file */
     for (u = 0; u < grp_dset_count; u++)
-        if (H5O_refresh_metadata_reopen(obj_ids[u], &obj_glocs[u], vol_connector, TRUE) < 0)
+        if (H5O_refresh_metadata_reopen(obj_ids[u], obj_apl_ids[u], &obj_glocs[u], vol_connector, TRUE) < 0)
             HGOTO_ERROR(H5E_ATOM, H5E_CLOSEERROR, FAIL, "can't refresh-close object")
 
     /* Unlock the file */
@@ -3764,7 +3819,6 @@ H5F__start_swmr_write(H5F_t *f)
 
 done:
     if (ret_value < 0 && setup) {
-
         /* Re-enable accumulator */
         f->shared->feature_flags |= (unsigned)H5FD_FEAT_ACCUMULATE_METADATA;
         if (H5FD_set_feature_flags(f->shared->lf, f->shared->feature_flags) < 0)
@@ -3800,6 +3854,14 @@ done:
     if (obj_paths)
         H5MM_xfree(obj_paths);
 
+    /* Free access property lists */
+    if (obj_apl_ids) {
+        for (u = 0; u < grp_dset_count; u++)
+            if (obj_apl_ids[u] != H5P_DEFAULT && obj_apl_ids[u] >= 0 && H5I_dec_ref(obj_apl_ids[u]) < 0)
+                HDONE_ERROR(H5E_ATOM, H5E_CANTDEC, FAIL, "decrementing property list ID failed")
+        H5MM_xfree(obj_apl_ids);
+    }
+
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5F__start_swmr_write() */
 
diff --git a/src/H5Oflush.c b/src/H5Oflush.c
index a622bd1..e02a084 100644
--- a/src/H5Oflush.c
+++ b/src/H5Oflush.c
@@ -334,7 +334,7 @@ H5O_refresh_metadata(hid_t oid, H5O_loc_t oloc)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "unable to refresh object")
 
         /* Re-open the object, re-fetching its metadata */
-        if ((H5O_refresh_metadata_reopen(oid, &obj_loc, connector, FALSE)) < 0)
+        if ((H5O_refresh_metadata_reopen(oid, H5P_DEFAULT, &obj_loc, connector, FALSE)) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "unable to refresh object")
 
         /* Restore the number of references on the VOL connector */
@@ -439,7 +439,8 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5O_refresh_metadata_reopen(hid_t oid, H5G_loc_t *obj_loc, H5VL_t *vol_connector, hbool_t start_swmr)
+H5O_refresh_metadata_reopen(hid_t oid, hid_t apl_id, H5G_loc_t *obj_loc, H5VL_t *vol_connector,
+                            hbool_t start_swmr)
 {
     void      *object = NULL;       /* Object for this operation */
     H5I_type_t type;                /* Type of object for the ID */
@@ -468,8 +469,13 @@ H5O_refresh_metadata_reopen(hid_t oid, H5G_loc_t *obj_loc, H5VL_t *vol_connector
             break;
 
         case H5I_DATASET:
+            /* Set dataset access property list in API context if appropriate */
+            if (H5CX_set_apl(&apl_id, H5P_CLS_DACC, oid, TRUE) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "can't set access property list info")
+
             /* Re-open the dataset */
-            if (NULL == (object = H5D_open(obj_loc, H5P_DATASET_ACCESS_DEFAULT)))
+            if (NULL ==
+                (object = H5D_open(obj_loc, apl_id == H5P_DEFAULT ? H5P_DATASET_ACCESS_DEFAULT : apl_id)))
                 HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENOBJ, FAIL, "unable to open dataset")
             if (!start_swmr) /* No need to handle multiple opens when H5Fstart_swmr_write() */
                 if (H5D_mult_refresh_reopen((H5D_t *)object) < 0)
diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h
index 44f383a..2f1e431 100644
--- a/src/H5Oprivate.h
+++ b/src/H5Oprivate.h
@@ -982,7 +982,7 @@ H5_DLL herr_t H5O_msg_get_flags(const H5O_loc_t *loc, unsigned type_id, uint8_t
 H5_DLL herr_t H5O_flush(H5O_loc_t *oloc, hid_t obj_id);
 H5_DLL herr_t H5O_flush_common(H5O_loc_t *oloc, hid_t obj_id);
 H5_DLL herr_t H5O_refresh_metadata(hid_t oid, H5O_loc_t oloc);
-H5_DLL herr_t H5O_refresh_metadata_reopen(hid_t oid, H5G_loc_t *obj_loc, H5VL_t *vol_driver,
+H5_DLL herr_t H5O_refresh_metadata_reopen(hid_t oid, hid_t apl_id, H5G_loc_t *obj_loc, H5VL_t *vol_driver,
                                           hbool_t start_swmr);
 
 /* Cache corking functions */