Re: [Xen-devel] [PATCH v3 5/5] libxl: add options to enable/disable emulated devices

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2016-01-20 at 12:57 +0100, Roger Pau Monne wrote:
> Allow enabling or disabling emulated devices from the libxl domain
> configuration file. For HVM guests with a device model all the emulated
> devices are enabled. For HVM guests without a device model no devices are
> enabled by default, although they can be enabled using the options
> provided.
> The arbiter of whether a combination is posible or not is always Xen,
> libxl
> doesn't do any kind of check.
> 
> This set of options is also propagated inside of the libxl migration record
> as part of the contents of the libxl_domain_build_info struct.

... and this is the real motivation for this change, not actually allowing
users to control all this AIUI.

Did you check that the fields updated using libxl_defbool_setdefault are
actually updated in the JSON copy and therefore propagated to the other
side of a migration as specific values and not as "pick a default"? I think
we don't want these changing on migration. I think/hope all this was
automatically handled by the work Wei did in the last release cycle.

> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Âdocs/man/xl.cfg.pod.5ÂÂÂÂÂÂÂ| 39 +++++++++++++++++++++++++++++++++++++++
> Âtools/libxl/libxl.hÂÂÂÂÂÂÂÂÂ| 11 +++++++++++
> Âtools/libxl/libxl_create.cÂÂ| 21 ++++++++++++++++++++-
> Âtools/libxl/libxl_types.idl |ÂÂ6 ++++++
> Âtools/libxl/libxl_x86.cÂÂÂÂÂ| 33 ++++++++++++++++++++++++++++-----
> Âtools/libxl/xl_cmdimpl.cÂÂÂÂ|ÂÂ7 +++++++
> Â6 files changed, 111 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 8899f75..46d4529 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1762,6 +1762,45 @@ See F<docs/misc/pci-device-reservations.txt> for
> more information.
> Â
> Â=back
> Â
> +=head3 HVM without a device model options
> +
> +This options can be used to change the set of emulated devices provided

"These..."

> +to guests without a device model. Note that Xen might not support all
> +possible combinations. By default HVM guests without a device model
> +don't have any of them enabled.

... and for those with a device model? The title and text suggest these
options are invalid/ignored in that case, but the code does actually honour
what the user specified in this case.

> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 92c95e5..8a21cda 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -519,6 +519,12 @@ libxl_domain_build_info = Struct("domain_build_info",[
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("serial_list",ÂÂÂÂÂÂ
> libxl_string_list),
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("rdm", libxl_rdm_reserve),
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("rdm_mem_boundary_memkb", MemKB),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("lapic",ÂÂÂÂÂÂÂÂÂÂÂÂlibxl_defbool),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("ioapic",ÂÂÂÂÂÂÂÂÂÂÂlibxl_defbool),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("rtc",ÂÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_defbool),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("power_management", libxl_defbool),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("pic",ÂÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_defbool),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("pit",ÂÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_defbool),

I wonder if these should go in a sub-struct. Although you might reasonably
argue that this is already such a dumping ground it doesn't matter...

> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ])),
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("pv", Struct(None, [("kernel", string),
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("slack_memkb", MemKB),
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 46cfafb..92f25fd 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -7,15 +7,38 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_domain_config *d_config,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂxc_domain_configuration_t
> *xc_config)
> Â{
> +ÂÂÂÂstruct libxl_domain_build_info *info = &d_config->b_info;
> Â
> -ÂÂÂÂif (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> -ÂÂÂÂÂÂÂÂd_config->b_info.device_model_version !=
> -ÂÂÂÂÂÂÂÂLIBXL_DEVICE_MODEL_VERSION_NONE) {
> -ÂÂÂÂÂÂÂÂ/* HVM domains with a device model. */

So, I'm not 100% clear on why this check and the corresponding logic to set
xc_config->emulation_flagsÂis not also sufficient for after migration.
Could you explain (and likely eventually add the rationale to the commit
message).

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