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

syscall: TestGetfsstat fails on some (VMware?) OS X builders #16937

Closed
bradfitz opened this issue Aug 31, 2016 · 13 comments
Closed

syscall: TestGetfsstat fails on some (VMware?) OS X builders #16937

bradfitz opened this issue Aug 31, 2016 · 13 comments

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Aug 31, 2016

--- FAIL: TestGetfsstat (0.00s)
    syscall_bsd_test.go:21: num fs = 5
    syscall_bsd_test.go:32: index 4 is an empty Statfs_t struct
FAIL
FAIL    syscall 0.165s
@bradfitz bradfitz added the OS-Darwin label Aug 31, 2016
@bradfitz bradfitz added this to the Go1.8 milestone Aug 31, 2016
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 4, 2016

@josharian and I see this on both 10.8 and 10.10, both on VMWare. I think VMWare is not really at fault, but now we're just getting a different number of mounted filesystems and finally seeing the mistake.

I suspect our syscall.Statfs_t struct is the wrong size or layout and the kernel is only giving us 4 filesystems worth of data because we're telling it a smaller bufsize than it's expecting.

/cc @ianlancetaylor

@bradfitz bradfitz changed the title syscall: TestGetfsstat fails on OS X 10.8 builder syscall: TestGetfsstat fails on some (VMware?) OS X builders Sep 5, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 5, 2016

CL https://golang.org/cl/28487 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 5, 2016
Updates #16937

Change-Id: I6d1b210c741269b58040bd68bf3b51644f891737
Reviewed-on: https://go-review.googlesource.com/28487
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 6, 2016

What is the output of the mount command run with no arguments?

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 6, 2016

Good question. I'll add more debugging.

I tried checking the C-vs-Go size, but the Go struct is at least how bug cgo thinks it is (C.sizeof_struct_etc), which makes sense since Go made the struct.

The failure after adding more debugging is:

--- FAIL: TestGetfsstat (0.00s)
    syscall_bsd_test.go:18: Getfsstat(nil, MNT_WAIT) = (5, <nil>)
    syscall_bsd_test.go:25: Getfsstat([]syscall.Statfs_t, MNT_WAIT) = (4, <nil>)
    syscall_bsd_test.go:33: index 4 is an empty Statfs_t struct
FAIL
FAIL    syscall 0.147s

I'll make it also log the mount output, and the contents of the non-zero struct entries.

gopherbot pushed a commit that referenced this issue Sep 6, 2016
Updates #16937

Change-Id: I98aa203176f8f2ca2fcca6e334a65bc60d6f824d
Reviewed-on: https://go-review.googlesource.com/28535
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 6, 2016

    syscall_bsd_test.go:45: mount: /dev/disk0s2 on / (hfs, local, journaled)
        devfs on /dev (devfs, local, nobrowse)
        map -hosts on /net (autofs, nosuid, automounted, nobrowse)
        map auto_home on /home (autofs, automounted, nobrowse)
        .host:/VMware Shared Folders on /Volumes/VMware Shared Folders (vmhgfs)
FAIL
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 6, 2016

CL https://golang.org/cl/28550 mentions this issue.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 6, 2016

@quentinmit raised a good point on https://golang.org/cl/28550 .... how does OS X's mount itself work?

dtruss says:

strace

Notably, it's passing a buffer size of 0x32d0, which is one element bigger than it needs to:

bradfitz@laptop ~$ cat size.go
package main

import (
    "syscall"
    "unsafe"
)

func main() {
    println(unsafe.Sizeof(syscall.Statfs_t{}))
}

bradfitz@laptop ~$ go run size.go
2168

bradfitz@laptop ~$ python
Python 2.7.10 (default, Oct 23 2015, 19:19:21) 
[GCC 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.0.59.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 0x32d0 / 2168
6

I suppose we could just do that too.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 6, 2016

OS X mount,
http://opensource.apple.com/source/diskdev_cmds/diskdev_cmds-582/mount.tproj/mount.c

... calls getmntinfo:

https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man3/getmntinfo.3.html

And its source:

https://opensource.apple.com/source/Libc/Libc-167/gen.subproj/getmntinfo.c

int
getmntinfo(mntbufp, flags)
    struct statfs **mntbufp;
    int flags;
{
    static struct statfs *mntbuf;
    static int mntsize;
    static long bufsize;

    if (mntsize <= 0 && (mntsize = getfsstat(0, 0, MNT_NOWAIT)) < 0)
        return (0);
    if (bufsize > 0 && (mntsize = getfsstat(mntbuf, bufsize, flags)) < 0)
        return (0);
    while (bufsize <= mntsize * sizeof(struct statfs)) {
        if (mntbuf)
            free(mntbuf);
        bufsize = (mntsize + 1) * sizeof(struct statfs);
        if ((mntbuf = (struct statfs *)malloc(bufsize)) == 0)
            return (0);
        if ((mntsize = getfsstat(mntbuf, bufsize, flags)) < 0)
            return (0);
    }
    *mntbufp = mntbuf;
    return (mntsize);
}

Notably:

        bufsize = (mntsize + 1) * sizeof(struct statfs);
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 7, 2016

No, the size doesn't matter at all.

Tested it on a VMware instance. MNT_NOWAIT (2) works, which is what mount does. MNT_WAIT (1) doesn't work.

@gopherbot gopherbot closed this in a4bdd64 Sep 7, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 7, 2016

CL https://golang.org/cl/28585 mentions this issue.

gopherbot pushed a commit to golang/sys that referenced this issue Sep 7, 2016
Same as https://golang.org/cl/28550 but for x/sys/unix.

Updates golang/go#16937

Change-Id: Ie93df8ff1c40a7f2d98f1fb3ecf6110330bf1cbc
Reviewed-on: https://go-review.googlesource.com/28585
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 7, 2016
Fixes test failure when VMWare's shared folder filesystem is present.

MNT_NOWAIT is what the mount(8) command does.

Fixes #16937

Change-Id: Id436185f544b7069db46c8716d6a0bf580b31da0
Reviewed-on: https://go-review.googlesource.com/28550
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-on: https://go-review.googlesource.com/28650
Run-TryBot: Chris Broadfoot <cbro@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Sep 23, 2016

@ALTree, only because it was a trybot run on an old commit which hadn't been rebased to tip and didn't have the updated test.

@ALTree
Copy link
Member

@ALTree ALTree commented Sep 23, 2016

Ah! Thanks and sorry for the noise. I will rebase my CL.

@golang golang locked and limited conversation to collaborators Sep 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.