Re: [gfvs] cdda backend

[Alexander Larsson <alexl redhat com>]

On Wed, 2007-12-19 at 00:58 -0500, David Zeuthen wrote:
> 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

I commited this with some small changes:
Renamed fuse_name to stable_name
Remove unused GVolumeMonitor *union_monitor added to GHalVolumeMonitor.

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

Sweet.

> > 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.

So, can there never be a situation where the GMount gets created before
the corresponding GVolume object is? And in that case, how does the
GVolume locate the mount?

> 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.

While I can sort of see the usecases for these, aren't we sort of
overcomplicating the API here? Is this really required? When and how
would the user force unmount or lazy unmount something? (Or the reverse,
if there is a case for one of these when should we not do it.)

>  - 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.

Now we're getting into really lowlevel bizarro things. I don't think we
should really expose this in a generic API. Can't this just be reported
in the error message for the G_IO_ERROR_BUSY error when unmounting.

I.E. the error dialog would say "Cannot unmount, because firefox (pid
35) is keeping files open on the volume", or "Cannot unmount, because 13
applications are keeping open files on the volume.". Maybe in the last
case we should at least give the name (and pid?) of one application so
that you can make progress on unmount by killing that.

>  - 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.

I do believe we want some form of mount options. For instance, a very
common request is to be able to specify the filename encoding of e.g. a
ftp share. 

However, I don't think the right place for this is as API arguments for
the mount call. Instead i think these are more like preferences for each
mount point, that we can store somewhere and that gvfs automatically
picks up each time you mount that particular share.

I haven't sat down and worked out all the details about this though.

>  - 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.

pasted here:

>  - have an operation on GInputFileStream to set/clear flags.
> 
> Am unsure whether there should be a dedicated SKIP_SCRATCHED_AREA flag
> or whether it should overload a standard one. We should probably also
> default to !SKIP_SCRATCHED_AREA.
> 
> Now, common sense dictates that it's probably not alright to add such
> public API to gio with the cdda:// backend being the only user; let me
> try to think of other users. Seems like we need some equivalent of
> being
> able to pass O_SYNC, O_DIRECT, O_NOATIME? Alex?

GFile is really designed to be a higher level API that can support
loading and saving user documents given the slightly looser semantics
you get from things like the ftp protocol and other weird virtual
filesystem backends. As such I think we should keep the API clear from
things that are very specific to local files.

All the flags you list are things that require very specific behaviour
of the backing filesystem, and that are generally used to implement
application specific i/o rather than handling "user document files". For
such code I really think its preferable to have the app use the actual
POSIX apis, that way there is less weird stuff in the gio API, and the
apps using this use less unnecessary layers and are imho clearer since
you can expect certain semantics when using posix.

The specific case of cdio and how to handle scratches is an example of
how things get weird when you try to use a virtual filesystem to
implement something that is almost just a filesystem, but with some
extra minor backend-specific quirk. Taken each by itself it might not
look that bad, but if you bend the API to take all of them into
consideration you risk that the total result turns into a weird mess.

Now, we could add something to GFileInputStream and GFileOutputStream to
allow you to set options. This is clearly not something that is part of
the requirements for loading and saving documents, and I'm sure it will
be abused in weird ways. However some uses of it might be important
enough that its worth having this wart on the API. I dunno. It seems a
dangerously small step from ioctl()...