[openbeosstorage] Re: DiskDevice API v2.1
- From: Ingo Weinhold <bonefish@xxxxxxxxxxxxxxx>
- To: openbeosstorage@xxxxxxxxxxxxx
- Date: Wed, 2 Apr 2003 17:28:29 +0200 (MEST)
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. I even think, it is a
bit confusing to use the BEmptySpace objects to create partitions. 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.
> 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.
> The BDiskSystem class provides info about supported child and parent
> systems for each disk system (i.e. intel extended would require an
> intel parent).
When introducing such a class, I would make it more complete. 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.
> class BEmptySpace {
> public:
> off_t Offset() const;
> off_t Size() const;
> int32 Index() const;
>
> BPartition* Parent() const;
>
> bool CanCreatePartition() const;
> status_t ValidateCreatePartition(
> off_t *start,
> off_t *size) const;
> BPartition* CreatePartition(off_t start, off_t size);
> // Partition creation this way would always create an
> // unformatted partition. Does this seem reasonable?
> };
As written above, I would drop the *CreatePartition() methods.
> class BPartition {
> public:
> BEmptySpace* EmptySpaceAt(int32 index) const;
>
> // I have resize and move separated because move could be filesystem
> // independent, whereas resize could not
I hope, you're right regarding move. The absolute addressing you found in
ISO9660 disk scares me a bit. Maybe we can ignore that.
> bool CanResize() const;
> bool CanMove() const;
> status_t ValidateResize(off_t*) const;
> status_t ValidateMove(off_t*) const;
> status_t Resize(off_t);
> status_t Move(off_t);
>
> bool CanEditParameters() const;
> status_t GetParameterEditor(
> BDiskScannerParameterEditor **editor,
> BDiskScannerParameterEditor **parentEditor);
>
> // Perhaps we should have something for checking a partition is
> // large/small enough for initialization with a given system? The
> // parameter editor could alternately check for this.
I would just add another method in BDiskSystem.
> bool CanInitialize() const;
> status_t GetInitializationParameterEditor(const char *system,
> BDiskScannerParameterEditor **editor) const;
> status_t Initialize(const char *diskSystem,
> const char *parameters);
>
> bool CanDelete() const;
> status_t Delete();
> };
>
> class BDiskDevice {
> public:
> int32 CommitUpdates(BMessenger progressMessenger);
> };
status_t CommitUpdates(bool synchronously = true,
BMessenger progressMessenger = BMessenger(),
BMessage *template = NULL);
:-)
> class BDiskSystem {
> public:
> bool SupportsParentSystem(const char *system) const;
> // True in most cases. NULL == raw device.
> bool SupportsChildSystem(const char *system) const;
> // False for most file systems, true for most partitioning
> // systems.
>
> bool IsPartitioningSystem() const;
> bool IsFileSystem() const;
> };
As said above, I would make them virtual. Moreover, the Supports*System()
should also have a BPartition* parameter. Otherwise partitioning systems
that allow only a limited number of levels can't give an authoritative
answer here. The BPartition* parameter could optionally be NULL to ask
whether it is supported in general.
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_t*) const;
virtual bool ValidateMove(BPartition*, off_t*) const;
virtual bool ValidateResizeChild(BPartition*, off_t*) const;
virtual bool ValidateMoveChild(BPartition*, off_t*) const;
virtual bool ValidateCreateChild(BPartition*, off_t*, off_t*) const;
virtual bool ValidateInitialize(BPartition*) const;
> class BDiskSystemList {
> public:
> // To be similar to BDiskDeviceList, offering updates as
> // systems are added and removed.
> };
>
> class BDiskDeviceRoster {
> public:
> status_t GetDiskSystems(BDiskSystemList *list) const;
> };
I think, I would prefer iteration methods instead (GetNext*(), Rewind*()).
Otherwise all add-ons would need to be in memory.
CU, Ingo
Other related posts:
