Rust bindings for qclib #5

bwalk-at-ibm · 2025-07-03T07:59:15Z

This pull request adds bindings for the Rust programming language.

The second patch with examples is optional but provides usage example for the bindings by reimplementing zhypinfo and zname in Rust.

bindings/rust/Cargo.toml

bindings/rust/src/layer.rs

bindings/rust/Cargo.toml

bwalk-at-ibm · 2025-07-09T13:55:36Z

bindings/rust/src/layer.rs

+        Ok(LayerCategory::from_raw(category))
+    }
+
+    impl_attr!(layer_name() -> String = LayerName);


@Stefan-Raspl: according to the qclib docs, qc_layer_name is only not available for KVM hypervisors. Is this an oversight in the documentation? If it is available for all layers, I make this a common attribute like the ones above.

Could you point at the exact piece of documentation with that statement?

It's missing in the available attributes for KVM hypervisors.

Rust bindings implement an FFI interface to the qclib C library and allows the usage of qclib in Rust context. The bindings come in two crates: * qclib-sys: contains the raw FFI interface to the C library. * qclib: contains a safe and idiomatic abstraction for data types and functions provided by the C library. Signed-off-by: Bjoern Walk <[email protected]>

Add native Rust implementations of zhypinfo and zname as an example for the usage of the bindings. Signed-off-by: Bjoern Walk <[email protected]>

bwalk-at-ibm · 2025-07-11T08:45:37Z

bindings/rust/src/lib.rs

+        if rc < 0 {
+            return Err(Error { code: rc });
+        }


I'll add handling rc > 0 as well (occurs when no consistent data is available after migration).

Stefan-Raspl · 2025-07-13T10:13:05Z

bindings/rust/Cargo.toml

+
+[package]
+name = "qclib"
+version = "2.0.0"


What's the significance of "version"? Should this reflect the actual package's version?

This is the version of the rust crate (library). I had it aligned with the version of the C library, but this will get awkward if we have changes in the crate and release a new version while the C library has not changed. I commented here before changing it. I'm open to suggestions since I'm not sure what the best handling is.

Stefan-Raspl · 2025-07-13T10:13:41Z

bindings/rust/Cargo.toml

@@ -0,0 +1,22 @@
+[workspace.package]
+edition = "2021"


Is this the year of the code?

No, it's the Rust edition (think C standard revisions like C89, C11 and so on) used for this crate.

Stefan-Raspl · 2025-07-13T10:15:07Z

bindings/rust/LICENSE

@@ -0,0 +1 @@
+../../LICENSE


Use a symlink instead? I assume Rust strictly requires a license file..?

It is one :)

Yes, to publish the crate Rust requires an explicit license.

Stefan-Raspl · 2025-07-13T10:17:53Z

bindings/rust/src/layer.rs

+                qc: self.qc,
+                layer: self.layer,
+            }),
+            LayerType::LparGroup => todo!(),


So this would be essentially a preview or beta version..?

If you prefer, sure. I just implemented what I needed. The other layers should be trivial to implement but I don't have a testing environment available, so I left them out.

I can implement the rest or we could change the version to -beta, let me know what you prefer.

Stefan-Raspl · 2025-07-13T10:27:10Z

bindings/rust/qclib-sys/src/lib.rs

+#![allow(nonstandard_style)]
+
+pub type qc_layer_type = ::std::os::raw::c_uint;
+pub const QC_LAYER_TYPE_CEC: qc_layer_type = 1;


I don't feel too well with these re-defines. Are these really necessary? I would have assumed all it takes is a couple 'extern "C"' wrapped functions, no?!
At the very least, we should look into automating the generation of these as part of the build procedure.

What are you worried about?

Yes, this file can be created automatically via bindgen (which in fact I ~~did~~tried, but the results were suboptimal). I didn't want to mess with the build scripts for qclib, but I can look into it if you prefer.

I need the type definitions and their corresponding values in Rust context, I don't see a way around them.

Stefan-Raspl · 2025-07-13T10:30:38Z

bindings/rust/src/layers/cec.rs

+        map: &mut serde_json::Map<String, serde_json::Value>,
+    ) -> Result<()> {
+        json_pair!(map <- Manufacturer: string? = self.manufacturer()?);
+        json_pair!(map <- Type: string? = self.machine_type()?);


Might be my lack of knowledge of Rust: This is for generating json output? We already have routines in qclib - why re-implement?!

Yes, it's for JSON output. The qc_export_json function that qclib provides dumps the JSON content to stdout and doesn't give the user any control over that.

I want to give the user more control, so I just create a JSON object and return that, allowing the user to process that how they prefer. It's done via a defacto standard dependencies for JSON handling and deserialization.

Stefan-Raspl · 2025-07-13T10:33:04Z

bindings/rust/src/lib.rs

+        let mut rc = 0;
+        let layers = unsafe { sys::qc_get_num_layers(self.as_raw(), &mut rc) };
+
+        if rc < 0 {


If I got this right, we're calling the C version of get_num_layers() here, but still check return codes. Why would we want to do that - to better the C version of the code...?

I don't understand, why wouldn't we check the return code? We are calling into the raw C function, so we need to check rc.

bwalk-at-ibm commented Jul 7, 2025

View reviewed changes

bindings/rust/Cargo.toml Outdated Show resolved Hide resolved

bwalk-at-ibm commented Jul 7, 2025

View reviewed changes

bindings/rust/src/layer.rs Outdated Show resolved Hide resolved

bwalk-at-ibm commented Jul 7, 2025

View reviewed changes

bindings/rust/Cargo.toml Outdated Show resolved Hide resolved

bwalk-at-ibm force-pushed the bwalk/bindings branch from 0c75339 to 180d921 Compare July 8, 2025 08:09

bwalk-at-ibm commented Jul 9, 2025

View reviewed changes

bwalk-at-ibm added 2 commits July 10, 2025 20:03

bindings: add examples for Rust bindings

3c41a17

Add native Rust implementations of zhypinfo and zname as an example for the usage of the bindings. Signed-off-by: Bjoern Walk <[email protected]>

bwalk-at-ibm force-pushed the bwalk/bindings branch from 180d921 to 3c41a17 Compare July 10, 2025 18:03

bwalk-at-ibm commented Jul 11, 2025

View reviewed changes

Stefan-Raspl reviewed Jul 13, 2025

View reviewed changes

		@@ -0,0 +1 @@
		../../LICENSE

Rust bindings for qclib #5

Are you sure you want to change the base?

Rust bindings for qclib #5

Uh oh!

Conversation

bwalk-at-ibm commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Stefan-Raspl Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwalk-at-ibm Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Stefan-Raspl Jul 13, 2025 •

edited

Loading

bwalk-at-ibm Jul 13, 2025 •

edited

Loading