[openbeosstorage] Re: DiskDevice API 2.2 remarks

> > some more thoughts on the updated DiskDevice API.
> > 
> > BDiskDevice:
> > -----------
> > 
> > * const char* DeviceType() const;

Ooops.

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

Okay.

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

Well, I was thinking of offering the ability to make read-only repair 
runs, i.e. verification runs. I'm not sure that's really necessary 
though.

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

I think I was playing around with that after taking out 
CountDescendents(), and forgot about it. You like it less, I take it?

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

Okay.

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

Yes.

> > * SetParameters(): Two editors, hence two parameter strings. :-)

Ooops. Yes. :-)

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

Okay.

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

Okay, they're gone.

> > * CreateChild(): I'd add a `BPartition **child' parameter for
> > convenience.

Hmmmm, okay.

> I wonder, why parameters aren't needed anymore.

Because CreateChild() always creates uninitialized partitions.

> > * DeleteChild(): The index is missing. 

Ooops.

> > It may be better to introduce
> > [Can]Delete() instead (or additionally).

Well, I had those in there, but I felt they fell under the same 
argument as the one use for eliminating 
BPartitionableSpace::CreatePartition(), i.e. the BPartition is suddenly 
going to disappear or at least be invalid.

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

I'm not sure yet. :-) I think I was hoping to hide most of the 
parent-level type info away, but I didn't know and still don't know 
that that's feasible. What's your take on that? If not, I do really 
like the Type() and ContentType() route.

-Tyler

Other related posts: