All of lore.kernel.org
 help / color / mirror / Atom feed
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 7/7] COLO-Proxy: Use socket to get checkpoint event.
Date: Mon, 20 Feb 2017 16:07:09 +0000	[thread overview]
Message-ID: <20170220160709.hr2fvbpxqqhpdw2j@citrix.com> (raw)
In-Reply-To: <1487297909-1885-8-git-send-email-zhangchen.fnst@cn.fujitsu.com>

On Fri, Feb 17, 2017 at 10:18:29AM +0800, Zhang Chen wrote:
> We use kernel colo proxy's way to get the checkpoint event
> from qemu colo-compare.
> Qemu colo-compare need add a API to support this(I will add this in qemu).
> 
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>  tools/libxl/libxl_colo.h         |  2 +
>  tools/libxl/libxl_colo_proxy.c   | 85 +++++++++++++++++++++++++++++++++++++---
>  tools/libxl/libxl_colo_restore.c | 11 ++++--
>  tools/libxl/libxl_colo_save.c    | 22 +++++++----
>  tools/libxl/libxl_nic.c          |  4 ++
>  tools/libxl/libxl_types.idl      |  4 +-
>  tools/libxl/xl_cmdimpl.c         |  4 ++
>  7 files changed, 114 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
> index 4746d8c..6c01b55 100644
> --- a/tools/libxl/libxl_colo.h
> +++ b/tools/libxl/libxl_colo.h
> @@ -69,6 +69,8 @@ struct libxl__colo_proxy_state {
>       *          False means use kernel colo proxy.
>       */
>      bool is_userspace_proxy;
> +    const char *checkpoint_host;
> +    const char *checkpoint_port;
>  };
>  
>  struct libxl__colo_save_state {
> diff --git a/tools/libxl/libxl_colo_proxy.c b/tools/libxl/libxl_colo_proxy.c
> index dd902fc..a6436b0 100644
> --- a/tools/libxl/libxl_colo_proxy.c
> +++ b/tools/libxl/libxl_colo_proxy.c
> @@ -18,9 +18,13 @@
>  #include "libxl_internal.h"
>  
>  #include <netlink/netlink.h>
> +#include <arpa/inet.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
>  
>  /* Consistent with the new COLO netlink channel in kernel side */
>  #define NETLINK_COLO 28
> +#define COLO_DEFAULT_WAIT_TIME 500000
>  
>  enum colo_netlink_op {
>      COLO_QUERY_CHECKPOINT = (NLMSG_MIN_TYPE + 1),
> @@ -76,6 +80,26 @@ static int colo_proxy_send(libxl__colo_proxy_state *cps, uint8_t *buff,
>      return ret;
>  }
>  
> +static int colo_userspace_proxy_recv(libxl__colo_proxy_state *cps,
> +                                     char *buff,
> +                                     unsigned int timeout_us)
> +{
> +    struct timeval tv;
> +    int ret;
> +
> +    STATE_AO_GC(cps->ao);
> +
> +    if (timeout_us) {
> +        tv.tv_sec = timeout_us / 1000000;
> +        tv.tv_usec = timeout_us % 1000000;
> +        setsockopt(cps->sock_fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));

Please check the return value of setsockopt and handle error
appropriately. If you don't care about the error, please add a comment.

> +    }
> +
> +    ret = recv(cps->sock_fd, buff, sizeof(buff),0);
> +
> +    return ret;
> +}
> +
>  /* error: return -1, otherwise return 0 */
>  static int64_t colo_proxy_recv(libxl__colo_proxy_state *cps, uint8_t **buff,
>                                 unsigned int timeout_us)
> @@ -153,8 +177,45 @@ 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)
> +    if (cps->is_userspace_proxy) {
> +        struct sockaddr_in addr;
> +        int port;
> +        char recvbuff[1024];
> +        char sendbuf[] = "COLO_USERSPACE_PROXY_INIT";
> +
> +        memset(&addr, 0, sizeof(addr));
> +        port = atoi(cps->checkpoint_port);
> +        addr.sin_family = AF_INET;
> +        addr.sin_port = htons(port);
> +        addr.sin_addr.s_addr = inet_addr(cps->checkpoint_host);
> +
> +        skfd = socket(AF_INET, SOCK_STREAM, 0);
> +        if (skfd < 0) {
> +            LOGD(ERROR, ao->domid, "can not create a TCP socket: %s",
> +                 strerror(errno));
> +            goto out;
> +        }
> +
> +        cps->sock_fd = skfd;
> +
> +        if (connect(skfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
> +            LOGD(ERROR, ao->domid, "connect error");
> +            goto out;
> +        }
> +
> +        ret = send(skfd, sendbuf, strlen(sendbuf),0);
> +        if (ret < 0)
> +            goto out;
> +
> +        ret = colo_userspace_proxy_recv(cps, recvbuff, COLO_DEFAULT_WAIT_TIME);
> +        if (ret < 0) {
> +            LOGD(ERROR, ao->domid, "Can't recv msg from qemu colo-compare: %s",
> +                 strerror(errno));
> +            goto out;
> +        }
> +
>          return 0;
> +    }
>  
>      skfd = socket(PF_NETLINK, SOCK_RAW, NETLINK_COLO);
>      if (skfd < 0) {
> @@ -247,8 +308,11 @@ 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)
> +    if (cps->is_userspace_proxy) {
> +        char sendbuf[] = "COLO_CHECKPOINT";

Please constify it.

> +        send(cps->sock_fd, sendbuf, strlen(sendbuf),0);

Space before "0".

>          return;
> +    }
>  
>      colo_proxy_send(cps, NULL, 0, COLO_CHECKPOINT);
>      /* TODO: need to handle if the call fails... */
> @@ -277,16 +341,25 @@ int colo_proxy_checkpoint(libxl__colo_proxy_state *cps,
>      struct nlmsghdr *h;
>      struct colo_msg *m;
>      int ret = -1;
> +    char recvbuff[1024];
>  
>      STATE_AO_GC(cps->ao);
>  
>      /*
> -     * enable userspace proxy mode, tmp sleep.
> -     * then we will add qemu API support this func.
> +     * enable userspace proxy mode.
> +     * Then we will add qemu API support for this func.
>       */
>      if (cps->is_userspace_proxy) {
> -        sleep(timeout_us / 1000000);
> -        return 0;
> +        ret = colo_userspace_proxy_recv(cps, recvbuff, timeout_us);
> +        if (ret <= 0)
> +            return 0;
> +
> +        if (!strcmp(recvbuff, "DO_CHECKPOINT")) {
> +            return 1;
> +        } else {
> +            LOGD(ERROR, ao->domid, "receive qemu colo-compare checkpoint error");
> +            return -1;

Please use goto style error handling if possible. Please refer to
CODING_STYLE file inside libxl directory.

> +        }
>      }
>  
>      size = colo_proxy_recv(cps, &buff, timeout_us);
> diff --git a/tools/libxl/libxl_colo_restore.c b/tools/libxl/libxl_colo_restore.c
> index c6d239a..065ea00 100644
> --- a/tools/libxl/libxl_colo_restore.c
> +++ b/tools/libxl/libxl_colo_restore.c
> @@ -613,7 +613,8 @@ static void colo_restore_preresume_cb(libxl__egc *egc,
>          }
>      }
>  
> -    colo_proxy_preresume(&crs->cps);
> +    if (!crs->cps.is_userspace_proxy)
> +        colo_proxy_preresume(&crs->cps);
>  
>      colo_restore_resume_vm(egc, crcs);
>  
> @@ -786,9 +787,11 @@ static void colo_setup_checkpoint_devices(libxl__egc *egc,
>      cds->ops = colo_restore_ops;
>  
>      crs->cps.ao = ao;
> -    if (colo_proxy_setup(&crs->cps)) {
> -        LOGD(ERROR, cds->domid, "COLO: failed to setup colo proxy for guest");
> -        goto out;
> +    if (!crs->cps.is_userspace_proxy) {
> +        if (colo_proxy_setup(&crs->cps)) {
> +            LOGD(ERROR, cds->domid, "COLO: failed to setup colo proxy for guest");
> +            goto out;
> +        }
>      }
>  
>      if (init_device_subkind(cds))
> diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
> index 91e3fce..a0cfc5a 100644
> --- a/tools/libxl/libxl_colo_save.c
> +++ b/tools/libxl/libxl_colo_save.c
> @@ -86,6 +86,7 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
>      libxl__checkpoint_devices_state *const cds = &dss->cds;
>      libxl__srm_save_autogen_callbacks *const callbacks =
>          &dss->sws.shs.callbacks.save.a;
> +    libxl_device_nic *nics;
>  
>      STATE_AO_GC(dss->ao);
>  
> @@ -110,24 +111,31 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
>          css->colo_proxy_script = GCSPRINTF("%s/colo-proxy-setup",
>                                             libxl__xen_script_dir_path());
>  
> -    /* If enable userspace proxy mode, we don't need VIF */
> -    if (css->cps.is_userspace_proxy)
> -        cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VBD);
> -    else
> -        cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VIF) |
> -                                 (1 << LIBXL__DEVICE_KIND_VBD);
> -
>      cds->ops = colo_ops;
>      cds->callback = colo_save_setup_done;
>      cds->ao = ao;
>      cds->domid = dss->domid;
>      cds->concrete_data = css;
>  
> +    /* If enable userspace proxy mode, we don't need VIF */
> +    if (css->cps.is_userspace_proxy) {
> +        cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VBD);
> +
> +        /* Use this args we can connect to qemu colo-compare */
> +        nics = libxl_device_nic_list(CTX, cds->domid, &cds->num_nics);
> +        css->cps.checkpoint_host = nics->colo_checkpoint_host;
> +        css->cps.checkpoint_port = nics->colo_checkpoint_port;
> +    } else {
> +        cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VIF) |
> +                                 (1 << LIBXL__DEVICE_KIND_VBD);
> +    }
> +
>      css->srs.ao = ao;
>      css->srs.fd = css->recv_fd;
>      css->srs.back_channel = true;
>      libxl__stream_read_start(egc, &css->srs);
>      css->cps.ao = ao;
> +

Stray blank line.

Wei.

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

  reply	other threads:[~2017-02-20 16:07 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-17  2:18 [PATCH V3 0/7] COLO-Proxy: Make Xen COLO use userspace colo-proxy Zhang Chen
2017-02-17  2:18 ` [PATCH V3 1/7] COLO-Proxy: Add remus command to open userspace proxy Zhang Chen
2017-02-20 15:50   ` Wei Liu
2017-02-21  2:49     ` Zhang Chen
2017-02-17  2:18 ` [PATCH V3 2/7] COLO-Proxy: Setup userspace colo-proxy on primary side Zhang Chen
2017-02-20 15:55   ` Wei Liu
2017-02-21  2:57     ` Zhang Chen
2017-02-21  9:53       ` Wei Liu
2017-02-21 11:03         ` Zhang Chen
2017-02-21 11:18           ` Wei Liu
2017-02-21 13:12             ` Zhang Chen
2017-02-17  2:18 ` [PATCH V3 3/7] tools/libxl: refactor do_domain_create() Zhang Chen
2017-02-20 15:50   ` Wei Liu
2017-02-21  2:48     ` Zhang Chen
2017-02-17  2:18 ` [PATCH V3 4/7] COLO-Proxy: Setup userspace colo-proxy on secondary side Zhang Chen
2017-02-17  2:18 ` [PATCH V3 5/7] COLO-Proxy: Add primary userspace colo proxy start args Zhang Chen
2017-02-20 15:59   ` Wei Liu
2017-02-21  3:07     ` Zhang Chen
2017-02-21  9:47       ` Wei Liu
2017-02-17  2:18 ` [PATCH V3 6/7] COLO-Proxy: Add secondary userspace colo-proxy " Zhang Chen
2017-02-17  2:18 ` [PATCH V3 7/7] COLO-Proxy: Use socket to get checkpoint event Zhang Chen
2017-02-20 16:07   ` Wei Liu [this message]
2017-02-21  6:24     ` Zhang Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170220160709.hr2fvbpxqqhpdw2j@citrix.com \
    --to=wei.liu2@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=biannm@cn.fujitsu.com \
    --cc=eddie.dong@intel.com \
    --cc=imhy.yang@gmail.com \
    --cc=lizhijian@cn.fujitsu.com \
    --cc=xen-devel@lists.xenproject.org \
    --cc=zhangchen.fnst@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.