Ticket #4635 (closed defect: invalid)

Opened 44 hours ago

Last modified 4 hours ago

Fuse mounted directory under kopia does not show contents in mc

Reported by: rahrah Owned by:
Priority: major Milestone:
Component: mc-vfs Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description

OS: Slackware64-current
LIBC: glibc-2.40
Kernel: 6.12.10
ARCH: x86_64

mc -V
GNU Midnight Commander 4.8.33
Built with GLib 2.82.4
Built with S-Lang 2.3.3 with terminfo database
Built with libssh2 1.11.1 
With builtin editor and aspell support
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalisation support
With multiple codepages support
With ext2fs attributes support
Virtual File Systems:
 cpiofs, tarfs, sfs, extfs, ftpfs, sftpfs, shell
Data types:
 char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64; uintmax_t: 64;

Description

Using mc with kopia (fuse) mounted snapshots. Cli commands work fine, but mc sees an empty directory.

I found mc 4.8.25 did not show this problem, but mc 4.8.26 did. I used a git bisection to find the issue.

Kopia

Kopia is a backup program written in Go. It uses go-fuse for its fuse kernel module interface. Backups are accessed in directory hierarchies. Inserted into the hierarchy path string are machine names and date and time stamps.

Kopia can be found here:

https://kopia.io

and

https://github.com/kopia/kopia/

The

mount -l

gives this after a kopia mount operation on my Linux test machine:

kopia on /mnt-test/kopia/mount type fuse.kopia (rw,nosuid,nodev,noatime,user_id=0,group_id=0,max_read=131072)

Git Bisection

I did a git bisection to find the commit causing the problem. The commit introducing the bad behaviour was:

commit 27de03754fb790c82e70356daf859ff4de11f85d (HEAD)
Author: Andrij Abyzov <aabyzov@slb.com>
Date:   Tue Nov 24 18:58:06 2020 +0100

    Ticket #3987: implement a workaround if readdir() system call returns with EINTR.

    On Linux >= 5.1, MC sometimes shows empty directpries on mounted CIFS
    shares. Rereading directory restores the directory content.

    (local_opendir): reopen directory, if first readdir() returns NULL and
    errno == EINTR.

    Signed-off-by: Andrij Abyzov <aabyzov@slb.com>
    Signed-off-by: Andrew Borodin <aborodin@vmail.ru>

 src/vfs/local/local.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)



Experimental Fix

Reverting 27de03754fb790c8

by using the following function fixed the problem:

/* ------------------- */

static void *
local_opendir (const vfs_path_t *vpath)
{
    DIR **local_info;
    DIR *dir = NULL;
    const char *path;

    path = vfs_path_get_last_path_str (vpath);
    dir = opendir (path);
    if (dir == NULL)
        return 0;

    local_info = (DIR **) g_new (DIR *, 1);
    *local_info = dir;

    return local_info;
}

/* ------------------- */

Original Code

/* ------------------- */

static void *
local_opendir (const vfs_path_t *vpath)
{
    DIR **local_info;
    DIR *dir = NULL;
    const char *path;

    path = vfs_path_get_last_path_str (vpath);

    /* On Linux >= 5.1, MC sometimes shows empty directories on mounted CIFS shares.
     * Rereading directory restores the directory content.
     *
     * Reopen directory, if first readdir() returns NULL and errno == EINTR.
     */
    while (dir == NULL)
    {
        dir = opendir (path);
        if (dir == NULL)
            return NULL;

        if (readdir (dir) == NULL && errno == EINTR)
        {
            closedir (dir);
            dir = NULL;
        }
        else
            rewinddir (dir);
    }

    local_info = (DIR **) g_new (DIR *, 1);
    *local_info = dir;

    return local_info;
}

/* ------------------- */

Notes on Testing

In my bisection tests, I, at first, mounted the kopia mount, then tried the various bisection builds. Doing this seemed to allow all the failed builds to work.

For more consistent test results, the kopia mount was umounted then remounted between tests. Indeed, opening the any dir in the mounted tree with a faulty local_open_dir() would certainly create the correct test conditions. Only good builds would then open the directory.

I'd be happy to provide more granular steps to reproduce this problem using kopia commands.

Possible Similar Bug

https://github.com/littlefs-project/littlefs-fuse/issues/43

Thank You

Thank you for all the time and effort you put into making Midnight Commander, and distributing it under the GPL.

Change History

comment:1 Changed 44 hours ago by andrew_b

Looks like rewinddir() isn't worked as expected.
See also #3987 and #4289.
Apparently won't fix.

Version 0, edited 44 hours ago by andrew_b (next)

comment:2 follow-up: ↓ 4 Changed 43 hours ago by zaytsev

Thanks for an extensive bug report. Could you please open a bug against kopia and littlefs and link them here? We believe that there is something wrong with rewinddir implementation on these FUSE filesystems...

comment:3 Changed 41 hours ago by zaytsev

  • Milestone changed from Future Releases to 4.8.34

Moving to current milestone while waiting for feedback.

comment:4 in reply to: ↑ 2 Changed 18 hours ago by rahrah

Replying to zaytsev:

Thanks for an extensive bug report. Could you please open a bug against kopia and littlefs and link them here? We believe that there is something wrong with rewinddir implementation on these FUSE filesystems...

Hi,

Thank you for getting back to me! Your responses spurred me on to investigate bisecting kopia, then on to bisect the go-fuse module on which it depends. I'm sorry I didn't do this before.

It turns out that kopia failed to work with mc, here:

commit 3a3bce5749d39fd06483022c7188bdb7a850c144
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Sep 30 18:29:57 2024 -0700

    build(deps): bump github.com/hanwen/go-fuse/v2 from 2.5.1 to 2.6.1 (#4147)

    Bumps [github.com/hanwen/go-fuse/v2](https://github.com/hanwen/go-fuse) from 2.5.1 to 2.6.1.
    - [Commits](https://github.com/hanwen/go-fuse/compare/v2.5.1...v2.6.1)

    ---
    updated-dependencies:
    - dependency-name: github.com/hanwen/go-fuse/v2
      dependency-type: direct:production
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

go-fuse (https://github.com/hanwen/go-fuse.git) added their
'bad' commit here:

commit e885cea8d4d40a5a9bb92bc3cef7193f2a316f59
Author: Han-Wen Nienhuys <hanwenn@gmail.com>
Date:   Mon Sep 9 13:31:33 2024 +0200

    fs: new directory API
    
    The new API follows the pattern of the other file API:
    
    InodeEmbedder can implement a OpendirHandle method, which returns a
    FileHandle. Directories can implement the following APIs,
    
     * FileSyncdirer
     * FileReaddirenter
     * FileReleasedirer
     * FileSeekdirer
    
    along with the opened directory, FOPEN_* flags may be returned. In a
    follow-up change, we'll demonstrate how to use FOPEN_CACHE_DIR for
    directory caching.
    
    This change is backward compatible in a compilation sense, but support
    for seeking in DirStreams has been removed: it was incorrect (it made
    the assumption that the dir offset was monotonically increasing),
    inefficient and complicated.
    
    Change-Id: I662713321f0f812e92e4059ee11e8b1427c6aa0f

 fs/api.go            |  30 ++++++++
 fs/bridge.go         | 191 +++++++++++++++++++++++++--------------------------
 fs/dir_test.go       | 166 ++++++++++++++++++++++++++++++++++++++++++++
 fs/dirstream.go      |  28 ++++++++
 fs/dirstream_unix.go |  42 ++++++++++-
 fs/loopback.go       |  13 ++--
 6 files changed, 366 insertions(+), 104 deletions(-)

###

go-fuse seemed to have fixed this bad commit here:

commit d6170d09d743644ccf6099744e5bad1d2c3e552f
Author: WeidiDeng <weidi_deng@icloud.com>
Date:   Thu Jan 9 14:10:41 2025 +0800

    fs: support seeking for dirStreamAsFile and dirArray
    
    These types are used as default implementations, we thus
    allow seeking in directories for simple FUSE file systems.
    
    Fixes https://github.com/hanwen/go-fuse/issues/549
    
    Change-Id: I420e27a9d00dce7f21b3b2bce8747ae06a9040ef

 fs/dirstream.go        | 33 +++++++++++++++++++++++++++++----
 fs/dirstream_darwin.go |  2 +-
 fs/dirstream_test.go   | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 5 deletions(-)

I can confirm that this fixes the problem for me with mc.

As you'll see, the discussion at

https://github.com/hanwen/go-fuse/issues/549

this is exactly the same issue with mc as I had.

I will report to kopia, upstream tomorrow as I've run out of time today.

Once again, thank you for getting back to me which spurred me to investigate further with git bisections on a go project dependency.

Forgive this next question, but is there a reason that this issue seems to occur only with mc? Is there something that is different in mc's use of readdir? Other file (horrible graphical) file managers seem to work.

Once again, thank you for all your work on mc!

comment:5 Changed 18 hours ago by rahrah

Hi,

I reported upstream to kopia, here:

https://github.com/kopia/kopia/issues/4376

===Rich

comment:6 Changed 4 hours ago by zaytsev

  • Status changed from new to closed
  • Resolution set to invalid
  • Milestone 4.8.34 deleted

I have added more documentation to explain what we are doing and why in f1e08a7838fc657d921ebe15c80ad96e266dd7aa.

I didn't include the discussion of SA_RESTART, because 1) my understanding is not deep enough 2) I think we aren't in complete control anyways and 3) my impression from the Go issues is that it will be ignored anyways.

The bottom line is that we have to decide, if we stick to our "smart" strategy and cater CIFS & NFS users on Linux (and possibly other OSes) better, or whether we stop the rewinddir tricks and have less problems with Linux users running FUSE systems that have problems with rewinddir.

It seems that it's a game that one can't win to me. So I'll probably suggest to stay with the extra hack for CIFS / NFS and hope that FUSE filesystem authors fix their systems.

Thanks for the report and investigation, at least I think I understand our own code a little bit better now. I will close this in mc.

Note: See TracTickets for help on using tickets.