Re: BDiskSystem::Supports*()/Validate*()

On 2003-07-27 at 16:13:33 [-0700], you wrote:
> On Sun, 27 Jul 2003 11:09:18 +0200 CEST "Axel Dörfler" <axeld@pinc-
> software.de> wrote:
> > "Ingo Weinhold" <bonefish-CFLBMwTPW48UNGrzBIF7/[email protected]> wrote:
> > [...]
> > > > rarely needed, I think it would be a good idea to have the
> > > > ContentSize() feature in there if we're going to support
> > > > non-destructive enlarging even when the contents of the 
> > > > partition
> > > > cannot (and are not) enlarged.
> > > OK, it's no problem to add it. The question is then, whether we
> > > want
> > > to
> > > allow resizing the contents individually
> > > (BPartition::ResizeContents(off_t)), or at least allow to pass 
> > > both
> > > size and content size to Resize():
> > > 
> > >     status_t Resize(off_t size, off_t contentSize);
> > > 
> > > CanResize() and ValidateResize() would need to be adjusted
> > > accordingly.
> > > Against separating the features speaks, that it's more work for 
> > > the
> > > API
> > > user, since they must take care of the order of applying the 
> > > resize
> > > operations (contents, partition when shrinking, the other way
> > > around
> > > when enlarging).
> > 
> > I think it would be okay if contentSize had a default parameter in
> > which case it'll be the same as size. OTOH I don't think those
> > changes
> > are worth the effort, since I cannot really imagine a situation 
> > where
> > you want to do that.
> 
> Do you mean all of the new suggestions or only the one for Resize()? 

I believe he is just referring to enlarging a partition without 
enlarging its contents.

> As
> I interpreted the vibrations on the list, ContentType() is liked, 
> since
> it allows us to deal more sensibly with non-resizable partition
> contents on otherwise resizable partitions (not nuking it as long as
> the partition size shrinks smaller than the content size). 

You mean "doesn't shrink", don't you?

> Resize()
> could remain unchanged, CanResize() too (save for providing a default
> parameter for `canResizeContents'), but, I think, ValidateResize()
> should nevertheless have an optional parameter `off_t *contentSize'
> instead of `bool resizeContents'. If a pointer is not supplied, the
> pointed to variable would not need to be set by the caller; the system
> returns in it the size the contents can be resized to. It would be the
> current content size, if the respective disk system does not support
> resizing, a suitable value otherwise. The caller can then pass true 
> for
> the `resizeContents' parameter of Resize(), when the value is equal to
> the current one, which nukes the contents, if the given partition size
> is too small to have the partition contain the contents, but leave it
> untouched otherwise.

I think that's a good way of handling the described semantics, but I 
also think Axel has a good point: why would you want to resize a 
partition but not its contents? Having ContentSize() around in case 
such a thing happens due to someone else's tools is probably a good 
idea, but doesn't it seem more natural to always resize the contents 
with the partition?

> 
> [...]
> > > > > But if the contents of the partition are supposed to be going
> > > > > away
> > > > > anyway, you could also just check:
> > > > > if (CanResize()) {
> > > > >     ...
> > > > > }
> > > > > without having to define an extra boolean parameter.
> > > That would work with the current version as well, when providing a
> > > default parameter for `canResizeContents'. I didn't, because I
> > > assumed
> > > that one would always be interested in the full truth, but it's no
> > > big
> > > deal to change that.
> > 
> > Perhaps we should do that, then?
> 
> Unless Tyler objects, I will do that.

The default parameter sounds good to me.

> > > > > OTOH for DriveSetup its use would probably nice, I have to
> > > > > admit:
> > > > > 
> > > > > bool canResizeContents;
> > > > > if (CanResize(&canResizeContents)) {
> > > > >     if (!canResizeContents)
> > > > >         warnUserAndAbortIfHeWantsTo();
> > > > > 
> > > > >     // resize the partition
> > > > >     ...
> > > > > }
> > > Yep.
> > 
> > Since DriveSetup is the main idea behind this API, maybe we just
> > ditch
> > my ResizeCapabilities() idea again, and keep what we have now?
> 
> I would be fine with that.

Me too.

> > > > > True, CanResize() wouldn't be needed anymore - yet the value 
> > > > > it
> > > > > returns is as confusing as in the current solution (IMO).
> > > > If we don't need CanResize(), then why not ditch it? The only
> > > > problem
> > > > I can see with ResizeCapabilities is that is doesn't match the
> > > > CanXYZ()
> > > > naming scheme.
> > > Yes, that's pretty much what I think too.
> > 
> > I could also undo my suggestions, and we stay with what we have -
> > since
> > it seems well suited for the use in DriveSetup and similar apps.
> 
> OK.
> 
> > > > I'm not sure I see what would be gained by splitting CanMove()
> > > > into
> > > > two
> > > > functions, though. If you really want to be able to get just the
> > > > info
> > > > returned by CanMove(), why not allow NULL to be passed for the
> > > > BObjectLists and provide default parameters?
> > > Same here, no big deal to do that.
> > 
> > Yes, that would be nice.
> 
> Fine. Then I'll do that.

Good deal.

> BTW, I was hoping to implement the job stuff and the first bits of
> physical partition manipulation this weekend, but unfortunately had no
> time to do anything. :-( I did a small biking tour today and used up
> the better part of yesterday to get my bicycle into a clean and well-
> working state (which two hours of riding in the rain pretty much undid
> :-( ). Anyway, I hope to get something done over the course of the
> week.

Good for you! I should really get my bike out sometime before the 
weather goes bad here again. I spent the weekend writing a paper and 
wrangling with LaTeX, I'm afraid, but I have been listening to mp3s off 
of UDF cdroms. :-)

-Tyler