bup fuse and the allow_other option

[Jon Dowland]

I've applied the following to bup as part of the Debian packaging:

diff --git a/cmd/fuse-cmd.py b/cmd/fuse-cmd.py
index 6488097..c03dd26 100755
--- a/cmd/fuse-cmd.py
+++ b/cmd/fuse-cmd.py
@@ -125,6 +125,5 @@ if opt.foreground:
f.fuse_args.setmod('foreground')
print f.multithreaded
f.multithreaded = False
-f.fuse_args.add('allow_other')

f.main()


By default, "allow_other" is disabled in Debian/Ubuntu and this line
caused an unpleasant backtrace. It looked fairly optional to me. An
ideal patch would probably be to test for the feature and warn if it is
not available. Allowing user-supplied fuse arguments at the command
line level would also let people optionally turn it on as on a
per-invocation basis. I haven't the time to write either of those
patches right now though :(

[Avery Pennarun]

On Wed, Apr 28, 2010 at 7:00 AM, Jon Dowland <jm...@debian.org> wrote:
> -f.fuse_args.add('allow_other')

>
> By default, "allow_other" is disabled in Debian/Ubuntu and this line
> caused an unpleasant backtrace.

Hmm, are you sure? I use unmodified fuse (and python-fuse) from
Debian, and I haven't run into this problem. What does the backtrace
look like? Why on earth would this feature be disabled?

> It looked fairly optional to me. An
> ideal patch would probably be to test for the feature and warn if it is
> not available.  Allowing user-supplied fuse arguments at the command
> line level would also let people optionally turn it on as on a
> per-invocation basis.  I haven't the time to write either of those
> patches right now though :(

Either of these would certainly be preferable to removing the
allow_other flag entirely. A major use case of 'bup fuse' is to allow
exporting the resulting filesystem via samba, which you can't easily
do unless you mount the file as the samba user, which is kind of
inconvenient.

I thought about using the normal fuse command line parsing stuff in
bup fuse, but I eventually decided against it because it had way too
much stuff; a lot of the options had nothing to do with bup, and were
just unnecessarily confusing. But making things like this
individually optional (using bup-style option parsing) would be fine.

Have fun,

Avery

[Jon Dowland]

On 28/04/10 14:08, Avery Pennarun wrote:
> On Wed, Apr 28, 2010 at 7:00 AM, Jon Dowland <jm...@debian.org> wrote:
>
>> -f.fuse_args.add('allow_other')
>>
>> By default, "allow_other" is disabled in Debian/Ubuntu and this line
>> caused an unpleasant backtrace.
>>
> Hmm, are you sure? I use unmodified fuse (and python-fuse) from
> Debian, and I haven't run into this problem. What does the backtrace
> look like? Why on earth would this feature be disabled?
>

Afraid so. It's disabled in /etc/fuse.conf only. The backtrace looks like:

$ bup fuse ~/mnt
fusermount: option allow_other only allowed if 'user_allow_other' is set
in /etc/fuse.conf
True
Traceback (most recent call last):
File "/usr/lib/bup/cmd/bup-fuse", line 131, in <module>
f.main()
File "/usr/lib/python2.6/dist-packages/fuse.py", line 754, in main
main(**d)
fuse.FuseError: filesystem initialization failed


How is this patch?

[Avery Pennarun]

On Wed, Apr 28, 2010 at 9:55 AM, Jon Dowland <jm...@debian.org> wrote:
> On 28/04/10 14:08, Avery Pennarun wrote:
>> On Wed, Apr 28, 2010 at 7:00 AM, Jon Dowland <jm...@debian.org> wrote:
>> Hmm, are you sure?  I use unmodified fuse (and python-fuse) from
>> Debian, and I haven't run into this problem.  What does the backtrace
>> look like?  Why on earth would this feature be disabled?
>
> Afraid so. It's disabled in /etc/fuse.conf only. The backtrace looks like:

Ah, ok. That's probably something about newer versions of Debian or
something. As long as it's only in the config file, it seems a lot
less evil :)

> $ bup fuse ~/mnt
> fusermount: option allow_other only allowed if 'user_allow_other' is set
> in /etc/fuse.conf
> True
> Traceback (most recent call last):
>  File "/usr/lib/bup/cmd/bup-fuse", line 131, in <module>
>    f.main()
>  File "/usr/lib/python2.6/dist-packages/fuse.py", line 754, in main
>    main(**d)
> fuse.FuseError: filesystem initialization failed
>
> How is this patch?

The patch is almost right, but you're missing a signed-off-by line.
Also, do you want to see if you can catch the "filesystem
initialization failed" exception and not print an ugly stack trace in
that case? If you don't have time, I'll take a look at it later.

Thanks,

Avery

[Jon Dowland]

On 28/04/10 15:15, Avery Pennarun wrote:
> The patch is almost right, but you're missing a signed-off-by line.
>

Thanks - revised one attached. I had a bit of a fiddle with git am /
format-patch but didn't get very far.

> Also, do you want to see if you can catch the "filesystem
> initialization failed" exception and not print an ugly stack trace in
> that case? If you don't have time, I'll take a look at it later.
>

Tricky: it's thrown by f.main() rather than f.fuse_args.add. The
following works, but isn't pretty:

--- cmd/fuse-cmd.py 2010-04-28 15:44:33.000000000 +0100
+++ /usr/lib/bup/cmd/bup-fuse 2010-04-28 16:03:36.677703676 +0100
@@ -127,4 +127,11 @@
f.multithreaded = False
f.fuse_args.add('allow_other')

-f.main()
+try:
+ f.main()
+except fuse.FuseError:
+ f.fuse_args.optlist -= set(['allow_other'])
+ f.main()

I can't (easily) see a way of distinguishing this failure from others
(in this case the exception's args field matches ('filesystem
initialization failed',), but I expect it does for many other possible
failures). I don't know whether that message might be localized. The
stderr output of fusermount leaks out to the terminal:

"fusermount: option allow_other only allowed if 'user_allow_other' is

set in /etc/fuse.conf", which again might be localized. There does not
appear to be a convenience function for removing an option from the
arguments, and I'm not sure how acceptable it is for me to poke at the
optlist member as I have here.

Also something about the above causes "True" to be printed to the screen
(stdout this time) if the exception is caught. I've never really messed
with Python exceptions before so I'm not sure why.

In all cases where the exception is not caused by this particular
problem, you will get two sets of error messages e.g.

$ bup fuse ~/mnt
fuse: failed to open /dev/fuse: Permission denied
fuse: failed to open /dev/fuse: Permission denied

True
Traceback (most recent call last):

File "/usr/lib/bup/cmd/bup-fuse", line 134, in <module>

f.main()
File "/usr/lib/python2.6/dist-packages/fuse.py", line 754, in main
main(**d)
fuse.FuseError: filesystem initialization failed

or

$ bup fuse ~/mnt
fuse: mountpoint is not empty
fuse: if you are sure this is safe, use the 'nonempty' mount option
fuse: mountpoint is not empty
fuse: if you are sure this is safe, use the 'nonempty' mount option

True
Traceback (most recent call last):

File "/usr/lib/bup/cmd/bup-fuse", line 134, in <module>

f.main()
File "/usr/lib/python2.6/dist-packages/fuse.py", line 754, in main
main(**d)
fuse.FuseError: filesystem initialization failed

(so in fact, this patch really sucks :))

[Avery Pennarun]

On Wed, Apr 28, 2010 at 11:27 AM, Jon Dowland <jm...@debian.org> wrote:
> On 28/04/10 15:15, Avery Pennarun wrote:
>> The patch is almost right, but you're missing a signed-off-by line.
>
> Thanks - revised one attached. I had a bit of a fiddle with git am /
> format-patch but didn't get very far.

There's more than one way to do it (tm), but my favourite way is to
just use the "-s" option to git commit. Format-patch can also add it,
but I don't really know why you'd want to add it at that phase.

>> Also, do you want to see if you can catch the "filesystem
>> initialization failed" exception and not print an ugly stack trace in
>> that case?  If you don't have time, I'll take a look at it later.
>>
> Tricky: it's thrown by f.main() rather than f.fuse_args.add. The
> following works, but isn't pretty:

Yeah, that's pretty gross. I'll apply your current patch as-is (ie.
without the better error handling) in the interests of
BetterThanLastTime. ie. it's strictly a functionality improvement,
since the stack trace is nothing new.

Have fun,

Avery