mattjala
commented
3 months ago This will add extra, currently unnecessary overhead. Unless there is a need for this now, I suggest that we defer this change until it is requested by a user.
Right now, the API context effectively stores two different vol connector property values - one on its FAPL, and one directly on the vol_connector_prop context field. Adding the FAPL to the Context State would enable the API context rely entirely rely on the FAPL for its VOL connector property value.
That wouldn't be super significant on its own, but would allow a threadsafe H5P implementation to implicitly resolve threadsafety problems with the VOL connector property - i.e. if all property modifications are threadsafe, then by having the context treat the VOL connector as a property, the context's handling of the VOL connector property will be threadsafe.
That said, I have no objection to shelving this PR and adding the FAPL at the time the above changes are relevant.
qkoziol
The API Context state is for use by VOL connectors that might not execute API calls in the order they were provided. It's used to save API Context State (e.g. property lists and a few other fields controlling the operation) and resume it later when the requested operation actually occurs at the VOL layer.
The API Context State is missing two property lists from the API Context - the FAPL and the DAPL. This seems to have occurred by accident because the the API Context State was created before property lists were added to the API Context, and not for any particular design reason.
Storing the FAPL and DAPL on the Context State is more consistent, and should provide future asynchronous-like VOL Connectors with more flexible control.
The API Context State struct itself isn't publicly exposed to applications, but VOL connectors use
H5VLretrieve_lib_state()/H5Vfree_lib_state(), which return and free the API Context State in an untyped buffer. This buffer shouldn't be introspected or modified at all by the VOL itself, so this isn't an API change.