Re: DiskDevice API 2.2 remarks

Tyler Dauwalder <tyler-cyCl/[email protected]>
Newsgroups gmane.os.openbeos.storage
Message-ID <[email protected]>
> > 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
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.