From 182507d0d1e35c4325f83080d9d605ce623f6c29 Mon Sep 17 00:00:00 2001 From: rob-deutsch Date: Thu, 20 Sep 2018 17:41:32 +1000 Subject: [PATCH 1/3] fixed test of raised fd limits Raising FD limits was erroring when the OS's max was at the maximum signed integer value. Switched the code to using uint64 instead of int64. fixed #5495 License: MIT Signed-off-by: Rob Deutsch --- cmd/ipfs/util/ulimit.go | 16 +++++++--------- cmd/ipfs/util/ulimit_freebsd.go | 19 ++++++++++++++----- cmd/ipfs/util/ulimit_unix.go | 10 +++++----- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/cmd/ipfs/util/ulimit.go b/cmd/ipfs/util/ulimit.go index 07c5f69c654..997c2a95160 100644 --- a/cmd/ipfs/util/ulimit.go +++ b/cmd/ipfs/util/ulimit.go @@ -16,9 +16,9 @@ var ( supportsFDManagement = false // getlimit returns the soft and hard limits of file descriptors counts - getLimit func() (int64, int64, error) + getLimit func() (uint64, uint64, error) // set limit sets the soft and hard limits of file descriptors counts - setLimit func(int64, int64) error + setLimit func(uint64, uint64) error ) // maxFds is the maximum number of file descriptors that go-ipfs @@ -56,9 +56,7 @@ func ManageFdLimit() error { return err } - max := int64(maxFds) - - if max <= soft { + if maxFds <= soft { return nil } @@ -67,25 +65,25 @@ func ManageFdLimit() error { // the hard limit acts as a ceiling for the soft limit // an unprivileged process may only set it's soft limit to a // alue in the range from 0 up to the hard limit - if err = setLimit(max, max); err != nil { + if err = setLimit(maxFds, maxFds); err != nil { if err != syscall.EPERM { return fmt.Errorf("error setting: ulimit: %s", err) } // the process does not have permission so we should only // set the soft value - if max > hard { + if maxFds > hard { return errors.New( "cannot set rlimit, IPFS_FD_MAX is larger than the hard limit", ) } - if err = setLimit(max, hard); err != nil { + if err = setLimit(maxFds, hard); err != nil { return fmt.Errorf("error setting ulimit wihout hard limit: %s", err) } } - fmt.Printf("Successfully raised file descriptor limit to %d.\n", max) + fmt.Printf("Successfully raised file descriptor limit to %d.\n", maxFds) return nil } diff --git a/cmd/ipfs/util/ulimit_freebsd.go b/cmd/ipfs/util/ulimit_freebsd.go index d332fab1500..945416ecce1 100644 --- a/cmd/ipfs/util/ulimit_freebsd.go +++ b/cmd/ipfs/util/ulimit_freebsd.go @@ -3,6 +3,9 @@ package util import ( + "errors" + "math" + unix "gx/ipfs/QmVGjyM9i2msKvLXwh9VosCTgP4mL91kC7hDmqnwTTx6Hu/sys/unix" ) @@ -12,16 +15,22 @@ func init() { setLimit = freebsdSetLimit } -func freebsdGetLimit() (int64, int64, error) { +func freebsdGetLimit() (uint64, uint64, error) { rlimit := unix.Rlimit{} err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit) - return rlimit.Cur, rlimit.Max, err + if (rlimit.Cur < 0) || (rlimit.Max < 0) { + return 0, 0, errors.New("invalid rlimits") + } + return uint64(rlimit.Cur), uint64(rlimit.Max), err } -func freebsdSetLimit(soft int64, max int64) error { +func freebsdSetLimit(soft uint64, max uint64) error { + if (soft > math.MaxInt64) || (max > math.MaxInt64) { + return errors.New("invalid rlimits") + } rlimit := unix.Rlimit{ - Cur: soft, - Max: max, + Cur: int64(soft), + Max: int64(max), } return unix.Setrlimit(unix.RLIMIT_NOFILE, &rlimit) } diff --git a/cmd/ipfs/util/ulimit_unix.go b/cmd/ipfs/util/ulimit_unix.go index 9e236b1be90..f85e3a5a6a1 100644 --- a/cmd/ipfs/util/ulimit_unix.go +++ b/cmd/ipfs/util/ulimit_unix.go @@ -12,16 +12,16 @@ func init() { setLimit = unixSetLimit } -func unixGetLimit() (int64, int64, error) { +func unixGetLimit() (uint64, uint64, error) { rlimit := unix.Rlimit{} err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit) - return int64(rlimit.Cur), int64(rlimit.Max), err + return rlimit.Cur, rlimit.Max, err } -func unixSetLimit(soft int64, max int64) error { +func unixSetLimit(soft uint64, max uint64) error { rlimit := unix.Rlimit{ - Cur: uint64(soft), - Max: uint64(max), + Cur: soft, + Max: max, } return unix.Setrlimit(unix.RLIMIT_NOFILE, &rlimit) } From 8cca57244c2da0276af0f12574463c63ab428907 Mon Sep 17 00:00:00 2001 From: rob-deutsch Date: Thu, 20 Sep 2018 17:53:03 +1000 Subject: [PATCH 2/3] supressed fd util printing to output Moved the fmt.Printf call from ManageFdLimit() to the calling code. ManageFdLimit() is called by tests and its annoying to have it output text License: MIT Signed-off-by: Rob Deutsch --- cmd/ipfs/daemon.go | 6 +++++- cmd/ipfs/util/ulimit.go | 18 ++++++++---------- cmd/ipfs/util/ulimit_test.go | 6 +++--- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/cmd/ipfs/daemon.go b/cmd/ipfs/daemon.go index a98d537f3a8..c68d67200aa 100644 --- a/cmd/ipfs/daemon.go +++ b/cmd/ipfs/daemon.go @@ -201,8 +201,12 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment managefd, _ := req.Options[adjustFDLimitKwd].(bool) if managefd { - if err := utilmain.ManageFdLimit(); err != nil { + if changedFds, newFdsLimit, err := utilmain.ManageFdLimit(); err != nil { log.Errorf("setting file descriptor limit: %s", err) + } else { + if changedFds { + fmt.Printf("Successfully raised file descriptor limit to %d.\n", newFdsLimit) + } } } diff --git a/cmd/ipfs/util/ulimit.go b/cmd/ipfs/util/ulimit.go index 997c2a95160..1cfb5da9f69 100644 --- a/cmd/ipfs/util/ulimit.go +++ b/cmd/ipfs/util/ulimit.go @@ -45,19 +45,19 @@ func setMaxFds() { // ManageFdLimit raise the current max file descriptor count // of the process based on the IPFS_FD_MAX value -func ManageFdLimit() error { +func ManageFdLimit() (changed bool, newLimit uint64, err error) { if !supportsFDManagement { - return nil + return false, 0, nil } setMaxFds() soft, hard, err := getLimit() if err != nil { - return err + return false, 0, err } if maxFds <= soft { - return nil + return false, 0, nil } // the soft limit is the value that the kernel enforces for the @@ -67,23 +67,21 @@ func ManageFdLimit() error { // alue in the range from 0 up to the hard limit if err = setLimit(maxFds, maxFds); err != nil { if err != syscall.EPERM { - return fmt.Errorf("error setting: ulimit: %s", err) + return false, 0, fmt.Errorf("error setting: ulimit: %s", err) } // the process does not have permission so we should only // set the soft value if maxFds > hard { - return errors.New( + return false, 0, errors.New( "cannot set rlimit, IPFS_FD_MAX is larger than the hard limit", ) } if err = setLimit(maxFds, hard); err != nil { - return fmt.Errorf("error setting ulimit wihout hard limit: %s", err) + return false, 0, fmt.Errorf("error setting ulimit wihout hard limit: %s", err) } } - fmt.Printf("Successfully raised file descriptor limit to %d.\n", maxFds) - - return nil + return true, maxFds, nil } diff --git a/cmd/ipfs/util/ulimit_test.go b/cmd/ipfs/util/ulimit_test.go index ed68e3c403d..2a5923f63fd 100644 --- a/cmd/ipfs/util/ulimit_test.go +++ b/cmd/ipfs/util/ulimit_test.go @@ -12,7 +12,7 @@ import ( func TestManageFdLimit(t *testing.T) { t.Log("Testing file descriptor count") - if err := ManageFdLimit(); err != nil { + if _, _, err := ManageFdLimit(); err != nil { t.Errorf("Cannot manage file descriptors") } @@ -41,7 +41,7 @@ func TestManageInvalidNFds(t *testing.T) { // call to check and set the maximum file descriptor from the env setMaxFds() - if err = ManageFdLimit(); err == nil { + if _, _, err := ManageFdLimit(); err == nil { t.Errorf("ManageFdLimit should return an error") } else if err != nil { flag := strings.Contains(err.Error(), @@ -79,7 +79,7 @@ func TestManageFdLimitWithEnvSet(t *testing.T) { t.Errorf("The maxfds is not set from IPFS_FD_MAX") } - if err = ManageFdLimit(); err != nil { + if _, _, err = ManageFdLimit(); err != nil { t.Errorf("Cannot manage file descriptor count") } From aada0cc1e27ec4cd12fb0cb7e2cc96a5ef3f9061 Mon Sep 17 00:00:00 2001 From: rob-deutsch Date: Thu, 20 Sep 2018 22:20:14 +1000 Subject: [PATCH 3/3] added freebsd cross-compile to 'make check' tests License: MIT Signed-off-by: Rob Deutsch --- mk/util.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mk/util.mk b/mk/util.mk index 449d6aa159f..fa6eae8e443 100644 --- a/mk/util.mk +++ b/mk/util.mk @@ -20,6 +20,8 @@ SUPPORTED_PLATFORMS += linux-amd64 SUPPORTED_PLATFORMS += darwin-386 SUPPORTED_PLATFORMS += darwin-amd64 +SUPPORTED_PLATFORMS += freebsd-amd64 + space:= space+= comma:=,