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

"Ingo Weinhold" <bonefish@xxxxxxxxxxxxxxx> wrote:
> > Shouldn't there better be a notification that the partitions are 
> > gone
> > =3D3F 
> > The reason why they are gone is okay to know, but not the other way 
> > around; knowing the reason and having to find out about the 
> > consequences yourself, right=3D3F
> That's a good point. I will change the notification events to 
> something 
> like B=5FDEVICE=5FPARTITION=5F{ADDED,REMOVED} then.

Yes, that would be nice (even with a "reason" field or something like 
this).

> > BTW, AFAIK, Tracker don't maintain a local list, but recreates the 
> > list everytime it's required.
> That does perhaps depend a bit on the point of view. The AutoMounter 
> "AutomountScan" thread (AutoMounter::WatchVolumes()) periodically 
> checks AutoMounter::fList (DeviceList) for changes and updates it if 
> necessary. Nevertheless on some occasions 
> AutoMounter::RescanDevices() 
> is invoked (maybe sometimes even unnecessarily), which completely 
> rebuilds the list.

Okay, sorry, then I got it wrong :-))

> > Yes, that's right, but again, I think it would make more sense if 
> > Tracker would receive a "new volume in town" notification, not a 
> > "FS 
> > has been initialized" one.
> OK, then I'd propose something like B=5FDEVICE=5FPARTITION=5FCHANGED for 
> that 
> case.

Is fine.

> > Right; and we can change the internals anyway. Perhaps we shouldn't 
> > make the current APIs public now anyway, so that we don't have to 
> > support them later on, if we decide to drop/replace them.
> No, the API should definitely not be public in R1. I guess, now I 
> also 
> remember, why I put the headers for the session/partition/fs module 
> API 
> in the `private' subtree. :-) Since I don't think, they should be 
> public in the first release -- but it seems that `headers/os/drivers' 
> is the correct place anyway.

headers/os/drivers is for kernel add-ons only - so that might be a good 
candidate, yes :-)
Perhaps we should even rename the fs=5Fdevice.h header to disk=5Fscanner.h 
in the os/kernel/ directory=3F
We could keep the headers in that directory (but don't have to, of 
course), and add a big disclaimer expressing their beta nature.

[...]
> > > Mmh, if you're not in a hurry...
> > Not at all, there are still more urgent issues, I think :-)
> Hehe. And skiing of course. ;-)

Oh, yes! Tomorrow I'll be heading to Switzerland!

[...]
> Yes, that's right. However, the device and session IDs are not very 
> prominent in the API anyway -- there's basically only the 
> BDiskDeviceRoster::*WithID() method to get an object for an ID, and 
> the 
> IDs sent with the notification messages. Therefore I'd like to keep 
> them for the time being and reconsider dropping them, when we've got 
> some more experience with the implementation.

Fine with me.

[...]
> Yep, VisitEach*() will make it, I guess. For the other iteration 
> strategy. Mmh, I actually don't like it -- a callback function plus a 
> void pointer as params is so non-OO...
> As a compromise one may offer a visitor subclass that takes a 
> callback 
> function and params on construction, thus functioning as a wrapper.

It wouldn't be a void pointer, it would be a pointer to a function 
that's equivalent to the current object method.
But anyway, we don't have to provide it - we are introducing a new API, 
so we should probably don't keep to much legacy ideas around. If 
everyone is complaining, we could still add them later :-)

> > Hm, OTOH I also don't see how one can get feedback when you call 
> > the 
> > ioctl() directly.
> That would require some support by the kernel and the concerned 
> driver. 
> For a function used that rarely, it's perhaps a bit of overkill.

Yup!

> > Perhaps you should ignore me and follow your main 
> > reason :-)
> Mmh, maybe I just do, what I can do best, and simply postpone the 
> decision. ;-)

Hehe :-)))

[...]
> > We could have a unique ID for every disk device that's just 
> > incremented 
> > with every change; that would be an easy check if the changes are 
> > allowed or not.
> > But that number would also have to be incremented in the local 
> > object 
> > if that caused the change, of course :-)
> I had something similar in mind, but I would make it a pure change 
> counter and not mix it with the ID concept.

Exactly, although I said "unique ID" I didn't meant it to mix with the 
ID concept :)

[...]
> BTW, I already started to implement the partitioning support in the 
> intel partition module. The actual writing of the partition map to 
> disk 
> is still missing, but that should be relatively simple.

Nice! I still haven't got a look at it, but I'll change that after my 
vacation :)

> Mmh, isn't mount() part of some POSIX standard=3F What we could do, is 
> to 
> keep it, but let it check whether the supplied void* parameter 
> (considering len too) looks like a string, and, if it does, pass it 
> to 
> the new API.

No, mount() is not part of the POSIX standard, it just happens to be 
part of almost every OS (at least AFAICT).
We could also keep the current scheme and let the file system decide if 
it wants to use the (to be extended) driver=5Fsettings API - we could 
just encourage but not enforce its usage.
But I am not yet sure why we shouldn't enforce it, just for the sake of 
consistency and simplicity for the user.

[... find=5Fdirectory]
> > If it already asks for the file system name, it needs one kernel 
> > call 
> > to do that - if it would replace that with the call to get the 
> > directy 
> > from the file system directly, that wouldn't change much, I think.
> You're absolutely right. It currently gets a fs=5Finfo from the kernel, 
> so it wouldn't make any difference performance-wise.

Of course only if it hasn't have to make this call anyway to get the 
mount point or whatever. Well, we'll see. I don't think that 
find=5Fdirectory() is time critical anyway :-)

[...]
> > Note, normally, a mounted file system shouldn't be able to access 
> > its 
> > entry anymore if changes took place for that one, but then, that's 
> > exactly what we want in the future with online resizing and such :)
> 
> Yeah! :-)
> 
> Regarding the further proceeding: I will have some time on the 
> weekend 
> and I plan to check in the proposed headers (incorporate the 
> discussed 
> changes before, of course), doxygen-document the API -- due to the 
> non-
> fixedness of the API maybe not too exhaustive -- and start with the 
> implementation.

Sounds good, although, as I mentioned several times now, I am skiing :-
))

Adios...
   Axel.

Other related posts: