Re: [PATCH V3 2/7] COLO-Proxy: Setup userspace colo-proxy on primary side

From: Wei Liu <wei.liu2@citrix.com>
To: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Li Zhijian <lizhijian@cn.fujitsu.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	eddie <eddie.dong@intel.com>, Yang Hongyang <imhy.yang@gmail.com>,
	Xen devel <xen-devel@lists.xenproject.org>,
	Bian Naimeng <biannm@cn.fujitsu.com>
Subject: Re: [PATCH V3 2/7] COLO-Proxy: Setup userspace colo-proxy on primary side
Date: Tue, 21 Feb 2017 09:53:20 +0000	[thread overview]
Message-ID: <20170221095320.aa65ev22kbdvr3cp@citrix.com> (raw)
In-Reply-To: <7120966a-6b84-6dba-06fb-003d7cb7da2d@cn.fujitsu.com>

On Tue, Feb 21, 2017 at 10:57:46AM +0800, Zhang Chen wrote:
> 
> 
> On 02/20/2017 11:55 PM, Wei Liu wrote:
> > On Fri, Feb 17, 2017 at 10:18:24AM +0800, Zhang Chen wrote:
> > > In this patch we close kernel COLO-Proxy on primary side.
> > > 
> > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> > > ---
> > >   tools/libxl/libxl_colo_proxy.c | 27 +++++++++++++++++++++++++++
> > >   tools/libxl/libxl_colo_save.c  |  9 +++++++--
> > >   2 files changed, 34 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/libxl/libxl_colo_proxy.c b/tools/libxl/libxl_colo_proxy.c
> > > index 0983f42..dd902fc 100644
> > > --- a/tools/libxl/libxl_colo_proxy.c
> > > +++ b/tools/libxl/libxl_colo_proxy.c
> > > @@ -152,6 +152,10 @@ int colo_proxy_setup(libxl__colo_proxy_state *cps)
> > >       STATE_AO_GC(cps->ao);
> > > +    /* If enable userspace proxy mode, we don't need setup kernel proxy */
> > > +    if (cps->is_userspace_proxy)
> > > +        return 0;
> > > +
> > >       skfd = socket(PF_NETLINK, SOCK_RAW, NETLINK_COLO);
> > >       if (skfd < 0) {
> > >           LOGD(ERROR, ao->domid, "can not create a netlink socket: %s", strerror(errno));
> > > @@ -222,6 +226,13 @@ out:
> > >   void colo_proxy_teardown(libxl__colo_proxy_state *cps)
> > >   {
> > > +    /*
> > > +     * If enable userspace proxy mode,
> > > +     * we don't need teardown kernel proxy
> > > +     */
> > > +    if (cps->is_userspace_proxy)
> > > +        return;
> > > +
> > >       if (cps->sock_fd >= 0) {
> > >           close(cps->sock_fd);
> > >           cps->sock_fd = -1;
> > > @@ -232,6 +243,13 @@ void colo_proxy_teardown(libxl__colo_proxy_state *cps)
> > >   void colo_proxy_preresume(libxl__colo_proxy_state *cps)
> > >   {
> > > +    /*
> > > +     * If enable userspace proxy mode,
> > > +     * we don't need preresume kernel proxy
> > > +     */
> > > +    if (cps->is_userspace_proxy)
> > > +        return;
> > > +
> > >       colo_proxy_send(cps, NULL, 0, COLO_CHECKPOINT);
> > >       /* TODO: need to handle if the call fails... */
> > >   }
> > > @@ -262,6 +280,15 @@ int colo_proxy_checkpoint(libxl__colo_proxy_state *cps,
> > >       STATE_AO_GC(cps->ao);
> > > +    /*
> > > +     * enable userspace proxy mode, tmp sleep.
> > > +     * then we will add qemu API support this func.
> > > +     */
> > > +    if (cps->is_userspace_proxy) {
> > > +        sleep(timeout_us / 1000000);
> > usleep is better.
> 
> OK.
> 
> > 
> > But in general I don't think sleeping in libxl is a good idea.
> > What is the reason that you need to sleep here?
> 
> In here we use this sleep to keep COLO period checkpoint,
> We can not do checkpoint continuously, that will make performance poor.
> After 7/7 we change this to
> ret = colo_userspace_proxy_recv(cps, recvbuff, timeout_us);
> 

I don't fully understand. Say, if I just use COLO with this sleep, will
it work?

> Thanks
> Zhang Chen
> 
> > 
> > Wei.
> > 
> > 
> > .
> > 
> 
> -- 
> Thanks
> Zhang Chen
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel