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

["Zhang, Chen" <chen.zhang@xxxxxxxxx>]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@xxxxxxxxxx]
> Sent: Friday, July 26, 2019 9:48 PM
> To: Zhang, Chen <chen.zhang@xxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; Zhang Chen <zhangckid@xxxxxxxxx>
> Subject: Re: [PATCH V2] tools/libxl: Add iothread support for COLO
> 
> 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.
> 

Make sense. I will add it in next version.

> > 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'?

Oh, it looks I misunderstood your means.
Do you think we just need hard code the iothread name here?
For example the "iothread-1"?

Thanks
Zhang Chen

> 
> Thanks,
> 
> --
> Anthony PERARD

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