[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind
From: |
Ian Campbell |
Subject: |
Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind |
Date: |
Wed, 18 Mar 2015 10:25:10 +0000 |
On Wed, 2015-03-18 at 15:32 +0800, Chen, Tiejun wrote:
> > I think the _libxl_ message needs to be just "Unable to detect graphics
> > passthru kind". i.e. it can't/shouldn't reference anything to do with xl
> > config options etc (which would make no sense if libvirt was being
> > used).
> >
> > That's not very user friendly though, so you may want to consider adding
> > a new specific error code for this case and returning it here, such that
> > xl or libvirt can then give a more comprehensible error message.
>
> But,
> args = libxl__build_device_model_args(gc, "stubdom-dm", guest_domid,
> guest_config, d_state, NULL);
> |
> + return libxl__build_device_model_args_new(gc, dm,guest_domid,
> guest_config, state, dm_state_fd);
> |
> + Currently we always return "NULL" inside.
>
> if (!args) {
> ret = ERROR_FAIL;
> goto out;
> }
>
> So I'm not very sure how to pass this new specific error code to xl/libvirt.
Bah, yes. I don't think it is reasonable to ask you to rework the error
handling here completely for this. Lets leave this as a potential future
enhancement.
> Thanks but I guess I'd better to paste this whole patch to avoid I'm
> still missing something :)
>
> ---
> tools/libxl/libxl.h | 6 ++++++
> tools/libxl/libxl_dm.c | 25 ++++++++++++++++++++++---
> tools/libxl/libxl_internal.h | 5 +++++
> tools/libxl/libxl_types.idl | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 14 ++++++++++++--
> 5 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 6bbc52d..62b3ae5 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -720,6 +720,12 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst,
> libxl_mac *src);
> #define LIBXL_HAVE_PSR_MBM 1
> #endif
>
> +/*
> + * libxl_domain_build_info has the u.hvm.gfx_passthru_kind field and
> + * the libxl_gfx_passthru_kind enumeration is defined.
> +*/
> +#define LIBXL_HAVE_GFX_PASSTHRU_KIND
> +
> typedef char **libxl_string_list;
> void libxl_string_list_dispose(libxl_string_list *sl);
> int libxl_string_list_length(const libxl_string_list *sl);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..045d48a 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -710,9 +710,6 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> flexarray_append(dm_args, "-net");
> flexarray_append(dm_args, "none");
> }
> - if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
> - flexarray_append(dm_args, "-gfx_passthru");
> - }
> } else {
> if (!sdl && !vnc) {
> flexarray_append(dm_args, "-nographic");
> @@ -757,6 +754,28 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> machinearg, max_ram_below_4g);
> }
> }
> +
> + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
> + switch (b_info->u.hvm.gfx_passthru_kind) {
> + case _info->u.hvm.gfx_passthru_kind:
> + if (libxl__is_igd_vga_passthru(gc, guest_config)) {
> + machinearg = GCSPRINTF("%s,igd-passthru=on",
> machinearg);
> + } else {
> + LOG(ERROR, "Unable to detect graphics passthru kind,"
> + " please set gfx_passthru_kind. See xl.cfg(5)
> for more"
> + " information.\n");
Please change this to "Unable to detect graphics passthru kind" to avoid
talking xl-isms in libxl.
> + return NULL;
> + }
> + break;
> + case LIBXL_GFX_PASSTHRU_KIND_IGD:
> + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
> + break;
This duplicates the code from above. I think this would be best done as:
static int libxl__detect_gfx_passthru_kind(libxl__gc *gc, guest_config)
{
if (b_info->u.hvm.gfx_passthru_kind != LIBXL_GFX_PASSTHRU_KIND_DEFAULT)
return 0;
if (libxl__is_igd_vga_passthru(gc, guest_config)) {
b_info->u.hvm.gfx_passthru_kind = LIBXL_GFX_PASSTHRU_KIND_IGD;
return 0;
}
LOG(ERROR, "Unable to detect graphics passthru kind");
return 1;
}
Then for the code in libxl__build_device_model_args_new:
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
if (!libxl__detect_gfx_passthru_kind(gc, guest_config))
return NULL
switch (b_info->u.hvm.gfx_passthru_kind) {
case LIBXL_GFX_PASSTHRU_KIND_IGD:
machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
break;
default:
LOG(ERROR, "unknown gfx_passthru_kind\n");
return NULL;
}
}
That is, a helper to try and autodetect kind if it is default and then a
single switch entry for each kind.
> + default:
> + LOG(WARN, "gfx_passthru_kind is invalid so ignored.\n");
Please return an error here, as I've shown above.
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index c97c62d..8912421 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3632,6 +3632,11 @@ static inline void
> libxl__update_config_vtpm(libxl__gc *gc,
> */
> void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
> const libxl_bitmap *sptr);
> +
> +#ifdef LIBXL_HAVE_GFX_PASSTHRU_KIND
No need for this ifdef.
Ian.
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, (continued)
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/11
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/11
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/12
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/12
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/13
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/15
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/16
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/17
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/17
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/18
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind,
Ian Campbell <=
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/18
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/19
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/19
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/20
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/20
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Ian Campbell, 2015/03/20
- Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind, Chen, Tiejun, 2015/03/20
[Qemu-devel] [v2][PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru, Tiejun Chen, 2015/03/10