Re: [Xen-devel] [PATCH V2] tools/libxl: Add iothread support for COLO

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Jul 26, 2019 at 02:43:00PM +0800, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@xxxxxxxxx>
> 
> Xen COLO and KVM COLO shared lots of code in Qemu.
> KVM COLO has added the iothread support, so we add it on Xen.

It would be useful to expand the comment of the commit and explain why the
change is required. I would add the following:

    The colo-compare object in QEMU now requires an `iothread' property
    since QEMU 2.11.

> Detail:
> https://wiki.qemu.org/Features/COLO
> 
> Signed-off-by: Zhang Chen <chen.zhang@xxxxxxxxx>
> ---

> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index b61399ce36..eda958eb4b 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -727,6 +727,7 @@ libxl_device_nic = Struct("device_nic", [
>      ("colo_filter_redirector1_queue", string),
>      ("colo_filter_redirector1_indev", string),
>      ("colo_filter_redirector1_outdev", string),
> +    ("colo_iothread", string),
>      ("colo_compare_pri_in", string),
>      ("colo_compare_sec_in", string),
>      ("colo_compare_out", string),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index e105bda2bb..0b8189f375 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -521,6 +521,8 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config 
> **config, char *token)
>          replace_string(&nic->colo_filter_redirector1_indev, oparg);
>      } else if (MATCH_OPTION("colo_filter_redirector1_outdev", token, oparg)) 
> {
>          replace_string(&nic->colo_filter_redirector1_outdev, oparg);
> +    } else if (MATCH_OPTION("colo_iothread", token, oparg)) {
> +        replace_string(&nic->colo_iothread, oparg);
>      } else if (MATCH_OPTION("colo_compare_pri_in", token, oparg)) {
>          replace_string(&nic->colo_compare_pri_in, oparg);
>      } else if (MATCH_OPTION("colo_compare_sec_in", token, oparg)) {

What I had in mind while reviewing the v1 of the patch was to remove
both `colo_iothread' and `colo_compare_iothread' from the libxl API and
xl config option. I don't think there are useful. Why did you keep
`colo_iothread'?

Thanks,

-- 
Anthony PERARD

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