internal/qmp-gen: generate implementations for events. #135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Seems to me that we should probably make @benlemasurier , thoughts? |
|
@mdlayher yeah, that makes the most sense to me |
|
Could you describe what you mean by making qmp.Event an interface with more words? I'm not seeing how this would make things prettier/easier, I think I'm missing something obvious. |
Running the generation pass pulled in some changes from upstream. Also a trivial change to qemu's strings to appease the presubmit.
|
Are you still interested in this? I'd like some guidance on how you'd like me to change qmp.Event, after which I can catch up this PR to intermediate changes. (Also, I have to implement a bit more stuff because the upstream spec introduced unions in event types) |
|
Absolutely, I totally forgot to get back to this, sorry! I'm at a conference this week but @benlemasurier ought to be able to provide some guidance. |
suryatmodulus
pushed a commit
to suryatmodulus/go-qemu
that referenced
this pull request
Oct 1, 2022
This involves some additional changes to overcome breaking changes from go-libvirt. These are the new changes in this revision of go-libvirt since the last release go-qemu has been using: * remove invitation to empty IRC channel * rpc_test: use Libvirt.addStream to add streams * MockLibvirt: deep-copy response before adding ID * monitor socket conn state in initLibvirtComms * libvirttest: use atomic to read "serial" * integration: wait for Disconnected before assert * add CODEOWNERS * Merge pull request digitalocean#152 from clarkwalcott/readme_update_rpc_link * Docs: Update README with working link * expose Libvirt.Disconnected method * Add NetworkUpdateCompat * update libvirt configuration step for new releases * search generated include directory for header files * ci updates (digitalocean#142) * integration tests (digitalocean#141) * take dialer as input instead of already existing connection (digitalocean#140) * support authentication via polkit (digitalocean#138) * fix some stream problems (digitalocean#137) * Merge pull request digitalocean#136 from digitalocean/jenni/fix_potential_segfault * write lock only needed for actual write * rm unneeded nil'ing of reader and writer * socket Disconnect close can race with nil of conn * Merge pull request digitalocean#135 from digitalocean/jenni/update_cgo * run go get golang.org/x/tools/go/internal/cgo@v0.1.1 * Merge pull request digitalocean#134 from digitalocean/jenni/add_socket_type * change socket to pointer ref * Merge branch 'master' of github.com:digitalocean/go-libvirt into jenni/add_socket_type * Merge pull request digitalocean#133 from digitalocean/jenni/fix_event_unsubscribe_race * reorder stream shutdown/delete * fix race in disconnect of event stream cleanup * add socket type to handle socket connection management * Merge pull request digitalocean#132 from digitalocean/jenni/fix_client_deadlock * fix typo and func conciseness * move listen and cleanup into a func * add timeout on waiting for disconnected * move temporary read error check to a function * Libvirt chan socketCleanupDone -> disconnected * fix accidental go fmt of generated files * add connect/disconnect to unit tests * add tests for callback cleanup on connection disconnect * fix rpc lookup test * fix deadlocked clients on lack of libvirt response * add more robust connection error checking * Merge pull request digitalocean#131 from digitalocean/jenni/fix_invalid_test_data * fix incorrect test data * Merge pull request digitalocean#130 from digitalocean/jenni/stream_shutdown * fix missing stream shutdown in lifecycle subscribe * Merge pull request digitalocean#129 from digitalocean/geoff/vet-cleanup * remove travis config file * Clean up some linter errors * Merge pull request digitalocean#125 from dmacvicar/dmacvicar_puberror * Run generators against libvirt 7.2.0 * Unexport From* (ErrorDomain) * golint * go fmt * libvirt.Error: expose only Code and Message * Run generators against libvirt 7.1.0 * go fmt * Make libvirtError public (libvirt.Error) * github actions (digitalocean#128) * internal/event/stream: fixes unsafe queue access (digitalocean#127) * Merge pull request digitalocean#124 from sam-github/check-wrapped-errors * Check wrapped errors for IsNotFound() * Fix two race conditions related to Disconnect (digitalocean#120) * Merge pull request digitalocean#119 from weizhouapache/readme-add-tls-port * README: Add missing tls port in example * README: Add example connect to libvirt via TLS over TCP (digitalocean#118) * Add overload for Connect() to allow other libvirt drivers (digitalocean#117)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So, for events, the current API in package qmp isn't great - it deserializes the event into map[string]interface{}, which means I have to re-serialize it so that I can json.Unmarshal out into the correct type.
If you have ideas on how to make this nicer, I'd love to hear it. The only things I can come up with involves stuffing some autogen'd types into the qmp package, and decoding the events to structs directly.