[PATCH preview 0/3] reviving minimal QAPI generation from 2021

[PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

This is just an extremely minimal extraction from the patches at
https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/,
limited to generating structs and enums from the QAPI schema.
It does not include them in any crate and does not compile them.

While I'm not going to work on this, I was curious how much work it
was to produce *some* kind of Rust QAPI struct, which could be a first
step towards using serde as an interface to C visitors, like this:

trait QapiType: FreeForeign {
    unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut <Self as FreeForeign>::Foreign, errp: *mut *mut bindings::Error);
}

fn to_c<T: QAPIType>(obj: &T) -> *mut <T as FreeForeign>::Foreign {
    let mut ptr = libc::calloc(...);
    let mut ser = QapiSerializer::<T>::new(ptr);
    obj.serialize(&mut ser).unwrap();
    ptr.cast()
}

unsafe fn from_c<T: QAPIType>(obj: *const <T as FreeForeign>::Foreign) -> T {
    let mut de = QapiDeserializer::new(T::visit, obj as *const c_void);
    let value = de::Deserialize::deserialize(&mut de).unwrap();
    de.end().unwrap();
    value
}

/* Generated code below: */

impl QapiType for UefiVariable {
    unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
        unsafe extern "C" visit_type_DumpGuestMemoryFormat(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
        unsafe { visit_type_DumpGuestMemoryFormat(v, name, obj, errp); }
    }
}

impl FreeForeign for UefiVariable {
    type Foreign = bindings::UefiVariable;

    unsafe fn free_foreign(p: *mut bindings::UefiVariable) {
        unsafe extern "C" qapi_free_UefiVariable(p: *mut bindings::UefiVariable);
        unsafe { qapi_free_UefiVariable(p); }
    }
}

impl CloneToForeign for UefiVariable {
    fn clone_to_foreign(&self) -> OwnedPointer<Self> {
        OwnedPointer::new(qapi::to_c(self))
    }
}

impl FromForeign for UefiVariable {
    unsafe fn cloned_from_foreign(obj: *const bindings::UefiVariable) -> Self {
        qapi::from_c(obj)
    }
}

The FFI types could be generated by qapi-gen, as in Marc-André's
proposal, or from bindgen.

I am not sure what approach is better---whether to use serde or to
automatically generate the marshaling and unmarshaling code; and whether
to use bindgen or generate C-compatible FFI types---but it made sense,
from the point of view of extracting "some" code from Marc-André's
proof of concept and enticing other people, :) to start from high-level
types.

Paolo

Marc-André Lureau (2):
  scripts/qapi: add QAPISchemaIfCond.rsgen()
  scripts/qapi: generate high-level Rust bindings

Paolo Bonzini (1):
  rust: make TryFrom macro more resilient

 meson.build                     |   4 +-
 rust/qemu-api-macros/src/lib.rs |   7 +-
 scripts/qapi/backend.py         |   4 +-
 scripts/qapi/common.py          |  16 ++
 scripts/qapi/main.py            |   4 +-
 scripts/qapi/rs.py              | 183 ++++++++++++++++++
 scripts/qapi/rs_types.py        | 320 ++++++++++++++++++++++++++++++++
 scripts/qapi/schema.py          |   4 +
 8 files changed, 535 insertions(+), 7 deletions(-)
 create mode 100644 scripts/qapi/rs.py
 create mode 100644 scripts/qapi/rs_types.py

-- 
2.49.0

[PATCH 1/3] rust: make TryFrom macro more resilient

[Paolo Bonzini]

If the enum includes values such as "Ok", "Err", or "Error", the TryInto
macro can cause errors.  Be careful and qualify identifiers with the full
path, or in the case of TryFrom<>::Error do not use the associated type
at all.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api-macros/src/lib.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 103470785e3..c18bb4e036f 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -203,8 +203,8 @@ fn derive_tryinto_body(
     Ok(quote! {
         #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
         match value {
-            #(#discriminants => Ok(#name::#discriminants),)*
-            _ => Err(value),
+            #(#discriminants => core::result::Result::Ok(#name::#discriminants),)*
+            _ => core::result::Result::Err(value),
         }
     })
 }
@@ -236,7 +236,8 @@ pub const fn from_bits(value: #repr) -> Self {
         impl core::convert::TryFrom<#repr> for #name {
             type Error = #repr;
 
-            fn try_from(value: #repr) -> Result<Self, Self::Error> {
+            #[allow(ambiguous_associated_items)]
+            fn try_from(value: #repr) -> Result<Self, #repr> {
                 #body
             }
         }
-- 
2.49.0

[PATCH 2/3] scripts/qapi: add QAPISchemaIfCond.rsgen()

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Generate Rust #[cfg(...)] guards from QAPI 'if' conditions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Link: https://lore.kernel.org/r/20210907121943.3498701-15-marcandre.lureau@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/qapi/common.py | 16 ++++++++++++++++
 scripts/qapi/schema.py |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index d7c8aa3365c..a8ea5792c11 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -199,6 +199,22 @@ def guardend(name: str) -> str:
                  name=c_fname(name).upper())
 
 
+def rsgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
+
+    def cfg(ifcond: Union[str, Dict[str, Any]]):
+        if isinstance(ifcond, str):
+            return ifcond
+        if isinstance(ifcond, list):
+            return ', '.join([cfg(c) for c in ifcond])
+        oper, operands = next(iter(ifcond.items()))
+        operands = cfg(operands)
+        return f'{oper}({operands})'
+
+    if not ifcond:
+        return ''
+    return '#[cfg(%s)]' % cfg(ifcond)
+
+
 def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
                cond_fmt: str, not_fmt: str,
                all_operator: str, any_operator: str) -> str:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cbe3b5aa91e..0fb151b5d89 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -39,6 +39,7 @@
     docgen_ifcond,
     gen_endif,
     gen_if,
+    rsgen_ifcond,
 )
 from .error import QAPIError, QAPISemError, QAPISourceError
 from .expr import check_exprs
@@ -65,6 +66,9 @@ def gen_endif(self) -> str:
     def docgen(self) -> str:
         return docgen_ifcond(self.ifcond)
 
+    def rsgen(self) -> str:
+        return rsgen_ifcond(self.ifcond)
+
     def is_present(self) -> bool:
         return bool(self.ifcond)
 
-- 
2.49.0

[PATCH 3/3] scripts/qapi: generate high-level Rust bindings

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Generate high-level idiomatic Rust code for the QAPI types, with to/from
translations for the C FFI.

- char* is mapped to String, scalars to there corresponding Rust types

- enums are simply aliased from FFI

- has_foo/foo members are mapped to Option<T>

- lists are represented as Vec<T>

- structures have Rust versions, with To/From FFI conversions

- alternate are represented as Rust enum

- unions are represented in a similar way as in C: a struct S with a "u"
  member (since S may have extra 'base' fields). However, the discriminant
  isn't a member of S, since Rust enum already include it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Link: https://lore.kernel.org/r/20210907121943.3498701-21-marcandre.lureau@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build              |   4 +-
 scripts/qapi/backend.py  |   4 +-
 scripts/qapi/main.py     |   4 +-
 scripts/qapi/rs.py       | 183 ++++++++++++++++++++++
 scripts/qapi/rs_types.py | 320 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 511 insertions(+), 4 deletions(-)
 create mode 100644 scripts/qapi/rs.py
 create mode 100644 scripts/qapi/rs_types.py

diff --git a/meson.build b/meson.build
index 967a10e80b8..cdfbc7241fb 100644
--- a/meson.build
+++ b/meson.build
@@ -3531,12 +3531,14 @@ qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py',
                      meson.current_source_dir() / 'scripts/qapi/introspect.py',
                      meson.current_source_dir() / 'scripts/qapi/main.py',
                      meson.current_source_dir() / 'scripts/qapi/parser.py',
+                     meson.current_source_dir() / 'scripts/qapi/rs_types.py',
                      meson.current_source_dir() / 'scripts/qapi/schema.py',
                      meson.current_source_dir() / 'scripts/qapi/source.py',
                      meson.current_source_dir() / 'scripts/qapi/types.py',
                      meson.current_source_dir() / 'scripts/qapi/features.py',
                      meson.current_source_dir() / 'scripts/qapi/visit.py',
-                     meson.current_source_dir() / 'scripts/qapi-gen.py'
+                     meson.current_source_dir() / 'scripts/qapi-gen.py',
+                     meson.current_source_dir() / 'scripts/qapi/rs.py',
 ]
 
 tracetool = [
diff --git a/scripts/qapi/backend.py b/scripts/qapi/backend.py
index 14e60aa67af..2e76370c72d 100644
--- a/scripts/qapi/backend.py
+++ b/scripts/qapi/backend.py
@@ -9,6 +9,7 @@
 from .introspect import gen_introspect
 from .schema import QAPISchema
 from .types import gen_types
+from .rs_types import gen_rs_types
 from .visit import gen_visit
 
 
@@ -35,7 +36,7 @@ def generate(self,
         """
 
 
-class QAPICBackend(QAPIBackend):
+class QAPICodeBackend(QAPIBackend):
 
     def generate(self,
                  schema: QAPISchema,
@@ -55,6 +56,7 @@ def generate(self,
 
         :raise QAPIError: On failures.
         """
+        gen_rs_types(schema, output_dir, prefix, builtins)
         gen_types(schema, output_dir, prefix, builtins)
         gen_features(schema, output_dir, prefix)
         gen_visit(schema, output_dir, prefix, builtins)
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 0e2a6ae3f07..4ad75e213f5 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -12,7 +12,7 @@
 import sys
 from typing import Optional
 
-from .backend import QAPIBackend, QAPICBackend
+from .backend import QAPIBackend, QAPICodeBackend
 from .common import must_match
 from .error import QAPIError
 from .schema import QAPISchema
@@ -27,7 +27,7 @@ def invalid_prefix_char(prefix: str) -> Optional[str]:
 
 def create_backend(path: str) -> QAPIBackend:
     if path is None:
-        return QAPICBackend()
+        return QAPICodeBackend()
 
     module_path, dot, class_name = path.rpartition('.')
     if not dot:
diff --git a/scripts/qapi/rs.py b/scripts/qapi/rs.py
new file mode 100644
index 00000000000..c1e76e074fb
--- /dev/null
+++ b/scripts/qapi/rs.py
@@ -0,0 +1,183 @@
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+QAPI Rust generator
+"""
+
+import os
+import re
+import subprocess
+from typing import NamedTuple, Optional
+
+from .common import POINTER_SUFFIX
+from .gen import QAPIGen
+from .schema import QAPISchemaModule, QAPISchemaVisitor
+
+
+# see to_snake_case() below
+snake_case = re.compile(r'((?<=[a-z0-9])[A-Z]|(?!^)[A-Z](?=[a-z]))')
+
+
+rs_name_trans = str.maketrans('.-', '__')
+
+
+# Map @name to a valid Rust identifier.
+# If @protect, avoid returning certain ticklish identifiers (like
+# keywords) by prepending raw identifier prefix 'r#'.
+def rs_name(name: str, protect: bool = True) -> str:
+    name = name.translate(rs_name_trans)
+    if name[0].isnumeric():
+        name = '_' + name
+    if not protect:
+        return name
+    # based from the list:
+    # https://doc.rust-lang.org/reference/keywords.html
+    if name in ('Self', 'abstract', 'as', 'async',
+                'await', 'become', 'box', 'break',
+                'const', 'continue', 'crate', 'do',
+                'dyn', 'else', 'enum', 'extern',
+                'false', 'final', 'fn', 'for',
+                'if', 'impl', 'in', 'let',
+                'loop', 'macro', 'match', 'mod',
+                'move', 'mut', 'override', 'priv',
+                'pub', 'ref', 'return', 'self',
+                'static', 'struct', 'super', 'trait',
+                'true', 'try', 'type', 'typeof',
+                'union', 'unsafe', 'unsized', 'use',
+                'virtual', 'where', 'while', 'yield'):
+        name = 'r#' + name
+    # avoid some clashes with the standard library
+    if name in ('String',):
+        name = 'Qapi' + name
+
+    return name
+
+
+def rs_type(c_type: str,
+            qapi_ns: Optional[str] = 'qapi::',
+            optional: Optional[bool] = False,
+            box: bool = False) -> str:
+    (is_pointer, _, is_list, c_type) = rs_ctype_parse(c_type)
+    # accepts QAPI types ('any', 'str', ...) as we translate
+    # qapiList to Rust FFI types here.
+    to_rs = {
+        'any': 'QObject',
+        'bool': 'bool',
+        'char': 'i8',
+        'double': 'f64',
+        'int': 'i64',
+        'int16': 'i16',
+        'int16_t': 'i16',
+        'int32': 'i32',
+        'int32_t': 'i32',
+        'int64': 'i64',
+        'int64_t': 'i64',
+        'int8': 'i8',
+        'int8_t': 'i8',
+        'null': 'QNull',
+        'number': 'f64',
+        'size': 'u64',
+        'str': 'String',
+        'uint16': 'u16',
+        'uint16_t': 'u16',
+        'uint32': 'u32',
+        'uint32_t': 'u32',
+        'uint64': 'u64',
+        'uint64_t': 'u64',
+        'uint8': 'u8',
+        'uint8_t': 'u8',
+        'String': 'QapiString',
+    }
+    if is_pointer:
+        to_rs.update({
+            'char': 'String',
+        })
+
+    if is_list:
+        c_type = c_type[:-4]
+
+    to_rs = to_rs.get(c_type)
+    if to_rs:
+        ret = to_rs
+    else:
+        ret = qapi_ns + c_type
+
+    if is_list:
+        ret = 'Vec<%s>' % ret
+    elif is_pointer and not to_rs and box:
+        ret = 'Box<%s>' % ret
+    if optional:
+        ret = 'Option<%s>' % ret
+    return ret
+
+
+class CType(NamedTuple):
+    is_pointer: bool
+    is_const: bool
+    is_list: bool
+    c_type: str
+
+
+def rs_ctype_parse(c_type: str) -> CType:
+    is_pointer = False
+    if c_type.endswith(POINTER_SUFFIX):
+        is_pointer = True
+        c_type = c_type[:-len(POINTER_SUFFIX)]
+    is_list = c_type.endswith('List')
+    is_const = False
+    if c_type.startswith('const '):
+        is_const = True
+        c_type = c_type[6:]
+
+    c_type = rs_name(c_type)
+    return CType(is_pointer, is_const, is_list, c_type)
+
+
+def to_camel_case(value: str) -> str:
+    # special case for last enum value
+    if value == '_MAX':
+        return value
+    raw_id = False
+    if value.startswith('r#'):
+        raw_id = True
+        value = value[2:]
+    value = ''.join('_' + word if word[0].isdigit()
+                    else word[:1].upper() + word[1:]
+                    for word in filter(None, re.split("[-_]+", value)))
+    if raw_id:
+        return 'r#' + value
+    return value
+
+
+def to_snake_case(value: str) -> str:
+    return snake_case.sub(r'_\1', value).lower()
+
+
+class QAPIGenRs(QAPIGen):
+    pass
+
+
+class QAPISchemaRsVisitor(QAPISchemaVisitor):
+
+    def __init__(self, prefix: str, what: str):
+        super().__init__()
+        self._prefix = prefix
+        self._what = what
+        self._gen = QAPIGenRs(self._prefix + self._what + '.rs')
+        self._main_module: Optional[str] = None
+
+    def visit_module(self, name: Optional[str]) -> None:
+        if name is None:
+            return
+        if QAPISchemaModule.is_user_module(name):
+            if self._main_module is None:
+                self._main_module = name
+
+    def write(self, output_dir: str) -> None:
+        self._gen.write(output_dir)
+
+        pathname = os.path.join(output_dir, self._gen.fname)
+        try:
+            subprocess.check_call(['rustfmt', pathname])
+        except FileNotFoundError:
+            pass
diff --git a/scripts/qapi/rs_types.py b/scripts/qapi/rs_types.py
new file mode 100644
index 00000000000..a182b33917a
--- /dev/null
+++ b/scripts/qapi/rs_types.py
@@ -0,0 +1,320 @@
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+QAPI Rust types generator
+"""
+
+from typing import List, Optional
+
+from .common import POINTER_SUFFIX, mcgen
+from .rs import (
+    QAPISchemaRsVisitor,
+    rs_ctype_parse,
+    rs_name,
+    rs_type,
+    to_camel_case,
+    to_snake_case,
+)
+from .schema import (
+    QAPISchema,
+    QAPISchemaEnumMember,
+    QAPISchemaEnumType,
+    QAPISchemaFeature,
+    QAPISchemaIfCond,
+    QAPISchemaObjectType,
+    QAPISchemaObjectTypeMember,
+    QAPISchemaType,
+    QAPISchemaVariants,
+)
+from .source import QAPISourceInfo
+
+
+objects_seen = set()
+
+
+def gen_rs_variants_to_tag(name: str,
+                           ifcond: QAPISchemaIfCond,
+                           variants: Optional[QAPISchemaVariants]) -> str:
+    ret = mcgen('''
+
+%(cfg)s
+impl From<&%(rs_name)sVariant> for %(tag)s {
+    fn from(e: &%(rs_name)sVariant) -> Self {
+        match e {
+    ''',
+                cfg=ifcond.rsgen(),
+                rs_name=rs_name(name),
+                tag=rs_type(variants.tag_member.type.c_type(), ''))
+
+    for var in variants.variants:
+        type_name = var.type.name
+        var_name = to_camel_case(rs_name(var.name))
+        patt = '(_)'
+        if type_name == 'q_empty':
+            patt = ''
+        ret += mcgen('''
+    %(cfg)s
+    %(rs_name)sVariant::%(var_name)s%(patt)s => Self::%(var_name)s,
+''',
+                     cfg=var.ifcond.rsgen(),
+                     rs_name=rs_name(name),
+                     var_name=var_name,
+                     patt=patt)
+
+    ret += mcgen('''
+        }
+    }
+}
+''')
+    return ret
+
+
+def gen_rs_variants(name: str,
+                    ifcond: QAPISchemaIfCond,
+                    variants: Optional[QAPISchemaVariants]) -> str:
+    ret = mcgen('''
+
+%(cfg)s
+#[derive(Clone,Debug)]
+pub enum %(rs_name)sVariant {
+''',
+                cfg=ifcond.rsgen(),
+                rs_name=rs_name(name))
+
+    for var in variants.variants:
+        type_name = var.type.name
+        var_name = to_camel_case(rs_name(var.name, False))
+        if type_name == 'q_empty':
+            ret += mcgen('''
+    %(cfg)s
+    %(var_name)s,
+''',
+                         cfg=var.ifcond.rsgen(),
+                         var_name=var_name)
+        else:
+            c_type = var.type.c_unboxed_type()
+            if c_type.endswith('_wrapper'):
+                c_type = c_type[6:-8]  # remove q_obj*-wrapper
+            ret += mcgen('''
+    %(cfg)s
+    %(var_name)s(%(rs_type)s),
+''',
+                         cfg=var.ifcond.rsgen(),
+                         var_name=var_name,
+                         rs_type=rs_type(c_type, ''))
+
+    ret += mcgen('''
+}
+''')
+
+    ret += gen_rs_variants_to_tag(name, ifcond, variants)
+
+    return ret
+
+
+def gen_rs_members(members: List[QAPISchemaObjectTypeMember],
+                   exclude: List[str] = None) -> str:
+    exclude = exclude or []
+    return [f"{m.ifcond.rsgen()} {to_snake_case(rs_name(m.name))}"
+            for m in members if m.name not in exclude]
+
+
+def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
+    ret = ''
+    for memb in members:
+        typ = rs_type(memb.type.c_type(), '', optional=memb.optional, box=True)
+        ret += mcgen('''
+    %(cfg)s
+    pub %(rs_name)s: %(rs_type)s,
+''',
+                     cfg=memb.ifcond.rsgen(),
+                     rs_type=typ,
+                     rs_name=to_snake_case(rs_name(memb.name)))
+    return ret
+
+
+def gen_rs_object(name: str,
+                  ifcond: QAPISchemaIfCond,
+                  base: Optional[QAPISchemaObjectType],
+                  members: List[QAPISchemaObjectTypeMember],
+                  variants: Optional[QAPISchemaVariants]) -> str:
+    if name in objects_seen:
+        return ''
+
+    if variants:
+        members = [m for m in members
+                   if m.name != variants.tag_member.name]
+
+    ret = ''
+    objects_seen.add(name)
+
+    if variants:
+        ret += gen_rs_variants(name, ifcond, variants)
+
+    ret += mcgen('''
+
+%(cfg)s
+#[derive(Clone, Debug)]
+pub struct %(rs_name)s {
+''',
+                 cfg=ifcond.rsgen(),
+                 rs_name=rs_name(name))
+
+    if base:
+        if not base.is_implicit():
+            ret += mcgen('''
+    // Members inherited:
+''',
+                         c_name=base.c_name())
+        base_members = base.members
+        if variants:
+            base_members = [m for m in base.members
+                            if m.name != variants.tag_member.name]
+        ret += gen_struct_members(base_members)
+        if not base.is_implicit():
+            ret += mcgen('''
+    // Own members:
+''')
+
+    ret += gen_struct_members(members)
+
+    if variants:
+        ret += mcgen('''
+    pub u: %(rs_type)sVariant,
+''', rs_type=rs_name(name))
+    ret += mcgen('''
+}
+''')
+    return ret
+
+
+def gen_rs_enum(name: str,
+                ifcond: QAPISchemaIfCond,
+                members: List[QAPISchemaEnumMember]) -> str:
+    # append automatically generated _max value
+    enum_members = members + [QAPISchemaEnumMember('_MAX', None)]
+
+    ret = mcgen('''
+
+%(cfg)s
+#[repr(u32)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, qemu_api_macros::TryInto)]
+pub enum %(rs_name)s {
+''',
+                cfg=ifcond.rsgen(),
+                rs_name=rs_name(name))
+
+    for member in enum_members:
+        ret += mcgen('''
+    %(cfg)s
+    %(c_enum)s,
+''',
+                     cfg=member.ifcond.rsgen(),
+                     c_enum=to_camel_case(rs_name(member.name)))
+    # picked the first, since that's what malloc0 does
+    # but arguably could use _MAX instead, or a qapi annotation
+    default = to_camel_case(rs_name(enum_members[0].name))
+    ret += mcgen('''
+}
+
+%(cfg)s
+impl Default for %(rs_name)s {
+    #[inline]
+    fn default() -> %(rs_name)s {
+        Self::%(default)s
+    }
+}
+''',
+                 cfg=ifcond.rsgen(),
+                 rs_name=rs_name(name),
+                 default=default)
+    return ret
+
+
+def gen_rs_alternate(name: str,
+                     ifcond: QAPISchemaIfCond,
+                     variants: Optional[QAPISchemaVariants]) -> str:
+    if name in objects_seen:
+        return ''
+
+    ret = ''
+    objects_seen.add(name)
+
+    ret += mcgen('''
+%(cfg)s
+#[derive(Clone, Debug)]
+pub enum %(rs_name)s {
+''',
+                 cfg=ifcond.rsgen(),
+                 rs_name=rs_name(name))
+
+    for var in variants.variants:
+        if var.type.name == 'q_empty':
+            continue
+        ret += mcgen('''
+        %(cfg)s
+        %(mem_name)s(%(rs_type)s),
+''',
+                     cfg=var.ifcond.rsgen(),
+                     rs_type=rs_type(var.type.c_unboxed_type(), ''),
+                     mem_name=to_camel_case(rs_name(var.name)))
+
+    ret += mcgen('''
+}
+''')
+    return ret
+
+
+class QAPISchemaGenRsTypeVisitor(QAPISchemaRsVisitor):
+
+    def __init__(self, prefix: str) -> None:
+        super().__init__(prefix, 'qapi-types')
+
+    def visit_begin(self, schema: QAPISchema) -> None:
+        # don't visit the empty type
+        objects_seen.add(schema.the_empty_object_type.name)
+        self._gen.preamble_add(
+            mcgen('''
+// generated by qapi-gen, DO NOT EDIT
+
+#![allow(non_camel_case_types)]
+
+use qemu_api::bindings::{QNull, QObject};
+
+'''))
+
+    def visit_object_type(self,
+                          name: str,
+                          info: Optional[QAPISourceInfo],
+                          ifcond: QAPISchemaIfCond,
+                          features: List[QAPISchemaFeature],
+                          base: Optional[QAPISchemaObjectType],
+                          members: List[QAPISchemaObjectTypeMember],
+                          variants: Optional[QAPISchemaVariants]) -> None:
+        if name.startswith('q_'):
+            return
+        self._gen.add(gen_rs_object(name, ifcond, base, members, variants))
+
+    def visit_enum_type(self,
+                        name: str,
+                        info: Optional[QAPISourceInfo],
+                        ifcond: QAPISchemaIfCond,
+                        features: List[QAPISchemaFeature],
+                        members: List[QAPISchemaEnumMember],
+                        prefix: Optional[str]) -> None:
+        self._gen.add(gen_rs_enum(name, ifcond, members))
+
+    def visit_alternate_type(self,
+                             name: str,
+                             info: QAPISourceInfo,
+                             ifcond: QAPISchemaIfCond,
+                             features: List[QAPISchemaFeature],
+                             variants: QAPISchemaVariants) -> None:
+        self._gen.add(gen_rs_alternate(name, ifcond, variants))
+
+
+def gen_rs_types(schema: QAPISchema, output_dir: str, prefix: str, builtins: bool) -> None:
+    # TODO: builtins?
+    vis = QAPISchemaGenRsTypeVisitor(prefix)
+    schema.visit(vis)
+    vis.write(output_dir)
-- 
2.49.0

Re: [PATCH 1/3] rust: make TryFrom macro more resilient

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

On Thu, Jun 5, 2025 at 2:11 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> If the enum includes values such as "Ok", "Err", or "Error", the TryInto
> macro can cause errors.  Be careful and qualify identifiers with the full
> path, or in the case of TryFrom<>::Error do not use the associated type
> at all.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  rust/qemu-api-macros/src/lib.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/
> lib.rs
> index 103470785e3..c18bb4e036f 100644
> --- a/rust/qemu-api-macros/src/lib.rs
> +++ b/rust/qemu-api-macros/src/lib.rs
> @@ -203,8 +203,8 @@ fn derive_tryinto_body(
>      Ok(quote! {
>          #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
>          match value {
> -            #(#discriminants => Ok(#name::#discriminants),)*
> -            _ => Err(value),
> +            #(#discriminants =>
> core::result::Result::Ok(#name::#discriminants),)*
> +            _ => core::result::Result::Err(value),
>          }
>      })
>  }
> @@ -236,7 +236,8 @@ pub const fn from_bits(value: #repr) -> Self {
>          impl core::convert::TryFrom<#repr> for #name {
>              type Error = #repr;
>
> -            fn try_from(value: #repr) -> Result<Self, Self::Error> {
> +            #[allow(ambiguous_associated_items)]
> +            fn try_from(value: #repr) -> Result<Self, #repr> {
>                  #body
>              }
>          }
> --
> 2.49.0
>
>

[-- Attachment #2: Type: text/html, Size: 2810 bytes --]

Re: [PATCH 2/3] scripts/qapi: add QAPISchemaIfCond.rsgen()

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

Hi

On Thu, Jun 5, 2025 at 2:11 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Generate Rust #[cfg(...)] guards from QAPI 'if' conditions.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Link:
> https://lore.kernel.org/r/20210907121943.3498701-15-marcandre.lureau@redhat.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/qapi/common.py | 16 ++++++++++++++++
>  scripts/qapi/schema.py |  4 ++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index d7c8aa3365c..a8ea5792c11 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -199,6 +199,22 @@ def guardend(name: str) -> str:
>                   name=c_fname(name).upper())
>
>
> +def rsgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
>

def rsgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:

to make type checking happy.

+
> +    def cfg(ifcond: Union[str, Dict[str, Any]]):
> +        if isinstance(ifcond, str):
> +            return ifcond
> +        if isinstance(ifcond, list):
> +            return ', '.join([cfg(c) for c in ifcond])
> +        oper, operands = next(iter(ifcond.items()))
> +        operands = cfg(operands)
> +        return f'{oper}({operands})'
> +
> +    if not ifcond:
> +        return ''
> +    return '#[cfg(%s)]' % cfg(ifcond)
> +
> +
>  def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
>                 cond_fmt: str, not_fmt: str,
>                 all_operator: str, any_operator: str) -> str:
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index cbe3b5aa91e..0fb151b5d89 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -39,6 +39,7 @@
>      docgen_ifcond,
>      gen_endif,
>      gen_if,
> +    rsgen_ifcond,
>  )
>  from .error import QAPIError, QAPISemError, QAPISourceError
>  from .expr import check_exprs
> @@ -65,6 +66,9 @@ def gen_endif(self) -> str:
>      def docgen(self) -> str:
>          return docgen_ifcond(self.ifcond)
>
> +    def rsgen(self) -> str:
> +        return rsgen_ifcond(self.ifcond)
> +
>      def is_present(self) -> bool:
>          return bool(self.ifcond)
>
> --
> 2.49.0
>
>

[-- Attachment #2: Type: text/html, Size: 3486 bytes --]

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3987 bytes --]

Hi Paolo,

On Thu, Jun 5, 2025 at 2:11 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> This is just an extremely minimal extraction from the patches at
>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
> ,
> limited to generating structs and enums from the QAPI schema.
> It does not include them in any crate and does not compile them.
>
>
Do you keep an up to date branch for those patches?

I fail to understand the advantage of going through Serde to
deserialize/serialize from C when you can have C types in Rust - having
less generated code? I also do not fully understand how that would work.

While I'm not going to work on this, I was curious how much work it
> was to produce *some* kind of Rust QAPI struct, which could be a first
> step towards using serde as an interface to C visitors, like this:
>
> trait QapiType: FreeForeign {
>     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut
> <Self as FreeForeign>::Foreign, errp: *mut *mut bindings::Error);
> }
>
> fn to_c<T: QAPIType>(obj: &T) -> *mut <T as FreeForeign>::Foreign {
>     let mut ptr = libc::calloc(...);
>     let mut ser = QapiSerializer::<T>::new(ptr);
>     obj.serialize(&mut ser).unwrap();
>     ptr.cast()
> }
>
> unsafe fn from_c<T: QAPIType>(obj: *const <T as FreeForeign>::Foreign) ->
> T {
>     let mut de = QapiDeserializer::new(T::visit, obj as *const c_void);
>     let value = de::Deserialize::deserialize(&mut de).unwrap();
>     de.end().unwrap();
>     value
> }
>
> /* Generated code below: */
>
> impl QapiType for UefiVariable {
>     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut
> bindings::UefiVariable, errp: *mut *mut bindings::Error) {
>         unsafe extern "C" visit_type_DumpGuestMemoryFormat(v:
> bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable,
> errp: *mut *mut bindings::Error) {
>         unsafe { visit_type_DumpGuestMemoryFormat(v, name, obj, errp); }
>

I guess  .._UefiVariable.


>     }
> }
>
> impl FreeForeign for UefiVariable {
>     type Foreign = bindings::UefiVariable;
>
>     unsafe fn free_foreign(p: *mut bindings::UefiVariable) {
>         unsafe extern "C" qapi_free_UefiVariable(p: *mut
> bindings::UefiVariable);
>         unsafe { qapi_free_UefiVariable(p); }
>     }
> }
>
> impl CloneToForeign for UefiVariable {
>     fn clone_to_foreign(&self) -> OwnedPointer<Self> {
>         OwnedPointer::new(qapi::to_c(self))
>     }
> }
>
> impl FromForeign for UefiVariable {
>     unsafe fn cloned_from_foreign(obj: *const bindings::UefiVariable) ->
> Self {
>         qapi::from_c(obj)
>     }
> }
>
> The FFI types could be generated by qapi-gen, as in Marc-André's
> proposal, or from bindgen.
>
> I am not sure what approach is better---whether to use serde or to
> automatically generate the marshaling and unmarshaling code; and whether
> to use bindgen or generate C-compatible FFI types---but it made sense,
> from the point of view of extracting "some" code from Marc-André's
> proof of concept and enticing other people, :) to start from high-level
> types.
>
> Paolo
>
> Marc-André Lureau (2):
>   scripts/qapi: add QAPISchemaIfCond.rsgen()
>   scripts/qapi: generate high-level Rust bindings
>
> Paolo Bonzini (1):
>   rust: make TryFrom macro more resilient
>
>  meson.build                     |   4 +-
>  rust/qemu-api-macros/src/lib.rs |   7 +-
>  scripts/qapi/backend.py         |   4 +-
>  scripts/qapi/common.py          |  16 ++
>  scripts/qapi/main.py            |   4 +-
>  scripts/qapi/rs.py              | 183 ++++++++++++++++++
>  scripts/qapi/rs_types.py        | 320 ++++++++++++++++++++++++++++++++
>  scripts/qapi/schema.py          |   4 +
>  8 files changed, 535 insertions(+), 7 deletions(-)
>  create mode 100644 scripts/qapi/rs.py
>  create mode 100644 scripts/qapi/rs_types.py
>
> --
> 2.49.0
>
>

[-- Attachment #2: Type: text/html, Size: 5277 bytes --]

Re: [PATCH 1/3] rust: make TryFrom macro more resilient

[Zhao Liu]

On Thu, Jun 05, 2025 at 12:11:22PM +0200, Paolo Bonzini wrote:
> Date: Thu,  5 Jun 2025 12:11:22 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 1/3] rust: make TryFrom macro more resilient
> X-Mailer: git-send-email 2.49.0
> 
> If the enum includes values such as "Ok", "Err", or "Error", the TryInto
> macro can cause errors.  Be careful and qualify identifiers with the full
> path, or in the case of TryFrom<>::Error do not use the associated type
> at all.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api-macros/src/lib.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Though I'm late...

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
> index 103470785e3..c18bb4e036f 100644
> --- a/rust/qemu-api-macros/src/lib.rs
> +++ b/rust/qemu-api-macros/src/lib.rs
> @@ -203,8 +203,8 @@ fn derive_tryinto_body(
>      Ok(quote! {
>          #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
>          match value {
> -            #(#discriminants => Ok(#name::#discriminants),)*
> -            _ => Err(value),
> +            #(#discriminants => core::result::Result::Ok(#name::#discriminants),)*
> +            _ => core::result::Result::Err(value),

error.rs is using std::result, so I think it's better to use std rather
than core if possible.

But there are many other cases like this where std and core are mixed,
maybe we can clean it up to always prefer the std.

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

On Tue, Jun 10, 2025 at 3:53 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> On Thu, Jun 5, 2025 at 2:11 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> This is just an extremely minimal extraction from the patches at
>> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/,
>> limited to generating structs and enums from the QAPI schema.
>> It does not include them in any crate and does not compile them.
>
> Do you keep an up to date branch for those patches?

I can add them to my rust-next branch but they can in theory just be
applied to upstream/master.

> I fail to understand the advantage of going through Serde to deserialize/serialize from C when you can have C types in Rust - having less generated code?

It's indeed trading generated Rust code for generic serializers and
deserializers + #[derive(Serializer, Deserializer)]. It's not the
primary reason but the size of your code generator patches was a bit
scary indeed and I don't want to inflict too much review effort on
QAPI maintainers.

But also, I'm not sure that the C types are needed at all for
implementing QAPI commands. The QAPI command generator could have a
directive like 'rust': 'qga::qmp'" directive (or maybe just
'language': 'rust', I don't know) that would generate Rust
marshaling/unmarshaling functions:

pub extern "C" fn qmp_guest_get_host_name(
        args: *mut QDict,
        ret: *mut *mut QObject,
        errp: *mut *mut bindings::Error) {
    let args = unsafe { from_qobject::<()>(args) };
    match ::qga::qmp::guest_get_host_name(args) {
        Ok(ref v) => unsafe { *ret = to_qobject::<GuestHostName>(v); },
        Err(err) => unsafe { err.propagate(errp); }
    }
}

and this (more specifically from_qobject/to_qobject) could also use
serde, this time to go directly from QObject to Rust and back. A
QObject serializer/deserializer should be easier to write (and very
similar to existing code in serde_json for
https://docs.rs/serde_json/latest/serde_json/enum.Value.html), so you
could reach the stage of your old posting with qapi-gen changes that
are little more than these two patches.

> I also do not fully understand how that would work.

To be honest neither do I entirely. But what I understood is that
Serde is based on a visitor API that is very similar to what we have
in QEMU. See the above serde_json link where it implements Serializer
and Deserializer, and the blog post at
https://ohadravid.github.io/posts/2025-05-serde-reflect/ should be a
good introduction too.

That said: the *possibility* of using Serde is why I started from the
native structs. I'm not wed to it and generating/converting the C
types remains a possibility as well.

Paolo

Re: [PATCH 3/3] scripts/qapi: generate high-level Rust bindings

[Zhao Liu]

On Thu, Jun 05, 2025 at 12:11:24PM +0200, Paolo Bonzini wrote:
> Date: Thu,  5 Jun 2025 12:11:24 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 3/3] scripts/qapi: generate high-level Rust bindings
> X-Mailer: git-send-email 2.49.0
> 
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Generate high-level idiomatic Rust code for the QAPI types, with to/from
> translations for the C FFI.
> 
> - char* is mapped to String, scalars to there corresponding Rust types
> 
> - enums are simply aliased from FFI
> 
> - has_foo/foo members are mapped to Option<T>
> 
> - lists are represented as Vec<T>
> 
> - structures have Rust versions, with To/From FFI conversions
>
> - alternate are represented as Rust enum
> 
> - unions are represented in a similar way as in C: a struct S with a "u"
>   member (since S may have extra 'base' fields). However, the discriminant
>   isn't a member of S, since Rust enum already include it.

Why not map the C union to rust union directly (in `pub enum %(rs_name)sVariant`)?

(latter comments are all about format nits)

...

> +%(cfg)s
> +impl From<&%(rs_name)sVariant> for %(tag)s {
> +    fn from(e: &%(rs_name)sVariant) -> Self {
> +        match e {
> +    ''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name),
> +                tag=rs_type(variants.tag_member.type.c_type(), ''))
> +
> +    for var in variants.variants:
> +        type_name = var.type.name
> +        var_name = to_camel_case(rs_name(var.name))
> +        patt = '(_)'
> +        if type_name == 'q_empty':
> +            patt = ''
> +        ret += mcgen('''
> +    %(cfg)s

This introduces extra \n, which will generate a blank line if there's
no cfg.

> +    %(rs_name)sVariant::%(var_name)s%(patt)s => Self::%(var_name)s,

So, I think it should be:

    %(cfg)s    %(rs_name)sVariant::%(var_name)s%(patt)s => Self::%(var_name)s,

> +''',
> +                     cfg=var.ifcond.rsgen(),
> +                     rs_name=rs_name(name),
> +                     var_name=var_name,
> +                     patt=patt)
> +
> +    ret += mcgen('''
> +        }
> +    }
> +}
> +''')
> +    return ret
> +
> +
> +def gen_rs_variants(name: str,
> +                    ifcond: QAPISchemaIfCond,
> +                    variants: Optional[QAPISchemaVariants]) -> str:
> +    ret = mcgen('''
> +
> +%(cfg)s
> +#[derive(Clone,Debug)]
> +pub enum %(rs_name)sVariant {
> +''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name))
> +
> +    for var in variants.variants:
> +        type_name = var.type.name
> +        var_name = to_camel_case(rs_name(var.name, False))
> +        if type_name == 'q_empty':
> +            ret += mcgen('''
> +    %(cfg)s
> +    %(var_name)s,

ditto,

    %(cfg)s    %(var_name)s,

> +''',
> +                         cfg=var.ifcond.rsgen(),
> +                         var_name=var_name)
> +        else:
> +            c_type = var.type.c_unboxed_type()
> +            if c_type.endswith('_wrapper'):
> +                c_type = c_type[6:-8]  # remove q_obj*-wrapper
> +            ret += mcgen('''
> +    %(cfg)s
> +    %(var_name)s(%(rs_type)s),

    %(cfg)s    %(var_name)s(%(rs_type)s),

> +''',
> +                         cfg=var.ifcond.rsgen(),
> +                         var_name=var_name,
> +                         rs_type=rs_type(c_type, ''))
> +
> +    ret += mcgen('''
> +}
> +''')
> +
> +    ret += gen_rs_variants_to_tag(name, ifcond, variants)
> +
> +    return ret
> +
> +
> +def gen_rs_members(members: List[QAPISchemaObjectTypeMember],
> +                   exclude: List[str] = None) -> str:
> +    exclude = exclude or []
> +    return [f"{m.ifcond.rsgen()} {to_snake_case(rs_name(m.name))}"
> +            for m in members if m.name not in exclude]
> +
> +
> +def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
> +    ret = ''
> +    for memb in members:
> +        typ = rs_type(memb.type.c_type(), '', optional=memb.optional, box=True)
> +        ret += mcgen('''
> +    %(cfg)s
> +    pub %(rs_name)s: %(rs_type)s,

    %(cfg)s    pub %(rs_name)s: %(rs_type)s,

> +''',
> +                     cfg=memb.ifcond.rsgen(),
> +                     rs_type=typ,
> +                     rs_name=to_snake_case(rs_name(memb.name)))
> +    return ret
> +
> +

...

> +%(cfg)s
> +#[repr(u32)]
> +#[derive(Copy, Clone, Debug, PartialEq, Eq, qemu_api_macros::TryInto)]
> +pub enum %(rs_name)s {
> +''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name))
> +
> +    for member in enum_members:
> +        ret += mcgen('''
> +    %(cfg)s
> +    %(c_enum)s,

    %(cfg)s    %(c_enum)s,

> +''',
> +                     cfg=member.ifcond.rsgen(),
> +                     c_enum=to_camel_case(rs_name(member.name)))
> +    # picked the first, since that's what malloc0 does
> +    # but arguably could use _MAX instead, or a qapi annotation
> +    default = to_camel_case(rs_name(enum_members[0].name))
> +    ret += mcgen('''
> +}
> +

...

> +def gen_rs_alternate(name: str,
> +                     ifcond: QAPISchemaIfCond,
> +                     variants: Optional[QAPISchemaVariants]) -> str:
> +    if name in objects_seen:
> +        return ''
> +
> +    ret = ''
> +    objects_seen.add(name)
> +
> +    ret += mcgen('''
> +%(cfg)s
> +#[derive(Clone, Debug)]
> +pub enum %(rs_name)s {
> +''',
> +                 cfg=ifcond.rsgen(),
> +                 rs_name=rs_name(name))
> +
> +    for var in variants.variants:
> +        if var.type.name == 'q_empty':
> +            continue
> +        ret += mcgen('''
> +        %(cfg)s
> +        %(mem_name)s(%(rs_type)s),

    %(cfg)s    %(mem_name)s(%(rs_type)s),

> +''',
> +                     cfg=var.ifcond.rsgen(),
> +                     rs_type=rs_type(var.type.c_unboxed_type(), ''),
> +                     mem_name=to_camel_case(rs_name(var.name)))
> +
> +    ret += mcgen('''
> +}
> +''')
> +    return ret

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Zhao Liu]

On Thu, Jun 05, 2025 at 12:11:21PM +0200, Paolo Bonzini wrote:
> Date: Thu,  5 Jun 2025 12:11:21 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH preview 0/3] reviving minimal QAPI generation from 2021
> X-Mailer: git-send-email 2.49.0
> 
> This is just an extremely minimal extraction from the patches at
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/,
> limited to generating structs and enums from the QAPI schema.
> It does not include them in any crate and does not compile them.
> 
> While I'm not going to work on this, I was curious how much work it
> was to produce *some* kind of Rust QAPI struct, which could be a first
> step towards using serde as an interface to C visitors, like this:
> 
> trait QapiType: FreeForeign {
>     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut <Self as FreeForeign>::Foreign, errp: *mut *mut bindings::Error);
> }
> 
> fn to_c<T: QAPIType>(obj: &T) -> *mut <T as FreeForeign>::Foreign {
>     let mut ptr = libc::calloc(...);
>     let mut ser = QapiSerializer::<T>::new(ptr);
>     obj.serialize(&mut ser).unwrap();
>     ptr.cast()
> }
> 
> unsafe fn from_c<T: QAPIType>(obj: *const <T as FreeForeign>::Foreign) -> T {
>     let mut de = QapiDeserializer::new(T::visit, obj as *const c_void);
>     let value = de::Deserialize::deserialize(&mut de).unwrap();
>     de.end().unwrap();
>     value
> }
> 
> /* Generated code below: */
> 
> impl QapiType for UefiVariable {
>     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
>         unsafe extern "C" visit_type_DumpGuestMemoryFormat(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
>         unsafe { visit_type_DumpGuestMemoryFormat(v, name, obj, errp); }
>     }
> }
> 
> impl FreeForeign for UefiVariable {
>     type Foreign = bindings::UefiVariable;

My question seems to be different from Marc-André's...

As patch 3 did, qapi will generate Rust types:

- char* is mapped to String, scalars to there corresponding Rust types
- enums are simply aliased from FFI
- has_foo/foo members are mapped to Option<T>
- lists are represented as Vec<T>
- structures have Rust versions, with To/From FFI conversions

It seems we still need some raw bindings (generated by bindgen) as the
`type Foreign`, and then implement Foreign traits for the Rust
structures generated by this patch.

For this example, UefiVariable is generated by qapi (patch 3) and
bindings::UefiVariable is generated by bindgen. Ah! I feel I'm wrong,
could you please correct me?

Thanks,
Zhao

>     unsafe fn free_foreign(p: *mut bindings::UefiVariable) {
>         unsafe extern "C" qapi_free_UefiVariable(p: *mut bindings::UefiVariable);
>         unsafe { qapi_free_UefiVariable(p); }
>     }
> }
> 
> impl CloneToForeign for UefiVariable {
>     fn clone_to_foreign(&self) -> OwnedPointer<Self> {
>         OwnedPointer::new(qapi::to_c(self))
>     }
> }
> 
> impl FromForeign for UefiVariable {
>     unsafe fn cloned_from_foreign(obj: *const bindings::UefiVariable) -> Self {
>         qapi::from_c(obj)
>     }
> }
> 
> The FFI types could be generated by qapi-gen, as in Marc-André's
> proposal, or from bindgen.
> 
> I am not sure what approach is better---whether to use serde or to
> automatically generate the marshaling and unmarshaling code; and whether
> to use bindgen or generate C-compatible FFI types---but it made sense,
> from the point of view of extracting "some" code from Marc-André's
> proof of concept and enticing other people, :) to start from high-level
> types.
> 

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

On Wed, Jun 11, 2025 at 9:52 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> As patch 3 did, qapi will generate Rust types:
>
> - char* is mapped to String, scalars to there corresponding Rust types
> - enums are simply aliased from FFI
> - has_foo/foo members are mapped to Option<T>
> - lists are represented as Vec<T>

Yep, so far so good.

> - structures have Rust versions, with To/From FFI conversions

The conversion part is not included in this minimal series. The raw
structure (the `type Foreign` as you point out) could be generated by
either bindgen or qapi-gen. Marc-André chose the latter for his old
prototype, I'm ambivalent.

> It seems we still need some raw bindings (generated by bindgen) as the
> `type Foreign`, and then implement Foreign traits for the Rust
> structures generated by this patch.

Yes. If using serde the implementation of the traits is very small,
and basically the same for all types. If not using serde, it would
need some (or most) of the infrastructure in Marc-André's original
series.

> For this example, UefiVariable is generated by qapi (patch 3) and
> bindings::UefiVariable is generated by bindgen. Ah! I feel I'm wrong,
> could you please correct me?

You're not wrong. :)

Paolo

> On Thu, Jun 05, 2025 at 12:11:21PM +0200, Paolo Bonzini wrote:
> > Date: Thu,  5 Jun 2025 12:11:21 +0200
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH preview 0/3] reviving minimal QAPI generation from 2021
> > X-Mailer: git-send-email 2.49.0
> >
> > This is just an extremely minimal extraction from the patches at
> > https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/,
> > limited to generating structs and enums from the QAPI schema.
> > It does not include them in any crate and does not compile them.
> >
> > While I'm not going to work on this, I was curious how much work it
> > was to produce *some* kind of Rust QAPI struct, which could be a first
> > step towards using serde as an interface to C visitors, like this:
> >
> > trait QapiType: FreeForeign {
> >     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut <Self as FreeForeign>::Foreign, errp: *mut *mut bindings::Error);
> > }
> >
> > fn to_c<T: QapiType>(obj: &T) -> *mut <T as FreeForeign>::Foreign {
> >     let mut ptr = libc::calloc(...);
> >     let mut ser = QapiSerializer::<T>::new(ptr);
> >     obj.serialize(&mut ser).unwrap();
> >     ptr.cast()
> > }
> >
> > unsafe fn from_c<T: QapiType>(obj: *const <T as FreeForeign>::Foreign) -> T {
> >     let mut de = QapiDeserializer::new(T::visit, obj as *const c_void);
> >     let value = de::Deserialize::deserialize(&mut de).unwrap();
> >     de.end().unwrap();
> >     value
> > }
> >
> > /* Generated code below: */
> >
> > impl QapiType for UefiVariable {
> >     unsafe fn visit(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
> >         unsafe extern "C" visit_type_UefiVariable(v: bindings::Visitor, name: *const c_char, obj: *mut bindings::UefiVariable, errp: *mut *mut bindings::Error) {
> >         unsafe { visit_type_UefiVariable(v, name, obj, errp); }
> >     }
> > }
> >
> > impl FreeForeign for UefiVariable {
> >     type Foreign = bindings::UefiVariable;
> >     unsafe fn free_foreign(p: *mut bindings::UefiVariable) {
> >         unsafe extern "C" qapi_free_UefiVariable(p: *mut bindings::UefiVariable);
> >         unsafe { qapi_free_UefiVariable(p); }
> >     }
> > }
> >
> > impl CloneToForeign for UefiVariable {
> >     fn clone_to_foreign(&self) -> OwnedPointer<Self> {
> >         OwnedPointer::new(qapi::to_c(self))
> >     }
> > }
> >
> > impl FromForeign for UefiVariable {
> >     unsafe fn cloned_from_foreign(obj: *const bindings::UefiVariable) -> Self {
> >         qapi::from_c(obj)
> >     }
> > }

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

On Wed, Jun 11, 2025 at 10:57 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Yes. If using serde the implementation of the traits is very small,
> and basically the same for all types. If not using serde, it would
> need some (or most) of the infrastructure in Marc-André's original
> series.

Looking more at it, the Rust->QObject and QObject->Rust parts *can* be
done with serde (following the model of serde_json's Value
(de)serializer) but the Rust<->C part has a problem.

To recap, Rust->C is the serialization and corresponds to output
visitors. C->Rust is the deserialization and corresponds to input
visitors.

For serialization, serde has a push model where the generated code
looks like this:

      let mut state =
          Serializer::serialize_struct(serializer, "S", 2);
      SerializeStruct::serialize_field(&mut state, "a", &self.a)?;
      SerializeStruct::serialize_field(&mut state, "b", &self.b)?;
      SerializeStruct::end(state)

whereas QAPI has a pull model where visit_type_* drives the process
and requests the fields one by one.

For deserialization, serde has a pull model where the generated code
asks for the field names one by one:

    fn visit_map<__A>(self, mut __map: __A)
        while let Some(key) =
            MapAccess::next_key::<__Field>(&mut __map)? {
                match __key { ... }
        }
    }

whereas QAPI has a push model where visit_type_* again drives the
process and sends fields one by one.

For commands this is not a problem because the real underlying
transformation is QObject->QObject and the intermediate steps (to and
from QObject) can use serde.

However, QOM property getters/setters (especially, but not
exclusively, for properties with compound types) remain a problem
since these use callbacks with a Visitor* argument. I see three
possibilities:

1) everything is done through an intermediate QObject step (e.g. for a
setter: Visitor->QObject with an input visitor, and QObject->Rust with
serde deserialization).
    + easy, Rust only sees serde
    + QMP commands use a single conversion step
    - inefficient

2) everything is done through an intermediate C step (e.g. for a
setter: Visitor->C with a visit_type_* function, and C->Rust with
generated code that does not need to use serde). There is still a
double conversion step, but it's more efficient than option 1
    + one framework (visitor)
    - double conversion for the QMP commands
    - lots of generated code

3) generating a Rust visit_type_* implementation as well, either in
qapi-gen (3a) or through a procedural macro (3b). This should not be
hard to write but it would remove a lot of the advantages from using
serde.
    + efficient
    + preserves single conversion for QMP commands
    - two frameworks

I am leaning towards option 1, i.e. keep using serde but only cover
conversions to and from QObject. The reason is that one future usecase
for Rust in QEMU is the UEFI variable store; that one also has some
Rust<->JSON conversions and could be served by either QObject or
serde_json. Either way, it'd be nice for the UEFI variable store to
remain within the Rust serde ecosystem and allow sharing code between
QEMU and Coconut SVSM. But I'm not so sure...

Paolo

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Zhao Liu]

On Thu, Jun 12, 2025 at 12:24:44PM +0200, Paolo Bonzini wrote:
> Date: Thu, 12 Jun 2025 12:24:44 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021
> 
> On Wed, Jun 11, 2025 at 10:57 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Yes. If using serde the implementation of the traits is very small,
> > and basically the same for all types. If not using serde, it would
> > need some (or most) of the infrastructure in Marc-André's original
> > series.
> 
> Looking more at it, the Rust->QObject and QObject->Rust parts *can* be
> done with serde (following the model of serde_json's Value
> (de)serializer) but the Rust<->C part has a problem.
> 
> To recap, Rust->C is the serialization and corresponds to output
> visitors. C->Rust is the deserialization and corresponds to input
> visitors.
> 
> For serialization, serde has a push model where the generated code
> looks like this:
> 
>       let mut state =
>           Serializer::serialize_struct(serializer, "S", 2);
>       SerializeStruct::serialize_field(&mut state, "a", &self.a)?;
>       SerializeStruct::serialize_field(&mut state, "b", &self.b)?;
>       SerializeStruct::end(state)
> 
> whereas QAPI has a pull model where visit_type_* drives the process
> and requests the fields one by one.
> 
> For deserialization, serde has a pull model where the generated code
> asks for the field names one by one:
> 
>     fn visit_map<__A>(self, mut __map: __A)
>         while let Some(key) =
>             MapAccess::next_key::<__Field>(&mut __map)? {
>                 match __key { ... }
>         }
>     }
> 
> whereas QAPI has a push model where visit_type_* again drives the
> process and sends fields one by one.
> 
> For commands this is not a problem because the real underlying
> transformation is QObject->QObject and the intermediate steps (to and
> from QObject) can use serde.
> 
> However, QOM property getters/setters (especially, but not
> exclusively, for properties with compound types) remain a problem
> since these use callbacks with a Visitor* argument. I see three
> possibilities:
> 
> 1) everything is done through an intermediate QObject step (e.g. for a
> setter: Visitor->QObject with an input visitor, and QObject->Rust with
> serde deserialization).
>     + easy, Rust only sees serde
>     + QMP commands use a single conversion step
>     - inefficient

This sounds like a natural approach.

> 2) everything is done through an intermediate C step (e.g. for a
> setter: Visitor->C with a visit_type_* function, and C->Rust with
> generated code that does not need to use serde).

I understand this step indicates to use bindgen to generate visit_type_*
bindings...

> There is still a
> double conversion step, but it's more efficient than option 1
>     + one framework (visitor)
>     - double conversion for the QMP commands
>     - lots of generated code

...if so, then yes, there would be too much generated and "unsafe"
codes, and further abstraction may be needed to ensure safety, similar
to the lots of work currently done in the qemu-api.

> 3) generating a Rust visit_type_* implementation as well, either in
> qapi-gen (3a) or through a procedural macro (3b).> This should not be
> hard to write but it would remove a lot of the advantages from using
> serde.
>     + efficient
>     + preserves single conversion for QMP commands
>     - two frameworks

The difference between the two frameworks is that one is for C and the
other is for Rust, which increases the maintenance burden.

> I am leaning towards option 1, i.e. keep using serde but only cover
> conversions to and from QObject.

Based on my understanding of the three options above, I also think that
option 1 is the more appropriate approach.

> The reason is that one future usecase
> for Rust in QEMU is the UEFI variable store; that one also has some
> Rust<->JSON conversions and could be served by either QObject or
> serde_json. Either way, it'd be nice for the UEFI variable store to
> remain within the Rust serde ecosystem and allow sharing code between
> QEMU and Coconut SVSM. But I'm not so sure...

Thanks,
Zhao

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

On Fri, Jun 13, 2025 at 7:36 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > 1) everything is done through an intermediate QObject step (e.g. for a
> > setter: Visitor->QObject with an input visitor, and QObject->Rust with
> > serde deserialization).
> >     + easy, Rust only sees serde
> >     + QMP commands use a single conversion step
> >     - inefficient
>
> This sounds like a natural approach.

Yep, serde is complex but the result is very natural. In some cases
serde has different defaults than QEMU, but attributes like
#[serde(skip_serializing_if = "Option::is_none")] can be automatically
generated by qapi-gen.

> > 2) everything is done through an intermediate C step (e.g. for a
> > setter: Visitor->C with a visit_type_* function, and C->Rust with
> > generated code that does not need to use serde).
>
> I understand this step indicates to use bindgen to generate visit_type_*
> bindings...

More than that, it's using qapi-gen to generate implementations like
the *Foreign traits used by Error. Marc-André wrote his own traits but
I suspect a lot of the code could be reused.

So I must give credit to Marc-André that there is a big advantage
here, in that a lot of the code is already written and it is simpler
than serde.

Paolo

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Markus Armbruster]

pylint isn't happy with the new module:

************* Module qapi.rs_types
scripts/qapi/rs_types.py:286:4: W0237: Parameter 'branches' has been renamed to 'variants' in overriding 'QAPISchemaGenRsTypeVisitor.visit_object_type' method (arguments-renamed)
scripts/qapi/rs_types.py:307:4: W0237: Parameter 'alternatives' has been renamed to 'variants' in overriding 'QAPISchemaGenRsTypeVisitor.visit_alternate_type' method (arguments-renamed)
scripts/qapi/rs_types.py:316:67: W0613: Unused argument 'builtins' (unused-argument)
scripts/qapi/rs_types.py:9:0: W0611: Unused POINTER_SUFFIX imported from common (unused-import)
scripts/qapi/rs_types.py:10:0: W0611: Unused rs_ctype_parse imported from rs (unused-import)
scripts/qapi/rs_types.py:18:0: W0611: Unused QAPISchemaEnumType imported from schema (unused-import)
scripts/qapi/rs_types.py:18:0: W0611: Unused QAPISchemaType imported from schema (unused-import)


Might be too early to bother with the type hints, but here goes anyway:

$ mypy scripts/qapi-gen.py 
scripts/qapi/schema.py:70: error: Argument 1 to "rsgen_ifcond" has incompatible type "str | dict[str, object] | None"; expected "str | dict[str, Any]"  [arg-type]
scripts/qapi/rs.py:99: error: Incompatible types in assignment (expression has type "str | None", variable has type "dict[str, str]")  [assignment]
scripts/qapi/rs.py:103: error: Unsupported left operand type for + ("None")  [operator]
scripts/qapi/rs.py:103: note: Left operand is of type "str | None"
scripts/qapi/rs.py:103: error: Incompatible types in assignment (expression has type "str | Any", variable has type "dict[str, str]")  [assignment]
scripts/qapi/rs.py:106: error: Incompatible types in assignment (expression has type "str", variable has type "dict[str, str]")  [assignment]
scripts/qapi/rs.py:108: error: Incompatible types in assignment (expression has type "str", variable has type "dict[str, str]")  [assignment]
scripts/qapi/rs.py:110: error: Incompatible types in assignment (expression has type "str", variable has type "dict[str, str]")  [assignment]
scripts/qapi/rs.py:111: error: Incompatible return value type (got "dict[str, str]", expected "str")  [return-value]
scripts/qapi/rs_types.py:47: error: Item "None" of "QAPISchemaVariants | None" has no attribute "tag_member"  [union-attr]
scripts/qapi/rs_types.py:49: error: Item "None" of "QAPISchemaVariants | None" has no attribute "variants"  [union-attr]
scripts/qapi/rs_types.py:84: error: Item "None" of "QAPISchemaVariants | None" has no attribute "variants"  [union-attr]
scripts/qapi/rs_types.py:116: error: Incompatible default for argument "exclude" (default has type "None", argument has type "list[str]")  [assignment]
scripts/qapi/rs_types.py:116: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
scripts/qapi/rs_types.py:116: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
scripts/qapi/rs_types.py:118: error: Incompatible return value type (got "list[str]", expected "str")  [return-value]
scripts/qapi/rs_types.py:251: error: Item "None" of "QAPISchemaVariants | None" has no attribute "variants"  [union-attr]
scripts/qapi/rs_types.py:309: error: Argument 2 of "visit_alternate_type" is incompatible with supertype "QAPISchemaVisitor"; supertype defines the argument type as "QAPISourceInfo | None"  [override]
scripts/qapi/rs_types.py:309: note: This violates the Liskov substitution principle
scripts/qapi/rs_types.py:309: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 15 errors in 3 files (checked 1 source file)

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 4093 bytes --]

Il mar 17 giu 2025, 09:49 Markus Armbruster <armbru@redhat.com> ha scritto:

> pylint isn't happy with the new module:
>

Fixed, will post a new version.

Can I pick your brain on whether to focus on Rust<->QObject conversion or
give the Rust structs the full visitor treatment? (See other messages in
reply to the cover letter).

Paolo


************* Module qapi.rs_types
> scripts/qapi/rs_types.py:286:4: W0237: Parameter 'branches' has been
> renamed to 'variants' in overriding
> 'QAPISchemaGenRsTypeVisitor.visit_object_type' method (arguments-renamed)
> scripts/qapi/rs_types.py:307:4: W0237: Parameter 'alternatives' has been
> renamed to 'variants' in overriding
> 'QAPISchemaGenRsTypeVisitor.visit_alternate_type' method (arguments-renamed)
> scripts/qapi/rs_types.py:316:67: W0613: Unused argument 'builtins'
> (unused-argument)
> scripts/qapi/rs_types.py:9:0: W0611: Unused POINTER_SUFFIX imported from
> common (unused-import)
> scripts/qapi/rs_types.py:10:0: W0611: Unused rs_ctype_parse imported from
> rs (unused-import)
> scripts/qapi/rs_types.py:18:0: W0611: Unused QAPISchemaEnumType imported
> from schema (unused-import)
> scripts/qapi/rs_types.py:18:0: W0611: Unused QAPISchemaType imported from
> schema (unused-import)
>
>
> Might be too early to bother with the type hints, but here goes anyway:
>
> $ mypy scripts/qapi-gen.py
> scripts/qapi/schema.py:70: error: Argument 1 to "rsgen_ifcond" has
> incompatible type "str | dict[str, object] | None"; expected "str |
> dict[str, Any]"  [arg-type]
> scripts/qapi/rs.py:99: error: Incompatible types in assignment (expression
> has type "str | None", variable has type "dict[str, str]")  [assignment]
> scripts/qapi/rs.py:103: error: Unsupported left operand type for +
> ("None")  [operator]
> scripts/qapi/rs.py:103: note: Left operand is of type "str | None"
> scripts/qapi/rs.py:103: error: Incompatible types in assignment
> (expression has type "str | Any", variable has type "dict[str, str]")
> [assignment]
> scripts/qapi/rs.py:106: error: Incompatible types in assignment
> (expression has type "str", variable has type "dict[str, str]")
> [assignment]
> scripts/qapi/rs.py:108: error: Incompatible types in assignment
> (expression has type "str", variable has type "dict[str, str]")
> [assignment]
> scripts/qapi/rs.py:110: error: Incompatible types in assignment
> (expression has type "str", variable has type "dict[str, str]")
> [assignment]
> scripts/qapi/rs.py:111: error: Incompatible return value type (got
> "dict[str, str]", expected "str")  [return-value]
> scripts/qapi/rs_types.py:47: error: Item "None" of "QAPISchemaVariants |
> None" has no attribute "tag_member"  [union-attr]
> scripts/qapi/rs_types.py:49: error: Item "None" of "QAPISchemaVariants |
> None" has no attribute "variants"  [union-attr]
> scripts/qapi/rs_types.py:84: error: Item "None" of "QAPISchemaVariants |
> None" has no attribute "variants"  [union-attr]
> scripts/qapi/rs_types.py:116: error: Incompatible default for argument
> "exclude" (default has type "None", argument has type "list[str]")
> [assignment]
> scripts/qapi/rs_types.py:116: note: PEP 484 prohibits implicit Optional.
> Accordingly, mypy has changed its default to no_implicit_optional=True
> scripts/qapi/rs_types.py:116: note: Use
> https://github.com/hauntsaninja/no_implicit_optional to automatically
> upgrade your codebase
> scripts/qapi/rs_types.py:118: error: Incompatible return value type (got
> "list[str]", expected "str")  [return-value]
> scripts/qapi/rs_types.py:251: error: Item "None" of "QAPISchemaVariants |
> None" has no attribute "variants"  [union-attr]
> scripts/qapi/rs_types.py:309: error: Argument 2 of "visit_alternate_type"
> is incompatible with supertype "QAPISchemaVisitor"; supertype defines the
> argument type as "QAPISourceInfo | None"  [override]
> scripts/qapi/rs_types.py:309: note: This violates the Liskov substitution
> principle
> scripts/qapi/rs_types.py:309: note: See
> https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
> Found 15 errors in 3 files (checked 1 source file)
>
>

[-- Attachment #2: Type: text/html, Size: 5553 bytes --]

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Markus Armbruster]

I don't know enough about Rust/serde to give advice.  I do know how to
make a fool of myself by asking dumb questions.

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Wed, Jun 11, 2025 at 10:57 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Yes. If using serde the implementation of the traits is very small,
>> and basically the same for all types. If not using serde, it would
>> need some (or most) of the infrastructure in Marc-André's original
>> series.
>
> Looking more at it, the Rust->QObject and QObject->Rust parts *can* be
> done with serde (following the model of serde_json's Value
> (de)serializer) but the Rust<->C part has a problem.
>
> To recap, Rust->C is the serialization and corresponds to output
> visitors. C->Rust is the deserialization and corresponds to input
> visitors.
>
> For serialization, serde has a push model where the generated code
> looks like this:
>
>       let mut state =
>           Serializer::serialize_struct(serializer, "S", 2);
>       SerializeStruct::serialize_field(&mut state, "a", &self.a)?;
>       SerializeStruct::serialize_field(&mut state, "b", &self.b)?;
>       SerializeStruct::end(state)
>
> whereas QAPI has a pull model where visit_type_* drives the process
> and requests the fields one by one.
>
> For deserialization, serde has a pull model where the generated code
> asks for the field names one by one:
>
>     fn visit_map<__A>(self, mut __map: __A)
>         while let Some(key) =
>             MapAccess::next_key::<__Field>(&mut __map)? {
>                 match __key { ... }
>         }
>     }
>
> whereas QAPI has a push model where visit_type_* again drives the
> process and sends fields one by one.
>
> For commands this is not a problem because the real underlying
> transformation is QObject->QObject and the intermediate steps (to and
> from QObject) can use serde.

Are you talking about commands implemented in Rust?

The existing data flow is roughly like this (I'm simplifying):

1. Parse JSON text into request QObject, pass to QMP core

2. Extract command name string and argument QDict

3. Look up generated command marshaller / unmarshaller, pass argument
   QDict to it

4. Unmarshall argument QDict with the QObject input visitor and
   generated visit_type_ARG()

5. Pass the C arguments to the handwritten command handler, receive the
   C return value

6. Marshall the return value into a QObject with the QObject output
   visitor and generated visit_type_RET(), return it to QMP core

7. Insert it into a response QObject

8. Unparse response QObject into JSON text

How would a Serde flow look like?

> However, QOM property getters/setters (especially, but not
> exclusively, for properties with compound types) remain a problem
> since these use callbacks with a Visitor* argument.

object_property_set() takes the new property value wrapped in an input
visitor.  The property setter extracts it using visit_type_FOOs() with
this input visitor as it sees fit.  Ideally, it uses exactly
visit_type_PROPTYPE().

object_property_get() takes an output visitor to be wrapped it around
the property value.  The property getter inserts it using
visit_type_FOOs() with this output visitor as it sees fit.  Ideally, it
uses exactly visit_type_PROPTYPE().

We sometimes use a QObject input / output visitor, and sometimes a
string input / output visitor.  The latter come with restrictions, and
are evolutionary dead ends.

The QObject visitors wrap a QObject, the string visitors wrap a string
(d'oh).

>                                                     I see three
> possibilities:
>
> 1) everything is done through an intermediate QObject step (e.g. for a
> setter: Visitor->QObject with an input visitor, and QObject->Rust with
> serde deserialization).
>     + easy, Rust only sees serde
>     + QMP commands use a single conversion step
>     - inefficient
>
> 2) everything is done through an intermediate C step (e.g. for a
> setter: Visitor->C with a visit_type_* function, and C->Rust with
> generated code that does not need to use serde). There is still a
> double conversion step, but it's more efficient than option 1
>     + one framework (visitor)
>     - double conversion for the QMP commands
>     - lots of generated code
>
> 3) generating a Rust visit_type_* implementation as well, either in
> qapi-gen (3a) or through a procedural macro (3b). This should not be
> hard to write but it would remove a lot of the advantages from using
> serde.
>     + efficient
>     + preserves single conversion for QMP commands
>     - two frameworks

I'm afraid this is too terse for ignorant me.

> I am leaning towards option 1, i.e. keep using serde but only cover
> conversions to and from QObject. The reason is that one future usecase
> for Rust in QEMU is the UEFI variable store; that one also has some
> Rust<->JSON conversions and could be served by either QObject or
> serde_json. Either way, it'd be nice for the UEFI variable store to
> remain within the Rust serde ecosystem and allow sharing code between
> QEMU and Coconut SVSM. But I'm not so sure...
>
> Paolo

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 4945 bytes --]

Il mer 18 giu 2025, 16:25 Markus Armbruster <armbru@redhat.com> ha scritto:

> I don't know enough about Rust/serde to give advice.  I do know how to
> make a fool of myself by asking dumb questions.
>

No dumb questions, only dumb answers.

> For commands this is not a problem because the real underlying
> > transformation is QObject->QObject and the intermediate steps (to and
> > from QObject) can use serde.
>
> Are you talking about commands implemented in Rust?
>

Yes. I will intersperse your text with the corresponding Rust/serde
implementation.

The existing data flow is roughly like this (I'm simplifying):
>
> 1. Parse JSON text into request QObject, pass to QMP core
> 2. Extract command name string and argument QDict
> 3. Look up generated command marshaller / unmarshaller, pass argument
>    QDict to it
>

Same so far since this is C code.

4. Unmarshall argument QDict with the QObject input visitor and
>    generated visit_type_ARG()
>

Unmarshall with QObject Deserializer, which talks to a serde-generated
Deserialize implementation.

5. Pass the C arguments to the handwritten command handler, receive the
>    C return value
>

Same.

6. Marshall the return value into a QObject with the QObject output
>    visitor and generated visit_type_RET(), return it to QMP core
>

Marshall with the QObject Serializer, which talks to a serde-generated
Serialize implementation.

7. Insert it into a response QObject
> 8. Unparse response QObject into JSON text
>

Same.

How would a Serde flow look like?
>

As described above, visitors are bypassed and the marshalling/unmarshalling
works directly at the QObject level.

Implementation-wise the main difference is that 1) the type code
(Serialize/Deserialize) is not the same for serialization and
desetialization, unlike visit_type_*() 2) the code generation is done by
serde instead of qapi-gen and we'd be mostly oblivious to how it works.

The Serializer and Deserializer should be about 1500 lines of Rust + tests,
and they would do the functionality of the QObject input and output
visitors.

> However, QOM property getters/setters (especially, but not
> > exclusively, for properties with compound types) remain a problem
> > since these use callbacks with a Visitor* argument.
>
> object_property_set() takes the new property value wrapped in an input
> visitor.  The property setter extracts it using visit_type_FOOs() with
> this input visitor as it sees fit.  Ideally, it uses exactly
> visit_type_PROPTYPE().
>
> object_property_get() takes an output visitor to be wrapped it around
> the property value.  The property getter inserts it using
> visit_type_FOOs() with this output visitor as it sees fit.  Ideally, it
> uses exactly visit_type_PROPTYPE().
>
> We sometimes use a QObject input / output visitor, and sometimes a
> string input / output visitor.  The latter come with restrictions, and
> are evolutionary dead ends.
>
> The QObject visitors wrap a QObject, the string visitors wrap a string
> (d'oh).
>

Yep. The string visitor is why we cannot just change getters and setters to
use QObject.

In this case, without writing a visit_type_*() implementation that can
write to a Rust struct, an intermediate QObject would be the only way to
turn a Visitor into a Rust data type. So I can imagine three ways to
operate:

* Keep using serde like for commands: in the callback that is invoked by
object_property_set() do Visitor->QObject->setter (yes that means double
conversion when the source visitor is and QObject visitor) or for the
getter case, getter->QObject->Visitor. This has the minimum amount of code
added to qapi-gen.

* Generate a visit_type_*() implementation that emits a Rust struct (i.e.
one that maps for example 'str' to a String and not a *mut c_char) and
forgo serde completely. Use this generated implementation everywhere: QOM
getters and setters, as well as QMP commands. This is how C code works.

* Generate rust->C (e.g. String->*mut c_char) and C->rust converters from
qapi-gen; use the existing C visit_type_*() to extract data from visitors
and then apply said converters to turn the data into a Rust struct, and
likewise in the other direction. This was the way Marc-André's prototype
worked.

I'm afraid this is too terse for ignorant me.
>

I tried to translate that. :)

Paolo


> > I am leaning towards option 1, i.e. keep using serde but only cover
> > conversions to and from QObject. The reason is that one future usecase
> > for Rust in QEMU is the UEFI variable store; that one also has some
> > Rust<->JSON conversions and could be served by either QObject or
> > serde_json. Either way, it'd be nice for the UEFI variable store to
> > remain within the Rust serde ecosystem and allow sharing code between
> > QEMU and Coconut SVSM. But I'm not so sure...
> >
> > Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 8535 bytes --]

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il mer 18 giu 2025, 16:25 Markus Armbruster <armbru@redhat.com> ha scritto:
>
>> I don't know enough about Rust/serde to give advice.  I do know how to
>> make a fool of myself by asking dumb questions.
>>
>
> No dumb questions, only dumb answers.
>
>> For commands this is not a problem because the real underlying
>> > transformation is QObject->QObject and the intermediate steps (to and
>> > from QObject) can use serde.
>>
>> Are you talking about commands implemented in Rust?
>>
>
> Yes. I will intersperse your text with the corresponding Rust/serde
> implementation.
>
>> The existing data flow is roughly like this (I'm simplifying):
>>
>> 1. Parse JSON text into request QObject, pass to QMP core
>>
>> 2. Extract command name string and argument QDict
>>
>> 3. Look up generated command marshaller / unmarshaller, pass argument
>>    QDict to it
>>
>
> Same so far since this is C code.
>
>> 4. Unmarshall argument QDict with the QObject input visitor and
>>    generated visit_type_ARG()
>
> Unmarshall with QObject Deserializer, which talks to a serde-generated
> Deserialize implementation.
>
>> 5. Pass the C arguments to the handwritten command handler, receive the
>>    C return value
>>
>
> Same.
>
>> 6. Marshall the return value into a QObject with the QObject output
>>    visitor and generated visit_type_RET(), return it to QMP core
>>
>
> Marshall with the QObject Serializer, which talks to a serde-generated
> Serialize implementation.
>
>> 7. Insert it into a response QObject
>>
>> 8. Unparse response QObject into JSON text
>>
>
> Same.
>>
>> How would a Serde flow look like?
>>
>
> As described above, visitors are bypassed and the marshalling/unmarshalling
> works directly at the QObject level.

We use the same code (handwritten QMP core) up to QObject and from
QObject on.

In between, we use QObject input/output visitors with generated C data
types for commands implemented in C, and Serde-generated
deserializers/serializers with generated Rust data types for commands
implemented in Rust.

Correct?

> Implementation-wise the main difference is that 1) the type code
> (Serialize/Deserialize) is not the same for serialization and
> desetialization, unlike visit_type_*() 2) the code generation is done by
> serde instead of qapi-gen and we'd be mostly oblivious to how it works.
>
> The Serializer and Deserializer should be about 1500 lines of Rust + tests,
> and they would do the functionality of the QObject input and output
> visitors.

We end up with two separate ways to do the same job, which is kind of
sad.

I gather the alternative would be to use generated QAPI visitors for the
generated Rust data types instead of Serde.  This would be unusual for
Rust code: Serde exists and works, so reinvent it would be wasteful and
dumb.

Gut feeling: sticking to how things are usually done (here: with Serde)
is the more prudent choice.

>> > However, QOM property getters/setters (especially, but not
>> > exclusively, for properties with compound types) remain a problem
>> > since these use callbacks with a Visitor* argument.
>>
>> object_property_set() takes the new property value wrapped in an input
>> visitor.  The property setter extracts it using visit_type_FOOs() with
>> this input visitor as it sees fit.  Ideally, it uses exactly
>> visit_type_PROPTYPE().
>>
>> object_property_get() takes an output visitor to be wrapped it around
>> the property value.  The property getter inserts it using
>> visit_type_FOOs() with this output visitor as it sees fit.  Ideally, it
>> uses exactly visit_type_PROPTYPE().
>>
>> We sometimes use a QObject input / output visitor, and sometimes a
>> string input / output visitor.  The latter come with restrictions, and
>> are evolutionary dead ends.
>>
>> The QObject visitors wrap a QObject, the string visitors wrap a string
>> (d'oh).
>>
>
> Yep. The string visitor is why we cannot just change getters and setters to
> use QObject.

The string visitors have long been a thorn in my side.

I wish QOM wasn't so annoyingly flexible.  I wish a property had to be
of QAPI type (possibly complex).  Less headaches.  Less boilerplate,
too.

Almost all QOM properties are.  And the few that aren't feel like bad
ideas to me.

> In this case, without writing a visit_type_*() implementation that can
> write to a Rust struct, an intermediate QObject would be the only way to
> turn a Visitor into a Rust data type. So I can imagine three ways to
> operate:
>
> * Keep using serde like for commands: in the callback that is invoked by
> object_property_set() do Visitor->QObject->setter (yes that means double
> conversion when the source visitor is and QObject visitor) or for the
> getter case, getter->QObject->Visitor. This has the minimum amount of code
> added to qapi-gen.

Recall the callback gets the new property value wrapped in an input
visitor.  Whereas a C setter extracts it into some C variable(s), a Rust
setter extracts it into a QObject, which it then passes to Serde for
deserialization into Rust variables.  Correct?

> * Generate a visit_type_*() implementation that emits a Rust struct (i.e.
> one that maps for example 'str' to a String and not a *mut c_char) and
> forgo serde completely. Use this generated implementation everywhere: QOM
> getters and setters, as well as QMP commands. This is how C code works.

This is the alternative mentioned above.

> * Generate rust->C (e.g. String->*mut c_char) and C->rust converters from
> qapi-gen; use the existing C visit_type_*() to extract data from visitors
> and then apply said converters to turn the data into a Rust struct, and
> likewise in the other direction. This was the way Marc-André's prototype
> worked.

Converters between C and Rust data types let us cut at the C data type
instead of at QObject.

We use the same code up to QAPI-generated C data type and from
QAPI-generated C data type on.

In between, we work directly with the C data type for properties
implemented in C, and convert to and from the Rust data type for
properties implemented in Rust.

Correct?

The simplest such converters convert via QObject.  Grossly inefficient
:)

Marc-André's prototype demonstrates efficient converters can be had.
The question is whether they're worth their keep.

>> I'm afraid this is too terse for ignorant me.
>>
>
> I tried to translate that. :)

Thank you!

Re: [PATCH preview 0/3] reviving minimal QAPI generation from 2021

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 3809 bytes --]

Il lun 23 giu 2025, 08:53 Markus Armbruster <armbru@redhat.com> ha scritto:

> We end up with two separate ways to do the same job, which is kind of
> sad.
>
> I gather the alternative would be to use generated QAPI visitors for the
> generated Rust data types instead of Serde.  This would be unusual for
> Rust code: Serde exists and works, so reinvent it would be wasteful and
> dumb.
>

My feeling as well.

The string visitors have long been a thorn in my side.
>
> I wish QOM wasn't so annoyingly flexible.  I wish a property had to be
> of QAPI type (possibly complex).  Less headaches.  Less boilerplate,
> too.
>
> Almost all QOM properties are.  And the few that aren't feel like bad
> ideas to me.
>

Makes sense. On the other hand, from the C point of view I admit that
passing a Visitor* or a QObject* to the setter would not be very
different—or possibly even a bit more code for the QObject*. Likewise for
passing a QObject** on the getter side.

So the problem isn't the flexibility... It's using/exploiting it.

> * Keep using serde like for commands: in the callback that is invoked by
> > object_property_set() do Visitor->QObject->setter (yes that means double
> > conversion when the source visitor is and QObject visitor) or for the
> > getter case, getter->QObject->Visitor. This has the minimum amount of
> code
> > added to qapi-gen.
>
> Recall the callback gets the new property value wrapped in an input
> visitor.  Whereas a C setter extracts it into some C variable(s), a Rust
> setter extracts it into a QObject, which it then passes to Serde for
> deserialization into Rust variables.  Correct?
>

Yes.

> * Generate a visit_type_*() implementation that emits a Rust struct (i.e.
> > one that maps for example 'str' to a String and not a *mut c_char) and
> > forgo serde completely. Use this generated implementation everywhere: QOM
> > getters and setters, as well as QMP commands. This is how C code works.
>
> This is the alternative mentioned above.
>

Yes.

> * Generate rust->C (e.g. String->*mut c_char) and C->rust converters from
> > qapi-gen; use the existing C visit_type_*() to extract data from visitors
> > and then apply said converters to turn the data into a Rust struct, and
> > likewise in the other direction. This was the way Marc-André's prototype
> > worked.
>
> Converters between C and Rust data types let us cut at the C data type
> instead of at QObject.
>
> We use the same code up to QAPI-generated C data type and from
> QAPI-generated C data type on.
>
> In between, we work directly with the C data type for properties
> implemented in C, and convert to and from the Rust data type for
> properties implemented in Rust.
>
> Correct?
>

Yes.

The simplest such converters convert via QObject.  Grossly inefficient
> :)
>
> Marc-André's prototype demonstrates efficient converters can be had.
> The question is whether they're worth their keep.
>

Exactly. What keeps me on the edge is that for QMP the result is still a
bit less efficient than straight QObject->Rust via Serde. For the QOM
property Visitor->C->Rust is better than Visitor->QObject->Rust, but is it
worth the extra amount of code generator? Leaving the code generation
business to Serde is appealing and it gets one of the two (QMP) as
efficient as it can be.

The Serde interface code is pretty much write once and forget, since
QObject hasn't changed substantially in 15 years. The qapi-gen code is
write once but stays more in the way of other Python code we maintain. On
the other hand the Serde code is probably going to be a bit on the
impenetrable side, and until someone writes it we don't know if the balance
may tilt back towards preferring Marc-André's ideas.

Paolo

[-- Attachment #2: Type: text/html, Size: 5905 bytes --]