FreeLists / openbeosstorage / [openbeosstorage] Re: BDiskSystem::Supports*()/Validate*()

[Tyler Dauwalder <tyler@xxxxxxxxxxxxx>]

On 2003-07-26 at 07:16:41 [-0700], you wrote:
> "Ingo Weinhold" <bonefish@xxxxxxxxxxxxxxx> wrote:
> > On Thu, 24 Jul 2003 04:37:52 +0200 CEST "Axel Dörfler" <axeld@pinc-
> > software.de> wrote:
> > > If you enlarge an existing partition, BFS won't even notice it (it
> > > will still operate properly but ignore the extra space).
> > The problem is, that we can't handle that in a consistent way, i.e.
> > shrinking the size will indeed corrupt the data. And when enlarging
> > first and shrinking then, we can't know and have to assume, that the
> > data are lost.
> 
> That's right, and it should be reflected by a warning along the lines
> "data on that partition is most probably destroyed" or similar.
> 
> > And I also wouldn't like to have e.g. a 1 GB FS residing on a 2 GB
> > partition without being able to get that info. What we could do, is
> > to
> > add a BPartition::ContentSize(). Then DriveSetup can give the user a
> > very good picture of the situation. Thoughts?
> 
> Might be a good idea, yes. OTOH some file systems might have size
> alignment requirements and cannot use the whole space - and if it
> always says something about 0.3% free on that partition it could be
> confusing for the user.
> Also, that case should only very rarely happen, so it could be too 
> much
> of a good thing as well.

We could decide to only display the content size if it differs by an 
empirically chosen significant amount. Even if it's theoretically only 
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.

> > > >   bool CanResize(bool *canResizeContents, bool *whileMounted =
> > > > NULL);
> > > 
> > > I am not sure if I like that way of method calling, even if I 
> > > might
> > > be a bit late :)
> > > I think it's strange that a CanResize() call needs and changes
> > > boolean values.
> > At least for all operations applicable on file systems the 
> > respective
> > Can*() methods takes a `whileMounted' parameter. Not required, but
> > optional. I think `canResizeContents' is not optional only for the
> > reason, that I can hardly imagine, why one would not be interested 
> > in
> > that info.
> 
> True enough, but it would always create the need to have another
> variable and check, even if what I want to know would be this:
> bool canResizeContents;
> if (CanResize(&canResizeContents) && canResizeContents) {
>     ...
> }
> 
> I think it would be clearer to write:
> if ((ResizeCapabilities() & (CAN_RESIZE | CAN_RESIZE_CONTENTS)) != 0) 
> {
>     ...
> }
> 
> 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.
> 
> OTOH for DriveSetup its use would probably nice, I have to admit:
> 
> bool canResizeContents;
> if (CanResize(&canResizeContents)) {
>     if (!canResizeContents)
>         warnUserAndAbortIfHeWantsTo();
> 
>     // resize the partition
>     ...
> }
> 
> > > I would understand a:
> > > enum resize_capabilities {
> > >     B_CAN_RESIZE_ITSELF = 1,
> > >     B_CAN_RESIZE_CONTENTS = 2,
> > >     B_RESIZE_WHILE_MOUNTED = 4
> > > };
> > > and:
> > > bool CanResize();
> > > resize_capabilities ResizeCapabilities();
> > > 
> > > better (or something to that extend). The functionality itself
> > > looks
> > > good, though :)
> > In fact, CanResize() wouldn't be needed at all, since all 
> > information
> > would be provided by the second method. It is even less clear, what
> > it
> > would return here -- whether the partition can be resized, or it's
> > contents, or both?
> 
> 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.

> 
> > > >   bool CanMove(bool *canMoveContents, bool *whileMounted = 
> > > >   NULL);
> > > 
> > > same as above.
> > 
> > Now the method looks even as scary as this:
> > 
> >     bool CanMove(BObjectList<BPartition> *unmovableDescendants,
> >                  BObjectList<BPartition> *movableOnlyIfUnmounted)
> > const;
> > 
> > So you would think of a `bool CanMove() const' and a second method
> > (status_t GetMovingProperies(...)).
> > 
> > Looks OK in principle. I'm a bit undecided though. Tyler?
> 
> Oh, didn't know it's already that complicated :-)
> But in which case does CanMove() return true? Only if
> unmovableDescendants is empty? In my understanding, you cannot move a
> partition when there is an unmovable descendant.

IIRC, the idea was to return whether or not the partition itself could 
be moved, separate from whether any of its children could be moved. 
That information would be returned in the two BObjectList parameters. 
The user then could choose to move only if all descendents allowed 
moving as well, or to move regardless and destroy the data in any child 
partitions that do not allow moving. 

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?

-Tyler