Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> sure what is the machine that we are emulating.

Does 1.6 here refer to the qemu version? If so then I think we need to
consciously decide that we are happy for Xen 4.5 to depend on qemu >=
1.6 and document it in README.

qemu 1.6 was release in August 2013, which is ~10 months ago, it'll be
approximately a year old when we release. Are we happy with this from a
distro PoV?

I think it is likely that distros which have an older qemu (and users
of such distros) will be using the qemu which comes with Xen (and is
naturally new enough) rather than expecting to use the system qemu.

At some point we really ought to grow an option to let the user choose
the machine...

> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> xen-platform device if requested. Choose slot 2 for the xen-platform
> device for compatibility with current installations. In case of Intel
> graphic passthrough, slot 2 might be needed by the grafic card. However

"graphics"

> now that we can specify the slot explicitly, it is easy to change the
> position of the xen-platform device on the PCI bus if graphic
> passthrough is enabled.
> 
> Move the machine options earlier, before any other emulated devices
> options. Otherwise the selected PCI slot for the xen-platform device is
> not available in QEMU.
> 
> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> differently from xenfv, the other QEMU machines do not have that option
> off by default.
> 
> This patch does not change the emulated environment in the guest.
> 
> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2

This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
via the 2 replies). Wrong link perhaps?

A Message-Id is generally a better identifier since I can feed it to the
archive of my choice and they aren't subject to e.g. future accidentally
renumberings etc.

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..fef684f 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -476,6 +476,29 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>          flexarray_vappend(dm_args, "-k", keymap, NULL);
>      }
>  
> +    flexarray_append(dm_args, "-machine");
> +    switch (b_info->type) {
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        flexarray_append(dm_args, "xenpv");
> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_pv[i]);
> +        break;
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> +        flexarray_append(dm_args, "-global");
> +        flexarray_append(dm_args, 
> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");

FWIW you can use flexarray_append_pair which more naturally gathers an
option and its argument together (improving readability).

(personally I think the -machine should be pulled into both cases using
this method, but that's not a prereq for accepting the patch IMHO)

Other than that the patch itself seems sound (although I dislike code
motion mixed with real changes, I suppose this one is small enough that
I can cope).

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel