Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fuse mounted directory under kopia does not show contents in mc #4635

Closed
mc-butler opened this issue Jan 25, 2025 · 6 comments
Closed

Fuse mounted directory under kopia does not show contents in mc #4635

mc-butler opened this issue Jan 25, 2025 · 6 comments
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress res: invalid The ticket is not a bug, or is a support request

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4635
Reporter rahrah (richard1hoyle@….com)

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 <[email protected]>
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 <[email protected]>
    Signed-off-by: Andrew Borodin <[email protected]>

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



Experimental Fix

Reverting 27de037

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

littlefs-project/littlefs-fuse#43

Thank You

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

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 25, 2025 at 16:55 UTC (comment 1)

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

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 25, 2025 at 17:20 UTC (comment 2)

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

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 25, 2025 at 19:10 UTC (comment 3)

  • Milestone changed from Future Releases to 4.8.34

Moving to current milestone while waiting for feedback.

@mc-butler
Copy link
Author

Changed by rahrah (richard1hoyle@….com) on Jan 26, 2025 at 18:06 UTC (comment 2.4)

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] <[email protected]>
    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 <[email protected]>
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 <[email protected]>
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

hanwen/go-fuse#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!

@mc-butler
Copy link
Author

Changed by rahrah (richard1hoyle@….com) on Jan 26, 2025 at 18:50 UTC (comment 5)

Hi,

I reported upstream to kopia, here:

kopia/kopia#4376

===Rich

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 27, 2025 at 8:51 UTC (comment 6)

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

I have added more documentation to explain what we are doing and why in [f1e08a7].

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress res: invalid The ticket is not a bug, or is a support request
Development

No branches or pull requests

1 participant