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

Use volume iterators to list volumes in windows #112

Merged
merged 6 commits into from
Jan 18, 2019

Conversation

jsoriano
Copy link
Member

GetLogicalDriveStrings doesn't list filesystems that are mounted with
access paths but don't have a drive letter. Use volume iterators to
list all volumes and obtain its access paths.

Reported on elastic/beats#8916

// all.bat?)
return e
}

var (
modkernel32 = syscall.NewLazyDLL("kernel32.dll")
modpsapi = syscall.NewLazyDLL("psapi.dll")
modntdll = syscall.NewLazyDLL("ntdll.dll")
modadvapi32 = syscall.NewLazyDLL("advapi32.dll")
Copy link
Member Author

@jsoriano jsoriano Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewkroh I had to revert these lines by hand, recent go versions use windows.NewLazySystemDLL, but as this package is named windows it fails.

We should move these generated syscall wrappers to other submodule, or rename this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What fails? It should be fine to import golang.org/x/sys/windows from within package windows. That might be a new dep that we didn't use previously though.

Copy link
Member Author

@jsoriano jsoriano Jan 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that it generates this code here:

       modkernel32 = NewLazySystemDLL("kernel32.dll")
       modpsapi    = NewLazySystemDLL("psapi.dll")
       modntdll    = NewLazySystemDLL("ntdll.dll")
       modadvapi32 = NewLazySystemDLL("advapi32.dll")

And NewLazySystemDLL doesn't exist in this package.

It seems that because it has some special handling for packages named windows: https://github.com/golang/go/blob/go1.10.5/src/syscall/mksyscall_windows.go#L735

I have seen that adding -systemdll=false to the parameters of mksyscall_windows generates the same code as before. I am adding it by now.

@jsoriano
Copy link
Member Author

With #111 we also got the type of filesystem, but we can try to obtain this info in any case also after this with https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getvolumeinformationw

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm happy to see the data is available outside of WMI.

sys/windows/syscall_windows.go Outdated Show resolved Hide resolved
sys/windows/syscall_windows.go Outdated Show resolved Hide resolved
sys/windows/syscall_windows.go Outdated Show resolved Hide resolved
sys/windows/syscall_windows.go Outdated Show resolved Hide resolved
// all.bat?)
return e
}

var (
modkernel32 = syscall.NewLazyDLL("kernel32.dll")
modpsapi = syscall.NewLazyDLL("psapi.dll")
modntdll = syscall.NewLazyDLL("ntdll.dll")
modadvapi32 = syscall.NewLazyDLL("advapi32.dll")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What fails? It should be fine to import golang.org/x/sys/windows from within package windows. That might be a new dep that we didn't use previously though.

Without this setting, it generates the syscall file as if it were inside
the `windows` package, with local references to functions there, what
fails.
@jsoriano
Copy link
Member Author

Thanks @andrewkroh for the fast review 🙂

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jsoriano jsoriano merged commit bfff7ba into elastic:master Jan 18, 2019
@jsoriano jsoriano deleted the volume-iterators branch January 18, 2019 11:08
@jsoriano jsoriano mentioned this pull request Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants