Switch H5FD interface and all VFDs to us (H5OPEN <id>) idiom

Move all VFDs into the 'H5FD' package, instead of their own individual packages. Stop using VFD's 'term' callback to reset the ID for the VFD. Use explicit 'register' and 'unregister' calls to do that. Use the 'term' callback only for dealing with VFD internal resource cleanup. A _ton_ of other misc. changes, but mainly to update and align the VFDs more with current best practices in the library. Signed-off-by: Quincey Koziol <[email protected]>
qkoziol · Oct 2, 2024 · 0abf90a · 0abf90a
1 parent edb4937
commit 0abf90a
Show file tree

Hide file tree

Showing 55 changed files with 1,519 additions and 1,356 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -236,6 +236,7 @@ set (H5FD_SOURCES
     ${HDF5_SRC_DIR}/H5FDmpi.c
     ${HDF5_SRC_DIR}/H5FDmpio.c
     ${HDF5_SRC_DIR}/H5FDmulti.c
+    ${HDF5_SRC_DIR}/H5FDmulti_int.c
     ${HDF5_SRC_DIR}/H5FDonion.c
     ${HDF5_SRC_DIR}/H5FDonion_header.c
     ${HDF5_SRC_DIR}/H5FDonion_history.c
@@ -246,6 +247,7 @@ set (H5FD_SOURCES
     ${HDF5_SRC_DIR}/H5FDspace.c
     ${HDF5_SRC_DIR}/H5FDsplitter.c
     ${HDF5_SRC_DIR}/H5FDstdio.c
+    ${HDF5_SRC_DIR}/H5FDstdio_int.c
     ${HDF5_SRC_DIR}/H5FDtest.c
     ${HDF5_SRC_DIR}/H5FDwindows.c
 )

diff --git a/src/H5.c b/src/H5.c
@@ -235,7 +235,7 @@ H5_init_library(void)
      * Also initialize some interfaces that might not be able to initialize
      * themselves soon enough.
      *
-     * Interfaces returning variables through a macro: H5E, H5O, H5P, H5T
+     * Interfaces returning variables through a macro: H5E, H5FD, H5O, H5P, H5T
      *
      * The link interface needs to be initialized so that the external link
      *   class is registered.
@@ -248,12 +248,15 @@ H5_init_library(void)
      * The dataspace interface needs to be initialized so that future IDs for
      *   dataspaces work.
      *
-     * The VOL interface needs to be initialized so that the default VOL
-     *   connector and the VOL-managed interfaces are set up.
+     * The VFD & VOL interfaces need to be initialized before the H5P interface
+     *   so that the default VFD and default VOL connector are ready for the
+     *   default FAPL.
      *
      */
     if (H5E_init() < 0)
         HGOTO_ERROR(H5E_FUNC, H5E_CANTINIT, FAIL, "unable to initialize error interface");
+    if (H5FD_init() < 0)
+        HGOTO_ERROR(H5E_FUNC, H5E_CANTINIT, FAIL, "unable to initialize VFL interface");
     if (H5VL_init_phase1() < 0)
         HGOTO_ERROR(H5E_FUNC, H5E_CANTINIT, FAIL, "unable to initialize vol interface");
     if (H5P_init_phase1() < 0)

diff --git a/src/H5FD.c b/src/H5FD.c
@@ -62,6 +62,9 @@ static herr_t H5FD__query(const H5FD_t *f, unsigned long *flags /*out*/);
 /* Package initialization variable */
 bool H5_PKG_INIT_VAR = false;
 
+/* Whether to ignore file locks when disabled (env var value) */
+htri_t H5FD_ignore_disabled_file_locks_p = FAIL;
+
 /*****************************/
 /* Library Private Variables */
 /*****************************/
@@ -92,6 +95,28 @@ static const H5I_class_t H5I_VFL_CLS[1] = {{
     (H5I_free_t)H5FD__free_cls /* Callback routine for closing objects of this class */
 }};
 
+/*-------------------------------------------------------------------------
+ * Function:    H5FD_init
+ *
+ * Purpose:     Initialize the interface from some other package.
+ *
+ * Return:      Success:	non-negative
+ *              Failure:	negative
+ *
+ *-------------------------------------------------------------------------
+ */
+herr_t
+H5FD_init(void)
+{
+    herr_t ret_value = SUCCEED; /* Return value */
+
+    FUNC_ENTER_NOAPI(FAIL)
+    /* FUNC_ENTER() does all the work */
+
+done:
+    FUNC_LEAVE_NOAPI(ret_value)
+} /* end H5FD_init() */
+
 /*-------------------------------------------------------------------------
  * Function:    H5FD__init_package
  *
@@ -104,6 +129,7 @@ static const H5I_class_t H5I_VFL_CLS[1] = {{
 herr_t
 H5FD__init_package(void)
 {
+    char  *lock_env_var = NULL; /* Environment variable pointer */
     herr_t ret_value = SUCCEED; /* Return value */
 
     FUNC_ENTER_PACKAGE
@@ -114,6 +140,61 @@ H5FD__init_package(void)
     /* Reset the file serial numbers */
     H5FD_file_serial_no_g = 0;
 
+    /* Check the use disabled file locks environment variable */
+    lock_env_var = getenv(HDF5_USE_FILE_LOCKING);
+    if (lock_env_var && !strcmp(lock_env_var, "BEST_EFFORT"))
+        H5FD_ignore_disabled_file_locks_p = true; /* Override: Ignore disabled locks */
+    else if (lock_env_var && (!strcmp(lock_env_var, "TRUE") || !strcmp(lock_env_var, "1")))
+        H5FD_ignore_disabled_file_locks_p = false; /* Override: Don't ignore disabled locks */
+    else
+        H5FD_ignore_disabled_file_locks_p = FAIL; /* Environment variable not set, or not set correctly */
+
+    /* Initialize all internal VFD drivers, so their driver IDs are set up */
+    if (H5FD__core_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register core VFD");
+#ifdef H5_HAVE_DIRECT
+    if (H5FD__direct_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register direct VFD");
+#endif
+    if (H5FD__family_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register family VFD");
+#ifdef H5_HAVE_LIBHDFS
+    if (H5FD__hdfs_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register hdfs VFD");
+#endif
+#ifdef H5_HAVE_IOC_VFD
+    if (H5FD__ioc_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register ioc VFD");
+#endif
+    if (H5FD__log_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register log VFD");
+#ifdef H5_HAVE_MIRROR_VFD
+    if (H5FD__mirror_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register mirror VFD");
+#endif
+#ifdef H5_HAVE_PARALLEL
+    if (H5FD__mpio_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register mpio VFD");
+#endif
+    if (H5FD__multi_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register multi VFD");
+    if (H5FD__onion_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register onion VFD");
+#ifdef H5_HAVE_ROS3_VFD
+    if (H5FD__ros3_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register ros3 VFD");
+#endif
+    if (H5FD__sec2_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register sec2 VFD");
+    if (H5FD__splitter_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register splitter VFD");
+    if (H5FD__stdio_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register stdio VFD");
+#ifdef H5_HAVE_SUBFILING_VFD
+    if (H5FD__subfiling_register() < 0)
+        HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register subfiling VFD");
+#endif
+
 done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5FD__init_package() */
@@ -143,6 +224,38 @@ H5FD_term_package(void)
     if (H5_PKG_INIT_VAR) {
         if (H5I_nmembers(H5I_VFL) > 0) {
             (void)H5I_clear_type(H5I_VFL, false, false);
+
+            /* Reset all internal VFD driver IDs */
+            H5FD__core_unregister();
+#ifdef H5_HAVE_DIRECT
+            H5FD__direct_unregister();
+#endif
+            H5FD__family_unregister();
+#ifdef H5_HAVE_LIBHDFS
+            H5FD__hdfs_unregister();
+#endif
+#ifdef H5_HAVE_IOC_VFD
+            H5FD__ioc_unregister();
+#endif
+            H5FD__log_unregister();
+#ifdef H5_HAVE_MIRROR_VFD
+            H5FD__mirror_unregister();
+#endif
+#ifdef H5_HAVE_PARALLEL
+            H5FD__mpio_unregister();
+#endif
+            H5FD__multi_unregister();
+            H5FD__onion_unregister();
+#ifdef H5_HAVE_ROS3_VFD
+            H5FD__ros3_unregister();
+#endif
+            H5FD__sec2_unregister();
+            H5FD__splitter_unregister();
+            H5FD__stdio_unregister();
+#ifdef H5_HAVE_SUBFILING_VFD
+            H5FD__subfiling_unregister();
+#endif
+
             n++; /*H5I*/
         }        /* end if */
         else {

diff --git a/src/H5FDcore.c b/src/H5FDcore.c
@@ -16,24 +16,21 @@
  *              access to small, temporary hdf5 files.
  */
 
-#include "H5FDdrvr_module.h" /* This source code file is part of the H5FD driver module */
+#include "H5FDmodule.h" /* This source code file is part of the H5FD module */
 
 #include "H5private.h"   /* Generic Functions            */
 #include "H5Eprivate.h"  /* Error handling               */
 #include "H5Fprivate.h"  /* File access                  */
-#include "H5FDprivate.h" /* File drivers                 */
 #include "H5FDcore.h"    /* Core file driver             */
+#include "H5FDpkg.h"     /* File drivers                 */
 #include "H5FLprivate.h" /* Free lists                   */
 #include "H5Iprivate.h"  /* IDs                          */
 #include "H5MMprivate.h" /* Memory management            */
 #include "H5Pprivate.h"  /* Property lists               */
 #include "H5SLprivate.h" /* Skip lists                   */
 
 /* The driver identification number, initialized at runtime */
-static hid_t H5FD_CORE_g = 0;
-
-/* Whether to ignore file locks when disabled (env var value) */
-static htri_t ignore_disabled_file_locks_s = FAIL;
+hid_t H5FD_CORE_id_g = H5I_INVALID_HID;
 
 /* The skip list node type.  Represents a region in the file. */
 typedef struct H5FD_core_region_t {
@@ -126,7 +123,6 @@ typedef struct H5FD_core_fapl_t {
 static herr_t  H5FD__core_add_dirty_region(H5FD_core_t *file, haddr_t start, haddr_t end);
 static herr_t  H5FD__core_destroy_dirty_list(H5FD_core_t *file);
 static herr_t  H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size);
-static herr_t  H5FD__core_term(void);
 static void   *H5FD__core_fapl_get(H5FD_t *_file);
 static H5FD_t *H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr);
 static herr_t  H5FD__core_close(H5FD_t *_file);
@@ -153,7 +149,7 @@ static const H5FD_class_t H5FD_core_g = {
     "core",                   /* name                 */
     MAXADDR,                  /* maxaddr              */
     H5F_CLOSE_WEAK,           /* fc_degree            */
-    H5FD__core_term,          /* terminate            */
+    NULL,                     /* terminate            */
     NULL,                     /* sb_size              */
     NULL,                     /* sb_encode            */
     NULL,                     /* sb_decode            */
@@ -443,85 +439,48 @@ H5FD__core_get_default_config(void)
 } /* end H5FD__core_get_default_config() */
 
 /*-------------------------------------------------------------------------
- * Function:    H5FD__init_package
+ * Function:    H5FD__core_register
  *
- * Purpose:     Initializes any interface-specific data or routines.
+ * Purpose:     Register the driver with the library.
  *
- * Return:      Non-negative on success/Negative on failure
+ * Return:      SUCCEED/FAIL
  *
  *-------------------------------------------------------------------------
  */
-static herr_t
-H5FD__init_package(void)
+herr_t
+H5FD__core_register(void)
 {
-    char  *lock_env_var = NULL; /* Environment variable pointer */
-    herr_t ret_value    = SUCCEED;
+    herr_t ret_value = SUCCEED; /* Return value */
 
     FUNC_ENTER_PACKAGE
 
-    /* Check the use disabled file locks environment variable */
-    lock_env_var = getenv(HDF5_USE_FILE_LOCKING);
-    if (lock_env_var && !strcmp(lock_env_var, "BEST_EFFORT"))
-        ignore_disabled_file_locks_s = true; /* Override: Ignore disabled locks */
-    else if (lock_env_var && (!strcmp(lock_env_var, "TRUE") || !strcmp(lock_env_var, "1")))
-        ignore_disabled_file_locks_s = false; /* Override: Don't ignore disabled locks */
-    else
-        ignore_disabled_file_locks_s = FAIL; /* Environment variable not set, or not set correctly */
-
-    if (H5FD_core_init() < 0)
-        HGOTO_ERROR(H5E_VFL, H5E_CANTINIT, FAIL, "unable to initialize core VFD");
+    if (H5I_VFL != H5I_get_type(H5FD_CORE_id_g))
+        if ((H5FD_CORE_id_g = H5FD_register(&H5FD_core_g, sizeof(H5FD_class_t), false)) < 0)
+            HGOTO_ERROR(H5E_VFL, H5E_CANTREGISTER, FAIL, "unable to register core driver");
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
-} /* H5FD__init_package() */
-
-/*-------------------------------------------------------------------------
- * Function:    H5FD_core_init
- *
- * Purpose:     Initialize this driver by registering the driver with the
- *              library.
- *
- * Return:      Success:    The driver ID for the core driver
- *              Failure:    H5I_INVALID_HID
- *
- *-------------------------------------------------------------------------
- */
-hid_t
-H5FD_core_init(void)
-{
-    hid_t ret_value = H5I_INVALID_HID; /* Return value */
-
-    FUNC_ENTER_NOAPI(H5I_INVALID_HID)
-
-    if (H5I_VFL != H5I_get_type(H5FD_CORE_g))
-        H5FD_CORE_g = H5FD_register(&H5FD_core_g, sizeof(H5FD_class_t), false);
-
-    /* Set return value */
-    ret_value = H5FD_CORE_g;
-
-done:
-    FUNC_LEAVE_NOAPI(ret_value)
-} /* end H5FD_core_init() */
+} /* end H5FD__core_register() */
 
 /*---------------------------------------------------------------------------
- * Function:    H5FD__core_term
+ * Function:    H5FD__core_unregister
  *
- * Purpose:     Shut down the VFD
+ * Purpose:     Reset library driver info.
  *
  * Returns:     SUCCEED (Can't fail)
  *
  *---------------------------------------------------------------------------
  */
-static herr_t
-H5FD__core_term(void)
+herr_t
+H5FD__core_unregister(void)
 {
     FUNC_ENTER_PACKAGE_NOERR
 
     /* Reset VFL ID */
-    H5FD_CORE_g = 0;
+    H5FD_CORE_id_g = H5I_INVALID_HID;
 
     FUNC_LEAVE_NOAPI(SUCCEED)
-} /* end H5FD__core_term() */
+} /* end H5FD__core_unregister() */
 
 /*-------------------------------------------------------------------------
  * Function:    H5Pset_core_write_tracking
@@ -822,9 +781,9 @@ H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr
     file->fi_callbacks = file_image_info.callbacks;
 
     /* Check the file locking flags in the fapl */
-    if (ignore_disabled_file_locks_s != FAIL)
+    if (H5FD_ignore_disabled_file_locks_p != FAIL)
         /* The environment variable was set, so use that preferentially */
-        file->ignore_disabled_file_locks = ignore_disabled_file_locks_s;
+        file->ignore_disabled_file_locks = H5FD_ignore_disabled_file_locks_p;
     else {
         /* Use the value in the property list */
         if (H5P_get(plist, H5F_ACS_IGNORE_DISABLED_FILE_LOCKS_NAME, &file->ignore_disabled_file_locks) < 0)

diff --git a/src/H5FDcore.h b/src/H5FDcore.h
@@ -16,7 +16,21 @@
 #ifndef H5FDcore_H
 #define H5FDcore_H
 
-#define H5FD_CORE       (H5FD_core_init())
+/* Public header files */
+#include "H5FDpublic.h" /* File drivers             */
+
+/* When this header is included from a private header, don't make calls to H5open() */
+#undef H5OPEN
+#ifndef H5private_H
+#define H5OPEN H5open(),
+#else /* H5private_H */
+#define H5OPEN
+#endif /* H5private_H */
+
+/** ID for the core VFD */
+#define H5FD_CORE       (H5OPEN H5FD_CORE_id_g)
+
+/** Identifier for the core VFD */
 #define H5FD_CORE_VALUE H5_VFD_CORE
 
 #ifdef __cplusplus
@@ -25,9 +39,9 @@ extern "C" {
 
 /** @private
  *
- * \brief Private initializer for the core VFD
+ * \brief ID for the core VFD
  */
-H5_DLL hid_t H5FD_core_init(void);
+H5_DLLVAR hid_t H5FD_CORE_id_g;
 
 /**
  * \ingroup FAPL