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

libmicrovmi python bindings #162

Closed
7 of 8 tasks
Wenzel opened this issue Jan 29, 2021 · 19 comments · Fixed by #165
Closed
7 of 8 tasks

libmicrovmi python bindings #162

Wenzel opened this issue Jan 29, 2021 · 19 comments · Fixed by #165
Labels
enhancement New feature or request python

Comments

@Wenzel
Copy link
Owner

Wenzel commented Jan 29, 2021

Create Python bindings to call libmicrovmi from Python

TODO (updated):

  • decide how to integrate this new library in our build system
  • expose DriverType and DriverInitParam
  • expose read_physical
  • package and upload to PyPI
  • update setup.py to accept --features parameter, and set features in RustExtension
  • create new Dockerfile based on quay.io/pypa/manylinux2014_x86_64 and install all the drivers required development headers
  • update build-wheels.sh to accept --features and pass them to setup.py
  • read all relevant fields of python/Cargo.toml to fill setup.py metadata

We should use the rust-cpython project

rust-cpython will help us to generate a Python extension module, wrapping the libmicrovmi Rust API.

how to integrate this new library

  1. create a separate repository
  2. use cargo workspace to manage both Rust library and Python extension in the same git repository, in tandem.
  3. maybe generate both with the same cargo.toml (I doubt this is possible)

@rageagainsthepc if you have an opinion

@Wenzel Wenzel mentioned this issue Jan 29, 2021
1 task
@Wenzel Wenzel added enhancement New feature or request python labels Jan 29, 2021
@Wenzel
Copy link
Owner Author

Wenzel commented Feb 2, 2021

After looking at rust-cpython, a couple of things:

  • there is no need to create a workspace, we can produce the same libmicrovmi.so that will be considered both the official C API, and the native Python extension
  • PyO3 is a fork of rust-cpython, at the time when it was maintained, and it proposes a better abstraction in my opinion. the only drawback was that it required nightly rust, but 6 months ago, it became available with stable rust

@rageagainsthepc
Copy link
Collaborator

the only drawback was that it required nightly rust, but 6 months ago, it became available with stable rust

that's awesome, my info was not up to date then, because I thought it was still nightly only. 👍

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 2, 2021

there is no need to create a workspace, we can produce the same libmicrovmi.so that will be considered both the official C API, and the native Python extension

actually not a good idea, since the library produced has undefined symbols to the Python API, requiring that any binary would link with Python libs.

                 U PyBaseObject_Type
                 U PyBytes_AsString
                 U PyBytes_Size
                 U _Py_Dealloc
                 U PyDict_Copy
                 U PyDict_DelItem
                 U PyDict_GetItem
                 U PyDict_Next
                 U PyDict_Size
                 U PyErr_Fetch
                 U PyErr_GivenExceptionMatches
                 U PyErr_NewException
                 U PyErr_NormalizeException
                 U PyErr_Print
                 U PyErr_PrintEx
                 U PyErr_Restore
                 U PyEval_SaveThread
                 U PyEval_ThreadsInitialized
                 U PyExc_AttributeError
                 U PyExc_BaseException
                 U PyExc_SystemError
                 U PyExc_TypeError
                 U PyExc_ValueError
                 U Py_Finalize
                 U PyGILState_Ensure
                 U PyGILState_Release
                 U Py_InitializeEx

so workspace and splitting is a good idea.

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

I investigated how to distribute the generate bindings.

maturin seems like the way to go for PyO3 bindings.
The tooling is good, however, I'm limited because maturin generates a wheel based on the crate name, and this isn't configurable.

The crate name right now is pymicrovmi, however this is only a conflict free value for the whole workspace, and the python package name should be microvmi.
A user should be able to do

pip install microvmi
and
from microvmi import Microvmi

Unfortunately, maturin doesn't allow to override this package name at the moment, and only uses the Cargo lib.name field.
This is an opened issue on maturin repo:
PyO3/maturin#313

Looking at this comment it looks like setuptools-rust is more flexible (yet older) alternative for now.

I will have a look at it

@rageagainsthepc
Copy link
Collaborator

What's wrong with pymicrovmi? After all many python bindings are named similarly if I remember correctly.

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

What's wrong with pymicrovmi?

I prefer the simplicity of having microvmi when you look for it on pypi and when you use the module than having a useless py prefix.
it's a python module, we know it's for python, there is no need for a prefix like this :)
it's just removing useless noise.

@rageagainsthepc
Copy link
Collaborator

Sure, I get that sentiment. I am just not sure whether it's worth pursuing other routes over. ;)

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

Sure, I get that sentiment. I am just not sure whether it's worth pursuing other routes over. ;)

I managed to get setuptools-rust working.
Maybe in a few months, maturin will have improved enough to edit the package name, who knows

@rageagainsthepc
Copy link
Collaborator

Are there any drawbacks at all if we are using setuptools-rust instead of maturin?

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

Maturin is described as:

This project is meant as a zero configuration replacement for setuptools-rust and milksnake.

So no, we are just not following the hype ... yet :)

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

Publishing wheels on PyPI is a pain in the ass.

PyPI will refuse linux_x86_64 platform tag

Uploading distributions to https://upload.pypi.org/legacy/
Uploading microvmi-0.0.1-cp36-cp36m-linux_x86_64.whl
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 1.15M/1.15M [00:01<00:00, 755kB/s]
NOTE: Try --verbose to see response content.
HTTPError: 400 Client Error: Binary wheel 'microvmi-0.0.1-cp36-cp36m-linux_x86_64.whl' has an unsupported platform tag 'linux_x86_64'. for url: https://upload.pypi.org/legacy/

reason: https://stackoverflow.com/a/59586096/3017219
so we need to generate the manylinux1 platform tag now, using this docker image:
https://github.com/pypa/manylinux

However, Rust can't be installed on the manylinux1 docker image, the Glibc is too old:

(venv)  wenzel@Strix  ~/Projets/rust/libmicrovmi   python_bindings  docker run --rm -v `pwd`:/io quay.io/pypa/manylinux1_x86_64 /io/python/build-wheels.sh
+ curl https://sh.rustup.rs -sSf
+ sh -s -- --default-toolchain stable -y
info: downloading installer
/tmp/tmp.ZfVxKoeK20/rustup-init: /lib64/libc.so.6: version `GLIBC_2.7' not found (required by /tmp/tmp.ZfVxKoeK20/rustup-init)
/tmp/tmp.ZfVxKoeK20/rustup-init: /lib64/libc.so.6: version `GLIBC_2.9' not found (required by /tmp/tmp.ZfVxKoeK20/rustup-init)

with manylinux2010, PyO3 won't compile:

       Fresh microvmi v0.1.10 (/io)
error: failed to run custom build command for `pyo3 v0.13.1`

Caused by:
  process didn't exit successfully: `/io/target/release/build/pyo3-66b94e3e1352bcf0/build-script-build` (exit code: 1)
  --- stderr
  /io/target/release/build/pyo3-66b94e3e1352bcf0/build-script-build: /lib64/libc.so.6: version `GLIBC_2.15' not found (required by /io/target/release/build/pyo3-66b94e3e1352bcf0/build-script-build)
  /io/target/release/build/pyo3-66b94e3e1352bcf0/build-script-build: /lib64/libc.so.6: version `GLIBC_2.14' not found (required by /io/target/release/build/pyo3-66b94e3e1352bcf0/build-script-build)
cargo rustc --lib --manifest-path Cargo.toml --features pyo3/extension-module --release --verbose -- --crate-type cdylib
error: cargo failed with code: 101

and with manylinux2014, auditwheel is unable to convert the platform tag to manylinux1:
➡️ actually now it works, and i have no idea why 😶

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 3, 2021

Okay, I uploaded a first version on PyPI to reserve the name, and generate a project specific API token (which is not possible without a first upload)
https://pypi.org/project/microvmi/ 🎉

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 4, 2021

About packaging, we have a problem.

Currently, the python library will rebuild libmicrovmi and embed it inside the Python extension.
To build the final binary distribution, we use manylinux, based on CentOS 6.
This means that inside this docker container, if we want to activate all linux drivers, we need to install Xen headers, libkvmi headers, FDP headers (virtualbox).
How can we install recent Xen headers (4.11.0) on CentOS ?
it's not as easy as apt-get install libxen-dev.

and for the rest, we might face an issue with outdated software that just won't compile, but not sure yet.

I have no idea to propose for now.

@rageagainsthepc
Copy link
Collaborator

Sounds tough, maybe we shouldn't use the official manylinux image then? We are not going to support older linux versions anyway.

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 5, 2021

Sounds tough, maybe we shouldn't use the official manylinux image then? We are not going to support older linux versions anyway.

the issue is that manylinux is the only way to upload a wheel on PyPI.
wheels for linux_x86_64 platform are not accepted anymore.

I managed to compile everything on this CentOS 6, so let's keep this for now.

other topic: read_physical on Python is slow, a relevant thread on passing large buffer from Rust to Python with PyO3:
PyO3/pyo3#1385

@rageagainsthepc
Copy link
Collaborator

the issue is that manylinux is the only way to upload a wheel on PyPI.
wheels for linux_x86_64 platform are not accepted anymore.

Sure, but you could use unofficial images based on newer linux distros. But if you already got it working there is no need to change it now of course ;)

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 6, 2021

Sure, but you could use unofficial images based on newer linux distros.

I don't think you can skip CentOS, the whole point is to use a very old ABI to have maximum compatibility, that's why they take CentOS 5/6.
So i'm not sure you could generate a "manylinux" wheel on ubuntu 20.04
The image we use is targeting manylinux2014:
https://www.python.org/dev/peps/pep-0599/

Similar to how PEP 571 and PEP 513 drew allowed shared libraries and their symbol versions from CentOS 5.11 and CentOS 6, respectively, a manylinux2014 platform tag will draw its libraries and symbol versions from CentOS 7, which will reach end-of-life on June 30th, 2024. [1]

And the reason we need this manylinux conversion is stated in this PEP:
https://www.python.org/dev/peps/pep-0513/#rationale

@rageagainsthepc
Copy link
Collaborator

I don't think you can skip CentOS, the whole point is to use a very old ABI to have maximum compatibility, that's why they take CentOS 5/6.

Well, imho you are already breaking this philosophy by installing a bunch of custom headers/libraries that are only available on newer systems (at least newer than CentOS 6). Looks to me like you are cheating a little, so to speak ;)

@Wenzel
Copy link
Owner Author

Wenzel commented Feb 23, 2021

PyO3 just added support for a custom wheel name:
PyO3/maturin#313 (comment)

I'm keeping the setuptools-rust based setup for now, but we could switch to maturin one day 👍

It actually doesn't event matter anymore since we providing a python library microvmi, which imports the Python extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants