Skip to content
This repository was archived by the owner on Jul 10, 2025. It is now read-only.

Commit a803e32

Browse files
committed
Add discussion updates.
1 parent 6f7d10b commit a803e32

File tree

1 file changed

+47
-2
lines changed

1 file changed

+47
-2
lines changed

rfcs/20200804-configurable-filesystems.md

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class FileSystem{
4646

4747
Since each filesystem will likely to have different set of tunable parameters, a `FilesystemConfig` object can be used to unify the API and allow discovery of existing tunable parameters at runtime. We propose a protobuf object with the following schema
4848

49-
``` proto
49+
```proto
5050
message FilesystemAttr{
5151
message ListValue {
5252
repeated bytes s = 2; // "list(string)"
@@ -132,4 +132,49 @@ This proposal will expose new methods to user to query and modify operational pa
132132
133133
## Questions and Discussion Topics
134134
135-
Seed this with open questions you require feedback on from the RFC process.
135+
### Comments and Altrenatives Came Out During Posting Period
136+
137+
During the posting period, some concerns about the protobuf passing through C C++ boundaries has been raised and alternative approaches has been discussed. @mihaimaruseac suggested following structure for crossing plugin-framework boundary.
138+
139+
```cpp
140+
typedef struct TF_Filesystem_Option_Value {
141+
int type_tag;
142+
int num_values;
143+
union {
144+
int64 inv_val;
145+
double real_val;
146+
struct {
147+
char* buf;
148+
int buf_length;
149+
} buffer_val;
150+
} *values; // owned
151+
} TF_Filesystem_Option_Value;
152+
153+
typedef struct TF_Filesystem_Option {
154+
char* name; // null terminated, owned
155+
char* description; // null terminated, owned
156+
int per_file; // bool actually, but bool is not a C type
157+
TF_Filesystem_Option_Value *value; // owned
158+
} TF_Filesystem_Option;
159+
```
160+
161+
On framework side these options can be translated to user friendly C++ and Python data structures and helper functions for getting and setting options can be provided in filesystem header file for plugins to use. With this approach all the buffer allocation and dealloactions will be done through allocator functions provided by plugins.
162+
163+
If this schema is prefered C layer methods become
164+
165+
```cpp
166+
void (*const get_filesystem_configuration)(TF_Filesystem_Option** options, int *num_options, TF_Status* status);
167+
void (*const set_filesystem_configuration)(const TF_Filesystem_Option** options, int num_options, TF_Status* status);
168+
}
169+
```
170+
171+
Alternatively API can be expanded with per-option getters and setters in which case methods similar to following would be added to filesystem API
172+
173+
```cpp
174+
void (*const get_filesystem_configuration_option)(const char* key, TF_Filesystem_Option *option, TF_Status* status);
175+
void (*const set_filesystem_configuration_option)(const TF_Filesystem_Option* option, TF_Status* status);
176+
void (*const get_filesystem_configuration_keys)(char** Keys, int *num_keys, TF_Status* status);
177+
178+
```
179+
180+
The first option has the advantage of smaller API surface for plugin developers to implement at the expense of bigger data size crossing the framework-plugin boundary. Adding per-option methods to the API can simplify data preparation for boudary crossing for filesystems that have very large configuration options.

0 commit comments

Comments
 (0)