Windows mount: unable to rename file if destination exists: Unknown error 526

frickenate commented 6 years ago

BUG REPORT

Environment:

Minikube version: v0.28.2
OS: Windows 10
VM Driver: virtualbox
ISO version: minikube-v0.28.1.iso

What happened:

Mounts created with minikube mount on a Windows host do not support mv (file renames) from a *nix guest when the destination file already exists - results in error and no rename taking place.

What you expected to happen:

Some kind of workaround on Windows hosts to allow the renaming/clobbering onto an existing file from *nix guests – even if it's just a (non-atomic) deletion of the existing file before doing the rename.

How to reproduce it:

Run the following from one Windows cmd or PowerShell on Windows. Note that 9p version 9p2000.L vs. default 9p2000.u makes no difference. The --ip passed due to separate bug #2442.

mkdir files
minikube.exe start
minikube.exe mount --9p-version 9p2000.L --ip 192.168.99.1 .\files:/mnt/host

Keep that terminal open to a) maintain the mount, and b) observe error message after the next step.

From a second Windows cmd or PowerShell:

minikube ssh
$ cd /mnt/host
$ touch hello
$ touch world
$ mv hello world

The output: mv: cannot move 'hello' to 'world': Unknown error 526

Switch back to the first Windows cmd/PowerShell where the mount process is running. Its output:

Rename C:\path\to\files/hello to world
rel  results in C:\path\to\files/world
rename C:\path\to\files/hello to C:\path\to\files/world gets
Cannot create a file when that file already exists.

Output of minikube logs (if applicable):

N/A

Anything else do we need to know:

Plea: regardless of an official stance/timeline on fixing this issue, if someone knows where exactly this problem originates, can you please share with me? When one runs mv hello world from a *nix guest, where can I find the code responsible for attempting a native Windows rename even though the OS does not permit renaming on top of an existing file? Is this logic part of minikube itself, or inherited via the use of a library that provides the 9p mounts?

I'm up against a tight deadline to demonstrate viability of k8s development, and am stuck with Windows for local dev (😢). ~~If I have to hack in a file-deletion-before-rename into an external library dependency and recompile minikube manually, I'm up for that.~~ Aside: I already tried VirtualBox shared folders, which failed for another reason (tldr: vboxsf mount from Windows to minikube, with hostPath mount from minikube to k8s pod's docker container - when Windows host writes to a file, the nested mount sees cached/corrupted file contents upon read).

Thanks!

frickenate commented 6 years ago

Found the primary issue, but then stumbled on a secondary problem.

Primary issue: in the Wstat() functions (duplicated across each system architecture), you are calling syscall.Rename() instead of os.Rename() - it just so happens that the os package's Rename() method properly handles renames of files that replace existing files (since 2015), while syscall.Rename() does not. The fix here should simply be to replace the use of syscall.Rename() with os.Rename() in each of the WStat() methods - and I've tested that this fix works.

However, I happened to stumble across a separate – technically unrelated – bug that is much (much!) scarier to ponder. For whatever reason, the inode of a file or directory reported to *nix is the size of the file in bytes... + 2 (possibly related to this struct field and its comment?). If you have a 5-byte file, its inode is assigned as 7; a 20-byte file has inode 22; a 998-byte file has inode 1000; and so on. *nix systems are effectively being told that all files/directories sharing the same file size point to the same physical file on disk. In addition to a myriad of other bugs this implementation detail will cause, it is impossible to rename one file having file size X to another file having the same file size X. If hello and world files have the same file size (ex: 10 bytes), then attempting to mv hello world results in the *nix error mv: 'hello' and 'world' are the same file - an assertion check that mv performs before reaching out to perform the rename at the filesystem level.

tldr; To directly resolve my original bug report, replacing all calls to syscall.Rename() with os.Rename() should suffice. Separately, inodes appear to be calculated as file size + 2, and that is just awful and asking for trouble.