Re: [gfvs] cdda backend

[David Zeuthen <david fubar dk>]

Hi,

Here's a new set of patches

http://people.freedesktop.org/~david/gio-4.patch
http://people.freedesktop.org/~david/gvfs-4.patch
http://people.freedesktop.org/~david/nautilus-4.patch

There's a description in ChangeLog and comments below

On Mon, 2007-12-17 at 10:16 +0100, Alexander Larsson wrote:
> >  1. The GVfs FUSE daemon never class close() on my fs backend when
> >     it's done reading.. this leads to leaks. Have you seen this?
> 
> The fuse daemon is mainly the work of hans petter. I haven't really
> looked into it or tried it that much.

OK, I've fixed this. Was simply a typo where the value was used instead
of the key in g_hash_table_remove().

> >  2. I have to set GMountSpec in try_mount(); it's not enough just
> >     setting it in do_mount(). Bug?
> 
> Yes, that shouldn't happen. Can you try to look into this?

I tried fixing it; no luck. Maybe tomorrow when I'm less tired :-)

> >  3. Once mounted, there's no way for the backend to change it's display
> >     name. I think I understand why you did this; it's because of the
> >     FUSE daemon right? E.g. you want to ensure that there are no
> >     name collisions to make it easier to figure out what directories
> >     to return in ~/.gvfs ...
> > 
> >     Anyway, the reason we want this is that initially we probably want
> >     the display name "Audio Disc" and if we (much later) acquire meta
> >     data for the disc we want to change it to "Tori Amos - Silent
> >     all these years" or whatever the title is.
> > 
> >     (same applies for the icon (think cover art); I haven't tested if
> >      this doesn't work)
> 
> Hmmm. Tricky. Its kinda weird having the name of the actual mountpoint
> change. Especially with it being the identifier for the mount in the
> fuse case. I think the best solution is to have the volume be called
> _("Audio Disc"), but then set the display name of the root of the disk
> to the actual disk title. Thats what will be displayed in many places
> like the window title for the toplevel directory.

This breaks if you have two audio discs in separate drives; what I've
done in the patch set is to provide a new function

 g_vfs_backend_set_fuse_name()

where I set the name "Audio Disc on sr0" etc. If a backend don't call
this function, the value from display_name() is used instead. The
implementation is a bit ugly (g_object_set|get_data on the GDaemonMount
object) but it works.

> > User inserts audio disc into drive. The HAL volume monitor notices this
> > and creates a GVolume object representing the unmounted audio disc. The
> > mount() method on the HAL volume monitor for said GVolume will simply do
> > the equivalent of
> > 
> >      $ gvfs-mount cdda://sr0
> 
> (This is just g_file_mount_enclosing_volume)

I've done this and it works great; I've also made sure that eject on the
drive (which is called by eject() on volumes and mounts in the hal
backend too) makes sure to unmount stuff and if any of these fails it
errors out in eject() without ejecting the media.

Useful if one of the mounts are busy and can't be unmounted (more on
that below).

> I don't see an obvious easy solution. It requires some work. The ideal
> thing to match with is an GMountSpec (which is basically the set of
> key-value pairs that define a gvfs mount). For a GDaemonMount this is
> availible in the GMountInfo object.
> 
> Thats a gvfs-specific object though, so we can't make that availible in
> the base GVolume apis. What if we add some new vtable calls in
> GVolumeMonitor like find_volume_for_new_mount() and
> find_mount_for_new_volume() which we have GUnionVolumeMonitor call on
> the installed backends when new, unowned volume/mounts are added.

I think I've come up with a pretty nice solution; see the new function
g_volume_monitor_adopt_orphan_volume() in the gio patch. 

I'm thinking this can also be used for the "favorite" servers like we
have today in gnome-vfs; e.g. we just implement a volume monitor that
creates GVolume objects that matches a list in gconf. Then when the
mount is actually created the volume monitor adopts the DaemonMount in
it's adopt_orphan_mount() function.

Other things in the patch set

 - the CDDA backend with the fixes suggested
 - teach gvfs-mount about unmounting too
 - fix a double free to avoid crash when unmounting
 - a new gvfs-less program (well.. shell script; remember to chmod a+x
   it when svn adding it!)
 - GDrive's can now emit the eject-button signal and this is hooked
   up into hal
 - Allow passing NULL for GMountOperation

The last two points has to do with that it needs to be possible to write
a gnome-volume-manager replacement that only uses gvfs's volume monitor.
In fact, I'm thinking of writing such a thing (and ideally get it into
Nautilus; that way we can nuke a preferences capplet. But that is
probably off-topic here.) mostly because you need it for automounting
file systems using gvfs backends (such as cdda://).

Other thoughts / questions about the API

 - Busy mounts. Right now the cdda:// backend refuses to unmount
   if there are open files on it. I think that's probably the right
   thing to do for any backend. Probably means we need to add flags
   to the unmount() calls; the flags used in Linux, e.g.
    - force unmount
    - lazy unmount
   comes to mind.

 - Further, if a mount can't be unmounted because it's busy (and
   we can't avoid that since we support mounts backed by kernel
   drivers), we probably want something like lsof(8) that Nautilus
   and other stuff can use to put up a dialog showing what apps
   is blocking the unmount. How about a list_open_files() method
   on GMount() that returns an array of
    - process id (is that portable? maybe need an abstraction)
    - icon
    - name
    - etc.
   Would need backend support for this too.

 - Mount options. Do we need this? At least for std file systems
   such as vfat and ntfs people still use it at least judging from
   bug reports. There's actually (very horrible) UI in gnome-mount for
    this crap

     http://people.freedesktop.org/~david/gm-prop/gm-prop2.png

   I don't know; maybe that's specific to each gvfs backend and the hal
   volume monitor. Ideally I want to ditch gnome-mount and just make the
   hal volume monitor call directly into HAL.

   The bigger question is if this is needed for gvfs; I can't think
   of any compelling uses for it save crap like choosing what ssh
   binary to use. Thoughts?

 - g_file_read() and g_file_read_async() probably needs to take a
   GFileReadFlags value. See my other mail about the cdda:// backend
   in this thread.

Thanks for looking at the patches.

       David