rustvmm_gen: Introduce `rustvmm_gen` #177

RuoqingHe · 2025-02-17T09:06:35Z

Summary of the PR

rustvmm_gen: Introduce lib_kernel_source.py
rustvmm_gen: Introduce lib_syscall.py
rustvmm_gen: Introduce rustvmm_gen

rustvmm_gen.py integrates kernel version checking, source extraction, compilation and header installation. Header files could be prepared with command:

./rustvmm_gen/rustvmm_gen.py --arch <arm64/x86_64/riscv> --version <X.Y/X.Y.Z> prepare

Source code required by seccompiler could be generated with command:

./rustvmm_gen/rustvmm_gen.py --arch <arm64/x86_64/riscv> --version <X.Y/X.Y.Z> generate_syscall

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

All commits in this PR have Signed-Off-By trailers (with
git commit -s), and the commit message has max 60 characters for the
summary and max 75 characters for each description line.
All added/changed functionality has a corresponding unit/integration
test.
All added/changed public-facing functionality has entries in the "Upcoming
Release" section of CHANGELOG.md (if no such section exists, please create one).
Any newly added unsafe code is properly documented.

roypat

Cool idea! I like having some centralized tools for dealing with all the bindings generation for our various crates. I do think the seccompiler-specific part should go into rust-vmm/seccompiler, however. What I'd find really useful would be to have some wrapper around bindgen-rs that can be used to generate the bindings in crates like linux-loader and kvm-bindings (and I maybe xen-sys?) as well, built on top of the kernel header fetching you included here. Do you think that's a possible extension? :o

As a high level comment, maybe let's rename rustvmm_gen/ to just be scripts/, or tools/, and drop the lib_ prefix from the library files, instead moving them to some lib subdirectory.

roypat · 2025-02-17T10:41:10Z

rustvmm_gen/lib_kernel_source.py

+def create_temp_dir(version):
+    temp_dir = os.path.join("/tmp", f"linux-{version}-prepare")
+
+    if os.path.exists(temp_dir):
+        shutil.rmtree(temp_dir)
+
+    try:
+        os.makedirs(temp_dir)
+        return temp_dir
+    except OSError as e:
+        raise RuntimeError(f"Failed to create temp directory: {e}") from e


Let's use something that relies on mkdtemp(2) here to avoid race conditions and the "delete if exists" dance: https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory

roypat · 2025-02-17T10:42:11Z

rustvmm_gen/lib_kernel_source.py

+    """
+    # Validate version format
+    if not re.match(r"^\d+\.\d+(\.\d+)?$", version):
+        return (False, "Invalid version format. Use X.Y or X.Y.Z")


Why this tuple of string and bool instead of just raising exceptions for errors, and returning void?

roypat · 2025-02-17T10:44:00Z

rustvmm_gen/lib_kernel_source.py

+
+
+def install_headers(src_dir, arch, install_path, cleanup=False):
+    os.chdir(src_dir)


This never chdirs back. Maybe with contextlib.chdir(src_dir):?

Removed, changed to make -C instead

roypat · 2025-02-17T10:44:50Z

rustvmm_gen/lib_syscall.py

This script is specific to rust-vmm/seccompiler, right? If so it should probably live in that repository

Yes, that lib file is specific to seccompiler, and I'm going to write according lib file for kvm-bindings and linux-loader, all of them are going to need installed headers to continue, WDYT 🤔

The generic parts, like downloading linux sources and installing headers, can live in rust-vmm-ci, and then there can be scripts in kvm-bindings/linux-leader/seccompiler that each call into the generic rust-vmm-ci scripts.

For kvm-bindings and linux-loader in particular there's probably some more common parts that can be moved into this repository (I was hoping some script that takes some .h files as input and produces .rs bindings from them?)

If we have the required headers, for kvm-bindings and linux-loader we are only one step away which is a invocation of bindgen, and modify something (like for serde support), IMO that's similar to seccompiler while seccompiler doesn't need bindgen

roypat · 2025-02-17T10:46:12Z

rustvmm_gen/lib_kernel_source.py

+            print(f"Cleaning temporary files at {parent_dir}")
+            shutil.rmtree(parent_dir)
+
+        return True


I'd also just drop the boolean return value here and simply raise an exception in the failure case

roypat · 2025-02-17T10:47:18Z

rustvmm_gen/lib_kernel_source.py

+            stderr=subprocess.STDOUT,
+            text=True,
+        )
+        print(result.stdout)


If we just print result.stdout anyway, why not stdout=subprocess.PIPE above?

roypat · 2025-02-17T10:48:44Z

rustvmm_gen/rustvmm_gen.py

+    except Exception as e:
+        print(f"\nError: {str(e)}")
+        exit(1)


Not really a need for this try-catch + exit, I think. The call to this function is the final statement in main(), so we hit an exception we'd exit with non-zero code anyway (and additionally printing "Error:" doesn't really add much imo), so can just drop this.

RuoqingHe · 2025-02-17T11:11:03Z

Sorry I forget to mark this as a draft, kvm-bindings generation is on its way 😂

Thanks @roypat for reviewing! ❤️

roypat · 2025-02-17T11:12:38Z

Sorry I forget to mark this as a draft, kvm-bindings generation is on its way 😂

Thanks @roypat for reviewing! ❤️

Ahahhaah, so sorry for jumping the gun! Looking forward to it then :D

`kernel_source.py` contains functions used to check version of desired kernel, download, extract kernel source to specific directory and get the headers out of it. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

`syscall.py` contains functions used to extract `syscall_table` from headers installed by `kernel_source.py` and generate according Rust code for specific architecture. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

Add `__init__.py` for python programs outside `lib` to reference functions inside lib. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

`rustvmm_gen.py` integrates kernel version checking, source extraction, compilation and header installation. Header files could be prepared with command: ```command ./rustvmm_gen/rustvmm_gen.py --arch <arm64/x86_64/riscv> --version <X.Y/X.Y.Z> prepare ``` Source code required by `seccompiler` could be generated with command: ```command ./rustvmm_gen/rustvmm_gen.py --arch <arm64/x86_64/riscv> --version <X.Y/X.Y.Z> generate_syscall ``` Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

RuoqingHe requested review from andreeaflorescu, lauralt, sameo, roypat and ShadowCurse as code owners February 17, 2025 09:06

RuoqingHe mentioned this pull request Feb 17, 2025

Update syscall to 6.12 rust-vmm/seccompiler#78

Open

4 tasks

roypat reviewed Feb 17, 2025

View reviewed changes

RuoqingHe marked this pull request as draft February 17, 2025 11:11

RuoqingHe force-pushed the rustvmm_gen branch from 99f5149 to 0686e7d Compare February 20, 2025 07:37

RuoqingHe added 4 commits February 20, 2025 15:41

scripts: Introduce kernel_source.py

a592de5

`kernel_source.py` contains functions used to check version of desired kernel, download, extract kernel source to specific directory and get the headers out of it. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

scripts: Introduce syscall.py

32a1e33

`syscall.py` contains functions used to extract `syscall_table` from headers installed by `kernel_source.py` and generate according Rust code for specific architecture. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

scripts: Setup lib folder

9652fb7

Add `__init__.py` for python programs outside `lib` to reference functions inside lib. Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>

RuoqingHe force-pushed the rustvmm_gen branch from 0686e7d to d871f25 Compare February 20, 2025 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustvmm_gen: Introduce `rustvmm_gen` #177

rustvmm_gen: Introduce `rustvmm_gen` #177

RuoqingHe commented Feb 17, 2025

roypat left a comment

roypat Feb 17, 2025

roypat Feb 17, 2025

roypat Feb 17, 2025

RuoqingHe Feb 20, 2025

roypat Feb 17, 2025

RuoqingHe Feb 20, 2025

roypat Feb 20, 2025

RuoqingHe Feb 20, 2025 •

edited

Loading

roypat Feb 17, 2025

roypat Feb 17, 2025

roypat Feb 17, 2025

RuoqingHe commented Feb 17, 2025

roypat commented Feb 17, 2025



		def install_headers(src_dir, arch, install_path, cleanup=False):
		os.chdir(src_dir)

rustvmm_gen: Introduce rustvmm_gen #177

Are you sure you want to change the base?

rustvmm_gen: Introduce rustvmm_gen #177

Conversation

RuoqingHe commented Feb 17, 2025

Summary of the PR

Requirements

roypat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RuoqingHe Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RuoqingHe commented Feb 17, 2025

roypat commented Feb 17, 2025

rustvmm_gen: Introduce `rustvmm_gen` #177

rustvmm_gen: Introduce `rustvmm_gen` #177

RuoqingHe Feb 20, 2025 •

edited

Loading