Re: [PATCH 00/16] Farewell rpcgen

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux