| Message ID | 20190610083336.18235-1-chen.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/libxl: Add iothread support for COLO | expand |
Hi All, Please give me some comments on this patch. This patch just add some missed parameter in qemu side. Thanks Zhang Chen > -----Original Message----- > From: Zhang, Chen > Sent: Monday, June 10, 2019 4:34 PM > To: Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; xen- > devel@lists.xenproject.org > Cc: Zhang Chen <zhangckid@gmail.com>; Zhang, Chen <chen.zhang@intel.com> > Subject: [PATCH] tools/libxl: Add iothread support for COLO > > From: Zhang Chen <chen.zhang@intel.com> > > Xen COLO and KVM COLO shared lots of code in Qemu. > KVM COLO has added the iothread support, so we add it on Xen. > > Detail: > https://wiki.qemu.org/Features/COLO > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > tools/libxl/libxl_dm.c | 14 +++++++++++--- > tools/libxl/libxl_types.idl | 2 ++ > tools/xl/xl_parse.c | 4 ++++ > 3 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index > f4fc96415d..6bb400efdf 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -1629,17 +1629,25 @@ static int > libxl__build_device_model_args_new(libxl__gc *gc, > nics[i].colo_filter_redirector1_queue, > nics[i].colo_filter_redirector1_outdev)); > } > + if (nics[i].colo_iothread) { > + flexarray_append(dm_args, "-object"); > + flexarray_append(dm_args, > + GCSPRINTF("iothread,id=%s", > + nics[i].colo_iothread)); > + } > if (nics[i].colo_compare_pri_in && > nics[i].colo_compare_sec_in && > nics[i].colo_compare_out && > - nics[i].colo_compare_notify_dev) { > + nics[i].colo_compare_notify_dev && > + nics[i].colo_compare_iothread) { > flexarray_append(dm_args, "-object"); > flexarray_append(dm_args, > - GCSPRINTF("colo- > compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", > + > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s, > + notify_dev=%s,iothread=%s", > nics[i].colo_compare_pri_in, > nics[i].colo_compare_sec_in, > nics[i].colo_compare_out, > - nics[i].colo_compare_notify_dev)); > + nics[i].colo_compare_notify_dev, > + nics[i].colo_compare_iothread)); > } > } > ioemu_nics++; > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index > b61399ce36..f0435a5177 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -727,10 +727,12 @@ 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), > ("colo_compare_notify_dev", string), > + ("colo_compare_iothread", string), > ("colo_sock_sec_redirector0_id", string), > ("colo_sock_sec_redirector0_ip", string), > ("colo_sock_sec_redirector0_port", string), diff --git a/tools/xl/xl_parse.c > b/tools/xl/xl_parse.c index e105bda2bb..cd16856910 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)) { @@ - > 529,6 +531,8 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config > **config, char *token) > replace_string(&nic->colo_compare_out, oparg); > } else if (MATCH_OPTION("colo_compare_notify_dev", token, oparg)) { > replace_string(&nic->colo_compare_notify_dev, oparg); > + } else if (MATCH_OPTION("colo_compare_iothread", token, oparg)) { > + replace_string(&nic->colo_compare_iothread, oparg); > } else if (MATCH_OPTION("colo_sock_sec_redirector0_id", token, oparg)) { > replace_string(&nic->colo_sock_sec_redirector0_id, oparg); > } else if (MATCH_OPTION("colo_sock_sec_redirector0_ip", token, oparg)) { > -- > 2.17.GIT
Ping~ Anyone have comments? Thanks Zhang Chen > -----Original Message----- > From: Zhang, Chen > Sent: Friday, June 21, 2019 2:00 PM > To: Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; xen- > devel@lists.xenproject.org > Cc: Zhang Chen <zhangckid@gmail.com> > Subject: RE: [PATCH] tools/libxl: Add iothread support for COLO > > Hi All, > > Please give me some comments on this patch. > This patch just add some missed parameter in qemu side. > > Thanks > Zhang Chen > > > -----Original Message----- > > From: Zhang, Chen > > Sent: Monday, June 10, 2019 4:34 PM > > To: Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; > > xen- devel@lists.xenproject.org > > Cc: Zhang Chen <zhangckid@gmail.com>; Zhang, Chen > > <chen.zhang@intel.com> > > Subject: [PATCH] tools/libxl: Add iothread support for COLO > > > > From: Zhang Chen <chen.zhang@intel.com> > > > > Xen COLO and KVM COLO shared lots of code in Qemu. > > KVM COLO has added the iothread support, so we add it on Xen. > > > > Detail: > > https://wiki.qemu.org/Features/COLO > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > --- > > tools/libxl/libxl_dm.c | 14 +++++++++++--- > > tools/libxl/libxl_types.idl | 2 ++ > > tools/xl/xl_parse.c | 4 ++++ > > 3 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index > > f4fc96415d..6bb400efdf 100644 > > --- a/tools/libxl/libxl_dm.c > > +++ b/tools/libxl/libxl_dm.c > > @@ -1629,17 +1629,25 @@ static int > > libxl__build_device_model_args_new(libxl__gc *gc, > > nics[i].colo_filter_redirector1_queue, > > nics[i].colo_filter_redirector1_outdev)); > > } > > + if (nics[i].colo_iothread) { > > + flexarray_append(dm_args, "-object"); > > + flexarray_append(dm_args, > > + GCSPRINTF("iothread,id=%s", > > + nics[i].colo_iothread)); > > + } > > if (nics[i].colo_compare_pri_in && > > nics[i].colo_compare_sec_in && > > nics[i].colo_compare_out && > > - nics[i].colo_compare_notify_dev) { > > + nics[i].colo_compare_notify_dev && > > + nics[i].colo_compare_iothread) { > > flexarray_append(dm_args, "-object"); > > flexarray_append(dm_args, > > - GCSPRINTF("colo- > > compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", > > + > > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=% > > + s, > > + notify_dev=%s,iothread=%s", > > nics[i].colo_compare_pri_in, > > nics[i].colo_compare_sec_in, > > nics[i].colo_compare_out, > > - nics[i].colo_compare_notify_dev)); > > + nics[i].colo_compare_notify_dev, > > + nics[i].colo_compare_iothread)); > > } > > } > > ioemu_nics++; > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index > > b61399ce36..f0435a5177 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -727,10 +727,12 @@ 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), > > ("colo_compare_notify_dev", string), > > + ("colo_compare_iothread", string), > > ("colo_sock_sec_redirector0_id", string), > > ("colo_sock_sec_redirector0_ip", string), > > ("colo_sock_sec_redirector0_port", string), diff --git > > a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index > > e105bda2bb..cd16856910 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)) { > > @@ - > > 529,6 +531,8 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config > > **config, char *token) > > replace_string(&nic->colo_compare_out, oparg); > > } else if (MATCH_OPTION("colo_compare_notify_dev", token, oparg)) { > > replace_string(&nic->colo_compare_notify_dev, oparg); > > + } else if (MATCH_OPTION("colo_compare_iothread", token, oparg)) { > > + replace_string(&nic->colo_compare_iothread, oparg); > > } else if (MATCH_OPTION("colo_sock_sec_redirector0_id", token, oparg)) { > > replace_string(&nic->colo_sock_sec_redirector0_id, oparg); > > } else if (MATCH_OPTION("colo_sock_sec_redirector0_ip", token, > > oparg)) { > > -- > > 2.17.GIT
Hi, Thanks for the patch, I've got plenty of question. On Mon, Jun 10, 2019 at 04:33:36PM +0800, Zhang Chen wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Xen COLO and KVM COLO shared lots of code in Qemu. > KVM COLO has added the iothread support, so we add it on Xen. > > Detail: > https://wiki.qemu.org/Features/COLO > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > tools/libxl/libxl_dm.c | 14 +++++++++++--- > tools/libxl/libxl_types.idl | 2 ++ > tools/xl/xl_parse.c | 4 ++++ > 3 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index f4fc96415d..6bb400efdf 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -1629,17 +1629,25 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, > nics[i].colo_filter_redirector1_queue, > nics[i].colo_filter_redirector1_outdev)); > } > + if (nics[i].colo_iothread) { > + flexarray_append(dm_args, "-object"); > + flexarray_append(dm_args, > + GCSPRINTF("iothread,id=%s", > + nics[i].colo_iothread)); This creates an iothread object, but it isn't used anywhere. What the purpose of it? Also, iothreads have options like "poll-grow", I don't know if you want to have that configurable or just keep the default values, just something to keep in mind. > + } > if (nics[i].colo_compare_pri_in && > nics[i].colo_compare_sec_in && > nics[i].colo_compare_out && > - nics[i].colo_compare_notify_dev) { > + nics[i].colo_compare_notify_dev && > + nics[i].colo_compare_iothread) { > flexarray_append(dm_args, "-object"); > flexarray_append(dm_args, > - GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s,iothread=%s", So, now iothread are mandatory? It would also mean that libxl cann't use QEMU older that 2.11, I think. Can't QEMU creates an iothread automatically if none are provided? Also, it looks like that if one of the colo-compare option is missing, the colo-compare object isn't created at all with no warning for the users of libxl. What's the difference between `colo_iothread' and `colo_compare_iothread' ? If a user only as the choice of a iothread id, why not have libxl create one on its own instead? Thanks,
> -----Original Message----- > From: Anthony PERARD [mailto:anthony.perard@citrix.com] > Sent: Tuesday, July 2, 2019 7:52 PM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; xen- > devel@lists.xenproject.org; Zhang Chen <zhangckid@gmail.com> > Subject: Re: [Xen-devel] [PATCH] tools/libxl: Add iothread support for COLO > > Hi, > > Thanks for the patch, I've got plenty of question. OK~ Very happy to discuss about that. > > On Mon, Jun 10, 2019 at 04:33:36PM +0800, Zhang Chen wrote: > > From: Zhang Chen <chen.zhang@intel.com> > > > > Xen COLO and KVM COLO shared lots of code in Qemu. > > KVM COLO has added the iothread support, so we add it on Xen. > > > > Detail: > > https://wiki.qemu.org/Features/COLO > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > --- > > tools/libxl/libxl_dm.c | 14 +++++++++++--- > > tools/libxl/libxl_types.idl | 2 ++ > > tools/xl/xl_parse.c | 4 ++++ > > 3 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index > > f4fc96415d..6bb400efdf 100644 > > --- a/tools/libxl/libxl_dm.c > > +++ b/tools/libxl/libxl_dm.c > > @@ -1629,17 +1629,25 @@ static int > libxl__build_device_model_args_new(libxl__gc *gc, > > nics[i].colo_filter_redirector1_queue, > > nics[i].colo_filter_redirector1_outdev)); > > } > > + if (nics[i].colo_iothread) { > > + flexarray_append(dm_args, "-object"); > > + flexarray_append(dm_args, > > + GCSPRINTF("iothread,id=%s", > > + nics[i].colo_iothread)); > > This creates an iothread object, but it isn't used anywhere. What the purpose of > it? No, colo compare use the iothread by the "colo_compare_iothread". > Also, iothreads have options like "poll-grow", I don't know if you want to have > that configurable or just keep the default values, just something to keep in > mind. Keep default. > > > + } > > if (nics[i].colo_compare_pri_in && > > nics[i].colo_compare_sec_in && > > nics[i].colo_compare_out && > > - nics[i].colo_compare_notify_dev) { > > + nics[i].colo_compare_notify_dev && > > + nics[i].colo_compare_iothread) { > > flexarray_append(dm_args, "-object"); > > flexarray_append(dm_args, > > - GCSPRINTF("colo- > compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", > > + > > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=% > > + s,notify_dev=%s,iothread=%s", > > So, now iothread are mandatory? It would also mean that libxl cann't use > QEMU older that 2.11, I think. > Can't QEMU creates an iothread automatically if none are provided? In current COLO design, iothread are mandatory, it's from Qemu maintainer's comments to make Colo-compare thread independent with Qemu main loop for better performance. I think libxl can use upstream Qemu to run COLO. Qemu can't creates iothread automatically, because it needs user input ID for iothread, then it will be used to other qemu module(need the ID). > > Also, it looks like that if one of the colo-compare option is missing, the colo- > compare object isn't created at all with no warning for the users of libxl. > > What's the difference between `colo_iothread' and `colo_compare_iothread' ? > "Colo_iothread" is iothread ID, "colo_compare_iothread" is colo compare used iothread's ID. In current COLO case, two ID must same. But for future or other case, it can different(maybe RX/TX use two iothread in future). > If a user only as the choice of a iothread id, why not have libxl create one on its > own instead? Because user also need input the iothread ID to colo-compare module. So we integrated the job here. Thanks Zhang Chen > > Thanks, > > -- > Anthony PERARD
On Tue, Jul 02, 2019 at 02:07:27PM +0000, Zhang, Chen wrote: > > On Mon, Jun 10, 2019 at 04:33:36PM +0800, Zhang Chen wrote: > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index > > > f4fc96415d..6bb400efdf 100644 > > > --- a/tools/libxl/libxl_dm.c > > > +++ b/tools/libxl/libxl_dm.c > > > @@ -1629,17 +1629,25 @@ static int > > libxl__build_device_model_args_new(libxl__gc *gc, > > > nics[i].colo_filter_redirector1_queue, > > > nics[i].colo_filter_redirector1_outdev)); > > > } > > > + if (nics[i].colo_iothread) { > > > + flexarray_append(dm_args, "-object"); > > > + flexarray_append(dm_args, > > > + GCSPRINTF("iothread,id=%s", > > > + nics[i].colo_iothread)); > > > > This creates an iothread object, but it isn't used anywhere. What the purpose of > > it? > > No, colo compare use the iothread by the "colo_compare_iothread". > > > > > > + } > > > if (nics[i].colo_compare_pri_in && > > > nics[i].colo_compare_sec_in && > > > nics[i].colo_compare_out && > > > - nics[i].colo_compare_notify_dev) { > > > + nics[i].colo_compare_notify_dev && > > > + nics[i].colo_compare_iothread) { > > > flexarray_append(dm_args, "-object"); > > > flexarray_append(dm_args, > > > - GCSPRINTF("colo- > > compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", > > > + > > > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=% > > > + s,notify_dev=%s,iothread=%s", > > > > So, now iothread are mandatory? It would also mean that libxl cann't use > > QEMU older that 2.11, I think. > > Can't QEMU creates an iothread automatically if none are provided? > > In current COLO design, iothread are mandatory, it's from Qemu maintainer's comments to make > Colo-compare thread independent with Qemu main loop for better performance. > I think libxl can use upstream Qemu to run COLO. > Qemu can't creates iothread automatically, because it needs user input ID for iothread, then it will be used to other qemu module(need the ID). > > > > > Also, it looks like that if one of the colo-compare option is missing, the colo- > > compare object isn't created at all with no warning for the users of libxl. > > > > What's the difference between `colo_iothread' and `colo_compare_iothread' ? > > > > "Colo_iothread" is iothread ID, "colo_compare_iothread" is colo compare used iothread's ID. > In current COLO case, two ID must same. > But for future or other case, it can different(maybe RX/TX use two iothread in future). > > > If a user only as the choice of a iothread id, why not have libxl create one on its > > own instead? > > Because user also need input the iothread ID to colo-compare module. What's a "colo-compare module"? Is it something outside of QEMU? If not, then I don't think users of libxl needs to provide a name for this iothread. Instead of adding "colo*_iothread" could you do something like the following? char *colo_compare_iothread_id = GCSPRINTF("colo-compare-iothread-%d", i); flexarray_append_pair(dm_args, "-object", GCSPRINTF("iothread,id=%s", colo_compare_iothread_id); // then use `colo_compare_iothread_id' when generating the // "colo-compare" option list: flexarray_append(dm_args, GCSPRINTF("colo-compare,...,iothread=%s", ..., colo_compare_iothread_id)); I think that will be more than enough for now. iothread can't be reused by something else that libxl create, and there can be only one "colo-compare" object in QEMU because the id is hard-coded in libxl. What do you thing? (I'm trying to make using COLO with libxl a little bit easier. libxl can be use to hide all the glory details of a QEMU command line.) Thanks,
> -----Original Message----- > From: Anthony PERARD [mailto:anthony.perard@citrix.com] > Sent: Wednesday, July 3, 2019 11:39 AM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; xen- > devel@lists.xenproject.org; Zhang Chen <zhangckid@gmail.com> > Subject: Re: [Xen-devel] [PATCH] tools/libxl: Add iothread support for COLO > > On Tue, Jul 02, 2019 at 02:07:27PM +0000, Zhang, Chen wrote: > > > On Mon, Jun 10, 2019 at 04:33:36PM +0800, Zhang Chen wrote: > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index > > > > f4fc96415d..6bb400efdf 100644 > > > > --- a/tools/libxl/libxl_dm.c > > > > +++ b/tools/libxl/libxl_dm.c > > > > @@ -1629,17 +1629,25 @@ static int > > > libxl__build_device_model_args_new(libxl__gc *gc, > > > > nics[i].colo_filter_redirector1_queue, > > > > nics[i].colo_filter_redirector1_outdev)); > > > > } > > > > + if (nics[i].colo_iothread) { > > > > + flexarray_append(dm_args, "-object"); > > > > + flexarray_append(dm_args, > > > > + GCSPRINTF("iothread,id=%s", > > > > + nics[i].colo_iothread)); > > > > > > This creates an iothread object, but it isn't used anywhere. What > > > the purpose of it? > > > > No, colo compare use the iothread by the "colo_compare_iothread". > > > > > > > > > + } > > > > if (nics[i].colo_compare_pri_in && > > > > nics[i].colo_compare_sec_in && > > > > nics[i].colo_compare_out && > > > > - nics[i].colo_compare_notify_dev) { > > > > + nics[i].colo_compare_notify_dev && > > > > + nics[i].colo_compare_iothread) { > > > > flexarray_append(dm_args, "-object"); > > > > flexarray_append(dm_args, > > > > - GCSPRINTF("colo- > > > compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s" > > > , > > > > + > > > > + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outd > > > > + ev=% > > > > + s,notify_dev=%s,iothread=%s", > > > > > > So, now iothread are mandatory? It would also mean that libxl cann't > > > use QEMU older that 2.11, I think. > > > Can't QEMU creates an iothread automatically if none are provided? > > > > In current COLO design, iothread are mandatory, it's from Qemu > > maintainer's comments to make Colo-compare thread independent with > Qemu main loop for better performance. > > I think libxl can use upstream Qemu to run COLO. > > Qemu can't creates iothread automatically, because it needs user input ID for > iothread, then it will be used to other qemu module(need the ID). > > > > > > > > Also, it looks like that if one of the colo-compare option is > > > missing, the colo- compare object isn't created at all with no warning for > the users of libxl. > > > > > > What's the difference between `colo_iothread' and > `colo_compare_iothread' ? > > > > > > > "Colo_iothread" is iothread ID, "colo_compare_iothread" is colo compare > used iothread's ID. > > In current COLO case, two ID must same. > > But for future or other case, it can different(maybe RX/TX use two iothread in > future). > > > > > If a user only as the choice of a iothread id, why not have libxl > > > create one on its own instead? > > > > Because user also need input the iothread ID to colo-compare module. > > What's a "colo-compare module"? Is it something outside of QEMU? > If not, then I don't think users of libxl needs to provide a name for this iothread. > Instead of adding "colo*_iothread" could you do something like the following? > > char *colo_compare_iothread_id = GCSPRINTF("colo-compare-iothread-%d", i); > flexarray_append_pair(dm_args, "-object", > GCSPRINTF("iothread,id=%s", colo_compare_iothread_id); // then > use `colo_compare_iothread_id' when generating the // "colo-compare" option > list: > flexarray_append(dm_args, GCSPRINTF("colo-compare,...,iothread=%s", > ..., colo_compare_iothread_id)); > > I think that will be more than enough for now. iothread can't be reused by > something else that libxl create, and there can be only one "colo-compare" > object in QEMU because the id is hard-coded in libxl. > > What do you thing? > > (I'm trying to make using COLO with libxl a little bit easier. libxl can be use to > hide all the glory details of a QEMU command line.) Hi Anthony, Yes, the colo-compare module is a part of Qemu. Your idea is good for now, easy to use, I can change it in next version. Thanks Zhang Chen > > Thanks, > > -- > Anthony PERARD
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index f4fc96415d..6bb400efdf 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -1629,17 +1629,25 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, nics[i].colo_filter_redirector1_queue, nics[i].colo_filter_redirector1_outdev)); } + if (nics[i].colo_iothread) { + flexarray_append(dm_args, "-object"); + flexarray_append(dm_args, + GCSPRINTF("iothread,id=%s", + nics[i].colo_iothread)); + } if (nics[i].colo_compare_pri_in && nics[i].colo_compare_sec_in && nics[i].colo_compare_out && - nics[i].colo_compare_notify_dev) { + nics[i].colo_compare_notify_dev && + nics[i].colo_compare_iothread) { flexarray_append(dm_args, "-object"); flexarray_append(dm_args, - GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s", + GCSPRINTF("colo-compare,id=c1,primary_in=%s,secondary_in=%s,outdev=%s,notify_dev=%s,iothread=%s", nics[i].colo_compare_pri_in, nics[i].colo_compare_sec_in, nics[i].colo_compare_out, - nics[i].colo_compare_notify_dev)); + nics[i].colo_compare_notify_dev, + nics[i].colo_compare_iothread)); } } ioemu_nics++; diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index b61399ce36..f0435a5177 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -727,10 +727,12 @@ 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), ("colo_compare_notify_dev", string), + ("colo_compare_iothread", string), ("colo_sock_sec_redirector0_id", string), ("colo_sock_sec_redirector0_ip", string), ("colo_sock_sec_redirector0_port", string), diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index e105bda2bb..cd16856910 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)) { @@ -529,6 +531,8 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token) replace_string(&nic->colo_compare_out, oparg); } else if (MATCH_OPTION("colo_compare_notify_dev", token, oparg)) { replace_string(&nic->colo_compare_notify_dev, oparg); + } else if (MATCH_OPTION("colo_compare_iothread", token, oparg)) { + replace_string(&nic->colo_compare_iothread, oparg); } else if (MATCH_OPTION("colo_sock_sec_redirector0_id", token, oparg)) { replace_string(&nic->colo_sock_sec_redirector0_id, oparg); } else if (MATCH_OPTION("colo_sock_sec_redirector0_ip", token, oparg)) {