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

  • From: Tyler Dauwalder <tyler@xxxxxxxxxxxxx>
  • To: openbeosstorage@xxxxxxxxxxxxx
  • Date: Mon, 21 Jul 2003 00:14:33 -0700

On 2003-07-18 at 07:41:35 [-0700], you wrote:
> On Thu, 17 Jul 2003 14:13:06 -0700 Tyler Dauwalder 
> <tyler@xxxxxxxxxxxxx
> > wrote:
> > On 2003-07-16 at 16:49:38 [-0700], you wrote:
> > > now that I implemented them all, I really start to wonder, whether
> > > most
> > > of the BDiskSystem::Supports*()/Validate*() are really needed at
> > > all.
> > > Some of them, like SupportsDefragmenting(), are completely
> > > equivalent
> > > to a BPartition method (CanDefragment() in this case). Or say, 
> > > they
> > > are
> > > even less handy, because one has a redundant parameter that the
> > > method
> > > needs to check (the disk system the method is invoked on must be
> > > the
> > > one the partition is formatted with (or its parent partition for
> > > some
> > > methods)).
> > > 
> > > Others, like BDiskSystem::SupportsResizing() give different
> > > information
> > > than BPartition::CanResize(). In fact the latter one is the result
> > > of
> > > an AND of BDiskSystem::SupportsResizing() and
> > > SupportsResizingChild().
> > > The question is: Are the individual pieces of information returned
> > > by
> > > the latter ones of interest or not?
> > 
> > I believe I had envisioned functions like BPartition::CanResize()
> > being
> > implemented by specifically calling BDiskSystem::SupportsResizing()
> > for
> > the appropriate disk system and then anding it with a call to
> > SupportsResizingChild(). Doing it like you have by calling the 
> > kernel
> > functions directly is fine, too, though. And as you mention, I guess
> > it's more efficient, too. :-)
> 
> Yep, it saves two syscalls (as I implemented it, initializing a
> BDiskSystem requires one). I think, it will even end up doing just one
> syscall, that gathers all the info in the kernel.
> 
> Anyway, I've been mulling over the question whether e.g. the pieces of
> information the BDiskSystem::SupportsResizing[Child]() methods return
> are actually useful for the API user. And I came to the conclusion,
> that they are indeed. E.g. if you have a primary or logical intel
> partition -- for which resizing will be implemented in the not so
> distant future -- being formatted with BFS -- which doesn't support
> resizing -- DriveSetup should nevertheless offer you the option to
> resize the partition. Warning you, of course, that it will destroy the
> data on the partition.

That's not how I had originally imagined things working, but I like it. 
:-) Seems very handy.

> So BPartition::Resize() needs something like a `bool force' parameter,
> indicating, that it shall not fail, when resizing the content fails; 
> or
> maybe better `bool resizeContents' specifying whether or not to resize
> the contents. At any rate that would make the semantics of CanResize()
> a bit confusing, I think (since it will just return false, although
> calling Resize() is possible). Another parameter might be useful:
> 
>   bool CanResize(bool *canResizeContents, bool *whileMounted = NULL);
> 
> The method would return, whether resizing is possible at all,
> canResizeContents would additionally tell you, whether the contained
> disk system can be resized as well, and if that is the case and the
> disk system is a file system, then whileMounted would provide the
> obvious information...

So CanResize() would return whether the parent allowed resizing the 
child, and canResizeContents would say whether the child could resize 
itself as well, correct?

> Now BPartition::ValidateResize() needs to be adjusted as well. If the
> additional parameter for Resize() is `resizeContents', then
> ValidateResize() should have the same parameter. If false, the methods
> returns a value suitable for the partitioning system, otherwise it
> returns one suitable also for the contained disk system. 

Okay.

> I'm not sure
> how to implement that BTW -- asking them alternately might cause an
> infinite loop. So either fail after one or two iterations, or just be
> happy, if the contained disk system is fine in principle, but proposes
> a smaller value.

Yes, something like that should be okay.

> The situation is more complicated for Move(). The (in-kernel) 
> algorithm
> would probably work like this: Validate, that the new offset is fine
> for the partitioning system responsible for the partition. Then
> recursively check, whether it is also OK for the contained disk system
> and the disk systems of all descendants. Then unmount all file systems
> of descendants that require it, move the partition
> (KDiskSystem::MoveChild()) and tell all disk systems within, that they
> have been moved (KDiskSystem::Move()).
> 
> Given that this is how it should work, DriveSetup should offer similar
> options as for resizing. As long as the partitioning system is able to
> move the partition, it should be enabled, and DriveSetup should warn
> the user, if any contained disk system cannot be moved (listing them
> all).

Okay.

> Thus in principle also Move() should have something like a `bool 
> force'
> parameter; or maybe `uint32 options' where options can be
> B_MOVE_PARTITION_ALL, B_MOVE_PARTITION_FORCE,
> B_MOVE_PARTITION_IGNORE_CONTENTS. 

Does B_MOVE_PARITION_ALL succeed iff you can move all partitions? And 
is FORCE then "move as many as you can, destroying those that say they 
can't be moved" and IGNORE "just move it and wipe out the contents"? 

> For CanMove() I'd think of:
> 
>   bool CanMove(bool *canMoveContents, bool *whileMounted = NULL);
> 
> canMoveContents will be set to false, if the contents of any 
> descendant
> cannot be moved. If the partition does contain a partitioning system,
> I'm not sure, what the best semantics would be for whileMounted. It
> could be that all descendants can remain mounted, or it could simply 
> be
> ignored in this case.
> 
> Instead of `canMoveContents' (or additionally) a `BObjectList<
> BPartition> *descendantsWithUnmovableContents' parameter could be
> added. Then DriveSetup could also find out, which partitions' contents
> would become invalid, when moving the partition is `forced'.

Having the extra info provided by the list might be nice. I think I 
would favor that method. Perhaps two lists: MovableWhileMounted, and 
Unmovable. Those movable only with unmounting are then easily 
determined as well (or instead make the second list 
MovableOnlyIfUnmounted, or something to that degree).

> Finally there's BPartition::ValidateMove(), which is a bit tricky. I
> think, it would be quite difficult to try to negotiate a value that 
> all
> affected disk systems are happy with. So I would only try to fine a
> value acceptable for the partitioning system responsible for the
> partition in question and its contained disk system, and fail after 
> one
> or two unsuccessful iterations. For the descendants I would only ask,
> if they are OK with the new value and just fail, if any of them is 
> not.

Sounds fair. We could always try something more thorough if we ever 
find ourselves with lots of free time on our hands. :-)

> Oh, I almost forgot: I would add the same parameter to ValidateMove()
> that is added to Move().

Sure.

> 
> > > So, what I think about is to
> > > 1) drop the BDiskSystem::Validate*() methods entirely,
> > 
> > Fine by me.
> 
> I was temporarily thinking about keeping ValidateResize[Child]() and
> ValidateMove[Child](), since that would allow the API user to 
> implement
> their one negotiation strategy, but I don't think, it would make that
> much sense.

No, I thought about that, too, but I don't see any reason either.

> > > So, the only remaining methods with a BPartition* parameters would
> > > be
> > > GetNextSupportedType(), which enumerates possible partition types
> > > for
> > > children of a partition, and IsSubSystemFor(), which tells whether
> > > the
> > > disk system is a subsystem of the system on the supplied partition
> > > (e.g. the `intel extended partition' system would be a subsystem
> > > for a
> > > `intel partition map' partition).
> > > 
> > > What do you think?
> > 
> > Looks good to me, I say go for it. :-)
> 
> Fine, will do. I hope, we haven't overlooked something important.

Me too. :-)

-Tyler

Other related posts: