The `--inode-file-handles` switch is basically just a best-effort suggestion: If a file handle cannot be generated, we still fall back to keeping an `O_PATH` FD for the inode in question. The rationale for this is: 1. Vivek proposed a variant where file handles were mandatory, and failure to generate file handles would result in an all-out error, quitting virtiofsd. However, I could not really imagine a case where this would be preferable behavior over it just working and falling back to `O_PATH` FDs, especially in the original context of using file handles only to reduce the number of open FDs. 2. In order not to quit at runtime, weâd have to somehow find out at start-up whether file handles will work for everything in the shared directory. Even if that were possible (I donât believe it is), it would be complex code to write. So weâd probably end up with complex test code at start-up, and might still run into errors at runtime.
Now two things have happened to change my opinion: 1. There are cases where `--inode-file-handles` should have fixed some concrete issue, but using it didnât change anything. For such cases, it would be nice to have some indication that it doesnât work as expected, so nobody has to run `ls /proc/$(pidof virtiofsd-rs)/fd | wc -l` to find out. 2. It turns out thereâs at least one other case for which file handles are useful besides just reducing the number of open FDs; and that is to get around NFSâs file renaming. For this case, it may indeed be preferable for users to receive an error of some sort instead of us falling back to `O_PATH` FDs and thus potentially exposing the less-than-ideal interaction that will then occur. Iâm now opening this (gitlab) issue to see whether someone has opinions on these (file handle usage) issues. I think we want different solutions for both. For the first one, we want to have warnings that sensibly log when we have to fall back. By âsensiblyâ?, I mean that most of the time, generating a file handle will either always or never work for a given mount, and so we should generally only warn once per mount (unless itâs a one-time issue like `name_to_handle_at()` returning `ENOMEM`). What Iâm much less sure about is how to log these warnings. Intuitively, Iâd log them at *warn* level, but the default level we have is *error*, so itâs unlikely that users would see them. Should we log them at level *error*? For the second case (NFS), I would throw an actual error somewhere. We really shouldnât quit virtiofsd over it, but we can return `EIO` to the guest or something. Off the top of my head, I have these rough ideas: * If generating a file handle fails on an NFS mount (discoverable through mountinfo), we not only log a warning, but return an error to the guest. Thatâd be a change in behavior, and perhaps surprising that NFS behaves differently than other filesystems, but it wouldnât require users to take special considerations. Weâd automatically interpret `--inode-file-handles` as strict as we deem sensible, depending on the situation. * Add some other mode (like `--inode-file-handles=mandatory` mentioned above) where any error to generate a file handle is passed through to the guest (not necessarily verbatim, perhaps just as a generic `EIO`). Users would be responsible to choose the behavior they need. On the plus side, on NFS weâd continue to by default fall back to `O_PATH` FDs as on any other FS, and the user needs to consciously pass a different parameter for more strict behavior. But thatâs also the downside, of course, the user needs to make this conscious decision, and if they donât, they may still run into the errors stemming from NFSâs file renaming. Perhaps to encourage users to take this decision, we could log a warning that `--inode-file-handles=mandatory` is recommended when we detect some mount in the shared directory to be NFS, if that parameter wasnât passed. Thoughts? --- https://gitlab.com/virtio-fs/virtiofsd-rs/-/issues/17
_______________________________________________ Virtio-fs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/virtio-fs
