Re: DiskDevice API 2.2 remarks

"Ingo Weinhold" <bonefish-CFLBMwTPW48UNGrzBIF7/[email protected]> wrote:

*sigh* My mail is completely mangled again. Tyler, I remember, Axel 
said, there would possibly be a mailing list setting=3F Have you had a 
closer look=3F

Now I see, that this actually isn't the mail, I wanted to reply to. 
Doesn't matter...

BDiskDeviceJob: I would replace Finished() with Status() (or just add 
the latter). A valid status would be one of:

B=5FDISK=5FDEVICE=5FJOB=5FUNINITIALIZED
B=5FDISK=5FDEVICE=5FJOB=5FSCHEDULED
B=5FDISK=5FDEVICE=5FJOB=5FIN=5FPROGRESS
B=5FDISK=5FDEVICE=5FJOB=5FIN=5FSUCCEEDED
B=5FDISK=5FDEVICE=5FJOB=5FIN=5FFAILED
(B=5FDISK=5FDEVICE=5FJOB=5FIN=5FCANCELED=3F)

CU, Ingo

Original mail edited for readability (replacing question marks is a bit 
of fighting wind mills, but... ;-):

> some more thoughts on the updated DiskDevice API.
> 
> BDiskDevice:
> -----------
> 
> * const char* DeviceType() const;
>   -----
> 
> BDiskDeviceRoster:
> -----------------
> 
> * `Do we want visit functions for disk systems and/or jobs=3F'
>   -> no, overkill
> 
> BDiskDisk:
> ---------
> 
> * What meaning has the `checkOnly' flag of SupportsRepairing()=3F Same 
> in 
> BPartition::[Can]Repair().
> 
> BPartition:
> ----------
> 
> * 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, hence 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