[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v2][PATCH] libxl: add one machine property to support
From: |
Wei Liu |
Subject: |
Re: [Qemu-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough |
Date: |
Tue, 3 Feb 2015 10:19:21 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, Feb 03, 2015 at 09:01:53AM +0800, Chen, Tiejun wrote:
>
> On 2015/2/2 20:19, Wei Liu wrote:
> >On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote:
> >>When we're working to support IGD GFX passthrough with qemu
> >>upstream, instead of "-gfx_passthru" we'd like to make that
> >>a machine option, "-machine xxx,-igd-passthru=on". This need
> >>to bring a change on tool side.
> >>
> >>Signed-off-by: Tiejun Chen <address@hidden>
> >>---
> >>v2:
> >>
> >>* Based on some discussions with Wei we'd like to keep both old
> >> option, -gfx_passthru, and new machine property option,
> >> "-machine xxx,-igd-passthru=on" at the same time but deprecate
> >> the old one. Then finally we remove the old one at that point
> >> that to give downstream (in this case, Xen) time to cope with the
> >> change.
> >>
> >
> >My suggestion has one premise -- if upstream QEMU has already released
> >that -gfx_passthru option. If there is no "old one" (in upstream QEMU)
> >at all, then there is nothing to keep and deprecate.
>
> Understood.
>
> >
> >> tools/libxl/libxl_dm.c | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >>diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >>index c2b0487..8405f0b 100644
> >>--- a/tools/libxl/libxl_dm.c
> >>+++ b/tools/libxl/libxl_dm.c
> >>@@ -701,6 +701,11 @@ static char **
> >>libxl__build_device_model_args_new(libxl__gc *gc,
> >
> >Note this function is upstream QEMU specfic.
>
> Yeah.
>
> >
> >> flexarray_append(dm_args, "-net");
> >> flexarray_append(dm_args, "none");
> >> }
> >>+ /*
> >>+ * Although we already introduce 'igd-passthru', but we'd like
> >>+ * to remove this until we give downstream time to cope with
> >>+ * the change.
> >>+ */
> >> if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
> >> flexarray_append(dm_args, "-gfx_passthru");
> >> }
> >
> >The comment contradicts what I know (or what I think I know). In our
> >last email exchange you said there was no "-gfx_passthru" in any version
> >of upstream QEMU.
> >
> >So, shouldn't you just remove this `if' statement?
>
> Right. So what about this?
>
> libxl: add one machine property to support IGD GFX passthrough
>
> When we're working to support IGD GFX passthrough with qemu
> upstream, we'd like to introduce a machine option,
> "-machine xxx,igd-passthru=on", to enable/disable that feature.
> And we also remove that old option, "-gfx_passthru", just from
> the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually
> no any qemu stream version really need or use that.
>
> Signed-off-by: Tiejun Chen <address@hidden>
>
Yes. I think a patch like this reflects the reality.
It would be nice, as Ian J suggested, to state which version of QEMU
upstream introduces that new option in commit message.
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index c2b0487..b888f19 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -701,9 +701,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");
> @@ -748,6 +745,11 @@ 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)) {
> + machinearg = libxl__sprintf(gc, "%s,igd-passthru=on",
> machinearg);
> + }
> +
Please use GCSPRINTF macro.
Wei.
> flexarray_append(dm_args, machinearg);
> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> flexarray_append(dm_args, b_info->extra_hvm[i]);
>
> Thanks
> Tiejun
>
> >
> >Wei.
> >
> >>@@ -748,6 +753,11 @@ 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)) {
> >>+ machinearg = libxl__sprintf(gc, "%s,igd-passthru=on",
> >>machinearg);
> >>+ }
> >>+
> >> flexarray_append(dm_args, machinearg);
> >> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL;
> >> i++)
> >> flexarray_append(dm_args, b_info->extra_hvm[i]);
> >>--
> >>1.9.1
> >
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, (continued)
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/09
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Ian Campbell, 2015/02/09
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/10
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/12
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Ian Campbell, 2015/02/18
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/26
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Ian Campbell, 2015/02/26
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/27
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Ian Campbell, 2015/02/27
Re: [Qemu-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough, Chen, Tiejun, 2015/02/02