Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Add solaris support #21

Merged
merged 4 commits into from
Mar 9, 2017
Merged

Add solaris support #21

merged 4 commits into from
Mar 9, 2017

Conversation

gfrey
Copy link
Contributor

@gfrey gfrey commented Feb 11, 2017

As solaris has a /proc this is pretty similar to linux. Restructured the code a little, so that only UnixProcess' Refresh method had to be implemented.

The assumption is that Executable() returns only the name of the binary started. On solaris this is truncated to 16 characters.

The UnixProcess seems to be generic enough, except for the Refresh
method, that contains linux specific stuff. Now it should be able to
support everything proc based.
This implements the UnixProcess' Refresh method for solaris.
@mitchellh
Copy link
Owner

This looks pretty good, but we'd like to try to not introduce a cgo requirement for any OS on this library. It looks like the code though could probably be structured a lot like the FreeBSD one by recreating the psinfo struct in Go and loading it there. Would you be open to that?

@gfrey
Copy link
Contributor Author

gfrey commented Feb 11, 2017

I tried to do so, but failed. Problem seems to be, that the /proc/<pid>/psinfo file has 336 bytes (reported by ls and when reading with python and c). But whenever I read that file with golang, I get 416 bytes. Pretty clueless at the moment.

This gets rid of the cgo dependency by using binary package to destruct
the psinfo struct (copied from /usr/include/sys/procfs.h).
@gfrey
Copy link
Contributor Author

gfrey commented Feb 12, 2017

Well, the solution is that whatever I did to evaluate the size of the /proc/<pid>/psinfo file, I did it with 32bit binaries. From man -s4 proc:

The format of the contents of any /proc file depends on the data model of the observer (the controlling process), not on the data model of the target process.

Now without the cgo dependency.

@jen20
Copy link

jen20 commented Mar 9, 2017

@mitchellh This LGTM and seems to work correctly on Illumos - merging will help port Nomad to Illumos.

@mitchellh
Copy link
Owner

@jen20 Sounds good, I just needed a verification + the removal of cgo. Both appear to now exist. :)

@mitchellh mitchellh merged commit 4fdf99a into mitchellh:master Mar 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants