[openbeosstorage] Re: First C Device API Draft

> 
> > CVS
> > absence of an idea for a better name, the kernel module bundling 
> > the 
> > functionality
> > for getting information about sessions and partitions of devices is 
> > called
> > partscan (for partition scanner, better suggestions are welcome :-) 
> 
> How about diskscan (more general, considering sessions)? Or 
> storage_device_scanner (descriptive, though long)? 

I like storage_device_scanner. It makes up a good TLA, SDS, that can be 
used for the header file and for prefixes -- unless this also stands 
for something one doesn't want to confuse it with.

> > and resides
> > in `src/add-ons/kernel/partscan'. The modules for sessions, 
> > partitions and
> > FSs go into the respective subdirs. Currently only one, 
> > `partition/intel',
> > being able to recognize DOS partition maps is implemented.
> > 
> > The headers for the modules live in `headers/private/partscan'. 
> > They 
> > contain
> > a bit of documentation about there API. The user API is defined in
> > `headers/os/kernel/fs_device.h'.
> > 
> > In `src/tests/add-ons/kernel/partscan' the user land versions of 
> > the 
> > modules
> > are built and there's also some test code.
> > 
> > As said, this is a first draft. Suggestions/ideas are very welcome.
> 
> So far just one question: what's the reason for passing "off_t 
> sessionOffset" and "off_t sessionSize" to the two partition functions 
> declared in partscan.h? Why not just pass a "struct session_info*" 
> (which seems clearer and easier to me)? 

The intention was to pass as little information as needed to do the 
job. If a session_info is passed, one either has to document properly 
which fields do have correct values and which don't -- which I somehow 
don't like for an input-only parameter -- or to make sure that indeed 
all fields are filled in correctly -- which may force the caller to 
retrieve unnecessary information.

In the case of get_nth_partition_info() and get_partition_module() the 
fields `index' (the documentation says the `session' field of the 
partition_info is required, but I think that's a lie) and `flags' were 
not needed. Currently it looks like one would request a session_info 
before anyway, though. So I tend to agree with your suggestion. The 
same holds for the partition module functions.

Let me throw in some issues, I'd like to get opinions about:

1) Does get_nth_partition_info() (the one to go into the kernel) look 
alright? Currently it is passed a session index. That requires that the 
session has to be found before. One could pass a session_info, but I'm 
a bit unsure, whether it is OK to rely on a structure the user level 
application could tamper with.

2) Regarding the get_{partition,session}_module() functions of the 
partscan module: I intended them as a way (for the kernel) to bypass 
the other functions, which each time must iterate through the list of 
session/partition modules to find the one responsible. Maybe it is 
better to add remove the get_*_module() functions and add a parameter 
to the other ones, used to return the module's name (first call) and 
pass it to the function (subsequent calls).

3) Regarding the extended_partition_info structure: (the comments in 
PartScanTest.cpp are swapped)
a) get_nth_partition_info() doesn't fill in the `device' field in the 
partition_info structure. It could get it via the B_GET_PARTITION_INFO 
ioctl(). Is it maybe better not to use partition_info, but rather to 
include only the other of its fields? The caller passed a FD to the 
device and certainly also has a path name. The field is shorter than it 
should be for a path name anyway.
b) get_nth_partition_info() doesn't fill in the `mounted_at' field in 
the partition_info. Currently I don't even know how to get it. Maybe 
omit it?

CU, Ingo

Other related posts: