Re: [Xen-devel] [XEN PATCH for-4.13 v3 10/10] libxl/xl: Overhaul passthrough setting logic

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Oct 11, 2019 at 05:55:49PM +0100, Ian Jackson wrote:
> LIBXL_PASSTHROUGH_UNKNOWN (aka "ENABLED" in an earlier uncommitted

I guess that's now LIBXL_PASSTHROUGH_UNSPECIFIED.

> version of this code) is doing double duty.  We actually need all of
> the following to be specificable:
>   * default ("unspecified"): enable PT iff we have devices to
                                         ^if

>     pass through specified in the initial config file.
>   * "enabled" (and fail if the platform doesn't support it).
>   * "disabled" (and reject future PT hotplug).
>   * "share_pt"/"sync_pt": enable PT and set a specific PT mode.
> 
> Defaulting and error checking should be done in libxl.  So, we make
> several changes here.
> 
> We introduce "enabled".  (And we document "unspecified".)
> 
> We move all of the error checking and defaulting code from xl into
> libxl.  Now, libxl__domain_config_setdefault has all of the necessary
> information to get this right.  So we can do it all there.  Choosing
> the specific mode is arch-specific.
> 
> We can also arrange to have only one place each which calculates
> (i) whether passthrough needs to be enabled because pt devices were
> specified (ii) whether pt_share can be used (for each arch).
> 
> xl now only has to parse the enum in the same way as it parses all
> other enums.
> 
> This change fixes a regression from earlier 4.13-pre: until recent
> changes, passthrough was only enabled by default if passthrough
> devices were specified.  We restore this behaviour.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> 
> ---
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index c0f88a7eaa..226b712cbd 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -631,6 +631,47 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc 
> *gc,
>      libxl_defbool_setdefault(&b_info->acpi, true);
>  }
>  
> +int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc,
> +                                            uint32_t domid,
> +                                            libxl_domain_config *d_config,
> +                                            const libxl_physinfo *physinfo)
> +{

[...]

> +}
> +                                        

There are 40 trailing white space here, any reason? :-).

Beside a few typos, the patch looks fine to me:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel