From ebe9a3966fcbd3e154d43dd31ba9b219c575e781 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Thu, 10 Dec 2020 16:30:07 -0600 Subject: Merge VOL framework versioning to 1.12 (#154) * Enforce VOL framework version compatibility when registering connectors. Also add a version for the connector itself, some refactoring on the register calls, and move the logic for matching / rejecting a VOL connector class from the plugin module to the VOL module. (#151) * Revise VOL framework version compatibility for the 1.12 release branch --- release_docs/RELEASE.txt | 8 ++++++ src/H5PLint.c | 29 ++++++------------- src/H5VL.c | 5 +++- src/H5VLconnector.h | 2 +- src/H5VLint.c | 72 ++++++++++++++++++++++++++++++++++++++++------- src/H5VLnative.c | 16 +++++++++-- src/H5VLpassthru.c | 2 +- src/H5VLpkg.h | 3 +- src/H5VLprivate.h | 5 ++-- src/H5VLpublic.h | 11 ++++++++ test/h5test.c | 1 + test/null_vol_connector.c | 2 +- test/vol.c | 21 ++++++++++++-- 13 files changed, 135 insertions(+), 42 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 3617de6..56ea804 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -161,6 +161,14 @@ New Features Library: -------- + - Added H5VL_VERSION macro that indicates the version of the VOL framework + implemented by a version of the library. Currently, compatibility + checking enforces that the 'version' field in the H5VL_class_t for + a VOL connector must match the version of the VOL framework for the + library when it is registered or dynamically loaded. + + (QAK - 2020/12/10) + - Added two new API routines for tracking library memory use: H5get_alloc_stats() and H5get_free_list_sizes(). diff --git a/src/H5PLint.c b/src/H5PLint.c index 0975a9e..3b9683b 100644 --- a/src/H5PLint.c +++ b/src/H5PLint.c @@ -354,30 +354,19 @@ H5PL__open(const char *path, H5PL_type_t type, const H5PL_key_t *key, hbool_t *s } case H5PL_TYPE_VOL: { - const H5VL_class_t *cls; + const void *cls; /* Get the plugin info */ - if (NULL == (cls = (const H5VL_class_t *)(*get_plugin_info)())) + if (NULL == (cls = (const void *)(*get_plugin_info)())) HGOTO_ERROR(H5E_PLUGIN, H5E_CANTGET, FAIL, "can't get VOL connector info from plugin") - /* Which kind of key are we looking for? */ - if (key->vol.kind == H5VL_GET_CONNECTOR_BY_NAME) { - /* If the plugin names match, we're done. Set the output parameters. */ - if (cls->name && !HDstrcmp(cls->name, key->vol.u.name)) { - *plugin_info = (const void *)cls; - *success = TRUE; - } /* end if */ - } /* end if */ - else { - /* Sanity check */ - HDassert(key->vol.kind == H5VL_GET_CONNECTOR_BY_VALUE); - - /* If the plugin values match, we're done. Set the output parameters. */ - if (cls->value == key->vol.u.value) { - *plugin_info = (const void *)cls; - *success = TRUE; - } /* end if */ - } /* end else */ + /* Ask VOL interface if this class is the one we are looking for and is compatible, etc */ + if (H5VL_check_plugin_load(cls, key, success) < 0) + HGOTO_ERROR(H5E_PLUGIN, H5E_CANTLOAD, FAIL, "VOL connector compatibility check failed") + + /* Check for finding the correct plugin */ + if (*success) + *plugin_info = cls; break; } diff --git a/src/H5VL.c b/src/H5VL.c index d60bd5f..92b1e4c 100644 --- a/src/H5VL.c +++ b/src/H5VL.c @@ -91,6 +91,9 @@ H5VLregister_connector(const H5VL_class_t *cls, hid_t vipl_id) if (!cls) HGOTO_ERROR(H5E_ARGS, H5E_UNINITIALIZED, H5I_INVALID_HID, "VOL connector class pointer cannot be NULL") + if (H5VL_VERSION != cls->version) + HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, + "VOL connector has incompatible version") if (!cls->name) HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, "VOL connector class name cannot be the NULL pointer") @@ -113,7 +116,7 @@ H5VLregister_connector(const H5VL_class_t *cls, hid_t vipl_id) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, H5I_INVALID_HID, "not a VOL initialize property list") /* Register connector */ - if ((ret_value = H5VL__register_connector(cls, TRUE, vipl_id)) < 0) + if ((ret_value = H5VL__register_connector_by_class(cls, TRUE, vipl_id)) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register VOL connector") done: diff --git a/src/H5VLconnector.h b/src/H5VLconnector.h index 2abdca3..022bdec 100644 --- a/src/H5VLconnector.h +++ b/src/H5VLconnector.h @@ -445,7 +445,7 @@ typedef struct H5VL_token_class_t { //! [H5VL_class_t_snip] typedef struct H5VL_class_t { /* Overall connector fields & callbacks */ - unsigned int version; /**< VOL connector class struct version # */ + unsigned version; /**< VOL connector class struct version # */ H5VL_class_value_t value; /**< Value to identify connector */ const char * name; /**< Connector name (MUST be unique!) */ unsigned cap_flags; /**< Capability flags for connector */ diff --git a/src/H5VLint.c b/src/H5VLint.c index 32aa8fb..aed397b 100644 --- a/src/H5VLint.c +++ b/src/H5VLint.c @@ -1075,7 +1075,7 @@ done: } /* end H5VL_file_is_same() */ /*------------------------------------------------------------------------- - * Function: H5VL_register_connector + * Function: H5VL__register_connector * * Purpose: Registers a new VOL connector as a member of the virtual object * layer class. @@ -1091,13 +1091,13 @@ done: *------------------------------------------------------------------------- */ hid_t -H5VL_register_connector(const void *_cls, hbool_t app_ref, hid_t vipl_id) +H5VL__register_connector(const void *_cls, hbool_t app_ref, hid_t vipl_id) { const H5VL_class_t *cls = (const H5VL_class_t *)_cls; H5VL_class_t * saved = NULL; hid_t ret_value = H5I_INVALID_HID; - FUNC_ENTER_NOAPI(H5I_INVALID_HID) + FUNC_ENTER_PACKAGE /* Check arguments */ HDassert(cls); @@ -1128,10 +1128,10 @@ done: } /* end if */ FUNC_LEAVE_NOAPI(ret_value) -} /* end H5VL_register_connector() */ +} /* end H5VL__register_connector() */ /*------------------------------------------------------------------------- - * Function: H5VL__register_connector + * Function: H5VL__register_connector_by_class * * Purpose: Registers a new VOL connector as a member of the virtual object * layer class. @@ -1148,7 +1148,7 @@ done: *------------------------------------------------------------------------- */ hid_t -H5VL__register_connector(const H5VL_class_t *cls, hbool_t app_ref, hid_t vipl_id) +H5VL__register_connector_by_class(const H5VL_class_t *cls, hbool_t app_ref, hid_t vipl_id) { H5VL_get_connector_ud_t op_data; /* Callback info for connector search */ hid_t ret_value = H5I_INVALID_HID; /* Return value */ @@ -1173,13 +1173,13 @@ H5VL__register_connector(const H5VL_class_t *cls, hbool_t app_ref, hid_t vipl_id } /* end if */ else { /* Create a new class ID */ - if ((ret_value = H5VL_register_connector(cls, app_ref, vipl_id)) < 0) + if ((ret_value = H5VL__register_connector(cls, app_ref, vipl_id)) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register VOL connector") } /* end else */ done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5VL__register_connector() */ +} /* end H5VL__register_connector_by_class() */ /*------------------------------------------------------------------------- * Function: H5VL__register_connector_by_name @@ -1233,7 +1233,7 @@ H5VL__register_connector_by_name(const char *name, hbool_t app_ref, hid_t vipl_i HGOTO_ERROR(H5E_VOL, H5E_CANTINIT, H5I_INVALID_HID, "unable to load VOL connector") /* Register the connector we loaded */ - if ((ret_value = H5VL_register_connector(cls, app_ref, vipl_id)) < 0) + if ((ret_value = H5VL__register_connector(cls, app_ref, vipl_id)) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register VOL connector ID") } /* end else */ @@ -1293,7 +1293,7 @@ H5VL__register_connector_by_value(H5VL_class_value_t value, hbool_t app_ref, hid HGOTO_ERROR(H5E_VOL, H5E_CANTINIT, H5I_INVALID_HID, "unable to load VOL connector") /* Register the connector we loaded */ - if ((ret_value = H5VL_register_connector(cls, app_ref, vipl_id)) < 0) + if ((ret_value = H5VL__register_connector(cls, app_ref, vipl_id)) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register VOL connector ID") } /* end else */ @@ -2341,3 +2341,55 @@ H5VL_wrap_register(H5I_type_t type, void *obj, hbool_t app_ref) done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5VL_wrap_register() */ + +/*------------------------------------------------------------------------- + * Function: H5VL_check_plugin_load + * + * Purpose: Check if a VOL connector matches the search criteria, and + * can be loaded. + * + * Note: Matching the connector's name / value, but the connector + * having an incompatible version is not an error, but means + * that the connector isn't a "match". Setting the SUCCEED + * value to FALSE and not failing for that case allows the + * plugin framework to keep looking for other DLLs that match + * and have a compatible version. + * + * Return: SUCCEED / FAIL + * + *------------------------------------------------------------------------- + */ +herr_t +H5VL_check_plugin_load(const H5VL_class_t *cls, const H5PL_key_t *key, hbool_t *success) +{ + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_NOAPI(FAIL) + + /* Sanity checks */ + HDassert(cls); + HDassert(key); + HDassert(success); + + /* Which kind of key are we looking for? */ + if (key->vol.kind == H5VL_GET_CONNECTOR_BY_NAME) { + /* Check if plugin name matches VOL connector class name */ + if (cls->name && !HDstrcmp(cls->name, key->vol.u.name)) + *success = TRUE; + } /* end if */ + else { + /* Sanity check */ + HDassert(key->vol.kind == H5VL_GET_CONNECTOR_BY_VALUE); + + /* Check if plugin value matches VOL connector class value */ + if (cls->value == key->vol.u.value) + *success = TRUE; + } /* end else */ + + /* Connector is a match, but might not be a compatible version */ + if (*success && cls->version != H5VL_VERSION) + *success = FALSE; + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5VL_check_plugin_load() */ diff --git a/src/H5VLnative.c b/src/H5VLnative.c index 0973e34..ead3beb 100644 --- a/src/H5VLnative.c +++ b/src/H5VLnative.c @@ -15,6 +15,16 @@ * using HDF5 VFDs. */ +/****************/ +/* Module Setup */ +/****************/ + +#define H5VL_FRIEND /* Suppress error about including H5VLpkg */ + +/***********/ +/* Headers */ +/***********/ + #include "H5private.h" /* Generic Functions */ #include "H5Aprivate.h" /* Attributes */ #include "H5Dprivate.h" /* Datasets */ @@ -25,7 +35,7 @@ #include "H5Oprivate.h" /* Object headers */ #include "H5Pprivate.h" /* Property lists */ #include "H5Tprivate.h" /* Datatypes */ -#include "H5VLprivate.h" /* Virtual Object Layer */ +#include "H5VLpkg.h" /* Virtual Object Layer */ #include "H5VLnative_private.h" /* Native VOL connector */ @@ -37,7 +47,7 @@ static herr_t H5VL__native_term(void); /* Native VOL connector class struct */ static const H5VL_class_t H5VL_native_cls_g = { - H5VL_NATIVE_VERSION, /* version */ + H5VL_VERSION, /* VOL class struct version */ H5VL_NATIVE_VALUE, /* value */ H5VL_NATIVE_NAME, /* name */ 0, /* capability flags */ @@ -176,7 +186,7 @@ H5VL_native_register(void) /* Register the native VOL connector, if it isn't already */ if (H5I_INVALID_HID == H5VL_NATIVE_ID_g) if ((H5VL_NATIVE_ID_g = - H5VL_register_connector(&H5VL_native_cls_g, TRUE, H5P_VOL_INITIALIZE_DEFAULT)) < 0) + H5VL__register_connector(&H5VL_native_cls_g, TRUE, H5P_VOL_INITIALIZE_DEFAULT)) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTINSERT, H5I_INVALID_HID, "can't create ID for native VOL connector") /* Set return value */ diff --git a/src/H5VLpassthru.c b/src/H5VLpassthru.c index 4e967d9..9cf8139 100644 --- a/src/H5VLpassthru.c +++ b/src/H5VLpassthru.c @@ -263,7 +263,7 @@ static herr_t H5VL_pass_through_optional(void *obj, int op_type, hid_t dxpl_id, /* Pass through VOL connector class struct */ static const H5VL_class_t H5VL_pass_through_g = { - H5VL_PASSTHRU_VERSION, /* version */ + H5VL_VERSION, /* VOL class struct version */ (H5VL_class_value_t)H5VL_PASSTHRU_VALUE, /* value */ H5VL_PASSTHRU_NAME, /* name */ 0, /* capability flags */ diff --git a/src/H5VLpkg.h b/src/H5VLpkg.h index c784e50..5799d2c 100644 --- a/src/H5VLpkg.h +++ b/src/H5VLpkg.h @@ -43,7 +43,8 @@ /******************************/ /* Package Private Prototypes */ /******************************/ -H5_DLL hid_t H5VL__register_connector(const H5VL_class_t *cls, hbool_t app_ref, hid_t vipl_id); +H5_DLL hid_t H5VL__register_connector(const void *cls, hbool_t app_ref, hid_t vipl_id); +H5_DLL hid_t H5VL__register_connector_by_class(const H5VL_class_t *cls, hbool_t app_ref, hid_t vipl_id); H5_DLL hid_t H5VL__register_connector_by_name(const char *name, hbool_t app_ref, hid_t vipl_id); H5_DLL hid_t H5VL__register_connector_by_value(H5VL_class_value_t value, hbool_t app_ref, hid_t vipl_id); H5_DLL htri_t H5VL__is_connector_registered_by_name(const char *name); diff --git a/src/H5VLprivate.h b/src/H5VLprivate.h index bb4e60f..581a24d 100644 --- a/src/H5VLprivate.h +++ b/src/H5VLprivate.h @@ -14,7 +14,7 @@ #define _H5VLprivate_H /* Include package's public header */ -#include "H5VLpublic.h" /* Generic Functions */ +#include "H5VLpublic.h" /* Generic Functions */ /* Private headers needed by this file */ @@ -67,7 +67,8 @@ H5_DLL herr_t H5VL_conn_copy(H5VL_connector_prop_t *value); H5_DLL herr_t H5VL_conn_free(const H5VL_connector_prop_t *info); /* Functions that deal with VOL connectors */ -H5_DLL hid_t H5VL_register_connector(const void *cls, hbool_t app_ref, hid_t vipl_id); +union H5PL_key_t; +H5_DLL herr_t H5VL_check_plugin_load(const H5VL_class_t *cls, const union H5PL_key_t *key, hbool_t *success); /* NOTE: The object and ID functions below deal in VOL objects (i.e.; * H5VL_object_t). Similar non-VOL calls exist in H5Iprivate.h. Use diff --git a/src/H5VLpublic.h b/src/H5VLpublic.h index 9c9d60a..a492291 100644 --- a/src/H5VLpublic.h +++ b/src/H5VLpublic.h @@ -25,6 +25,17 @@ /* Public Macros */ /*****************/ +/** + * \ingroup H5VLDEF + * \brief Version # of VOL class struct & callbacks + * + * \details Each VOL connector must set the 'version' field in the H5VL_class_t + * struct to the version of the H5VL_class_t struct that the connector + * implements. The HDF5 library will reject connectors with + * incompatible structs. + */ +#define H5VL_VERSION 0 + /* VOL connector identifier values * These are H5VL_class_value_t values, NOT hid_t values! */ diff --git a/test/h5test.c b/test/h5test.c index dbd1a90..7a4bf2d 100644 --- a/test/h5test.c +++ b/test/h5test.c @@ -2039,6 +2039,7 @@ h5_get_dummy_vol_class(void) /* Fill in the minimum parameters to make a VOL connector class that * can be registered. */ + vol_class->version = H5VL_VERSION; vol_class->name = "dummy"; return vol_class; diff --git a/test/null_vol_connector.c b/test/null_vol_connector.c index 095169c..400064c 100644 --- a/test/null_vol_connector.c +++ b/test/null_vol_connector.c @@ -26,7 +26,7 @@ /* The VOL class struct */ static const H5VL_class_t null_vol_g = { - 0, /* version */ + H5VL_VERSION, /* VOL class struct version */ NULL_VOL_CONNECTOR_VALUE, /* value */ NULL_VOL_CONNECTOR_NAME, /* name */ 0, /* capability flags */ diff --git a/test/vol.c b/test/vol.c index b662cb8..6076fca 100644 --- a/test/vol.c +++ b/test/vol.c @@ -42,7 +42,7 @@ const char *FILENAME[] = {"native_vol_test", NULL}; * functionality. */ static const H5VL_class_t fake_vol_g = { - 0, /* version */ + H5VL_VERSION, /* VOL class struct version */ FAKE_VOL_VALUE, /* value */ FAKE_VOL_NAME, /* name */ 0, /* capability flags */ @@ -181,6 +181,7 @@ test_vol_registration(void) htri_t is_registered = FAIL; hid_t vol_id = H5I_INVALID_HID; hid_t vol_id2 = H5I_INVALID_HID; + H5VL_class_t *bad_fake_vol_class = NULL; TESTING("VOL registration"); @@ -204,6 +205,19 @@ test_vol_registration(void) if (H5Pclose(lapl_id) < 0) TEST_ERROR; + /* Test registering a VOL connector with an incompatible version # */ + if (NULL == (bad_fake_vol_class = HDmalloc(sizeof(H5VL_class_t)))) + TEST_ERROR; + HDmemcpy(bad_fake_vol_class, &fake_vol_g, sizeof(H5VL_class_t)); + bad_fake_vol_class->version = H5VL_VERSION + 1; + H5E_BEGIN_TRY { + vol_id = H5VLregister_connector(bad_fake_vol_class, H5P_DEFAULT); + } H5E_END_TRY; + if (H5I_INVALID_HID != vol_id) + FAIL_PUTS_ERROR("should not be able to register a connector with an incompatible version #"); + HDfree(bad_fake_vol_class); + bad_fake_vol_class = NULL; + /* Load a VOL interface * The vipl_id does nothing without a VOL that needs it, but we do need to * test creating a property list of that class and passing it along as a @@ -277,8 +291,11 @@ error: H5Pclose(vipl_id); } H5E_END_TRY; - return FAIL; + if (bad_fake_vol_class) + HDfree(bad_fake_vol_class); + + return FAIL; } /* end test_vol_registration() */ /*------------------------------------------------------------------------- -- cgit v0.12