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

Enable build on IBM i PASE (variant of AIX) #1415

Closed
wants to merge 6 commits into from

Conversation

ThePrez
Copy link

@ThePrez ThePrez commented Feb 19, 2019

In this commit, I am enabling this important package to build on IBM i. For now, this means a simple "no op" for several of the operations that IBM i PASE does not support.

In the future, me or my team will fill in the missing capability. With this PR, I intend to allow pip install of psutil to succeed (resulting applications may still work if errors are handled elegantly). That'll be much better than failed installs!

@giampaolo
Copy link
Owner

UIhm... adding support for a brand new exotic platform. I like it. =)
If I remember correctly by returning NULL like that you're going to cause SystemError exception not set exceptions when you call those functions. At the very least you should do this instead:

return PyErr_SetString(PyExc_NotImplementedError, "not implemented on IBM PASE");

I'm not sure how to proceed with this. What are the main blockers for implementing those functions on IBM PASE? Of course that would be much more desirable, even though I know it's not gonna be a piece of cake...
Also CC-ing our good old AIX expert @wiggin15. ;)

@ThePrez
Copy link
Author

ThePrez commented Feb 19, 2019

The lack of libperfstat on IBM i, combined with a lack of /proc means that some performance metrics are quite difficult to get. We do have the getprocs() subroutine with limited ability, so that can give us some. We can also get some information by calling into the non-AIX parts of the OS (you were right that it is an exotic platform!). For a complete match of the AIX functions, however, it's possibly a couple months of work the data is scattered throughout various databases and API's. Hence the desire for an interim version.

I'll go ahead with the return PyErr_SetString(PyExc_NotImplementedError, "not implemented on IBM PASE"); change. Thanks for that!

@ThePrez
Copy link
Author

ThePrez commented Feb 19, 2019

Added the PyErr_SetStrings (it returns void so I needed a new line). Thanks!

@giampaolo
Copy link
Owner

Could you paste the output of make test? I would like to see how this looks like.

@ThePrez
Copy link
Author

ThePrez commented Feb 19, 2019

Sure. The make test isn't pretty, presumably because of the missing functions

make test
make install
make[1]: Entering directory '/home/JGORZINS/psutil'
make build
make[2]: Entering directory '/home/JGORZINS/psutil'
# make sure setuptools is installed (needed for 'develop' / edit mode)
python -c "import setuptools"
PYTHONWARNINGS=all python setup.py build
running build
running build_py
running build_ext
building 'psutil._psutil_aix' extension
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/_psutil_common.c -o build/temp.os400-powerpc64-3.6/psutil/_psutil_common.o
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/_psutil_posix.c -o build/temp.os400-powerpc64-3.6/psutil/_psutil_posix.o
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/_psutil_aix.c -o build/temp.os400-powerpc64-3.6/psutil/_psutil_aix.o
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/arch/aix/net_connections.c -o build/temp.os400-powerpc64-3.6/psutil/arch/aix/net_connections.o
psutil/arch/aix/net_connections.c: In function 'process_file':
psutil/arch/aix/net_connections.c:187:35: warning: passing argument 2 of 'read_unp_addr' makes integer from pointer without a cast [-Wint-conversion]
             if (read_unp_addr(Kd, unp.unp_addr, unix_laddr_str,
                                   ^~~
psutil/arch/aix/net_connections.c:37:1: note: expected 'KA_T {aka long long unsigned int}' but argument is of type 'struct mbuf *'
 read_unp_addr(
 ^~~~~~~~~~~~~
psutil/arch/aix/net_connections.c:198:35: warning: passing argument 2 of 'read_unp_addr' makes integer from pointer without a cast [-Wint-conversion]
             if (read_unp_addr(Kd, unp.unp_addr, unix_raddr_str,
                                   ^~~
psutil/arch/aix/net_connections.c:37:1: note: expected 'KA_T {aka long long unsigned int}' but argument is of type 'struct mbuf *'
 read_unp_addr(
 ^~~~~~~~~~~~~
psutil/arch/aix/net_connections.c: In function 'psutil_net_connections':
psutil/arch/aix/net_connections.c:261:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
         for (i = 0; i < p->pi_maxofile; i++) {
                       ^
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/arch/aix/common.c -o build/temp.os400-powerpc64-3.6/psutil/arch/aix/common.o
psutil/arch/aix/common.c: In function 'psutil_kread':
psutil/arch/aix/common.c:31:12: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (br != len) {
            ^~
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -O2 -g -maix64 -O2 -g -maix64 -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=552 -DPSUTIL_AIX=1 -I/QOpenSys/pkgs/include/python3.6m -c psutil/arch/aix/ifaddrs.c -o build/temp.os400-powerpc64-3.6/psutil/arch/aix/ifaddrs.o
/QOpenSys/pkgs/lib/python3.6/config-3.6m/ld_so_aix gcc -pthread -bI:/QOpenSys/pkgs/lib/python3.6/config-3.6m/python.exp -Wl,-blibpath:/QOpenSys/pkgs/lib:/QOpenSys/usr/lib build/temp.os400-powerpc64-3.6/psutil/_psutil_common.o build/temp.os400-powerpc64-3.6/psutil/_psutil_posix.o build/temp.os400-powerpc64-3.6/psutil/_psutil_aix.o build/temp.os400-powerpc64-3.6/psutil/arch/aix/net_connections.o build/temp.os400-powerpc64-3.6/psutil/arch/aix/common.o build/temp.os400-powerpc64-3.6/psutil/arch/aix/ifaddrs.o -L/QOpenSys/pkgs/lib -lutil -o build/lib.os400-powerpc64-3.6/psutil/_psutil_aix.so
PYTHONWARNINGS=all python setup.py build_ext -i
running build_ext
copying build/lib.os400-powerpc64-3.6/psutil/_psutil_aix.so -> psutil
copying build/lib.os400-powerpc64-3.6/psutil/_psutil_posix.so -> psutil
rm -rf tmp
python -c "import psutil"  # make sure it actually worked
make[2]: Leaving directory '/home/JGORZINS/psutil'
PYTHONWARNINGS=all python setup.py develop `python -c "import sys; print('' if hasattr(sys, 'real_prefix') else '--user')"`
running develop
running egg_info
writing psutil.egg-info/PKG-INFO
writing dependency_links to psutil.egg-info/dependency_links.txt
writing top-level names to psutil.egg-info/top_level.txt
reading manifest file 'psutil.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'psutil.egg-info/SOURCES.txt'
running build_ext
copying build/lib.os400-powerpc64-3.6/psutil/_psutil_aix.so -> psutil
copying build/lib.os400-powerpc64-3.6/psutil/_psutil_posix.so -> psutil
Creating /home/JGORZINS/.local/lib/python3.6/site-packages/psutil.egg-link (link to .)
Adding psutil 5.5.2 to easy-install.pth file

Installed /home/JGORZINS/psutil
Processing dependencies for psutil==5.5.2
Finished processing dependencies for psutil==5.5.2
rm -rf tmp
make[1]: Leaving directory '/home/JGORZINS/psutil'
PYTHONWARNINGS=all PSUTIL_TESTING=1 PSUTIL_DEBUG=1 python psutil/tests/__main__.py
Traceback (most recent call last):
  File "/home/JGORZINS/psutil/psutil/_common.py", line 341, in wrapper
    ret = self._cache[fun]
AttributeError: _cache

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 338, in wrapper
    return fun(self, *args, **kwargs)
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 428, in create_time
    return self._proc_basic_info()[proc_info_map['create_time']]
  File "/home/JGORZINS/psutil/psutil/_common.py", line 344, in wrapper
    return fun(self)
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 385, in _proc_basic_info
    return cext.proc_basic_info(self.pid, self._procfs_path)
FileNotFoundError: [Errno 2] No such file or directory: '/proc/1140905/psinfo'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 465, in _init
    self.create_time()
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 801, in create_time
    self._create_time = self._proc.create_time()
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 349, in wrapper
    raise NoSuchProcess(self.pid, self._name)
psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists (pid=1140905)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "psutil/tests/__main__.py", line 23, in <module>
    from psutil.tests import PYTHON_EXE
  File "/home/JGORZINS/psutil/psutil/tests/__init__.py", line 169, in <module>
    HAS_MEMORY_FULL_INFO = 'uss' in psutil.Process().memory_full_info()._fields
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 438, in __init__
    self._init(pid)
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 478, in _init
    raise NoSuchProcess(pid, None, msg)
psutil.NoSuchProcess: psutil.NoSuchProcess no process found with pid 1140905
make: *** [Makefile:113: test] Error 1

@giampaolo
Copy link
Owner

giampaolo commented Feb 19, 2019

Mmm... tests don't run and this is weird:

PYTHONWARNINGS=all PSUTIL_TESTING=1 PSUTIL_DEBUG=1 python psutil/tests/__main__.py
Traceback (most recent call last):
  File "/home/JGORZINS/psutil/psutil/_common.py", line 341, in wrapper
    ret = self._cache[fun]
AttributeError: _cache

Looks related to a change I made in 5.5.1. I suspect we'd get the same error on plain AIX (and unfortunately I don't have an AIX to test against).

@giampaolo
Copy link
Owner

Actually that's not weird (I misread the exception), but still it seems tests didn't run.

@ThePrez
Copy link
Author

ThePrez commented Feb 19, 2019

Yeah, I blindly figured it has something to do with the complete braindeadedness of basic functions. Didn't read much into it

@wiggin15
Copy link
Collaborator

Wouldn't it be better to "ifdef out" the entire functions, their exports from the C extension, and their interface from the main psutil file, like we do for other functions - e.g. psutil_proc_threads which is ifdef'd by CURR_VERSION_THREAD, and has HAS_THREADS for its availability? I think it would look a little cleaner in the code and will prevent NotImplementedError exceptions.
Also for net_kernel_structs.h/net_connections.c - we can just leave that C file out of Extension.sources in setup.py to avoid having to ifdef out more code.

@giampaolo
Copy link
Owner

@wiggin15 while I'm at it I wanted to update doc describing platform support. Do you know what AIX versions do we support exactly?

@wiggin15
Copy link
Collaborator

The oldest version I could test was 6.1 TL 8. It should work on this version and later.

@ThePrez
Copy link
Author

ThePrez commented Feb 21, 2019

@wiggin15 , that would be a good way to go about it also. I can work on that (I'll need a couple days to get to it). Thanks!

@ThePrez
Copy link
Author

ThePrez commented Feb 26, 2019

Refactored based on @wiggin15's suggestion (makes a lot more sense!). Thanks!

@giampaolo
Copy link
Owner

Can we take a look at make test?

@giampaolo
Copy link
Owner

(...in a txt file if you can)

@ThePrez
Copy link
Author

ThePrez commented Feb 26, 2019

... but now make test and subsequent usage pops out AttributeError

python -c "import psutil"  # make sure it actually worked
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 166, in <module>
    from . import _psaix as _psplatform
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 188, in <module>
    disk_io_counters = cext.disk_io_counters
AttributeError: module 'psutil_aix' has no attribute 'disk_io_counters'```
Not sure the best way to address that

@giampaolo
Copy link
Owner

Yes, with the #ifdefs you managed to compile the C extension but the upper _psaix.py module will have to take the differences into account. Generally speaking what you want to do is this (and do the same for others):

def _not_supported(*args, **kw):
    raise NotImplementedError("not supported on this platform")


if hasattr(cext, 'disk_io_counters'):
    disk_io_counters = cext.disk_io_counters
else:
    disk_io_counters = _not_supported

@ThePrez
Copy link
Author

ThePrez commented Feb 26, 2019

In an effort to get make test working, I added implementations for proc_basic_info (already there) and list_pids (new). This works around failures stemming from an absent /proc on IBM i.

Now, however, stuck here when I run make test:

Traceback (most recent call last):
  File "psutil/tests/__main__.py", line 94, in <module>
    main()
  File "psutil/tests/__main__.py", line 91, in main
    run_suite()
  File "/home/JGORZINS/psutil/psutil/tests/__init__.py", line 834, in run_suite
    result = unittest.TextTestRunner(verbosity=VERBOSITY).run(get_suite())
  File "/home/JGORZINS/psutil/psutil/tests/__init__.py", line 828, in get_suite
    suite.addTest(unittest.defaultTestLoader.loadTestsFromName(tm))
  File "/QOpenSys/pkgs/lib/python3.6/unittest/loader.py", line 153, in loadTestsFromName
    module = __import__(module_name)
  File "/home/JGORZINS/psutil/psutil/tests/test_misc.py", line 411, in <module>
    class TestWrapNumbers(unittest.TestCase):
  File "/home/JGORZINS/psutil/psutil/tests/test_misc.py", line 624, in TestWrapNumbers
    not psutil.disk_io_counters() or not psutil.net_io_counters(),
  File "/home/JGORZINS/psutil/psutil/__init__.py", line 2131, in disk_io_counters
    rawdict = _psplatform.disk_io_counters(**kwargs)
  File "/home/JGORZINS/psutil/psutil/_psaix.py", line 110, in _not_supported
    raise NotImplementedError("not supported on this platform")
NotImplementedError: not supported on this platform
make: *** [Makefile:113: test] Error 1

This is probably a simple case where we need to disable those tests (probably in test_misc.py). I haven't yet figured out the proper way to do that.

psutil/_psaix.py Outdated Show resolved Hide resolved
@giampaolo
Copy link
Owner

giampaolo commented Feb 26, 2019

It's OK if tests are failing with NotImplementedError for now. I just want to start getting a general sense of how things looks like. As for tests: this is such an exotic platform that I'm not willing to support it in the test suite by adding skip decorators all around so don't worry about that.

@ThePrez
Copy link
Author

ThePrez commented Feb 27, 2019

Works for me. When I get close to AIX equivalence, we can discuss the test suite more sensibly.

@ThePrez
Copy link
Author

ThePrez commented Feb 27, 2019

At this point, I think I've done what I can do without serious rip-up of the existing AIX path. I"m ready for this to land when you are. Many things don't work, but an install of psutil works and that makes me very happy.

In a future PR, I will split IBM i into its own source files for both the native and python side, because:

  • The usage of /proc is significant (and IBM i doesn't have it)
  • Many functions can be implemented in pure Python on IBM i through use of SQL

... but I would love for this interim version to go live in the meantime.

@giampaolo
Copy link
Owner

Please attach the test output.

@ThePrez
Copy link
Author

ThePrez commented Feb 27, 2019

make test still fails to run, quitting early with the same error as before (#1415 (comment))

@giampaolo
Copy link
Owner

giampaolo commented Feb 27, 2019

For the APIs which fail at import time you can do this for now:

IPASE = os.uname().sysname == 'OS400'

if IPASE:
    def disk_io_counters(*args, **kw):
        return {}
else:
    disk_io_counters = cext.disk_io_counters

At this point, I think I've done what I can do without serious rip-up of the existing AIX path. I"m ready for this to land when you are. Many things don't work, but an install of psutil works and that makes me very happy.
I would love for this interim version to go live in the meantime.

Generally speaking, this is not what I have in mind. Other than merely being able to import psutil this PR should provide at the very least the basic functionality, like system-wide CPU and memory metrics (which is missing now). Where it's too difficult in C it's also OK to parse a subprocess command output in pure python (but we'll get to that later), or we can give up and return NotImplementedError, but we're not there yet. Let's not rush it.

@ThePrez
Copy link
Author

ThePrez commented Feb 28, 2019

@giampaolo, that's fair enough. Other than system-wide CPU and memory metrics, are there any other specific things you would want implemented before accepting the new platform support? I'll work on those.

In the meantime, I'll start from scratch, pulling IBM i into its own modules for both the Python and C code. It probably makes sense to close this PR until I have something more fully-baked.

@ThePrez ThePrez closed this Feb 28, 2019
@giampaolo
Copy link
Owner

giampaolo commented Feb 28, 2019

I would say you can exclude the most difficult ones for now, which are likely gonna be:

  • _psaix.cpu_count_physical (maybe)
  • psutil.net_connections
  • psutil.Process.connections

Process.open_files() may also be challenging. Perhaps the current bits in _psaix.py using subprocess module are gonna work already and you can reuse them.

As for where to put the C code: I think it will be more clear once you'll start adding stuff, but if you feel the need to split then do it in a psutil/arch/aix/ibase.c which you include from _psutil_aix.c which still remains the main module.

Also: can this be considered an AIX platform the same way FreeBSD OpenBSD NetBSD are "BSD"?

@ThePrez
Copy link
Author

ThePrez commented Feb 28, 2019

Quite often, we treat IBM i as an AIX variant. In fact, Python self-identifies as "AIX" as you can see. That is because IBM i runs an AIX runtime on top of an IBM i kernel. Often we say we're AIX and address some minor differences.

For psutil stuff, though, things might be different. Since performance tracking and process management aren't done by the AIX kernel we run, we need to call out to other parts of the system for lots of things (my other PR #1444 demonstrates). I'm guessing 70% of the functions will end up different from AIX, which makes me think a different implementation altogether might be the best choice, especially since we may want to use the ibm_db and itoolkit libraries. It would have some duplicate work, but would avoid a lot of #ifdefs and mysterious hasattr checks.

I'm 100% fine either way, just want you to understand that we're only about 30% similar to AIX. Thanks for your patience and assistance! It's going to be rocking once this is all working!

@giampaolo
Copy link
Owner

giampaolo commented Mar 1, 2019

For now keep _psaix.py and feel free to split into arch/aix/iphase.c if you see the C code differs too much. There's already plenty of both C and Python code which you can reuse (I did the same for BSD). Splitting the implementation in 2 separate py and C files would basically imply this is a new platform for which I'm not willing to add support in the doc. Also I don't want to differentiate psutil.IBASE constant from psutil.AIX. It's so exotic that it has to be considered the same as an AIX, so psutil.AIX will be True.

Also what is ibm_db_dbi? Is it on PYPI? That would mean introducing a dependency, which would be a first, so it's questionable (e.g. we didn't use pywin32 module on Windows).

Feel free to use ibm_db_dbi lib to start modeling the interface and to get familiar with the code, but a pure implementation in C or by using subprocess module should be preferred (in this order). Since this looks still premature I suggest you play with this in your private clone until you don't get the tests running, but feel free to ask for direction here. Also you may want take a look at:
https://github.com/giampaolo/psutil/blob/master/docs/DEVGUIDE.rst

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.

None yet

3 participants