From: Anthony PERARD <anthony.perard@citrix.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: <xen-devel@lists.xenproject.org>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH v2 01/21] libxl: don't ignore the return value from xc_cpuid_apply_policy
Date: Wed, 28 Apr 2021 16:13:08 +0100 [thread overview]
Message-ID: <YIl7hMpnN5lxXJ+m@perard> (raw)
In-Reply-To: <20210413140140.73690-2-roger.pau@citrix.com>
On Tue, Apr 13, 2021 at 04:01:19PM +0200, Roger Pau Monne wrote:
> Also change libxl__cpuid_legacy to propagate the error from
> xc_cpuid_apply_policy into callers.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> Changes since 1:
> - Return ERROR_FAIL on error.
> ---
> tools/libs/light/libxl_cpuid.c | 15 +++++++++++----
> tools/libs/light/libxl_create.c | 5 +++--
> tools/libs/light/libxl_dom.c | 2 +-
> tools/libs/light/libxl_internal.h | 4 ++--
> tools/libs/light/libxl_nocpuid.c | 5 +++--
> 5 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
> index 289c59c7420..539fc4869e6 100644
> --- a/tools/libs/light/libxl_cpuid.c
> +++ b/tools/libs/light/libxl_cpuid.c
> @@ -419,11 +419,13 @@ int libxl_cpuid_parse_config_xend(libxl_cpuid_policy_list *cpuid,
> return 0;
> }
>
> -void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, bool restore,
> - libxl_domain_build_info *info)
> +int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, bool restore,
> + libxl_domain_build_info *info)
> {
> + GC_INIT(ctx);
> bool pae = true;
> bool itsc;
> + int rc;
>
> /*
> * Gross hack. Using libxl_defbool_val() here causes libvirt to crash in
> @@ -462,8 +464,13 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, bool restore,
> itsc = (libxl_defbool_val(info->disable_migrate) ||
> info->tsc_mode == LIBXL_TSC_MODE_ALWAYS_EMULATE);
>
> - xc_cpuid_apply_policy(ctx->xch, domid, restore, NULL, 0,
> - pae, itsc, nested_virt, info->cpuid);
> + rc = xc_cpuid_apply_policy(ctx->xch, domid, restore, NULL, 0,
> + pae, itsc, nested_virt, info->cpuid);
> + if (rc)
> + LOGE(ERROR, "Failed to apply CPUID policy");
Is logging `errno` is going to be accurate enough ? The
xc_cpuid_apply_policy() seems to only set `rc` and never change `errno`
directly. It would often return "-errno" or an error code. There's one
instance where we have "rc = -EOPNOTSUPP" but `errno` doesn't seems to
be change to the same value (when checking "featureset).
So maybe having "LOGEV(ERROR, -rc, ...)" would be better?
And it should be `r` instead of `rc`. The latter is for libxl error
code, the former for system call/libxc.
> +
> + GC_FREE;
> + return rc ? ERROR_FAIL : 0;
The rest looks good.
Thanks,
--
Anthony PERARD
next prev parent reply other threads:[~2021-04-28 15:13 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-13 14:01 [PATCH v2 00/21] libs/guest: new CPUID/MSR interface Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 01/21] libxl: don't ignore the return value from xc_cpuid_apply_policy Roger Pau Monne
2021-04-28 15:13 ` Anthony PERARD [this message]
2021-04-13 14:01 ` [PATCH v2 02/21] libs/guest: rename xc_get_cpu_policy_size to xc_cpu_policy_get_size Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 03/21] libs/guest: introduce xc_cpu_policy_t Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 04/21] libs/guest: introduce helper to fetch a system cpu policy Roger Pau Monne
2021-04-14 13:28 ` Jan Beulich
2021-04-13 14:01 ` [PATCH v2 05/21] libs/guest: introduce helper to fetch a domain " Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 06/21] libs/guest: introduce helper to serialize a " Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 07/21] tools: switch existing users of xc_get_{system,domain}_cpu_policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 08/21] libs/guest: introduce a helper to apply a cpu policy to a domain Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 09/21] libs/guest: allow fetching a specific CPUID leaf from a cpu policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 10/21] tests/cpu-policy: add sorted MSR test Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 11/21] libs/guest: allow fetching a specific MSR entry from a cpu policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 12/21] libs/guest: allow updating a cpu policy CPUID data Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 13/21] libs/guest: allow updating a cpu policy MSR data Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 14/21] libs/guest: introduce helper to check cpu policy compatibility Roger Pau Monne
2021-04-14 13:36 ` Jan Beulich
2021-04-22 8:22 ` Roger Pau Monné
2021-04-22 8:31 ` Jan Beulich
2021-04-13 14:01 ` [PATCH v2 15/21] libs/guest: obtain a compatible cpu policy from two input ones Roger Pau Monne
2021-04-14 13:49 ` Jan Beulich
2021-04-22 9:42 ` Roger Pau Monné
2021-04-22 9:58 ` Jan Beulich
2021-04-22 10:34 ` Roger Pau Monné
2021-04-22 10:48 ` Jan Beulich
2021-04-22 10:56 ` Roger Pau Monné
2021-04-22 11:05 ` Jan Beulich
2021-04-22 11:37 ` Roger Pau Monné
2021-04-22 11:42 ` Jan Beulich
2021-04-22 12:07 ` Roger Pau Monné
2021-04-22 12:08 ` Jan Beulich
2021-04-21 10:22 ` Wei Liu
2021-04-21 11:26 ` Roger Pau Monné
2021-04-21 16:42 ` Wei Liu
2021-04-13 14:01 ` [PATCH v2 16/21] libs/guest: make a cpu policy compatible with older Xen versions Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 17/21] libs/guest: introduce helper set cpu topology in cpu policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 18/21] libs/guest: rework xc_cpuid_xend_policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 19/21] libs/guest: apply a featureset into a cpu policy Roger Pau Monne
2021-04-13 14:01 ` [PATCH v2 20/21] libs/{light,guest}: implement xc_cpuid_apply_policy in libxl Roger Pau Monne
2021-04-28 16:19 ` Anthony PERARD
2021-04-13 14:01 ` [PATCH v2 21/21] libs/guest: (re)move xc_cpu_policy_apply_cpuid Roger Pau Monne
2021-04-28 16:45 ` Anthony PERARD
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=YIl7hMpnN5lxXJ+m@perard \
--to=anthony.perard@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.com \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).