Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export QMP functions #1340

Merged
merged 3 commits into from
Oct 30, 2024
Merged

Export QMP functions #1340

merged 3 commits into from
Oct 30, 2024

Conversation

bensmrs
Copy link
Contributor

@bensmrs bensmrs commented Oct 28, 2024

Closes: #1226

@stgraber
Copy link
Member

@bensmrs I wonder, wouldn't it be cleaner if we just added a single command, run_command which would take one un-named argument (the command) and treat all kwargs as the arguments?

So we'd then have run_qmp that handles the raw JSON in/out QMP and run_command that lets you use a nicer syntax like:

run_command("qom-get", path="/machine/peripheral/dev-incus_root", property="serial")

I'm basically trying to avoid us having to duplicate the entire QMP command list in here.

@stgraber
Copy link
Member

Actually, I think I'd be willing to make a bit of a compromise because I do like the shorter syntax when you don't need to name all the arguments, but I still don't want to set a precedent which will get us to have to implement every single QMP command.

So I think we may want to go with:

Generic:

  • run_qmp(JSON)
  • run_command(CMD, key=value...)

Specific:

  • qom_{get,list,set}
  • object_{add,del}
  • device_{add,del}
  • blockdev_{add,del}
  • chardev_{add,change,remove}
  • netdev_{add,del}

Those are all modern QMP commands for core objects so not terribly likely to change or go away and the most likely to be called from the hook.

The other more niche commands can go through run_command where for commands with no arguments (quit/system_reset), this will remain pretty nice and short.

@bensmrs
Copy link
Contributor Author

bensmrs commented Oct 28, 2024

I expected the remark but I agree with your last message, I’ll look at it tomorrow

@stgraber
Copy link
Member

Thanks!

@bensmrs
Copy link
Contributor Author

bensmrs commented Oct 29, 2024

Just for a bit of context, I’m exploring the call stack to get the position in the Starlark code where there is a problem (in big scriptlets, it’s pretty useful). I haven’t found any other way, and Starlark doesn’t seem to offer much introspection into AST nodes, so that should do the trick…

@github-actions github-actions bot added the Documentation Documentation needs updating label Oct 30, 2024
@stgraber stgraber merged commit 03eb8c1 into lxc:main Oct 30, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating
Development

Successfully merging this pull request may close these issues.

Addition of helper functions in the QEMU scriptlet
2 participants