Created attachment 289083 [details] [review]
daemon: Move random string generation into shared lib

Random string generation is used in a few different places, so share the
implementation.

Created attachment 289084 [details] [review]
daemon: Move seek type conversion to shared library

Since converting a GSeekType into an lseek type is repeated in a few
places, move it into shared code.

Created attachment 289085 [details] [review]
Add an nfs backend based on libnfs

Add an nfs backend based on libnfs to make userspace mounting and usage
of nfs shares easier.  The backend is written in a single-threaded,
asynchronous style.  Performance measurements show around 60-70 MiB/s
throughput on 1GbE.

To make use of it, simply mount the share with gvfs-mount or Nautilus
with the following syntax:
nfs://host/export/path

Authentication is simple, based on UNIX uid.

Since this is a userspace nfs client, it comes with the caveat that the
mount needs to be exported with "insecure" on Linux (or some equivalent
for other NFS servers) so that it allows connections from port numbers
higher than 1023.  Alternatively, a special capability can be given to
the binary:
sudo setcap 'cap_net_bind_service=+ep' /path/to/executable

Thanks for your work, but the backend doesn't have lot of usage if we can't use privileged ports :-( Capabilities could be set using setcap e.g. in a %post section in spec probably. However I'm not sure about the portability since capabilities are based only on withdrawn of standard and are implemented only on Linux based system. Are the capabilities enabled on all platforms? Also what about filesystems without extended attributes? Also does it work together with SELinux? 

There is already closed bug as WONTFIX for nfs backend from this reasons, see:
Bug 479944.

(In reply to comment #4)
> Thanks for your work, but the backend doesn't have lot of usage if we can't use
> privileged ports :-(

As far as I know, the built in OS X Finder client for NFS has the same limitation.

> Capabilities could be set using setcap e.g. in a %post
> section in spec probably. However I'm not sure about the portability since
> capabilities are based only on withdrawn of standard and are implemented only
> on Linux based system. Are the capabilities enabled on all platforms? Also what
> about filesystems without extended attributes? Also does it work together with
> SELinux? 

Capabilities work on all recent Linux kernels.
Most filesystems that would store binaries support xattrs (e.g. ext*, xfs, btrfs, tmpfs).
Yes, it works with SELinux.
This capabilities approach is already taken by some binaries on a default Fedora install, e.g. /usr/bin/ping.
Regarding other OSes, obviously they won't be able to use the setcap feature but some backends (e.g. mtp) don't even support non-Linux platforms at all.

Regardless, I don't think having the NFS backend adds much maintenance overhead or cause any problems for users if they don't use it.

> 
> There is already closed bug as WONTFIX for nfs backend from this reasons, see:
> Bug 479944.

"No, we don't support NFS in gvfs. Maybe one day, patches are welcome."

So here's a patch :-)

There wasn't a libnfs when bug 479944 was closed. Or maybe people remembered the horror that was the home-made implementation of nfs in gnome-vfs:
https://git.gnome.org/browse/archive/gnome-vfs/tree/modules

I support this new nfs backend that doesn't require us to implement the NFS protocol ourselves!

For what it's worth, I would use this backend, so this is me throwing what little weight behind it.

Thanks for the work Ross.

(In reply to comment #5)
> (In reply to comment #4)
> > Thanks for your work, but the backend doesn't have lot of usage if we can't use
> > privileged ports :-(
> 
> As far as I know, the built in OS X Finder client for NFS has the same
> limitation.
> 
> > Capabilities could be set using setcap e.g. in a %post
> > section in spec probably. However I'm not sure about the portability since
> > capabilities are based only on withdrawn of standard and are implemented only
> > on Linux based system. Are the capabilities enabled on all platforms? Also what
> > about filesystems without extended attributes? Also does it work together with
> > SELinux? 
> 
> Capabilities work on all recent Linux kernels.
> Most filesystems that would store binaries support xattrs (e.g. ext*, xfs,
> btrfs, tmpfs).
> Yes, it works with SELinux.
> This capabilities approach is already taken by some binaries on a default
> Fedora install, e.g. /usr/bin/ping.
> Regarding other OSes, obviously they won't be able to use the setcap feature
> but some backends (e.g. mtp) don't even support non-Linux platforms at all.

Ok, this is enough for me :-) (I support this backend also, I just expressed my fears before...)

I have to check those patches...

Review of attachment 289083 [details] [review]:

It looks good, thanks, just few notes:

gvfs_randomize_string could be used also in link_to_tmp() in metatree.c, couldn't be?

::: daemon/gvfsdaemonutils.c
@@ +225,3 @@
+ **/
+void
+gvfs_randomize_string (char *str, int len)

Please put the second parameter to the newline as it is done on other places in the file.

::: daemon/gvfsdaemonutils.h
@@ +41,3 @@
 						     GFileType         type);
 
+void         gvfs_randomize_string (char *str, int len);

Please fix indentation of params to be same as on previous lines (also put each param on a newline).

Review of attachment 289084 [details] [review]:

Looks good otherwise, thanks!

::: daemon/gvfsbackendafc.c
@@ -1356,2 @@
       g_vfs_job_failed(job, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
                        _("Invalid seek type"));

The seek error messages should be unified and used also in other backends (but this is different bug).

::: daemon/gvfsdaemonutils.h
@@ +43,3 @@
 void         gvfs_randomize_string (char *str, int len);
 
+int          gvfs_seek_type_to_lseek (GSeekType type);

Also fix the indentation...

Review of attachment 289084 [details] [review]:

::: daemon/gvfsbackendsmb.c
@@ +862,3 @@
       g_vfs_job_failed (G_VFS_JOB (job),
+                        G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+                        _("Unsupported seek type"));

Also this whitespace change isn't probably intentional...

@@ +1341,3 @@
       g_vfs_job_failed (G_VFS_JOB (job),
+                        G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+                        _("Unsupported seek type"));

dtto

Review of attachment 289085 [details] [review]:

The backend looks overall good and handles almost all possible functionality. It is lot of work behind, thanks for that! 

I'm still thinking about whether it would be better to use sync libnfs api if there is this option. It would be harder to take care about race conditions, but overall the code could be pretty simplier. (e.g. replace method consists about 18 functions, it is terrifying)

The nfs_source thing should be also checked by somebody else, I haven't any experiences at all with implementing GSource. Would be good if Alex could look, we should notice him about the backend anyway...

There are some comments:

::: daemon/gvfsbackendnfs.c
@@ +1,2 @@
+/* GIO - GLib Input, Output and Streaming Library
+ * 

Remove the white space at the end of line.

@@ +156,3 @@
+      nfs_destroy_context (nfs_source->ctx);
+      g_source_destroy (source);
+      g_source_unref (source);

Are the _destroy and _unref calls necessary? It should be called on finalize...

@@ +194,3 @@
+      nfs_destroy_context (nfs_source->ctx);
+      g_source_destroy (source);
+      g_source_unref (source);

dtto

@@ +232,3 @@
+  for (ptr = export_list; ptr; ptr = ptr->ex_next)
+    {
+      if (strstr (mount_spec->mount_prefix, ptr->ex_dir) == mount_spec->mount_prefix)

Wouldn't be better to use g_str_has_prefix instead?

@@ +239,3 @@
+            {
+              char *s = mount_spec->mount_prefix + this_exportlen;
+              if ((*s == '/' || *s == '\0') && this_exportlen > exportlen)

If pathlen > this_exportlen is true, could be *s == '\0' ever true?

@@ +269,3 @@
+  if (err)
+    {
+      g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);

Maybe would be better to say something about privileged ports in case of EACCESS.

@@ +352,3 @@
+                                    G_IO_ERROR,
+                                    G_IO_ERROR_IS_DIRECTORY,
+                                    _("Can't open directory"));

Shouldn't be g_vfs_job_failed called after nfs_close_async is closed?

@@ +424,3 @@
+
+static char *
+set_type_from_mode (GFileInfo *info, uint64_t mode)

Couldn't we use gvfs_file_info_populate_content_types instead?

@@ +484,3 @@
+  if (g_file_attribute_matcher_matches (matcher,
+                                        G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME))
+    {

Couldn't we use gvfs_file_info_populate_names_as_local instead the following code?

@@ +808,3 @@
+      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

copy_handle_free?

@@ +833,3 @@
+      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

dtto

@@ +854,3 @@
+      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

dtto

@@ +869,3 @@
+      handle->srcfh = data;
+      nfs_create_async (ctx,
+                        handle->dest, O_TRUNC, 0600,

Are the permissions correct? Shouldn't be same as source file has?

@@ +1178,3 @@
+
+      if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
+          !handle->is_symlink && S_ISDIR (handle->mode))

Is this check really needed? It should fail in replace_stat_cb already if it is directory.

@@ +1186,3 @@
+        }
+      else if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
+               (!handle->is_symlink && handle->nlink <= 1))

Wouldn't be better to do the backup only if it is specified by op_job->make_backup (I know it is done also in glocalfile.c, but we are now on network)?

@@ +1298,3 @@
+  else if (err == -EEXIST)
+    {
+      nfs_stat64_async (ctx, op_job->filename, replace_stat_cb, job);

Wouldn't be better to call lstat first and skip stat if it isn't symlink?

@@ +1358,3 @@
+                    O_EXCL,
+                    (flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,
+                    create_cb, job);

What about G_FILE_CREATE_REPLACE_DESTINATION?

@@ +1759,3 @@
+
+static char *
+build_filename (GVfsJobEnumerate *op_job, GFileInfo *item)

Isn't this function redundant?

@@ +1770,3 @@
+  if (handle->readlink_list || handle->symlink_list || handle->access_list)
+    {
+      char *path;

const char *filename = g_file_info_get_name (item);

@@ +1774,3 @@
+      if (handle->readlink_list)
+        {
+          path = build_filename (handle->op_job, handle->readlink_list->data);

path = g_build_filename (op_job->filename, filename, NULL);

@@ +1779,3 @@
+      else if (handle->symlink_list)
+        {
+          path = build_filename (handle->op_job, handle->symlink_list->data);

dtto

@@ +1784,3 @@
+      else if (handle->access_list)
+        {
+          path = build_filename (handle->op_job, handle->access_list->data);

dtto

@@ +1808,3 @@
+      struct nfsdirent *d;
+
+      g_vfs_job_succeeded (job);

Shouldn't this be called as a last command?

@@ +2063,3 @@
+
+      g_file_info_set_is_symlink (op_job->file_info, S_ISLNK (st->nfs_mode));
+      nfs_stat64_async (ctx, op_job->filename, stat_cb, job);

We don't need to call nfs_stat64_async if S_ISLNK (st->nfs_mode) is false.

@@ +2465,3 @@
+      g_vfs_job_failed_literal (G_VFS_JOB (job),
+                                G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+                                _("Not supported"));

G_IO_ERROR_CANT_CREATE_BACKUP should be used instead with message like _("Backups are not yet supported."), see other backends.

::: daemon/gvfsbackendnfs.h
@@ +1,2 @@
+/* GIO - GLib Input, Output and Streaming Library
+ * 

Remove the white space at the end of line.

Alex, could you look at?

Review of attachment 289085 [details] [review]:

Thanks for reviewing this. Here's some responses to your review, I'll give the rest later.

Indeed the async api is a bit more complicated than the sync api; however, the replace implementation would be fairly complex even with the sync api (e.g. you still need to do things like copy the backup file in some cases, etc). See glocalfileoutputstream.c for a long and complex implementation of replace using a sync api.

Nevertheless, with the sync api, you can't easily parallelize the work which limits performance. To achieve line rate in push() and pull(), you need to use the async api which is one of the reasons why people always complain about performance of the smb backend (it uses a sync api).

Also, I don't feel like redoing the implementation :-)

::: daemon/gvfsbackendnfs.c
@@ +1,2 @@
+/* GIO - GLib Input, Output and Streaming Library
+ * 

OK

@@ +156,3 @@
+      nfs_destroy_context (nfs_source->ctx);
+      g_source_destroy (source);
+      g_source_unref (source);

force_unmount is an asynchronous operation. If you don't destroy the source, it isn't removed from the main loop and you end up poll()ing on a bad fd which causes problems until the backend finally exits. Destroying and unrefing the source removes it from the main loop so _prepare() and _dispatch() are never called afterwards.

@@ +194,3 @@
+      nfs_destroy_context (nfs_source->ctx);
+      g_source_destroy (source);
+      g_source_unref (source);

As above.

@@ +232,3 @@
+  for (ptr = export_list; ptr; ptr = ptr->ex_next)
+    {
+      if (strstr (mount_spec->mount_prefix, ptr->ex_dir) == mount_spec->mount_prefix)

OK

@@ +1358,3 @@
+                    O_EXCL,
+                    (flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,
+                    create_cb, job);

It is not relevant for try_create because create never replaces the destination.

@@ +1808,3 @@
+      struct nfsdirent *d;
+
+      g_vfs_job_succeeded (job);

No. With enumerate, you indicate partial success by calling g_vfs_job_succeeded, then call g_vfs_job_enumerate_done at the end. See the smb backend for an example.

@@ +2465,3 @@
+      g_vfs_job_failed_literal (G_VFS_JOB (job),
+                                G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+                                _("Not supported"));

No, it fails with G_IO_ERROR_NOT_SUPPORTED so that it falls back to the default move implementation (i.e. copy+delete).

Review of attachment 289085 [details] [review]:

::: daemon/gvfsbackendnfs.c
@@ +239,3 @@
+            {
+              char *s = mount_spec->mount_prefix + this_exportlen;
+              if ((*s == '/' || *s == '\0') && this_exportlen > exportlen)

No, good catch!

@@ +269,3 @@
+  if (err)
+    {
+      g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);

OK

@@ +352,3 @@
+                                    G_IO_ERROR,
+                                    G_IO_ERROR_IS_DIRECTORY,
+                                    _("Can't open directory"));

It's not necessary in this case because reporting the error to the user is unrelated to whether the file handle is open or not. This allows us to save having (yet another) callback just to report the error to the user.

@@ +424,3 @@
+
+static char *
+set_type_from_mode (GFileInfo *info, uint64_t mode)

set_type_from_mode is called from query_info_on_{read,write} where you don't have a filename but gvfs_file_info_populate_content_types requires a filename. Additionally, the doc for it says it should not be used for directories.

Perhaps at a later point we can come up with some utility function which is useful for multiple backends (gvfs_file_info_populate_content_types is not used at all).

@@ +484,3 @@
+  if (g_file_attribute_matcher_matches (matcher,
+                                        G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME))
+    {

OK

@@ +808,3 @@
+      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

OK

@@ +833,3 @@
+      nfs_close_async (ctx, handle->destfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

OK

@@ +854,3 @@
+      nfs_close_async (ctx, handle->srcfh, null_cb, NULL);
+      handle->cb (FALSE, handle->private_data);
+      g_slice_free (CopyHandle, handle);

OK

@@ +869,3 @@
+      handle->srcfh = data;
+      nfs_create_async (ctx,
+                        handle->dest, O_TRUNC, 0600,

I was setting the perms afterwards in replace_backup_cb but there's no reason not to do it here instead.

I also changed a few places in the backup code to not fail if permission is denied (e.g. setting ownership on the backup file is not essential so it shouldn't cause the whole operation to fail).

@@ +1178,3 @@
+
+      if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) &&
+          !handle->is_symlink && S_ISDIR (handle->mode))

Yes. In replace_stat_cb, there is a possibility that you can replace a symlink to a directory if REPLACE_DEST is specified so it only fails if !REPLACE_DEST and it is a directory.

In the above, now that we know whether it is a symlink or not, we catch the case where REPLACE_DEST is specified, it is not a symlink, and it is a directory.

(I redid this part anyway based on your subsequent comment.)

@@ +1186,3 @@
+        }
+      else if ((op_job->flags & G_FILE_CREATE_REPLACE_DESTINATION) ||
+               (!handle->is_symlink && handle->nlink <= 1))

The backup is only done if op_job->make_backup is true (see replace_temp_cb and close_write_cb). Writing first to a temp file anyway is just good practice so that the replacement of the destination file is more "atomic". This is the way it is done in the afp, smb and sftp backends.

@@ +1298,3 @@
+  else if (err == -EEXIST)
+    {
+      nfs_stat64_async (ctx, op_job->filename, replace_stat_cb, job);

Yes it would :-)

@@ +1759,3 @@
+
+static char *
+build_filename (GVfsJobEnumerate *op_job, GFileInfo *item)

Sort of. See below...

@@ +1770,3 @@
+  if (handle->readlink_list || handle->symlink_list || handle->access_list)
+    {
+      char *path;

You can't do this, because "item" is different depending on which if statement is true. Anyway, I changed it, repeating a variant of the following lines in each if:
const char *filename = g_file_info_get_name (handle->readlink_list->data);
path = g_build_filename (handle->op_job->filename, filename, NULL);

@@ +1774,3 @@
+      if (handle->readlink_list)
+        {
+          path = build_filename (handle->op_job, handle->readlink_list->data);

As above.

@@ +1779,3 @@
+      else if (handle->symlink_list)
+        {
+          path = build_filename (handle->op_job, handle->symlink_list->data);

As above.

@@ +1784,3 @@
+      else if (handle->access_list)
+        {
+          path = build_filename (handle->op_job, handle->access_list->data);

As above.

@@ +2063,3 @@
+
+      g_file_info_set_is_symlink (op_job->file_info, S_ISLNK (st->nfs_mode));
+      nfs_stat64_async (ctx, op_job->filename, stat_cb, job);

OK

::: daemon/gvfsbackendnfs.h
@@ +1,2 @@
+/* GIO - GLib Input, Output and Streaming Library
+ * 

OK

Created attachment 291328 [details] [review]
Add an nfs backend based on libnfs

Add an nfs backend based on libnfs to make userspace mounting and usage
of nfs shares easier.  The backend is written in a single-threaded,
asynchronous style.  Performance measurements show around 60-70 MiB/s
throughput on 1GbE.

To make use of it, simply mount the share with gvfs-mount or Nautilus
with the following syntax:
nfs://host/export/path

Authentication is simple, based on UNIX uid.

Since this is a userspace nfs client, it comes with the caveat that the
mount needs to be exported with "insecure" on Linux (or some equivalent
for other NFS servers) so that it allows connections from port numbers
higher than 1023.  Alternatively, a special capability can be given to
the binary:
sudo setcap 'cap_net_bind_service=+ep' /path/to/executable

OK, I think I've addressed all the review comments (thanks!).  I've posted a new version of the nfs backend patch.

To see just the changes, have a look at https://git.gnome.org/browse/gvfs/log/?h=nfs

libnfs 1.9.6 has been released with the required API changes so I have update the branch to depend on this version.

Review of attachment 291328 [details] [review]:

Overall this seems like some pretty good work, and we should get it in.
I didn't do a super thorough review of the details, but i gave it a glance
over and here are some highlevel comments.

::: daemon/gvfsbackendnfs.c
@@ +108,3 @@
+event_to_condition (int events)
+{
+  GIOCondition condition = 0;

These are actually not necessary, as glib configure picks up the system definition of these during configure and uses the same values.
See GLIB_SYSDEP_POLL*.

@@ +230,3 @@
+  /* Find the longest matching mount. E.g. if the given mount_prefix is
+   * /some/long/path * and there exist two mounts, /some and /some/long, match
+   * against the latter. */

I worry a bit about this.
In the case of the above, if we first access /some and then later /some/long then the local cache would mean we use the first mountinfo to access files in /some/long.
However, if we looked in /some/long first and later /some, then we will get two different mounts in a confusing way.

I don't know the exact NFS semantics here, does the same full path *always* point to the same file? If so we should always use the shortest match instead.
Of course, that may have different mount options, so if the mount there fails we would have to try the inner one.

Does this seem right to you?

@@ +409,3 @@
+      GVfsJobRead *op_job = G_VFS_JOB_READ (job);
+
+      memcpy (op_job->buffer, data, err);

This copy seems a bit ridicolous. Is there no way to steal the buffer, or have libnfs write into a buffer we allocated.

@@ +931,3 @@
+                        op_job->filename,
+                        O_TRUNC,
+                        (op_job->flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,

This is a bit weird, as the umask is from the backend startup and unaffected by what umask the *client* currently has set.
So, once mounted you can never create files with a different umask?
Not sure if this is the ideal behaviour, although it is kind of a fringe detail.

Review of attachment 291328 [details] [review]:

Thanks for the review.  I need to try and work out something sane for when there are multiple exported mount points in a hierarchy.

::: daemon/gvfsbackendnfs.c
@@ +108,3 @@
+event_to_condition (int events)
+{
+  GIOCondition condition = 0;

OK, that removes some cruft :-)

@@ +230,3 @@
+  /* Find the longest matching mount. E.g. if the given mount_prefix is
+   * /some/long/path * and there exist two mounts, /some and /some/long, match
+   * against the latter. */

Yes, this looks like it will cause problems... I think I may ask libnfs upstream about the best way of solving this.  There may be trouble accessing a filesystem hierarchy with multiple mounts in it. E.g. if the server exports /dir and something else is mounted on /dir/mnt, then the client is expected to mount both /dir and /dir/mnt separately. Could this be handled with shadow mounts in gvfs?

@@ +409,3 @@
+      GVfsJobRead *op_job = G_VFS_JOB_READ (job);
+
+      memcpy (op_job->buffer, data, err);

Yes indeed, it is a bit silly. With the current API, this is the best we can do. I will try get something upstream to fix this; however, at the moment, I don't think this should block anything moving forward.

Mostly, the latency or bandwidth of the connection is by far the limiting factor and one extra copy doesn't matter *that* much.

@@ +931,3 @@
+                        op_job->filename,
+                        O_TRUNC,
+                        (op_job->flags & G_FILE_CREATE_PRIVATE ? 0600 : 0666) & ~op_backend->umask,

How can I get the client umask?

I thought it would be better to use the user's default umask inherited through the backend's environment than some constant like 002 which the user can't configure at all.

I don't know of a better way of doing this...

So Ronnie Sahlberg (upstream) says that when it comes to overlapping nfs mounts, you need to match against the longest mountpoint.

Suppose a server exports /data and /data/tmp. With a normal nfs client, to access /data/tmp/file, the client would then need to mount /data and /data/tmp otherwise /data/tmp will just be an empty directory.

NFS guarantees that you can only see a file through a single export.

The problem is that if we mount /data and then use g_file_get_child(file, "tmp") on that we will not reach the /data/tmp mount if its not already mounted.

What if we mount *all* unmounted submounts of the shortest match?

Ronnie has improved the latest libnfs master so that it automatically handles any submounts transparently to the application, all gvfs needs to do is mount the shortest matching one (which it does already). Awesome!

I discussed with Alex on IRC about umask handling and there's currently no way to do it any better.

I don't think there're any more outstanding issues currently.

(In reply to comment #23)
> I don't think there're any more outstanding issues currently.

What about the first note in the Comment 19 (about event_to_condition stuff)?

Whoops, forgot about that one.

I have pushed a fix to the nfs branch for that. Thanks

As I promised I've looked on the code after your fixes. The fixes looks good, but there are some another comments:

diff --git a/daemon/gvfsbackendnfs.c b/daemon/gvfsbackendnfs.c
index 845576f..def21fe 100644
--- a/daemon/gvfsbackendnfs.c
+++ b/daemon/gvfsbackendnfs.c
@@ -91,6 +91,7 @@ g_vfs_backend_nfs_finalize (GObject *object)
 {
   GVfsBackendNfs *backend = G_VFS_BACKEND_NFS (object);
 
+// Wouldn't be better to do following code in unmount, to be sure that _prepare() and _dispatch() will never be called?
   if (backend->source)
     {
       g_source_destroy (backend->source);
@@ -122,6 +123,7 @@ nfs_source_prepare (GSource *source, gint *timeout)
       g_vfs_backend_force_unmount (G_VFS_BACKEND (backend));
       nfs_destroy_context (nfs_source->ctx);
       backend->ctx = NULL;
+// maybe would be cleaner to implement nfs_source_finalize and clear the context there
       g_source_destroy (source);
       g_source_unref (source);
       backend->source = NULL;
@@ -159,6 +161,7 @@ nfs_source_dispatch (GSource *source, GSourceFunc callback, gpointer user_data)
       g_vfs_backend_force_unmount (G_VFS_BACKEND (backend));
       nfs_destroy_context (nfs_source->ctx);
       backend->ctx = NULL;
+// dtto
       g_source_destroy (source);
       g_source_unref (source);
       backend->source = NULL;
@@ -197,6 +200,7 @@ do_mount (GVfsBackend *backend,
   /* Find the longest matching mount. E.g. if the given mount_prefix is
    * /some/long/path * and there exist two mounts, /some and /some/long, match
    * against the latter. */
+// didn't you say that libnfs is handling submounts transparently, isn't this code redundant in this case?
   for (ptr = export_list; ptr; ptr = ptr->ex_next)
     {
       if (g_str_has_prefix (mount_spec->mount_prefix, ptr->ex_dir))
@@ -213,6 +217,8 @@ do_mount (GVfsBackend *backend,
                 }
             }
           else if (this_exportlen > exportlen)
+// would be good to add comment that this_exportlen == pathlen (mount_prefix == ex_dir)
+// maybe this condition statement is redundant and simple "else" would be enough
             {
               export = ptr->ex_dir;
               exportlen = this_exportlen;
@@ -246,6 +252,7 @@ do_mount (GVfsBackend *backend,
         {
           g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);
         }
+// g_free (export);
       return;
     }
 
@@ -276,7 +283,7 @@ do_mount (GVfsBackend *backend,
   g_mount_spec_set_mount_prefix (nfs_mount_spec, export);
   g_vfs_backend_set_mount_spec (backend, nfs_mount_spec);
   g_mount_spec_unref (nfs_mount_spec);
-
+// g_free (export);
   /* cache the process's umask for later */
   op_backend->umask = umask (0);
   umask (op_backend->umask);
@@ -400,6 +407,7 @@ try_read (GVfsBackend *backend,
 }
 
 static char *
+// static const char *?
 set_type_from_mode (GFileInfo *info, uint64_t mode)
 {
   GFileType type = G_FILE_TYPE_UNKNOWN;
@@ -1596,6 +1604,7 @@ typedef struct
   GSList *readlink_list;
   GSList *symlink_list;
   GSList *access_list;
+// I think one list would be enough for it, you can chain all operations for each entry... 
   gboolean requires_access;
   GVfsJobEnumerate *op_job;
 } EnumerateHandle;
@@ -1712,24 +1721,27 @@ enumerate_continue (EnumerateHandle *handle, struct nfs_context *ctx)
           const char *filename = g_file_info_get_name (handle->readlink_list->data);
           path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_readlink_async (ctx, path, enumerate_readlink_cb, handle);
+// g_free (path);?
         }
       else if (handle->symlink_list)
         {
           const char *filename = g_file_info_get_name (handle->symlink_list->data);
           path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_stat64_async (ctx, path, enumerate_stat_cb, handle);
+// g_free (path);?
         }
       else if (handle->access_list)
         {
           const char *filename = g_file_info_get_name (handle->access_list->data);
           path = g_build_filename (handle->op_job->filename, filename, NULL);
           nfs_access2_async (ctx, path, enumerate_access_cb, handle);
+// g_free (path);?
         }
       g_free (path);
     }
   else
     {
+// g_slice_free (EnumerateHandle, handle);?
       g_vfs_job_enumerate_done (handle->op_job);
     }
 }
@@ -1856,6 +1868,7 @@ enumerate_cb (int err, struct nfs_context *ctx, void *data, void *private_data)
     }
   else
     {
+// g_slice_free (EnumerateHandle, handle);?
       g_vfs_job_failed_from_errno (job, -err);
     }
 }
@@ -1895,6 +1908,7 @@ stat_access_cb (int err,
       g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_READ, err & R_OK);
       g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE, err & W_OK);
       g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, err & X_OK);
+// I'm still thinking about if we shouldn't rename "err" to "ret", or something like that, because it is weird if we are using "err" as a value...
     }
 
   if (g_file_attribute_matcher_matches (op_job->attribute_matcher,
@@ -2413,7 +2427,7 @@ try_move (GVfsBackend *backend,
                                 _("Not supported"));
       return TRUE;
     }
-
+// What about G_FILE_COPY_NOFOLLOW_SYMLINKS?
   nfs_lstat64_async (op_backend->ctx, source, move_stat_source_cb, job);
 
   return TRUE;

I want to look on replace code tomorrow yet again...

(In reply to comment #26)
> @@ -1712,24 +1721,27 @@ enumerate_continue (EnumerateHandle *handle, struct
nfs_context *ctx)
>            path = g_build_filename (handle->op_job->filename, filename, NULL);
>            nfs_access2_async (ctx, path, enumerate_access_cb, handle);
> +// g_free (path);?
>          }
>        g_free (path);

Sorry, this isn't obviously problem...

Thanks for the review!

(In reply to comment #26)
> As I promised I've looked on the code after your fixes. The fixes looks good,
> but there are some another comments:
> 
> diff --git a/daemon/gvfsbackendnfs.c b/daemon/gvfsbackendnfs.c
> index 845576f..def21fe 100644
> --- a/daemon/gvfsbackendnfs.c
> +++ b/daemon/gvfsbackendnfs.c
> @@ -91,6 +91,7 @@ g_vfs_backend_nfs_finalize (GObject *object)
>  {
>    GVfsBackendNfs *backend = G_VFS_BACKEND_NFS (object);
> 
> +// Wouldn't be better to do following code in unmount, to be sure that
> _prepare() and _dispatch() will never be called?

Done.

>    if (backend->source)
>      {
>        g_source_destroy (backend->source);
> @@ -122,6 +123,7 @@ nfs_source_prepare (GSource *source, gint *timeout)
>        g_vfs_backend_force_unmount (G_VFS_BACKEND (backend));
>        nfs_destroy_context (nfs_source->ctx);
>        backend->ctx = NULL;
> +// maybe would be cleaner to implement nfs_source_finalize and clear the
> context there

Done.

>        g_source_destroy (source);
>        g_source_unref (source);
>        backend->source = NULL;
> @@ -159,6 +161,7 @@ nfs_source_dispatch (GSource *source, GSourceFunc callback,
> gpointer user_data)
>        g_vfs_backend_force_unmount (G_VFS_BACKEND (backend));
>        nfs_destroy_context (nfs_source->ctx);
>        backend->ctx = NULL;
> +// dtto

Done.

>        g_source_destroy (source);
>        g_source_unref (source);
>        backend->source = NULL;
> @@ -197,6 +200,7 @@ do_mount (GVfsBackend *backend,
>    /* Find the longest matching mount. E.g. if the given mount_prefix is
>     * /some/long/path * and there exist two mounts, /some and /some/long, match
>     * against the latter. */
> +// didn't you say that libnfs is handling submounts transparently, isn't this
> code redundant in this case?

It handles submounts transparently, but still offers them as available mount points. So the code needs to look for the shortest matching mount point so that libnfs handles the submounts within it transparently. I've updated it to use the shortest matching mount.

>    for (ptr = export_list; ptr; ptr = ptr->ex_next)
>      {
>        if (g_str_has_prefix (mount_spec->mount_prefix, ptr->ex_dir))
> @@ -213,6 +217,8 @@ do_mount (GVfsBackend *backend,
>                  }
>              }
>            else if (this_exportlen > exportlen)
> +// would be good to add comment that this_exportlen == pathlen (mount_prefix
> == ex_dir)
> +// maybe this condition statement is redundant and simple "else" would be
> enough

I've added some comments.  That condition is needed so that it chooses the shortest matching mount.

>              {
>                export = ptr->ex_dir;
>                exportlen = this_exportlen;
> @@ -246,6 +252,7 @@ do_mount (GVfsBackend *backend,
>          {
>            g_vfs_job_failed_from_errno (G_VFS_JOB (job), -err);
>          }
> +// g_free (export);

Done.

>        return;
>      }
> 
> @@ -276,7 +283,7 @@ do_mount (GVfsBackend *backend,
>    g_mount_spec_set_mount_prefix (nfs_mount_spec, export);
>    g_vfs_backend_set_mount_spec (backend, nfs_mount_spec);
>    g_mount_spec_unref (nfs_mount_spec);
> -
> +// g_free (export);

Done.

>    /* cache the process's umask for later */
>    op_backend->umask = umask (0);
>    umask (op_backend->umask);
> @@ -400,6 +407,7 @@ try_read (GVfsBackend *backend,
>  }
> 
>  static char *
> +// static const char *?

Done.

>  set_type_from_mode (GFileInfo *info, uint64_t mode)
>  {
>    GFileType type = G_FILE_TYPE_UNKNOWN;
> @@ -1596,6 +1604,7 @@ typedef struct
>    GSList *readlink_list;
>    GSList *symlink_list;
>    GSList *access_list;
> +// I think one list would be enough for it, you can chain all operations for
> each entry... 

I think this would be harder, because not all operations are required for each entry so you'd need to keep extra state per operation to know what operation to start with.

>    gboolean requires_access;
>    GVfsJobEnumerate *op_job;
>  } EnumerateHandle;
> @@ -1712,24 +1721,27 @@ enumerate_continue (EnumerateHandle *handle, struct
> nfs_context *ctx)
>            const char *filename = g_file_info_get_name
> (handle->readlink_list->data);
>            path = g_build_filename (handle->op_job->filename, filename, NULL);
>            nfs_readlink_async (ctx, path, enumerate_readlink_cb, handle);
> +// g_free (path);?
>          }
>        else if (handle->symlink_list)
>          {
>            const char *filename = g_file_info_get_name
> (handle->symlink_list->data);
>            path = g_build_filename (handle->op_job->filename, filename, NULL);
>            nfs_stat64_async (ctx, path, enumerate_stat_cb, handle);
> +// g_free (path);?
>          }
>        else if (handle->access_list)
>          {
>            const char *filename = g_file_info_get_name
> (handle->access_list->data);
>            path = g_build_filename (handle->op_job->filename, filename, NULL);
>            nfs_access2_async (ctx, path, enumerate_access_cb, handle);
> +// g_free (path);?
>          }
>        g_free (path);
>      }
>    else
>      {
> +// g_slice_free (EnumerateHandle, handle);?

Done.

>        g_vfs_job_enumerate_done (handle->op_job);
>      }
>  }
> @@ -1856,6 +1868,7 @@ enumerate_cb (int err, struct nfs_context *ctx, void
> *data, void *private_data)
>      }
>    else
>      {
> +// g_slice_free (EnumerateHandle, handle);?

Done.

>        g_vfs_job_failed_from_errno (job, -err);
>      }
>  }
> @@ -1895,6 +1908,7 @@ stat_access_cb (int err,
>        g_file_info_set_attribute_boolean (info,
> G_FILE_ATTRIBUTE_ACCESS_CAN_READ, err & R_OK);
>        g_file_info_set_attribute_boolean (info,
> G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE, err & W_OK);
>        g_file_info_set_attribute_boolean (info,
> G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, err & X_OK);
> +// I'm still thinking about if we shouldn't rename "err" to "ret", or
> something like that, because it is weird if we are using "err" as a value...

I'd prefer keeping it as "err" for consistency with all the other callback functions and the upstream declaration:
https://github.com/sahlberg/libnfs/blob/master/include/nfsc/libnfs.h#L103

>      }
> 
>    if (g_file_attribute_matcher_matches (op_job->attribute_matcher,
> @@ -2413,7 +2427,7 @@ try_move (GVfsBackend *backend,
>                                  _("Not supported"));
>        return TRUE;
>      }
> -
> +// What about G_FILE_COPY_NOFOLLOW_SYMLINKS?
>    nfs_lstat64_async (op_backend->ctx, source, move_stat_source_cb, job);
> 

As with local files, G_FILE_COPY_NOFOLLOW_SYMLINKS is ignored for g_file_move (it would be pretty weird to move a file through a symlink...). The GIO docs should probably be updated.

>
>   return TRUE;
>
> I want to look on replace code tomorrow yet again...

I have added a comment which describes the basic flow of the replace code.

Thanks for the fixes!

(In reply to comment #28)
> > @@ -197,6 +200,7 @@ do_mount (GVfsBackend *backend,
> >    /* Find the longest matching mount. E.g. if the given mount_prefix is
> >     * /some/long/path * and there exist two mounts, /some and /some/long, match
> >     * against the latter. */
> > +// didn't you say that libnfs is handling submounts transparently, isn't this
> > code redundant in this case?
> 
> It handles submounts transparently, but still offers them as available mount
> points. So the code needs to look for the shortest matching mount point so that
> libnfs handles the submounts within it transparently. I've updated it to use
> the shortest matching mount.

I found bug probably (maybe in libnfs, but not debugging it):

$ less /etc/exports
/path/a localhost(ro,sync)
/path/a/b localhost(rw,sync)

$ gvfs-mount nfs://localhost/path/a/b
$ gvfs-mount -l | grep nfs
Mount(0): a on localhost -> nfs://localhost/path/a
$ gvfs-ls nfs://localhost/path/a
b
$ gvfs-ls nfs://localhost/path/a/b
Error: No such file or directory

When I unexport /path/a, gvfs-ls nfs://localhost/path/a/b works correctly...

> >    for (ptr = export_list; ptr; ptr = ptr->ex_next)
> >      {
> >        if (g_str_has_prefix (mount_spec->mount_prefix, ptr->ex_dir))
> > @@ -213,6 +217,8 @@ do_mount (GVfsBackend *backend,
> >                  }
> >              }
> >            else if (this_exportlen > exportlen)
> > +// would be good to add comment that this_exportlen == pathlen (mount_prefix
> > == ex_dir)
> > +// maybe this condition statement is redundant and simple "else" would be
> > enough
> 
> I've added some comments.  That condition is needed so that it chooses the
> shortest matching mount.

Makes sense now with the fixes. 

> >  set_type_from_mode (GFileInfo *info, uint64_t mode)
> >  {
> >    GFileType type = G_FILE_TYPE_UNKNOWN;
> > @@ -1596,6 +1604,7 @@ typedef struct
> >    GSList *readlink_list;
> >    GSList *symlink_list;
> >    GSList *access_list;
> > +// I think one list would be enough for it, you can chain all operations for
> > each entry... 
> 
> I think this would be harder, because not all operations are required for each
> entry so you'd need to keep extra state per operation to know what operation to
> start with.

I see, I missed this before, hmm still it could be done with one or two lists, but ok, don't care about...

> > @@ -1895,6 +1908,7 @@ stat_access_cb (int err,
> >        g_file_info_set_attribute_boolean (info,
> > G_FILE_ATTRIBUTE_ACCESS_CAN_READ, err & R_OK);
> >        g_file_info_set_attribute_boolean (info,
> > G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE, err & W_OK);
> >        g_file_info_set_attribute_boolean (info,
> > G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, err & X_OK);
> > +// I'm still thinking about if we shouldn't rename "err" to "ret", or
> > something like that, because it is weird if we are using "err" as a value...
> 
> I'd prefer keeping it as "err" for consistency with all the other callback
> functions and the upstream declaration:
> https://github.com/sahlberg/libnfs/blob/master/include/nfsc/libnfs.h#L103

Ok :-)

> 
> >      }
> > 
> >    if (g_file_attribute_matcher_matches (op_job->attribute_matcher,
> > @@ -2413,7 +2427,7 @@ try_move (GVfsBackend *backend,
> >                                  _("Not supported"));
> >        return TRUE;
> >      }
> > -
> > +// What about G_FILE_COPY_NOFOLLOW_SYMLINKS?
> >    nfs_lstat64_async (op_backend->ctx, source, move_stat_source_cb, job);
> > 
> 
> As with local files, G_FILE_COPY_NOFOLLOW_SYMLINKS is ignored for g_file_move
> (it would be pretty weird to move a file through a symlink...). The GIO docs
> should probably be updated.

Ok, https://bugzilla.gnome.org/show_bug.cgi?id=743848

> >
> >   return TRUE;
> >
> > I want to look on replace code tomorrow yet again...
> 
> I have added a comment which describes the basic flow of the replace code.

Super, maybe would be good to add a note that it is almost identical code as for local files, just async.

(In reply to comment #29)
> Thanks for the fixes!
> 
> (In reply to comment #28)
> > > @@ -197,6 +200,7 @@ do_mount (GVfsBackend *backend,
> > >    /* Find the longest matching mount. E.g. if the given mount_prefix is
> > >     * /some/long/path * and there exist two mounts, /some and /some/long, match
> > >     * against the latter. */
> > > +// didn't you say that libnfs is handling submounts transparently, isn't this
> > > code redundant in this case?
> > 
> > It handles submounts transparently, but still offers them as available mount
> > points. So the code needs to look for the shortest matching mount point so that
> > libnfs handles the submounts within it transparently. I've updated it to use
> > the shortest matching mount.
> 
> I found bug probably (maybe in libnfs, but not debugging it):
> 
> $ less /etc/exports
> /path/a localhost(ro,sync)
> /path/a/b localhost(rw,sync)
> 
> $ gvfs-mount nfs://localhost/path/a/b
> $ gvfs-mount -l | grep nfs
> Mount(0): a on localhost -> nfs://localhost/path/a
> $ gvfs-ls nfs://localhost/path/a
> b
> $ gvfs-ls nfs://localhost/path/a/b
> Error: No such file or directory
> 
> When I unexport /path/a, gvfs-ls nfs://localhost/path/a/b works correctly...

This should be fixed in the latest libnfs code.

> 
> > >    for (ptr = export_list; ptr; ptr = ptr->ex_next)
> > >      {
> > >        if (g_str_has_prefix (mount_spec->mount_prefix, ptr->ex_dir))
> > > @@ -213,6 +217,8 @@ do_mount (GVfsBackend *backend,
> > >                  }
> > >              }
> > >            else if (this_exportlen > exportlen)
> > > +// would be good to add comment that this_exportlen == pathlen (mount_prefix
> > > == ex_dir)
> > > +// maybe this condition statement is redundant and simple "else" would be
> > > enough
> > 
> > I've added some comments.  That condition is needed so that it chooses the
> > shortest matching mount.
> 
> Makes sense now with the fixes. 
> 
> > >  set_type_from_mode (GFileInfo *info, uint64_t mode)
> > >  {
> > >    GFileType type = G_FILE_TYPE_UNKNOWN;
> > > @@ -1596,6 +1604,7 @@ typedef struct
> > >    GSList *readlink_list;
> > >    GSList *symlink_list;
> > >    GSList *access_list;
> > > +// I think one list would be enough for it, you can chain all operations for
> > > each entry... 
> > 
> > I think this would be harder, because not all operations are required for each
> > entry so you'd need to keep extra state per operation to know what operation to
> > start with.
> 
> I see, I missed this before, hmm still it could be done with one or two lists,
> but ok, don't care about...
> 
> > > @@ -1895,6 +1908,7 @@ stat_access_cb (int err,
> > >        g_file_info_set_attribute_boolean (info,
> > > G_FILE_ATTRIBUTE_ACCESS_CAN_READ, err & R_OK);
> > >        g_file_info_set_attribute_boolean (info,
> > > G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE, err & W_OK);
> > >        g_file_info_set_attribute_boolean (info,
> > > G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, err & X_OK);
> > > +// I'm still thinking about if we shouldn't rename "err" to "ret", or
> > > something like that, because it is weird if we are using "err" as a value...
> > 
> > I'd prefer keeping it as "err" for consistency with all the other callback
> > functions and the upstream declaration:
> > https://github.com/sahlberg/libnfs/blob/master/include/nfsc/libnfs.h#L103
> 
> Ok :-)
> 
> > 
> > >      }
> > > 
> > >    if (g_file_attribute_matcher_matches (op_job->attribute_matcher,
> > > @@ -2413,7 +2427,7 @@ try_move (GVfsBackend *backend,
> > >                                  _("Not supported"));
> > >        return TRUE;
> > >      }
> > > -
> > > +// What about G_FILE_COPY_NOFOLLOW_SYMLINKS?
> > >    nfs_lstat64_async (op_backend->ctx, source, move_stat_source_cb, job);
> > > 
> > 
> > As with local files, G_FILE_COPY_NOFOLLOW_SYMLINKS is ignored for g_file_move
> > (it would be pretty weird to move a file through a symlink...). The GIO docs
> > should probably be updated.
> 
> Ok, https://bugzilla.gnome.org/show_bug.cgi?id=743848
> 
> > >
> > >   return TRUE;
> > >
> > > I want to look on replace code tomorrow yet again...
> > 
> > I have added a comment which describes the basic flow of the replace code.
> 
> Super, maybe would be good to add a note that it is almost identical code as
> for local files, just async.

Done.

(In reply to comment #30)
> (In reply to comment #29)
> > (In reply to comment #28)
> > I found bug probably (maybe in libnfs, but not debugging it):
> > 
> > $ less /etc/exports
> > /path/a localhost(ro,sync)
> > /path/a/b localhost(rw,sync)
> > 
> > $ gvfs-mount nfs://localhost/path/a/b
> > $ gvfs-mount -l | grep nfs
> > Mount(0): a on localhost -> nfs://localhost/path/a
> > $ gvfs-ls nfs://localhost/path/a
> > b
> > $ gvfs-ls nfs://localhost/path/a/b
> > Error: No such file or directory
> > 
> > When I unexport /path/a, gvfs-ls nfs://localhost/path/a/b works correctly...
> 
> This should be fixed in the latest libnfs code.

You are right, seems to be fixed (I probably did not finish rebuilding before...).

It's fixed because I fixed it yesterday :-)

Ah, ok, thanks :-) 

When I was playing with nfs, I realized that Nautilus offers deleting/renaming/trashing/cutting even if a nfs share is readonly, so probably following attributes should be set to avoid this:

G_FILE_ATTRIBUTE_ACCESS_CAN_TRASH
G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE
G_FILE_ATTRIBUTE_ACCESS_CAN_RENAME

That's hopefully my last notice ;-)

(In reply to comment #33)
> Ah, ok, thanks :-) 
> 
> When I was playing with nfs, I realized that Nautilus offers
> deleting/renaming/trashing/cutting even if a nfs share is readonly, so probably
> following attributes should be set to avoid this:
> 
> G_FILE_ATTRIBUTE_ACCESS_CAN_TRASH
> G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE
> G_FILE_ATTRIBUTE_ACCESS_CAN_RENAME
> 
> That's hopefully my last notice ;-)

Good spot, should be fixed now.

Great, looks good, but...

I have two shares:
/a localhost(ro,sync)
/a/b localhost(rw,sync)

The access rights are:
$ gvfs-info -a "access" nfs://localhost/a/b
uri: nfs://localhost/a/b
attributes:
  access::can-read: TRUE
  access::can-write: TRUE
  access::can-execute: TRUE
  access::can-delete: FALSE
  access::can-trash: FALSE
  access::can-rename: FALSE

$ gvfs-info -a "access" nfs://localhost/a/b/file
uri: nfs://localhost/a/b/file
attributes:
  access::can-read: TRUE
  access::can-write: TRUE
  access::can-execute: FALSE
  access::can-delete: TRUE
  access::can-trash: FALSE
  access::can-rename: TRUE

However Nautilus doesn't allow deleting and renaming on this file, or should be the rights as follows (as are when nfs://localhost/a isn't exported)?
$ gvfs-info -a "access"  nfs://localhost/a/b
uri:  nfs://localhost/a/b
attributes:
  access::can-read: TRUE
  access::can-write: TRUE
  access::can-execute: TRUE
  access::can-delete: TRUE
  access::can-trash: FALSE
  access::can-rename: TRUE

Not sure from docs, but it seems to me there is a bug in Nautilus, do you agree?

(In reply to comment #35)
> Great, looks good, but...
> 
> I have two shares:
> /a localhost(ro,sync)
> /a/b localhost(rw,sync)
> 
> The access rights are:
> $ gvfs-info -a "access" nfs://localhost/a/b
> uri: nfs://localhost/a/b
> attributes:
>   access::can-read: TRUE
>   access::can-write: TRUE
>   access::can-execute: TRUE
>   access::can-delete: FALSE
>   access::can-trash: FALSE
>   access::can-rename: FALSE
> 
> $ gvfs-info -a "access" nfs://localhost/a/b/file
> uri: nfs://localhost/a/b/file
> attributes:
>   access::can-read: TRUE
>   access::can-write: TRUE
>   access::can-execute: FALSE
>   access::can-delete: TRUE
>   access::can-trash: FALSE
>   access::can-rename: TRUE
> 
> However Nautilus doesn't allow deleting and renaming on this file, or should be
> the rights as follows (as are when nfs://localhost/a isn't exported)?
> $ gvfs-info -a "access"  nfs://localhost/a/b
> uri:  nfs://localhost/a/b
> attributes:
>   access::can-read: TRUE
>   access::can-write: TRUE
>   access::can-execute: TRUE
>   access::can-delete: TRUE
>   access::can-trash: FALSE
>   access::can-rename: TRUE
> 
> Not sure from docs, but it seems to me there is a bug in Nautilus, do you
> agree?

Well it was actually a bug in try_enumerate(), it gave the wrong results for:
gvfs-ls -a "access::*"  nfs://localhost/a/b

It should be fixed now.

(Most of the time, Nautilus seems to get its information from enumerate, not query_info and they can (but shouldn't) be different...)

Merged in 874bc00bd19c6853cedcea610e8ac035c8486ca7. Thanks for the reviews!

*** Bug 756047 has been marked as a duplicate of this bug. ***

*** Bug 634916 has been marked as a duplicate of this bug. ***