Re: First C Device API Draft

"Ingo Weinhold" <bonefish-CFLBMwTPW48UNGrzBIF7/[email protected]>
Newsgroups gmane.os.openbeos.storage
Message-ID <6287379640-BeMail@graete>
> 
> > 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.