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