[openbeosstorage] Re: DiskDevice API v2.1

  • From: "Ingo Weinhold" <bonefish@xxxxxxxxxxxxxxx>
  • To: openbeosstorage@xxxxxxxxxxxxx
  • Date: Fri, 04 Apr 2003 00:13:06 +0200 CEST

Tyler Dauwalder <tyler@xxxxxxxxxxxxx> wrote:
> 
> On 2003-04-02 at 07:28:29 [-0800], Ingo Weinhold wrote:
> > 
> > On Tue, 1 Apr 2003, Tyler Dauwalder wrote:
> > 
> > > I had the time, so here are some proposed changes for better
> > > partition management support. BPartitions would maintain lists of
> > > BEmptySpace objects, each of which represents the largest 
> > > contiguous
> > > space between child BPartitions that could be filled with a new
> > > BPartition.
> > 
> > Mmh, I don't know, if I see the benefits of this. 
> 
> The main motivation is that it gives the partition code a chance to 
> declare what regions of unused space are valid for creating child 
> partitions. 

This might indeed be a good idea. Since the systems have some 
administrative overhead (the PTSs in the intel system), it could be 
quite convenient.

> > I even think, it is 
> > a
> > bit confusing to use the BEmptySpace objects to create partitions.
> 
> This I agree with now that you mention it. :-)
> 
> > At
> > least to my intuition empty space is a `non-thing', so 
> > encapsulating 
> > it in
> > a class is somehow... er... strange. I know that it is not uncommon 
> > to
> > manage free resources (e.g. `free' lists in memory management), but 
> > that
> > is just to manage information. Just imagine, you have BDiskSpace 
> > object,
> > call a method on it (CreatePartition()) and suddenly the object is 
> > gone.
> > If at all, I'd only provide getter methods by BEmptySpace.
> 
> I think I still like the idea of the BEmptySpace class (maybe 
> BPartitionableSpace would be better=3F), but I agree with moving the 
> creation functions into the parent partition (i.e. 
> CreateChildPartition). Does that sound better=3F

CreateChild() for consistency. Though BEmptySpace sounds suitable in 
the context of the DiskDevice API, I foresee a name clash when the 
indispensable Astronomer Kit is introduced in R2. ;-) So, 
BPartitionableSpace is better IMO.

> > > The ValidateXYZ() functions would be used to bounds/error check
> > > modifications before actually solidifying said modification with 
> > > the
> > > corresponding XYZ() function. The BDiskDevice::CommitUpdates()
> > > function would then actually perform the device's batched 
> > > physical
> > > updates.
> > 
> > Maybe it would be better to have CommitUpdates() in BPartition 
> > rather 
> > than
> > in BDiskDevice. If we provided a partition locking, the siblings of 
> > the
> > affected partition wouldn't need to be locked. Not sure though.
> 
> That's okay with me. I think we should have a way of finding out 
> which 
> partitions are locked and busy though (i.e. IsLocked()), or would you 
> plan on Lock() being non-blocking=3F

Yep, an IsLocked() makes sense. A non-blocking is preferable, I think.

[...]
> > Actually,
> > those objects could be returned by the add-ons. The class should 
> > then 
> > have
> > ValidateXYZ(BPartition*,...) (implemented by derived classes), 
> > which 
> > are
> > invoked by their BPartition::ValidateXYZ() counterparts. This 
> > means, 
> > that
> > the add-ons for the used systems must be kept loaded while the API 
> > is
> > used, but that shouldn't harm that much, I think.
> > 
> > An alternative -- I guess, you envisioned it like that -- is to not 
> > make
> > the BDiskSystem methods virtual, but let them call into the kernel, 
> > where
> > the repective module is asked. I don't think, I like that better.
> 
> Why do you prefer the userland version=3F

I reconsidered that, and I'm about to change my mind. :-)
The reason, why I tended to like the userland version more, was that it 
is more lightweight -- a simple virtual function called vs. a syscall. 
But the API is not exactly performance critical and, I would be 
surprised, if thousand or more invocations per second would have any 
significant impact on modern hardware. So, even live checking of 
partition positions, while the user drags a slider or something like 
that, should be no problem at all.

The big plus on the kernel version's side, is that using a partition's 
BDiskSystem doesn't require the respective userland add-on to be 
loaded. Only the kernel module, which needs to be loaded all the time 
anyway.

[...]
> > >     bool CanInitialize() const;
> > >     status=5Ft GetInitializationParameterEditor(const char *system,
> > >                BDiskScannerParameterEditor **editor) const;
> > >     status=5Ft Initialize(const char *diskSystem,
> > >                      const char *parameters);
> > >
> > >     bool CanDelete() const;
> > >     status=5Ft Delete();
> > > };
> > >
> > > class BDiskDevice {
> > > public:
> > >     int32 CommitUpdates(BMessenger progressMessenger);
> > > };
> > 
> > status=5Ft CommitUpdates(bool synchronously =3D true,
> >     BMessenger progressMessenger =3D BMessenger(),
> >     BMessage *template =3D NULL);
> > 
> > :-)
> 
> Can do, but now you have to explain to me why you're so fond of 
> synchronous operation, because I still don't get it (and if you told 
> me 
> back when we were working on the MIME update functions, it obviously 
> didn't stick :-). 

I'm not *that* fond of synchronous operation. I just don't see the need 
to force the API user to have it done in another thread. E.g. if you 
have a worker thread, that couldn't proceed anyway until the disk is 
set up, you would just waste resources.

[...]
> > Furthermore there should be:
> > 
> >   virtual const char *Name() const;
> > 
> >   virtual bool SupportsResizing(BPartition *partition) const;
> >   virtual bool SupportsResizingChild(BPartition *child) const;
> >   virtual bool SupportsMoving(BPartition *partition) const;
> >   virtual bool SupportsMovingChild(BPartition *child) const;
> > 
> >   virtual bool ValidateResize(BPartition*, off=5Ft*) const;
> >   virtual bool ValidateMove(BPartition*, off=5Ft*) const;
> >   virtual bool ValidateResizeChild(BPartition*, off=5Ft*) const;
> >   virtual bool ValidateMoveChild(BPartition*, off=5Ft*) const;
> > 
> >   virtual bool ValidateCreateChild(BPartition*, off=5Ft*, off=5Ft*) 
> > const;
> > 
> >   virtual bool ValidateInitialize(BPartition*) const;
> 
> Sounds good, though should ValidateInitialize include an off=5Ft *size 
> as 
> well=3F

To return the size the system would actually use, so that the caller is 
e.g. able to warn the user, if space would be wasted when trying to 
initialize the partition with a FS with a smaller maximal size=3F

[...]

CU, Ingo


Other related posts: