Re: volume:// uri support



On Thu, 2007-12-13 at 01:17 -0500, David Zeuthen wrote: 
> Hey Alex,
> 
> I've attached some preliminary patches to introduce a new volume:// name
> space. The thinking is that g_file_get_uri() will return a persistent
> URI if a local file is on user-visible media.
> 
> (Emphasis on user-visible. As in the media is visible in the UI, e.g.
> computer://, Nautilus's sidebar and the file chooser). Notably files on
> your rootfs (which we hide in the UI) will still use file:// URI's.)
> 
> For example this snippet
> 
>  GFile *f;
>  f = g_vfs_get_file_for_path ("/media/disk/path/to/file.txt");
>  uri = g_file_get_uri (f);
>  printf ("uri is %s", uri);
> 
>  f = g_vfs_get_file_uri ("volume://name=davidz%20data;uuid=1234/path/to/file.txt");
>  uri = g_file_get_path (f);
>  printf ("path is %s", uri);
> 
> will print
> 
>  volume://name=davidz%20data;uuid=1234/path/to/file.txt
>  /media/disk/path/to/file.txt

Its sort of neat, but just automatically doing this for all file uris
will cause a lot of grief, with apps not being able to load local files
etc. If we really do something like this it really has to be optional
somehow to the apps.

Also, I don't think this should be in libgio itself. Its really
something that would be part of the unix desktop and not something that
makes sense on other platforms. So, this should go into gvfs.

> Implementation-wise there's little to no overhead except that
> 
>  - g_vfs_get_file_from_uri() may block the very first time it's
>    called with a volume:// URI since it needs to construct a
>    GVolumeMonitor object
> 
>  - ditto for g_file_get_uri()
> 
> I don't think these are real problems; we already block for GDaemonFile
> all the time IIRC. Other notes:

On the contrary, this is a huge problem. GFile was very deliberately
designed to be a low-overhead object. Constructing, walking and checking
aspects of GFiles should *never* do I/O itself, only actual I/O
operations should. It is for all means and purposes a refcounted
path/uri string.

Furthermore, you actually construct a GVolumeMonitor in each process.
This will mean all processes start monitoring /etc/mtab (possibly even
polling it if monitoring is not availible). Not only is this heavy, but
there is no guarantee that it will work, as it requires much more of the
app using gio than a mere sync I/O operation, such as a running
mainloop, which may not be true.

Take your g_local_file_get_uri_scheme() change for instance, for each
call it does a stat-walk of its parents to find the mount,
reads /etc/mtab and tries to look for the mountpath. Then you made
g_local_file_has_uri_scheme() call this (adding an extra memory
allocation on this fast path even though this call was added in addition
to get_uri_scheme() just to avoid that). Now, this function is called
all over nautilus to check if things are in the trash, on the desktop or
whatever, and they are expected to be fast non-io operations (in
nautilus *all* i/o operations should be explicitly async). This is gonna
murder performance...

GDaemonFile never fully resolves its mountpoints until an actual i/o
operation is run, so it doesn't run in to this. Or, actually, this is
not strictly true, as I had to add a single get_mount_info_sync() call
to g_daemon_file_get_path to support the fuse stuff. But this is only
done once per gvfs mount, on the i/o first operation (or get_path) and
it never happens for local files. So while non-ideal its not as much of
a problem it could be. 

> - We probably want g_volume_monitor_get() to keep around a ref to
>    the singleton when it's called and release that ref, say, 5 seconds
>    later via a timeout. This is to avoid constructing and tearing down 
>    volume monitors all the time. Thoughts?

There is no guarantee that a timeout will be called at all. For sync
calls there might not even be a mainloop. (Think of for instance a
commandline app like gvfs-ls.) You just can not use the volume monitor
to implement sync I/O calls, it is designed for implementing a UI.

Now, this doesn't mean the entire idea has to be given up. We just need
to realize the fact that creating a volume: uri from a file: uri is an
I/O operation, and we cannot hide it in the non-I/O parts of GFile. Once
you've made this observation everything get much easier.

No changes are necessary for the local file implementation, we just have
to add a single i/o operation to create a persistant reference GFile
object (g_file_make_persistant_reference()?), and implement that. Your
volume: implementation is really not platform native native, and as such
should live in gvfs, not libgio. This means that it has to be
implemented by the gvfs loadable module, like the hal volume monitor.

When handling such a volume: uri there is no need to do any I/O for
construction, get_uri_scheme, get_uri, get_child, etc. All you need to
do is textual path work, and then only at the time an actual I/O
operation happens you resolve the uuid and either call the local file
implementation (via g_vfs_get_local() or return a G_IO_ERROR_NOT_MOUNTED
(and then you can handle requesting the media in g_mount_for_location()
which e.g. nautilus will call automatically).

The only hard thing to implement is g_volume_file_get_path(), which has
the same problem as GDaemonFile has wrt fuse. The only possible solution
I can see is to have a single per-mount cached uuid -> mount path
resolver which is used for all i/o ops, use it in get_path() and hope
that the user did some i/o to the volume before the first call to
get_path()... Again, not ideal, but not disastrous either.

> - Obviously things don't work when e.g. doubleclicking the icon
>    representing /media/disk/path/to/file.txt because gedit isn't
>    yet using gio (on my system). This is because Nautilus passes
>    a volume:// URI. 
> 
> About the latter; there's a couple of options
> 
>  1. Introduce g_file_get_stable_uri() and always make g_file_get_uri()
>     return file:// URI's for local files

Given the above, this is the only workable solution.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]