From: Igor Mammedov <imammedo@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH 12/15] plugin: propagate errors
Date: Mon, 7 Dec 2020 17:53:12 +0100 [thread overview]
Message-ID: <20201207175312.57617740@redhat.com> (raw)
In-Reply-To: <20201202081854.4126071-13-pbonzini@redhat.com>
On Wed, 2 Dec 2020 03:18:51 -0500
Paolo Bonzini <pbonzini@redhat.com> wrote:
> qemu_finish_machine_init currently can only exit QEMU if it fails.
> Prepare for giving it proper error propagation, and possibly for
> adding a plugin_add monitor command that calls an accelerator
> method.
>
> While at it, make all errors from plugin_load look the same.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/qemu/plugin.h | 4 ++--
> linux-user/main.c | 4 +---
> plugins/loader.c | 34 +++++++++++++++++-----------------
> softmmu/vl.c | 4 +---
> 4 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index ab790ad105..841deed79c 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -45,7 +45,7 @@ static inline void qemu_plugin_add_opts(void)
> }
>
> void qemu_plugin_opt_parse(const char *optarg, QemuPluginList *head);
> -int qemu_plugin_load_list(QemuPluginList *head);
> +int qemu_plugin_load_list(QemuPluginList *head, Error **errp);
>
> union qemu_plugin_cb_sig {
> qemu_plugin_simple_cb_t simple;
> @@ -199,7 +199,7 @@ static inline void qemu_plugin_opt_parse(const char *optarg,
> exit(1);
> }
>
> -static inline int qemu_plugin_load_list(QemuPluginList *head)
> +static inline int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
> {
> return 0;
> }
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 24d1eb73ad..750a01118f 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -671,9 +671,7 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
> trace_init_file();
> - if (qemu_plugin_load_list(&plugins)) {
> - exit(1);
> - }
> + qemu_plugin_load_list(&plugins, &error_fatal);
>
> /* Zero out regs */
> memset(regs, 0, sizeof(struct target_pt_regs));
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 8ac5dbc20f..5cb9794fda 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -150,7 +150,7 @@ static uint64_t xorshift64star(uint64_t x)
> return x * UINT64_C(2685821657736338717);
> }
>
> -static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info)
> +static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, Error **errp)
> {
> qemu_plugin_install_func_t install;
> struct qemu_plugin_ctx *ctx;
> @@ -163,37 +163,37 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info)
>
> ctx->handle = g_module_open(desc->path, G_MODULE_BIND_LOCAL);
> if (ctx->handle == NULL) {
> - error_report("%s: %s", __func__, g_module_error());
> + error_setg(errp, "Could not load plugin %s: %s", desc->path, g_module_error());
> goto err_dlopen;
> }
>
> if (!g_module_symbol(ctx->handle, "qemu_plugin_install", &sym)) {
> - error_report("%s: %s", __func__, g_module_error());
> + error_setg(errp, "Could not load plugin %s: %s", desc->path, g_module_error());
> goto err_symbol;
> }
> install = (qemu_plugin_install_func_t) sym;
> /* symbol was found; it could be NULL though */
> if (install == NULL) {
> - error_report("%s: %s: qemu_plugin_install is NULL",
> - __func__, desc->path);
> + error_setg(errp, "Could not load plugin %s: qemu_plugin_install is NULL",
> + desc->path);
> goto err_symbol;
> }
>
> if (!g_module_symbol(ctx->handle, "qemu_plugin_version", &sym)) {
> - error_report("TCG plugin %s does not declare API version %s",
> - desc->path, g_module_error());
> + error_setg(errp, "Could not load plugin %s: plugin does not declare API version %s",
> + desc->path, g_module_error());
> goto err_symbol;
> } else {
> int version = *(int *)sym;
> if (version < QEMU_PLUGIN_MIN_VERSION) {
> - error_report("TCG plugin %s requires API version %d, but "
> - "this QEMU supports only a minimum version of %d",
> - desc->path, version, QEMU_PLUGIN_MIN_VERSION);
> + error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
> + "this QEMU supports only a minimum version of %d",
> + desc->path, version, QEMU_PLUGIN_MIN_VERSION);
> goto err_symbol;
> } else if (version > QEMU_PLUGIN_VERSION) {
> - error_report("TCG plugin %s requires API version %d, but "
> - "this QEMU supports only up to version %d",
> - desc->path, version, QEMU_PLUGIN_VERSION);
> + error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
> + "this QEMU supports only up to version %d",
> + desc->path, version, QEMU_PLUGIN_VERSION);
> goto err_symbol;
> }
> }
> @@ -220,8 +220,8 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info)
> rc = install(ctx->id, info, desc->argc, desc->argv);
> ctx->installing = false;
> if (rc) {
> - error_report("%s: qemu_plugin_install returned error code %d",
> - __func__, rc);
> + error_setg(errp, "Could not load plugin %s: qemu_plugin_install returned error code %d",
> + desc->path, rc);
> /*
> * we cannot rely on the plugin doing its own cleanup, so
> * call a full uninstall if the plugin did not yet call it.
> @@ -263,7 +263,7 @@ static void plugin_desc_free(struct qemu_plugin_desc *desc)
> * Note: the descriptor of each successfully installed plugin is removed
> * from the list given by @head.
> */
> -int qemu_plugin_load_list(QemuPluginList *head)
> +int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
> {
> struct qemu_plugin_desc *desc, *next;
> g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
> @@ -283,7 +283,7 @@ int qemu_plugin_load_list(QemuPluginList *head)
> QTAILQ_FOREACH_SAFE(desc, head, entry, next) {
> int err;
>
> - err = plugin_load(desc, info);
> + err = plugin_load(desc, info, errp);
> if (err) {
> return err;
> }
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index e5f3c42049..0f63d80472 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2417,9 +2417,7 @@ static void qemu_init_board(void)
> }
>
> /* process plugin before CPUs are created, but once -smp has been parsed */
> - if (qemu_plugin_load_list(&plugin_list)) {
> - exit(1);
> - }
> + qemu_plugin_load_list(&plugin_list, &error_fatal);
>
> /* From here on we enter MACHINE_PHASE_INITIALIZED. */
> machine_run_board_init(current_machine);
next prev parent reply other threads:[~2020-12-07 16:55 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-02 8:18 [PATCH 00/15] Finish cleaning up qemu_init Paolo Bonzini
2020-12-02 8:18 ` [PATCH 01/15] remove preconfig state Paolo Bonzini
2020-12-07 13:57 ` Igor Mammedov
2020-12-07 14:11 ` Paolo Bonzini
2020-12-07 15:14 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 02/15] vl: remove separate preconfig main_loop Paolo Bonzini
2020-12-07 14:02 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 03/15] vl: allow -incoming defer with -preconfig Paolo Bonzini
2020-12-02 8:18 ` [PATCH 04/15] vl: extract softmmu/runstate.c Paolo Bonzini
2020-12-02 8:18 ` [PATCH 05/15] vl: extract softmmu/globals.c Paolo Bonzini
2020-12-02 8:18 ` [PATCH 06/15] vl: move all generic initialization out of vl.c Paolo Bonzini
2020-12-07 14:19 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 07/15] chardev: do not use machine_init_done Paolo Bonzini
2020-12-07 15:15 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 08/15] machine: introduce MachineInitPhase Paolo Bonzini
2020-12-07 15:28 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 09/15] machine: record whether nvdimm= was set Paolo Bonzini
2020-12-07 15:40 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 10/15] vl: make qemu_get_machine_opts static Paolo Bonzini
2020-12-07 16:07 ` Igor Mammedov
2020-12-07 16:38 ` Paolo Bonzini
2020-12-08 2:32 ` Daniel Henrique Barboza
2020-12-08 10:55 ` Igor Mammedov
2020-12-08 11:05 ` [PATCH] ppc/spapr: cleanup -machine pseries,nvdimm=X handling Igor Mammedov
2020-12-08 16:46 ` [PATCH v2] " Igor Mammedov
2020-12-08 17:24 ` Daniel Henrique Barboza
2020-12-08 18:35 ` Igor Mammedov
2020-12-08 2:16 ` [PATCH 10/15] vl: make qemu_get_machine_opts static Daniel Henrique Barboza
2020-12-08 8:13 ` Paolo Bonzini
2020-12-02 8:18 ` [PATCH 11/15] qtest: add a QOM object for qtest Paolo Bonzini
2020-12-07 16:24 ` Igor Mammedov
2020-12-07 16:43 ` Paolo Bonzini
2020-12-07 16:57 ` Igor Mammedov
2020-12-07 17:22 ` Paolo Bonzini
2020-12-08 11:11 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 12/15] plugin: propagate errors Paolo Bonzini
2020-12-02 11:33 ` Alex Bennée
2020-12-07 16:53 ` Igor Mammedov [this message]
2020-12-02 8:18 ` [PATCH 13/15] memory: allow creating MemoryRegions before accelerators Paolo Bonzini
2020-12-07 16:38 ` Igor Mammedov
2020-12-07 16:40 ` Paolo Bonzini
2020-12-07 17:06 ` Igor Mammedov
2020-12-02 8:18 ` [PATCH 14/15] null-machine: do not create a default memdev Paolo Bonzini
2020-12-07 16:43 ` Igor Mammedov
2020-12-11 23:24 ` Paolo Bonzini
2020-12-14 11:53 ` Igor Mammedov
2020-12-14 13:24 ` Paolo Bonzini
2020-12-02 8:18 ` [PATCH 15/15] monitor: allow quitting while in preconfig state Paolo Bonzini
2020-12-07 16:45 ` Igor Mammedov
2020-12-07 14:12 ` [PATCH 00/15] Finish cleaning up qemu_init no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201207175312.57617740@redhat.com \
--to=imammedo@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.