[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.

That one's got my vote too. 

> 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.

That's the main thing: it seems like you'll just have a session_info 
lying around ready to be used anyway.

> 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.

I think the user level functions look okay accepting indices like they 
do. To me it makes it clearer which params are input and which are 
output. 

> 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).

Not decided yet... :)

> 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.

Well, they had a path name at one time, but I could see that it might 
be handy to be able to get that info back. Is it that costly to do the 
ioctl() that it would matter? And separately, are we at liberty to muck 
around with struct sizes here?

> 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?

Couldn't we eventually get it rather easily once we have our own 
kernel? Seems like another "could be handy" kind of field to me. I'd 
rather leave it in.

-Tyler

Other related posts: