[PATCH 00/16] Farewell rpcgen

Daniel P. Berrangé posted 16 patches 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230308163913.338952-1-berrange@redhat.com
build-aux/Makefile.in                         |   1 +
build-aux/meson.build                         |   5 +
build-aux/syntax-check.mk                     |  42 +-
libvirt.spec.in                               |   2 +-
meson.build                                   |  13 +-
scripts/meson.build                           |   2 +
scripts/rpcgen/main.py                        |  90 ++
scripts/rpcgen/meson.build                    |  16 +
scripts/rpcgen/rpcgen/ast.py                  | 270 ++++++
scripts/rpcgen/rpcgen/generator.py            | 509 ++++++++++++
scripts/rpcgen/rpcgen/lexer.py                | 213 +++++
scripts/rpcgen/rpcgen/meson.build             |   7 +
scripts/rpcgen/rpcgen/parser.py               | 497 +++++++++++
scripts/rpcgen/rpcgen/visitor.py              | 156 ++++
scripts/rpcgen/tests/demo.c                   | 495 +++++++++++
scripts/rpcgen/tests/demo.h                   | 264 ++++++
scripts/rpcgen/tests/demo.x                   | 127 +++
scripts/rpcgen/tests/meson.build              |  20 +
scripts/rpcgen/tests/simple.x                 |  35 +
scripts/rpcgen/tests/test_demo.c              | 782 ++++++++++++++++++
scripts/rpcgen/tests/test_demo_enum.bin       | Bin 0 -> 4 bytes
.../tests/test_demo_enum_fixed_array.bin      | Bin 0 -> 52 bytes
.../tests/test_demo_enum_pointer_null.bin     | Bin 0 -> 4 bytes
.../tests/test_demo_enum_pointer_set.bin      | Bin 0 -> 8 bytes
.../rpcgen/tests/test_demo_enum_scalar.bin    | Bin 0 -> 4 bytes
.../test_demo_enum_variable_array_empty.bin   | Bin 0 -> 4 bytes
.../test_demo_enum_variable_array_set.bin     | Bin 0 -> 16 bytes
.../tests/test_demo_int_fixed_array.bin       | Bin 0 -> 12 bytes
.../tests/test_demo_int_pointer_null.bin      | Bin 0 -> 4 bytes
.../tests/test_demo_int_pointer_set.bin       | Bin 0 -> 8 bytes
scripts/rpcgen/tests/test_demo_int_scalar.bin | Bin 0 -> 4 bytes
.../test_demo_int_variable_array_empty.bin    | Bin 0 -> 4 bytes
.../test_demo_int_variable_array_set.bin      | Bin 0 -> 16 bytes
.../tests/test_demo_opaque_fixed_array.bin    | Bin 0 -> 12 bytes
.../test_demo_opaque_variable_array_empty.bin | Bin 0 -> 4 bytes
.../test_demo_opaque_variable_array_set.bin   | Bin 0 -> 8 bytes
.../test_demo_string_variable_array_empty.bin | Bin 0 -> 4 bytes
.../test_demo_string_variable_array_set.bin   | Bin 0 -> 12 bytes
scripts/rpcgen/tests/test_demo_struct.bin     | Bin 0 -> 8 bytes
.../tests/test_demo_struct_fixed_array.bin    | Bin 0 -> 136 bytes
.../tests/test_demo_struct_pointer_null.bin   | Bin 0 -> 4 bytes
.../tests/test_demo_struct_pointer_set.bin    | Bin 0 -> 12 bytes
.../rpcgen/tests/test_demo_struct_scalar.bin  |   1 +
.../test_demo_struct_variable_array_empty.bin | Bin 0 -> 4 bytes
.../test_demo_struct_variable_array_set.bin   | Bin 0 -> 28 bytes
.../tests/test_demo_test_struct_all_types.bin | Bin 0 -> 1752 bytes
scripts/rpcgen/tests/test_demo_union_case.bin | Bin 0 -> 8 bytes
.../rpcgen/tests/test_demo_union_default.bin  | Bin 0 -> 8 bytes
.../tests/test_demo_union_fixed_array.bin     | Bin 0 -> 168 bytes
.../tests/test_demo_union_no_default_case.bin | Bin 0 -> 8 bytes
.../tests/test_demo_union_pointer_null.bin    | Bin 0 -> 4 bytes
.../tests/test_demo_union_pointer_set.bin     | Bin 0 -> 12 bytes
.../rpcgen/tests/test_demo_union_scalar.bin   | Bin 0 -> 8 bytes
.../test_demo_union_variable_array_empty.bin  | Bin 0 -> 4 bytes
.../test_demo_union_variable_array_set.bin    | Bin 0 -> 28 bytes
.../test_demo_union_void_default_case.bin     | Bin 0 -> 8 bytes
.../test_demo_union_void_default_default.bin  |   1 +
scripts/rpcgen/tests/test_generator.py        |  60 ++
scripts/rpcgen/tests/test_lexer.py            | 116 +++
scripts/rpcgen/tests/test_parser.py           |  91 ++
src/admin/admin_remote.c                      |  50 +-
src/admin/meson.build                         |   8 +-
src/locking/meson.build                       |   8 +-
src/logging/meson.build                       |   8 +-
src/lxc/meson.build                           |  12 +-
src/remote/meson.build                        |   8 +-
src/remote/remote_driver.c                    | 754 ++++++-----------
src/rpc/gendispatch.pl                        |  60 +-
src/rpc/genprotocol.pl                        | 144 ----
src/rpc/meson.build                           |   9 +-
src/rpc/virnetmessage.c                       | 704 ++++++++++++++++
src/rpc/virnetmessage.h                       |  88 ++
72 files changed, 4897 insertions(+), 771 deletions(-)
create mode 100755 scripts/rpcgen/main.py
create mode 100644 scripts/rpcgen/meson.build
create mode 100644 scripts/rpcgen/rpcgen/ast.py
create mode 100644 scripts/rpcgen/rpcgen/generator.py
create mode 100644 scripts/rpcgen/rpcgen/lexer.py
create mode 100644 scripts/rpcgen/rpcgen/meson.build
create mode 100644 scripts/rpcgen/rpcgen/parser.py
create mode 100644 scripts/rpcgen/rpcgen/visitor.py
create mode 100644 scripts/rpcgen/tests/demo.c
create mode 100644 scripts/rpcgen/tests/demo.h
create mode 100644 scripts/rpcgen/tests/demo.x
create mode 100644 scripts/rpcgen/tests/meson.build
create mode 100644 scripts/rpcgen/tests/simple.x
create mode 100644 scripts/rpcgen/tests/test_demo.c
create mode 100644 scripts/rpcgen/tests/test_demo_enum.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_fixed_array.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_null.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_scalar.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_fixed_array.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_null.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_scalar.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_opaque_fixed_array.bin
create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_fixed_array.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_null.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_scalar.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_test_struct_all_types.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_case.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_default.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_fixed_array.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_no_default_case.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_null.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_scalar.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_empty.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_set.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_case.bin
create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_default.bin
create mode 100644 scripts/rpcgen/tests/test_generator.py
create mode 100644 scripts/rpcgen/tests/test_lexer.py
create mode 100644 scripts/rpcgen/tests/test_parser.py
delete mode 100755 src/rpc/genprotocol.pl
[PATCH 00/16] Farewell rpcgen
Posted by Daniel P. Berrangé 1 year, 1 month ago
This series something I was hacking on a little while back in an
attempt to make our RPC layer more maintainable. There are many
aspects I'm unhappy about with current code

 * When serializing a message we have no clue how big
   it will be, but xdrmem_create wants a fixed size,
   so we have to keep trying to serialize in a loop
   making it bigger each time

 * We don't control memory allocation/free'ing directly
   so we can't do a virSecureErase on fields inside the
   RPC message struct that handle secrets.

 * The XDR API is generally unpleasant to use as it is
   outside our virNetMessage object. Ideally we would
   be reading/writing directly from/to the virNetMessage
   buffer with APIs on virNetMessage,instead of indirectly
   via a XDR object.

 * We want more from XDR than it actually gives us. Our
   XDR protocol files have annotations to express what
   we want our code generator todo, or for ACLs. The
   relationship between the structs and the message
   numbers is implicit. Essentially we've defined our
   own language indirectly via comments, and then
   parse this with regexes which is horrid.

 * The code rpcgen creates is poor quality which we have
   to post-process to fix bugs/problems. It also lacks
   support for modern features like g_auto.

Anyway, in a fit of rage I looked at the XDR RFC and thought..

This language is trivial, why do we need to outsource to
rpcgen and libtirpc instead of dealing with it directly.

This small series moves in that direction. It creates an
XDR language lexer and parser, and then a code generator
which emits code that is (nearly) identical to what rpcgen
would emit. This is sufficient to eliminate rpcgen usage
and support g_auto. Since we're still using libtirpc
at this stage we can be confident we're still doing the
same thing on the wire. I've got some unit tests too
with the rpcgen generation to validate stuff.

The next step is to change the code generator so that
instead of generating code for libtirpc APIs, it will
instead directly speak virNetMessage APIs. That would
give us full control over our RPC stack guaranteed to
be platform portable instead of fighting slight differences
in RPC libraries (eg xdr_quad vs xdr_int64 madness).

I was going to wait until I had written such code before
sending this series, but I've got diverted onto other more
important tasks. So here at least is what I have so far.

After that foundation is done, we are in a place where
we can actually do more innovative things. For example
we can directly extend the XDR protocol language if we
like, turning our magic comments into properly parsable
constructs.

With this, we would be in a position to replace our
Perl RPC client/server dispatch code generators with
something that is more supportable in Python. The python
code would work with properly represented objects and
formal parsers and not regexes and anonymous complex
perl data structures.

Daniel P. Berrangé (16):
  rpcgen: drop type-puning workarounds
  build-aux: skip E203 and W503 flake8 checks
  build-aux: introduce 'black' tool for python formatting
  rpcgen: add an XDR protocol lexer
  rpcgen: add an XDR protocol abstract syntax tree
  rpcgen: add an XDR protocol parser
  rpcgen: define a visitor API for XDR protocol specs
  rpcgen: add a C code generator for XDR protocol specs
  rpcgen: add test case for XDR serialization
  rpcgen: define entrypoint for running new rpcgen impl
  build: switch over to new rpc generator code
  rpcgen: add g_auto function support
  rpc: use g_auto for client RPC return parameters
  admin: use g_auto for client RPC return parameters
  remote: use g_auto for client RPC return parameters
  rpc: add helpers for XDR type serialization

 build-aux/Makefile.in                         |   1 +
 build-aux/meson.build                         |   5 +
 build-aux/syntax-check.mk                     |  42 +-
 libvirt.spec.in                               |   2 +-
 meson.build                                   |  13 +-
 scripts/meson.build                           |   2 +
 scripts/rpcgen/main.py                        |  90 ++
 scripts/rpcgen/meson.build                    |  16 +
 scripts/rpcgen/rpcgen/ast.py                  | 270 ++++++
 scripts/rpcgen/rpcgen/generator.py            | 509 ++++++++++++
 scripts/rpcgen/rpcgen/lexer.py                | 213 +++++
 scripts/rpcgen/rpcgen/meson.build             |   7 +
 scripts/rpcgen/rpcgen/parser.py               | 497 +++++++++++
 scripts/rpcgen/rpcgen/visitor.py              | 156 ++++
 scripts/rpcgen/tests/demo.c                   | 495 +++++++++++
 scripts/rpcgen/tests/demo.h                   | 264 ++++++
 scripts/rpcgen/tests/demo.x                   | 127 +++
 scripts/rpcgen/tests/meson.build              |  20 +
 scripts/rpcgen/tests/simple.x                 |  35 +
 scripts/rpcgen/tests/test_demo.c              | 782 ++++++++++++++++++
 scripts/rpcgen/tests/test_demo_enum.bin       | Bin 0 -> 4 bytes
 .../tests/test_demo_enum_fixed_array.bin      | Bin 0 -> 52 bytes
 .../tests/test_demo_enum_pointer_null.bin     | Bin 0 -> 4 bytes
 .../tests/test_demo_enum_pointer_set.bin      | Bin 0 -> 8 bytes
 .../rpcgen/tests/test_demo_enum_scalar.bin    | Bin 0 -> 4 bytes
 .../test_demo_enum_variable_array_empty.bin   | Bin 0 -> 4 bytes
 .../test_demo_enum_variable_array_set.bin     | Bin 0 -> 16 bytes
 .../tests/test_demo_int_fixed_array.bin       | Bin 0 -> 12 bytes
 .../tests/test_demo_int_pointer_null.bin      | Bin 0 -> 4 bytes
 .../tests/test_demo_int_pointer_set.bin       | Bin 0 -> 8 bytes
 scripts/rpcgen/tests/test_demo_int_scalar.bin | Bin 0 -> 4 bytes
 .../test_demo_int_variable_array_empty.bin    | Bin 0 -> 4 bytes
 .../test_demo_int_variable_array_set.bin      | Bin 0 -> 16 bytes
 .../tests/test_demo_opaque_fixed_array.bin    | Bin 0 -> 12 bytes
 .../test_demo_opaque_variable_array_empty.bin | Bin 0 -> 4 bytes
 .../test_demo_opaque_variable_array_set.bin   | Bin 0 -> 8 bytes
 .../test_demo_string_variable_array_empty.bin | Bin 0 -> 4 bytes
 .../test_demo_string_variable_array_set.bin   | Bin 0 -> 12 bytes
 scripts/rpcgen/tests/test_demo_struct.bin     | Bin 0 -> 8 bytes
 .../tests/test_demo_struct_fixed_array.bin    | Bin 0 -> 136 bytes
 .../tests/test_demo_struct_pointer_null.bin   | Bin 0 -> 4 bytes
 .../tests/test_demo_struct_pointer_set.bin    | Bin 0 -> 12 bytes
 .../rpcgen/tests/test_demo_struct_scalar.bin  |   1 +
 .../test_demo_struct_variable_array_empty.bin | Bin 0 -> 4 bytes
 .../test_demo_struct_variable_array_set.bin   | Bin 0 -> 28 bytes
 .../tests/test_demo_test_struct_all_types.bin | Bin 0 -> 1752 bytes
 scripts/rpcgen/tests/test_demo_union_case.bin | Bin 0 -> 8 bytes
 .../rpcgen/tests/test_demo_union_default.bin  | Bin 0 -> 8 bytes
 .../tests/test_demo_union_fixed_array.bin     | Bin 0 -> 168 bytes
 .../tests/test_demo_union_no_default_case.bin | Bin 0 -> 8 bytes
 .../tests/test_demo_union_pointer_null.bin    | Bin 0 -> 4 bytes
 .../tests/test_demo_union_pointer_set.bin     | Bin 0 -> 12 bytes
 .../rpcgen/tests/test_demo_union_scalar.bin   | Bin 0 -> 8 bytes
 .../test_demo_union_variable_array_empty.bin  | Bin 0 -> 4 bytes
 .../test_demo_union_variable_array_set.bin    | Bin 0 -> 28 bytes
 .../test_demo_union_void_default_case.bin     | Bin 0 -> 8 bytes
 .../test_demo_union_void_default_default.bin  |   1 +
 scripts/rpcgen/tests/test_generator.py        |  60 ++
 scripts/rpcgen/tests/test_lexer.py            | 116 +++
 scripts/rpcgen/tests/test_parser.py           |  91 ++
 src/admin/admin_remote.c                      |  50 +-
 src/admin/meson.build                         |   8 +-
 src/locking/meson.build                       |   8 +-
 src/logging/meson.build                       |   8 +-
 src/lxc/meson.build                           |  12 +-
 src/remote/meson.build                        |   8 +-
 src/remote/remote_driver.c                    | 754 ++++++-----------
 src/rpc/gendispatch.pl                        |  60 +-
 src/rpc/genprotocol.pl                        | 144 ----
 src/rpc/meson.build                           |   9 +-
 src/rpc/virnetmessage.c                       | 704 ++++++++++++++++
 src/rpc/virnetmessage.h                       |  88 ++
 72 files changed, 4897 insertions(+), 771 deletions(-)
 create mode 100755 scripts/rpcgen/main.py
 create mode 100644 scripts/rpcgen/meson.build
 create mode 100644 scripts/rpcgen/rpcgen/ast.py
 create mode 100644 scripts/rpcgen/rpcgen/generator.py
 create mode 100644 scripts/rpcgen/rpcgen/lexer.py
 create mode 100644 scripts/rpcgen/rpcgen/meson.build
 create mode 100644 scripts/rpcgen/rpcgen/parser.py
 create mode 100644 scripts/rpcgen/rpcgen/visitor.py
 create mode 100644 scripts/rpcgen/tests/demo.c
 create mode 100644 scripts/rpcgen/tests/demo.h
 create mode 100644 scripts/rpcgen/tests/demo.x
 create mode 100644 scripts/rpcgen/tests/meson.build
 create mode 100644 scripts/rpcgen/tests/simple.x
 create mode 100644 scripts/rpcgen/tests/test_demo.c
 create mode 100644 scripts/rpcgen/tests/test_demo_enum.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_fixed_array.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_null.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_scalar.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_fixed_array.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_null.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_scalar.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_opaque_fixed_array.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_fixed_array.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_null.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_scalar.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_test_struct_all_types.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_case.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_default.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_fixed_array.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_no_default_case.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_null.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_scalar.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_empty.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_set.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_case.bin
 create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_default.bin
 create mode 100644 scripts/rpcgen/tests/test_generator.py
 create mode 100644 scripts/rpcgen/tests/test_lexer.py
 create mode 100644 scripts/rpcgen/tests/test_parser.py
 delete mode 100755 src/rpc/genprotocol.pl

-- 
2.39.1

Re: [PATCH 00/16] Farewell rpcgen
Posted by Michal Prívozník 1 year, 1 month ago
On 3/8/23 17:38, Daniel P. Berrangé wrote:
>

> 
> Daniel P. Berrangé (16):
>   rpcgen: drop type-puning workarounds
>   build-aux: skip E203 and W503 flake8 checks
>   build-aux: introduce 'black' tool for python formatting
>   rpcgen: add an XDR protocol lexer
>   rpcgen: add an XDR protocol abstract syntax tree
>   rpcgen: add an XDR protocol parser
>   rpcgen: define a visitor API for XDR protocol specs
>   rpcgen: add a C code generator for XDR protocol specs
>   rpcgen: add test case for XDR serialization
>   rpcgen: define entrypoint for running new rpcgen impl
>   build: switch over to new rpc generator code
>   rpcgen: add g_auto function support
>   rpc: use g_auto for client RPC return parameters
>   admin: use g_auto for client RPC return parameters
>   remote: use g_auto for client RPC return parameters
>   rpc: add helpers for XDR type serialization
> 

>  72 files changed, 4897 insertions(+), 771 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

but please do see individual replies before pushing. I'm suggesting some
squash-ins.

Michal

Re: [PATCH 00/16] Farewell rpcgen
Posted by Daniel P. Berrangé 5 months, 2 weeks ago
FYI, this series was fully acked and I completely forgot
to then push it. Todays' fix for rpcgen on macOS reminded
me about this. I've tested that with this series applied,
macOS is fine, and the only rebase conflicts were trivial
on meson.build and syntax-check.mk. So I'm intending to
push this series, minus the last patch.

On Wed, Mar 08, 2023 at 11:38:57AM -0500, Daniel P. Berrangé wrote:
> This series something I was hacking on a little while back in an
> attempt to make our RPC layer more maintainable. There are many
> aspects I'm unhappy about with current code
> 
>  * When serializing a message we have no clue how big
>    it will be, but xdrmem_create wants a fixed size,
>    so we have to keep trying to serialize in a loop
>    making it bigger each time
> 
>  * We don't control memory allocation/free'ing directly
>    so we can't do a virSecureErase on fields inside the
>    RPC message struct that handle secrets.
> 
>  * The XDR API is generally unpleasant to use as it is
>    outside our virNetMessage object. Ideally we would
>    be reading/writing directly from/to the virNetMessage
>    buffer with APIs on virNetMessage,instead of indirectly
>    via a XDR object.
> 
>  * We want more from XDR than it actually gives us. Our
>    XDR protocol files have annotations to express what
>    we want our code generator todo, or for ACLs. The
>    relationship between the structs and the message
>    numbers is implicit. Essentially we've defined our
>    own language indirectly via comments, and then
>    parse this with regexes which is horrid.
> 
>  * The code rpcgen creates is poor quality which we have
>    to post-process to fix bugs/problems. It also lacks
>    support for modern features like g_auto.
> 
> Anyway, in a fit of rage I looked at the XDR RFC and thought..
> 
> This language is trivial, why do we need to outsource to
> rpcgen and libtirpc instead of dealing with it directly.
> 
> This small series moves in that direction. It creates an
> XDR language lexer and parser, and then a code generator
> which emits code that is (nearly) identical to what rpcgen
> would emit. This is sufficient to eliminate rpcgen usage
> and support g_auto. Since we're still using libtirpc
> at this stage we can be confident we're still doing the
> same thing on the wire. I've got some unit tests too
> with the rpcgen generation to validate stuff.
> 
> The next step is to change the code generator so that
> instead of generating code for libtirpc APIs, it will
> instead directly speak virNetMessage APIs. That would
> give us full control over our RPC stack guaranteed to
> be platform portable instead of fighting slight differences
> in RPC libraries (eg xdr_quad vs xdr_int64 madness).
> 
> I was going to wait until I had written such code before
> sending this series, but I've got diverted onto other more
> important tasks. So here at least is what I have so far.
> 
> After that foundation is done, we are in a place where
> we can actually do more innovative things. For example
> we can directly extend the XDR protocol language if we
> like, turning our magic comments into properly parsable
> constructs.
> 
> With this, we would be in a position to replace our
> Perl RPC client/server dispatch code generators with
> something that is more supportable in Python. The python
> code would work with properly represented objects and
> formal parsers and not regexes and anonymous complex
> perl data structures.
> 
> Daniel P. Berrangé (16):
>   rpcgen: drop type-puning workarounds
>   build-aux: skip E203 and W503 flake8 checks
>   build-aux: introduce 'black' tool for python formatting
>   rpcgen: add an XDR protocol lexer
>   rpcgen: add an XDR protocol abstract syntax tree
>   rpcgen: add an XDR protocol parser
>   rpcgen: define a visitor API for XDR protocol specs
>   rpcgen: add a C code generator for XDR protocol specs
>   rpcgen: add test case for XDR serialization
>   rpcgen: define entrypoint for running new rpcgen impl
>   build: switch over to new rpc generator code
>   rpcgen: add g_auto function support
>   rpc: use g_auto for client RPC return parameters
>   admin: use g_auto for client RPC return parameters
>   remote: use g_auto for client RPC return parameters
>   rpc: add helpers for XDR type serialization
> 
>  build-aux/Makefile.in                         |   1 +
>  build-aux/meson.build                         |   5 +
>  build-aux/syntax-check.mk                     |  42 +-
>  libvirt.spec.in                               |   2 +-
>  meson.build                                   |  13 +-
>  scripts/meson.build                           |   2 +
>  scripts/rpcgen/main.py                        |  90 ++
>  scripts/rpcgen/meson.build                    |  16 +
>  scripts/rpcgen/rpcgen/ast.py                  | 270 ++++++
>  scripts/rpcgen/rpcgen/generator.py            | 509 ++++++++++++
>  scripts/rpcgen/rpcgen/lexer.py                | 213 +++++
>  scripts/rpcgen/rpcgen/meson.build             |   7 +
>  scripts/rpcgen/rpcgen/parser.py               | 497 +++++++++++
>  scripts/rpcgen/rpcgen/visitor.py              | 156 ++++
>  scripts/rpcgen/tests/demo.c                   | 495 +++++++++++
>  scripts/rpcgen/tests/demo.h                   | 264 ++++++
>  scripts/rpcgen/tests/demo.x                   | 127 +++
>  scripts/rpcgen/tests/meson.build              |  20 +
>  scripts/rpcgen/tests/simple.x                 |  35 +
>  scripts/rpcgen/tests/test_demo.c              | 782 ++++++++++++++++++
>  scripts/rpcgen/tests/test_demo_enum.bin       | Bin 0 -> 4 bytes
>  .../tests/test_demo_enum_fixed_array.bin      | Bin 0 -> 52 bytes
>  .../tests/test_demo_enum_pointer_null.bin     | Bin 0 -> 4 bytes
>  .../tests/test_demo_enum_pointer_set.bin      | Bin 0 -> 8 bytes
>  .../rpcgen/tests/test_demo_enum_scalar.bin    | Bin 0 -> 4 bytes
>  .../test_demo_enum_variable_array_empty.bin   | Bin 0 -> 4 bytes
>  .../test_demo_enum_variable_array_set.bin     | Bin 0 -> 16 bytes
>  .../tests/test_demo_int_fixed_array.bin       | Bin 0 -> 12 bytes
>  .../tests/test_demo_int_pointer_null.bin      | Bin 0 -> 4 bytes
>  .../tests/test_demo_int_pointer_set.bin       | Bin 0 -> 8 bytes
>  scripts/rpcgen/tests/test_demo_int_scalar.bin | Bin 0 -> 4 bytes
>  .../test_demo_int_variable_array_empty.bin    | Bin 0 -> 4 bytes
>  .../test_demo_int_variable_array_set.bin      | Bin 0 -> 16 bytes
>  .../tests/test_demo_opaque_fixed_array.bin    | Bin 0 -> 12 bytes
>  .../test_demo_opaque_variable_array_empty.bin | Bin 0 -> 4 bytes
>  .../test_demo_opaque_variable_array_set.bin   | Bin 0 -> 8 bytes
>  .../test_demo_string_variable_array_empty.bin | Bin 0 -> 4 bytes
>  .../test_demo_string_variable_array_set.bin   | Bin 0 -> 12 bytes
>  scripts/rpcgen/tests/test_demo_struct.bin     | Bin 0 -> 8 bytes
>  .../tests/test_demo_struct_fixed_array.bin    | Bin 0 -> 136 bytes
>  .../tests/test_demo_struct_pointer_null.bin   | Bin 0 -> 4 bytes
>  .../tests/test_demo_struct_pointer_set.bin    | Bin 0 -> 12 bytes
>  .../rpcgen/tests/test_demo_struct_scalar.bin  |   1 +
>  .../test_demo_struct_variable_array_empty.bin | Bin 0 -> 4 bytes
>  .../test_demo_struct_variable_array_set.bin   | Bin 0 -> 28 bytes
>  .../tests/test_demo_test_struct_all_types.bin | Bin 0 -> 1752 bytes
>  scripts/rpcgen/tests/test_demo_union_case.bin | Bin 0 -> 8 bytes
>  .../rpcgen/tests/test_demo_union_default.bin  | Bin 0 -> 8 bytes
>  .../tests/test_demo_union_fixed_array.bin     | Bin 0 -> 168 bytes
>  .../tests/test_demo_union_no_default_case.bin | Bin 0 -> 8 bytes
>  .../tests/test_demo_union_pointer_null.bin    | Bin 0 -> 4 bytes
>  .../tests/test_demo_union_pointer_set.bin     | Bin 0 -> 12 bytes
>  .../rpcgen/tests/test_demo_union_scalar.bin   | Bin 0 -> 8 bytes
>  .../test_demo_union_variable_array_empty.bin  | Bin 0 -> 4 bytes
>  .../test_demo_union_variable_array_set.bin    | Bin 0 -> 28 bytes
>  .../test_demo_union_void_default_case.bin     | Bin 0 -> 8 bytes
>  .../test_demo_union_void_default_default.bin  |   1 +
>  scripts/rpcgen/tests/test_generator.py        |  60 ++
>  scripts/rpcgen/tests/test_lexer.py            | 116 +++
>  scripts/rpcgen/tests/test_parser.py           |  91 ++
>  src/admin/admin_remote.c                      |  50 +-
>  src/admin/meson.build                         |   8 +-
>  src/locking/meson.build                       |   8 +-
>  src/logging/meson.build                       |   8 +-
>  src/lxc/meson.build                           |  12 +-
>  src/remote/meson.build                        |   8 +-
>  src/remote/remote_driver.c                    | 754 ++++++-----------
>  src/rpc/gendispatch.pl                        |  60 +-
>  src/rpc/genprotocol.pl                        | 144 ----
>  src/rpc/meson.build                           |   9 +-
>  src/rpc/virnetmessage.c                       | 704 ++++++++++++++++
>  src/rpc/virnetmessage.h                       |  88 ++
>  72 files changed, 4897 insertions(+), 771 deletions(-)
>  create mode 100755 scripts/rpcgen/main.py
>  create mode 100644 scripts/rpcgen/meson.build
>  create mode 100644 scripts/rpcgen/rpcgen/ast.py
>  create mode 100644 scripts/rpcgen/rpcgen/generator.py
>  create mode 100644 scripts/rpcgen/rpcgen/lexer.py
>  create mode 100644 scripts/rpcgen/rpcgen/meson.build
>  create mode 100644 scripts/rpcgen/rpcgen/parser.py
>  create mode 100644 scripts/rpcgen/rpcgen/visitor.py
>  create mode 100644 scripts/rpcgen/tests/demo.c
>  create mode 100644 scripts/rpcgen/tests/demo.h
>  create mode 100644 scripts/rpcgen/tests/demo.x
>  create mode 100644 scripts/rpcgen/tests/meson.build
>  create mode 100644 scripts/rpcgen/tests/simple.x
>  create mode 100644 scripts/rpcgen/tests/test_demo.c
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_fixed_array.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_null.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_pointer_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_scalar.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_enum_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_fixed_array.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_null.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_pointer_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_scalar.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_int_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_opaque_fixed_array.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_opaque_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_string_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_fixed_array.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_null.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_pointer_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_scalar.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_struct_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_test_struct_all_types.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_case.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_default.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_fixed_array.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_no_default_case.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_null.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_pointer_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_scalar.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_empty.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_variable_array_set.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_case.bin
>  create mode 100644 scripts/rpcgen/tests/test_demo_union_void_default_default.bin
>  create mode 100644 scripts/rpcgen/tests/test_generator.py
>  create mode 100644 scripts/rpcgen/tests/test_lexer.py
>  create mode 100644 scripts/rpcgen/tests/test_parser.py
>  delete mode 100755 src/rpc/genprotocol.pl
> 
> -- 
> 2.39.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org