FreeLists / openbeosstorage / [openbeosstorage] DiskDevice API 2.2 remarks

["Ingo Weinhold" <bonefish@xxxxxxxxxxxxxxx>]

Hi,

some more thoughts on the updated DiskDevice API.

BDiskDevice:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

* =5Fconst=5F char* DeviceType() const;

BDiskDeviceRoster:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

* `Do we want visit functions for disk systems and/or jobs=3F'
  -> no, overkill

BDiskDisk:
=3D=3D=3D=3D=3D=3D=3D=3D=3D

* What meaning has the `checkOnly' flag of SupportsRepairing()=3F Same in 
BPartition::[Can]Repair().

BPartition:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

* VisitSubtree(): I thought, we agreed on VisitEachDescendent()=3F

* CanResizeWhileMounted(): I'd rather add a boolean return parameter to 
CanResize().

* GetParameterEditor(): What was `parentEditor' needed for again=3F I 
suspect, in case of a partition with a FS, it is an editor for the 
partition properties, while `editor' would provide FS parameter 
editing.

* SetParameters(): Two editors =3D> two parameter strings. :-)

* CanInitialize(): Should get a `const char *diskSystem' parameter.

* ValidateSetParameters(), ValidateInitialize(): I don't think, they 
make much sense. For one, the `parameters' parameter is not very 
helpful -- the supplied argument can not be delete, nor the value 
returned for it instead. More importantly, the only way to get a 
parameter string is to ask a parameter editor. And it should be safe to 
assume that it won't return nonsense. If the API user supplies an 
invalid string, the job simply fails.

* CreateChild(): I'd add a `BPartition **child' parameter for 
convenience. I wonder, why parameters aren't needed anymore.

* DeleteChild(): The index is missing. It may be better to introduce 
[Can]Delete() instead (or additionally).

Now, I finally know, what bothered me a bit. A partition actually has 
two types, an outer and inner type. The former one is assigned by the 
parent partition, while the latter one describes the contents. The same 
holds, of course, for the parameters. I wonder, if we should make that 
explicit in the API and have Type() and ContentType(). That way we 
could also drop the BDiskDevice::DeviceType(). CanEditParameters(), 
GetParameterEditor() and SetParameters() would each get a second, 
`Content', version.
What do you think=3F

CU, Ingo