Machine mount sync package - initial version - #10332

Why do you need name+"_", the file is going to be suffixed either way?

f, err := ioutil.TempFile(filepath.Split(path))

This could be a global var.

And assigned at L96.

Sorry but I can't see any advantages of this approach. However, I see one big disadvantage which is an introduction of global variable used only in one place as a fallback to incomplete options(used in tests) inside the type that is meant to be used as singleton.

I do not quite follow, since you're using a global variable either way (machine.DefaultLogger) - I just commented to do the same here - use a global variable instead of allocating new default logger each time.

The root logger machine.DefaultLogger is a parent for other loggers and it uses config.Konfig.Debug variable during its initialization.

Actually, I'm avoiding unnecessary allocation by not defining global variable. Please note, that this branch is a fallback which normally will not be executed.

I'm not sure these fine-grained locks are correct. Two different threads can simultaneously pass this check and both can assign a new *synced value with the same id to the map. While if the whole body was protected with a single lock, the second thread would fail here.

the funny fact is that we don't care.. even if IDs are the same (which is not possible in valid logic), the only inconsistency will be that we fetch index twice (or just load it from cache) and swap the file atomically.

This is a trade off I had to make if I want to add mounts concurrently (during kd initialization). newSync may be slow and I want to avoid synchronous initialization process.

I will add:

s.mu.Lock()
if _, ok := s.syncs[id]; ok {
    s.mu.Unlock()
    return //some fancy error here
}
s.syncs[id] = sd
s.mu.Unlock()

prob MkdirAll is enough, it already handles Stat as fast path https://golang.org/src/os/path.go?s=537:766

Thanks - TIL