Re: DiskDevice API 2.2 remarks

On 2003-04-08 at 12:01:20 [-0700], Ingo Weinhold wrote:
> Tyler Dauwalder <tyler-cyCl/[email protected]> wrote:
> [...]
> > > > 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.
> 
> Ah, OK. Doesn't harm at least. BTW, the idea is to send encountered
> errors via notifications? I hope that doesn't have impact on
> performance. 

Yes, but only to the provided progressMessenger, not through the 
watching system.

> Although the FS could try to bundle subsequent errors
> messages in one notification message (like `Blocks 123456 to 198235
> contain a funny potpourri of garbage. Better start searching your
> backup.').

:-). Yes, something like that. :-)

> > > > 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?
> 
> Yes. Since we have VisitEachChild(), VisitEachDescendant() would be
> consequent.

Fair enough.

> [...]
> > > > * CreateChild(): I'd add a `BPartition **child' parameter for
> > > > convenience.
> > 
> > Hmmmm, okay.
> 
> We can make it a `= NULL' default parameter, so one doesn't have to
> supply it. It's just, that you can't be sure, at which index the new
> partition ends up.

Okay, I can handle that.

> > > I wonder, why parameters aren't needed anymore.
> > 
> > Because CreateChild() always creates uninitialized partitions.
> 
> Right, but nevertheless the parameters the parent disk system
> associates with the partition (as opposed to the content parameters,
> which are indeed not needed when creating an uninitialized partition)
> are needed. I would allow NULL as argument, in which case the
> partitioning system has to use defaults.

Hmmm, okay.

> [...]
> > > > 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.
> 
> Although at least the name Delete() should be hint enough for the user
> to not be too surprised. ;-) However, since one can only delete child
> partitions (not devices) DeleteChild() is probably better. Maybe we
> should add an additional DeleteChild(BPartition *child) version.

DeleteChild(child->Index()) ?

> > > > 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?
> 
> Actually, I don't think it is. Same with parameters + content
> parameters.

Darn. :-)

> > If not, I do really
> > like the Type() and ContentType() route.
> 
> OK, then let's go for it.

Sounds good.

-Tyler