Re: [gfvs] cdda backend

[Alexander Larsson <alexl redhat com>]

On Wed, 2007-12-19 at 16:43 -0500, David Zeuthen wrote:

> > > 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.
> 
> Btw, is this how we should implement it? Or did you have anything else
> in mind? I really miss my "public_html @ p.fd.o" link that comes up
> automatically - I use it a lot when doing screen shots to share. 
> 
> I think if we do the feature this way (a gvfs module that links in
> gconf) it can be written in a day or less.

Well, Its certainly a good way to implement connect to server as it
existed before. However, the question is whether this is the best design
in the new gio world. In the pre-gio world there were no "mounts" for a
remote share like your example, so it didn't appear anywhere. Connect to
server was added to sort of make it into a mount. But in the new world
we actually have a mount object, so the need to make it look like there
is a mount is gone. 

However, as you say this is used for another thing, namely easy access
to the share. In the gio world this would mean having a GVolume for the
GMount so that it is easy to mount. At the very least this would involve
changing how connect to server is exposed in the UI and what it is
called. There is also how it relates to bookmarks, which is another
not-quite-the-same way to quickly get to places.

So, I guess the question is, in what way is it better to have
"public_html @ p.fd.o" be a volume than using a bookmark for this?

> > 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?
> 
> That's a good question; it's not supported in the API yet because that
> situation can never happen for the use I wrote it for (cdda:// backend).
> 
> How about a another vtable entry in the GVolumeMonitor
> 
>  gboolean (*request_adoption_of_orphan_mount) (GVolume *adopter,
>                                                GMount *orphan);
> 
> that each implementation can implement? Alternatively we can put this
> method on the GMount interface but that seems wrong since it's only
> something volume monitors would ever want to do.

The volume monitor would recurse over all volumes and call this?

Isn't it better to have something like: 

 GMount * (*find_orphan_mount_for_adoption) (GVolumeMonitor *monitor,
                                             GVolume *adopter)
 

> > > 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. 
> 
> Btw, what are you thoughts about this? I feel strongly this is the right
> thing to do .. otherwise you can lose data.
> 
> I'd even go as far and say that the GVfsBackend base class should
> automatically return G_IO_ERROR_BUSY on unmount() when there are pending
> jobs. At least that would be the default unmount() operation unless you
> override it.

Sure, thats probably a good thing.

> > >  - 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.
> 
> Right, I definitely think we want the dialog to look something like this
> 
>  +-------------------------------------------------------+
>  | Cannot unmount "public_html @ p.fd.o", the following  |
>  | applications are still using it                       |
>  |                                                       |
>  | [icon] GEdit - Secret Stuff.txt                [kill] |
>  | [icon] GEdit - My Diary.txt                    [kill] |
>  | [icon] Tomboy                                  [kill] |
>  | [icon] /bin/cat                                [kill] |
>  |                                                       |
>  |                                              [Cancel] |
>  +-------------------------------------------------------+
>  (dunno what icon / wording to use for [kill] though. Maybe
>   there could also be a [switch to] button that dismisses
>   the dialog (or not?) and takes you to the application)
> 
> Stuffing all these details into the GError doesn't seem at all
> reasonable. So I think a simple g_mount_list_open_files() function is
> what we need (need an async function too).

I really think you are overreacting a bit on this. For regular unix
mounts it is often the case that you get stuck with busy mounts, because
applications with cwd inside the mount keep it open, or due to a dnotify
watch. However, for gvfs mounts the only time something is busy is if
there is actually an ongoing operation like a file copy, loading a file
or something like that. This is far less likely to happen when you
unmount a volume, and its far easier to be aware of this (for instance
you'd have a copy file progress notification icon for the file copy, or
just loading a file in gedit).

Furthermore, since we can make sure we can succeed 100% with a forced
unmount, having a blunt instrument like killing an application that is
doing an operation on the volume seems like the wrong thing to do. I
can't think of any reason you'd want to do this instead of forcefully
unmounting and have the apps doing the operation get an i/o error.

So, adding this chunk of lowlevel weird stuff (where you have to expose
nonportable things like pids, etc) to the supported portable highlevel
API for this very rare case that we can handle well with a force unmount
anyway seems like a bad idea.

Now, maybe you're thinking of using this for unmounting in the hal
backend were we're talking about unmounting unix filesystems, which do
not support a nice force unmount and are more likely to be busy. Yes,
this is more of a problem, but do we really need to expose the
implementation of lsof in the highlevel API. Isn't it better to just
encapsulate the whole thing (dialog/widget + getting the data for it) in
a single call.


> Now, this dialog by itself _could_ offer the option of "Forcibly
> unmount", "Unmount anyway" or whatever but I think that's bad UI. Maybe
> some other desktop than GNOME using gvfs wants that though.
> 
> However, the main reason you want the moral equivalent of "umount -f" is
> for the networking applet (e.g. nm-applet). Basically if a network
> connection (including VPN) goes down you want to forcible unmount any
> network shares that used that connection (and tell the user, maybe via a
> notification, what applications are affected). That implies the
> networking applet needs to know not only whether a GMount is network
> connected but also the name of the endpoint. (See below for API
> proposal.)
> 
> Now, you could propose that the backends themselves should deal with
> networking connections going away (by listening to e.g. NetworkManager
> events or whatever) but I don't think that's reasonable at all. You want
> them to do one thing and do that thing good.

I'm not sure this is actually all that important On a connection drop,
either of these two will happen:
1) The mount is doing i/o on the tcp connection (due to user operations
or just idle chatter). This will cause a fatal i/o error in the gvfs
backend, which will generally lead to the volume being unmounted. This
has to be handled no matter what, as these things could happen for other
reasons.
2) The mount doesn't need to do i/o at the moment, and is fine with the
connection temporarily not being availible. If the connection later
comes back things will continue working as if there was never a problem.
If the user tries to do i/o we're back on case 1.

For case 1, forcibly unmounting doesn't make a difference. For case two
it might actually hurt, since a temporary drop of network will
unnecessarily cause all network mounts to go down. Of course, someone
might reason that its a good thing that you immediately see that the
volume is down without having to realize this when trying to use the
volume.

Anyway, that is a later discussion, because I agree that it makes sense
to have a force unmount flag (for the BUSY case above at the very
least).

> So I think the conclusion is that we do want to pass GMountUnmountFlags
> to unmount() on GMount. With a single G_MOUNT_UNMOUNT_FORCE defined in
> that enumeration. Thoughts?

Agree.
 
> > 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.
> 
> Sounds good to me; no need to complicate the API. However, I think we
> want a remount() method on GMount() that either errors out if remounting
> is not possible, or remounts the mount with the new options. Otherwise
> you'll end up with
> 
> http://people.freedesktop.org/~david/gm-prop/gm-prop2.png
> 
> or similar to tell the user to remount.  That would be bad. Thoughts?

Hmm, for gvfs backends the config storage could be in gconf which
wouldn't need a remount operation for re-reading the config options. On
the other hand, you could use the remount operation for local mounts
too, and maybe some config options affect the actual login phase.

I'll add a remount operation to GMount.

> > 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()...
> 
> How about something simple like 
> 
>  gboolean g_input_stream_set_option (GInputStream *stream, 
>                                      const char   *key, 
>                                      const char   *value,
>                                      GError       *error);
> 
>  char *   g_input_stream_get_option (GInputStream *stream, 
>                                      const char   *key,
>                                      GError       *error);
> 
> with async versions and ditto for output stream too? (Would it make
> sense to have a GStream base class for this and other things?) 

I certainly don't want this on the lowlevel stream apis. This is clearly
not part of the minimal stream abstraction. Yes, many stream types have
specific options, like for instance the various sockopts. However, these
should be exposed in a type safe way in the correct place. So, the
setsockopt() stuff should be part of the GSocketInputStream API.

The question is do we want to add some such specific calls for the file
streams (like GFileInputStream) so that we can do crack in gvfs
backends. At the moment i'd lean towards no. The use case for it in gio
could as well be handled by exposing the files in two places like
"/Track 1.wav" and "/noskip/Track 1.wav", with the /noskip directory
marked as hidden. Both ways are imho hacks, but the directory hack at
least does not affect the public API.

> Another thought: we probably need a way to learn more about the GDrive,
> GVolume and GMount objects than just their names and icons. First, cf.
> the network applet use, we need to know if the mount is networked or not
> and if it is, what hostname is it using. Simple proposal
> 
>  gboolean g_volume_is_local         (GVolume *volume);
>  char *   g_volume_get_foreign_host (GVolume *volume);
> 
>  gboolean g_mount_is_local         (GMount *mount);
>  char *   g_mount_get_foreign_host (GMount *mount);
> 
>  (not sure we need the same on GDrive; I think these are always local)

The whole _is_local() thing is very tricky. I deliberately went with
g_file_is_native() instead of g_file_is_local() due to problems with
is_local in gnome-vfs. Its really hard to define what a "local"
filesystem is. Is a nfs mount to a fast file server in the local network
a remote share? Its likely faster than a local disk, so for purposes of
detecting if it is "fast" (e.g. to decide whether to thumbnail or not)
it is "local", but then again its on the network. 

Things gets even more complicated when one adds things like external usb
disks and file stores on bluetooth devices. These are non-local in some
sense, but not in the traditional tcp/ip sense.

So, I don't think this is a good idea really. At least in this generic
fashion. If we have some specific implementation usecase we can add
something for that specific case maybe. For instance
"g_mount_is_on_network (GMount *mount, ip_address, netmask)" might be
needed in your network applet example. (Although I'm not sure exactly
what it is you want it to do.)

> It would be good to know more precisely the "purpose" of a GDrive,
> GVolume or GMount in question. Right now all we expose is the name and
> icon and I think that not sufficient for applications like F-Spot,
> Rhythmbox, Banshee etc. We ought to export API for this.
> 
> There are (at least) three sources from which we can get this
> information
> 
>  a. if a GVolume stems from the gvfsd-gphoto2 backend chances are
>     there's photos (or music) on the mount; similar if it stems
>     from the cdda:// backend there will be music on it.
> 
>  b. Things like GPS readers, music players, cameras: we have this kind
>     of information from HAL whitelists. The obex-ftp backend will know
>     it's a phone. Things like that.
> 
>  c. One can sniff for things like DCIM/ on the file system itself to
>     infer there's photos on it. Probably other things too (e.g. the
>     iPod has a predictable file system layout).
> 
> So the question is how to export good API with this information and
> where to put it. I think it would be good to separate "we know the
> device is XYZ" from "looks like there is this XYZ kind of files on the
> file system".
> 
> A simple proposal is this

... snipped ...

I think this is an interesting idea. I would use strings as your patch
has them, but instead of using things like "x-drive/optical" in the
actual API i would just do "optical" and then do the x-drive/ mimetype
style mapping when implementing the desktop hacks.

However, I'm not 100% certain exactly what kind of use cases you have
for this. If we know a mount is probably for photos, what would we do
with it? Are photo applications supposed to only list photo mounts when
browsing for files? This strikes me as a risk if things are misdetected,
and a limitation if you want to go outside the bounds. 

It seems this is mainly useful as "hints" to the user. One could use it
to for instance put the right kind of icon for the mount, or an emblem
for a directory in nautilus. 

There is also the issue that these kinds of classifications are weak
(uncertain) and limited (not a lot of detail). For instance, your
G_DRIVE_CLASSIFICATION_OPTICAL_BURN classification is really not enough
for a real cd burner applications. It would need to know much more
details like what type of media it can burn, etc. But we don't want to
keep extending GDrive for this, as that would just turn it into HAL,
which is not the goal. HAL is a fine API itself, and if you need that
kind of lowlevel information you should just use it (and imho n-c-b
should!). 

GVolumeMonitor was designed with the goal of being able to implement a
file manager sidebar and a file selector, including showing references
to existing filesystem "roots" and mounting and handling removable and
remote media. It can use a lot of availible or guessed lowlevel
information to get as good names and icons as possible, but it was never
meant to be a hardware abstraction layer.

If you want to extend this goal, you need to describe exactly what is
the target of the new API. What should you be able to do with it, and
what not. Then we can discuss how this is best achieved.