[openbeosstorage] Re: The New Device API (tm)

  • From: Ingo Weinhold <bonefish@xxxxxxxxxxxxxxx>
  • To: openbeosstorage@xxxxxxxxxxxxx
  • Date: Wed, 29 Jan 2003 18:41:20 +0100 (MET)

On Tue, 28 Jan 2003, Tyler Dauwalder wrote:

[...]
> > In fact, it could work quite well to completely drop BVolume[Roster]
> > and incorporate their functionality into the Device API. But we are of
> > course not free to do that.
>
> Technically speaking, we *are* free to do that, aren't we? Certainly,
> BVolume must be kept around for compatibility, but it could be
> deprecated and superseded by the Device API.

Yep, that's right. Though, I tend to agree on Axel's recent comment,
that the average application is happy with BVolume. Using an API that can
do anything from gathering precise information about the layout of your
devices to low level formatting, partitioning and initializing, sounds a
bit like overkill, when you only want to get a listing of the currently
mounted volumes.

> The only immediate
> reason I can see for needing to keep BVolume around (vs. moving
> its functionality into the Device API) is if a BVolume object can
> be created for a mounted file image. But I'm afraid I really don't
> understand the low level differences between the two (i.e. at
> which point the system differentiates between a mounted device
> and a mounted file image). Could someone explain that for me?

Actually I think, there is no real difference. A FS doesn't care what it
operates on, if it can only get a file descriptor. And other entities
aren't involved as I see it.

The only reason for not having them in the Device API, is that you can't
(and quite likely don't even want) to deal with unmounted image files. But
in principle it should be possible to handle them in the Device API as
well. One could create a BDevice with a virtual session and partition,
when it is mounted and remove it when being unmounted (i.e. similar to
removing a hot pluggable device).

A nice feature would be, if the system (and also the Device API) would
allow to register files as devices. Then one could also partition an
image. If the session modules would also support creating sessions, then
one could easily master CDs as images files. Just brain storming... :-)

> That aside, do you see other reasons for keeping BVolume[Roster]
> that I don't?

The main reason to keep it would be to have a lightweight API for the
usual needs.

> > bool EachDevice(BDiskDeviceVisitor *visitor, BDiskDevice *device =
> > NULL);
> > bool Each[XYZ]Partition(BDiskDeviceVisitor *visitor, BDiskDevice *
> > device = NULL, BPartition **partition = NULL);
>
> Are you meaning these two+ functions specifically, or all the EachXXX
> functions? I think having a consistent interface all around for the
> various EachXXX() functions might be rather nice.

I was thinking of all the BDiskDeviceRoster iteration methods, but the
ones in BDiskDevice and BSession I intended to leave unchanged. In the
latter cases it is simply more convenient to directly return the
respective object, which is impossible in BDiskDeviceRoster.

> > They would return `true', if terminated early, and in that case set the
> > return parameters (if supplied) accordingly. I believe, that if one is
> > interested in the concerned partition, then one usually wants the
> > object to get more info (moreover this can be done with zero overhead).
>
> This bugs me: why does the function return an affirmative when
> it fails and a negative when it succeeds? The only reason I can
> see is to match the error checking sematics of the other versions
> that return pointers, thus returning NULL on success. But if we
> standardize the interface, wouldn't it make sense to use the
> more obvious bool values for success and failure?

The results don't mean success or failure. They indicate only, whether the
visitor requested to terminate the iteration. E.g. if you write a visitor,
that searches for a partition with a certain property, then a return value
of `true' means `found' -- the BDiskDevice version directly returns the
respective partition instead.

> > > And I would like "ForEach" instead of "Each" better,
> >
> > Yep, or even DoForEach* as in BList. I copied the naming from the old
> > Device API.
> >
> > > but I am not a native speaker.
> >
> > Me neither. :-)
> > Tylaahhhhh! ;-)
>
> Sorry, I'm here now. :-) I like "VisitEachXYZ()" best, but "DoForEachXYZ()"
> seems reasonable, too.

OK.

> > > Should Type() be a string=3F Wouldn't it be better if we had IDs,
> > > like
> > > B=5FINTEL=5FPARTITION =3D 'intl'=3F But I might be wrong here :-)
> >
> > For Intel style partitioning Type() is the interpreted PartitionCode()
> > (which I personally don't like, but nobody listens to me ;-), for
> > partitioning systems using strings it's the respective string.
>
> How about a standardized encoding of the PartitionCode() value in the
> Type() string for unrecognized codes, i.e. ("Unrecognized Type 0xfe")?
> That would discourage use of the partition code over the type string
> unless absolutely necessary, but still make it available. To tell you
> the truth, I'd like this anyway as a user. Seeing "Unrecognized Type"
> in DriveSetup or whatever is a lot less illuminating that "Unrecognized
> Type 0xfe".

Very good idea! I will change the partition module accordingly.

> > > I would also like to get rid of the IsBFS(), IsHFS(), IsOFS() methods
> > > -
> > >  those are strange convenience functions to me.
> >
> > Well, yes, they are. The only alternative would be to test
> > FileSystemShortName() for "bfs",... though.
>
> I can see why IsBFS() might be oft used, but are IsHFS() and IsOFS()
> really needed anymore? And really, shouldn't the checks for attribute
> or index support be used rather than just seeing if the fs is bfs?
> That being true, I wouldn't see any good reason to keep any of those
> functions.

Right. I was perhaps thinking too much of the Tracker and its mount
settings.

> > > PublishDevice() should not be necessary - at least I would like to
> > > get
> > > rid of it, and let the kernel automatically create them as needed.
> > > Ideas welcome, I haven't thought much about it, I just don't like the
> > > way it's currently done :-)
> >
> > The most simple way is to always publish all (non-empty, non-hidden)
> > partitions, but that's probably not what you are looking for?
>
> Is there a good reason *not* to always publish all of them?

None I know of.

> The way
> it's currently done has always seemed like an odd way of doing things.
>
> If PublishDevice() *is* kept, shouldn't an UnpublishDevice() be available?

Yes. But I think the better approach is indeed, as Axel proposes, to move
that completely into the kernel.

> > > > Regarding the naming: I find BDevice simply too general. BDisk
> > > > would
> > > > be
> > > > fine in principle, but there may be potential for mixing it up with
> > > > BVolume. BStorageDevice would be most accurate, but also a bit
> > > > longer.
> > > > :-P
> > >
> > > BDiskDevice is fine, I would like BStorageDevice better - although
> > > BDiskDevice better covers the topic, I think (dunno why, though).
> >
> > I personally don't mind. Tyler? Lurkers?
>
> /dev/disk => BDiskDevice makes sense to me. BStorageDevice is more
> accurately general, yes, but considering we've already got ourselves
> a disk_scanner API, I think sticking with BDiskDevice would be the
> better choice. Unless we feel like changing to storage_scanner...
> :-P :-)

OK, I consider that the final word on that matter. BDiskDevice it shall
be. :-)

CU, Ingo


Other related posts: