mount module API

[Floris Bruynooghe]

Hi

Currently it is possible to create invalid mount objects by
instantiating the classes from Python. Would it be more appropriate
for their init method to raise an exception (it's pretty much
obesolete otherwise really, the `mounti' member is initialised to NULL
by Python anyway)?

But what exception would be appropriate? RuntimeError?

Regards
Floris

--
Debian GNU/Linux -- The Power of Freedom
www.debian.org | www.gnu.org | www.kernel.org

[Floris Bruynooghe]

Hello

I played aroud more with the mount module and the result is what I've
checked in to the mountrefactor branch. The changes are quite
substantial and you might not like them hence I made the branch.
Anyway here the rationale:

The .space attribute returned a tuple and fetched the information live
from the system. This was very different from the rest of psi which
takes a snapshot at instantiation time. So I modified psi_mountinfo
to contain the space information too (and extended it a little to
include all statvfs(2) info, why not).

This raises a new issue though, statvfs(2) on a remote filesystem is
subject to arbirary timeouts, and when getting the list of mounted
filesystems this might be undesirable. Hence I added a boolean flag
"remote" to the factory method to only return local filesystems.

Lastly this change meant that the ability to retrieve updated
size information from a mount object was gone, so I added the
.refresh() method I've been thinking of adding to other objects too.

Another change I think is required is the way the sizes are exported
to python. I don't think the original way works, it might overflow.
Now the raw statvfs info is stored in the mountinfo structure so that
the descriptors of MountBase can do the calculation using Python long
objects which can have arbitrary sizes.

Other then that I changed a few attribute names to what I think is
more natural. I'm not convince on all attribute names, e.g. maybe the
size info should be using the statvfs names bfree, bavail, ifree,
iavail etc.

So what the impression? This any good to replace the mount module in
the default branch?

Cheers

[Chris Miles]

On 24/06/2009, at 9:06 AM, Floris Bruynooghe wrote:

>
> Hello
>
> I played aroud more with the mount module and the result is what I've
> checked in to the mountrefactor branch. The changes are quite
> substantial and you might not like them hence I made the branch.
> Anyway here the rationale:
>
> The .space attribute returned a tuple and fetched the information live
> from the system. This was very different from the rest of psi which
> takes a snapshot at instantiation time. So I modified psi_mountinfo
> to contain the space information too (and extended it a little to
> include all statvfs(2) info, why not).

I'd like PSI to be consistent with its "snapshot" model, so am +1 for
this change.

>
> This raises a new issue though, statvfs(2) on a remote filesystem is
> subject to arbirary timeouts, and when getting the list of mounted
> filesystems this might be undesirable. Hence I added a boolean flag
> "remote" to the factory method to only return local filesystems.

+1. Not including remote filesystems by default is sensible. I assume
statvfs(2) can hang indefinitely on hard NFS mounts that have gone
away, which is a real pain.

>
> Lastly this change meant that the ability to retrieve updated
> size information from a mount object was gone, so I added the
> .refresh() method I've been thinking of adding to other objects too.

+1. That is a handy addition.

>
> Another change I think is required is the way the sizes are exported
> to python. I don't think the original way works, it might overflow.
> Now the raw statvfs info is stored in the mountinfo structure so that
> the descriptors of MountBase can do the calculation using Python long
> objects which can have arbitrary sizes.
>
> Other then that I changed a few attribute names to what I think is
> more natural. I'm not convince on all attribute names, e.g. maybe the
> size info should be using the statvfs names bfree, bavail, ifree,
> iavail etc.

I haven't thought much about the attribute names, but don't mind your
suggestion.

However, I think "mount_options" containing an underscore while
"mountpoint" does not feels inconsistent. Change "mountpoint" to
"mount_point" ?

>
>
> So what the impression? This any good to replace the mount module in
> the default branch?

I didn't have a chance to look at the first version of mount, but this
looks good to me. I have updated the mountrefactor branch to at least
compile on OS X. I'll push that back up. I haven't updated it yet
with all the missing features yet though (like remote flag and free/
available attributes).

Cheers
Chris

[Floris Bruynooghe]

On Wed, Jun 24, 2009 at 12:20:34PM +1000, Chris Miles wrote:
> However, I think "mount_options" containing an underscore while
> "mountpoint" does not feels inconsistent. Change "mountpoint" to
> "mount_point" ?

I quite like "mountpoint", I'd rather change mount_options to
something else...

Floris

[Chris Miles]

Sure. I couldn't think of anything better than just "options", which
might be good enough.

Cheers
Chris