diff mbox

[RFC,1/6] COLO-Proxy: Add remus command to open userspace proxy

Message ID 1485412569-7431-2-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Chen Jan. 26, 2017, 6:36 a.m. UTC
Add remus '-p' to enable userspace colo proxy(in qemu).

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 docs/man/xl.pod.1.in          |  4 ++++
 tools/libxl/libxl_colo.h      |  5 +++++
 tools/libxl/libxl_colo_save.c |  2 ++
 tools/libxl/libxl_types.idl   | 17 +++++++++--------
 tools/libxl/xl_cmdimpl.c      | 13 ++++++++++++-
 tools/libxl/xl_cmdtable.c     |  3 ++-
 6 files changed, 34 insertions(+), 10 deletions(-)

Comments

Wei Liu Jan. 27, 2017, 5:05 p.m. UTC | #1
On Thu, Jan 26, 2017 at 02:36:04PM +0800, Zhang Chen wrote:
> Add remus '-p' to enable userspace colo proxy(in qemu).
> 
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>  docs/man/xl.pod.1.in          |  4 ++++
>  tools/libxl/libxl_colo.h      |  5 +++++
>  tools/libxl/libxl_colo_save.c |  2 ++
>  tools/libxl/libxl_types.idl   | 17 +++++++++--------
>  tools/libxl/xl_cmdimpl.c      | 13 ++++++++++++-
>  tools/libxl/xl_cmdtable.c     |  3 ++-
>  6 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
> index 09c1faa..b5fb7c1 100644
> --- a/docs/man/xl.pod.1.in
> +++ b/docs/man/xl.pod.1.in
> @@ -553,6 +553,10 @@ Disable disk replication. Requires enabling unsafe mode.
>  Enable COLO HA. This conflicts with B<-i> and B<-b>, and memory
>  checkpoint compression must be disabled.
>  
> +=item B<-p>
> +
> +Enable userspace COLO Proxy. Must open with B<-c>.
> +

Use userspace COLO Proxy. This option must be used in conjunction with B<-c>.

>  =back
>  
>  =item B<pause> I<domain-id>
> diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
> index 682275c..4746d8c 100644
> --- a/tools/libxl/libxl_colo.h
> +++ b/tools/libxl/libxl_colo.h
> @@ -64,6 +64,11 @@ struct libxl__colo_proxy_state {
>  
>      int sock_fd;
>      int index;
> +    /*
> +     * Private, True means use userspace colo proxy
> +     *          False means use kernel colo proxy.
> +     */
> +    bool is_userspace_proxy;
>  };
>  
>  struct libxl__colo_save_state {
> diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
> index 6277770..eb8336c 100644
> --- a/tools/libxl/libxl_colo_save.c
> +++ b/tools/libxl/libxl_colo_save.c
> @@ -101,6 +101,8 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
>      css->qdisk_setuped = false;
>      css->qdisk_used = false;
>      libxl__ev_child_init(&css->child);
> +    css->cps.is_userspace_proxy =
> +        libxl_defbool_val(dss->remus->userspace_colo_proxy);
>  
>      if (dss->remus->netbufscript)
>          css->colo_proxy_script = libxl__strdup(gc, dss->remus->netbufscript);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index a612d1f..1bd2057 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -844,14 +844,15 @@ libxl_sched_credit2_params = Struct("sched_credit2_params", [
>      ], dispose_fn=None)
>  
>  libxl_domain_remus_info = Struct("domain_remus_info",[
> -    ("interval",     integer),
> -    ("allow_unsafe", libxl_defbool),
> -    ("blackhole",    libxl_defbool),
> -    ("compression",  libxl_defbool),
> -    ("netbuf",       libxl_defbool),
> -    ("netbufscript", string),
> -    ("diskbuf",      libxl_defbool),
> -    ("colo",         libxl_defbool)
> +    ("interval",             integer),
> +    ("allow_unsafe",         libxl_defbool),
> +    ("blackhole",            libxl_defbool),
> +    ("compression",          libxl_defbool),
> +    ("netbuf",               libxl_defbool),
> +    ("netbufscript",         string),
> +    ("diskbuf",              libxl_defbool),
> +    ("colo",                 libxl_defbool),
> +    ("userspace_colo_proxy", libxl_defbool)

Please add a LIBXL_HAVE macro in libxl.h.

>      ])
>  
>  libxl_event_type = Enumeration("event_type", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 7e8a8ae..905c5f6 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8893,7 +8893,7 @@ int main_remus(int argc, char **argv)
>  
>      memset(&r_info, 0, sizeof(libxl_domain_remus_info));
>  
> -    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ec", NULL, "remus", 2) {
> +    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ecp", NULL, "remus", 2) {
>      case 'i':
>          r_info.interval = atoi(optarg);
>          break;
> @@ -8923,6 +8923,9 @@ int main_remus(int argc, char **argv)
>          break;
>      case 'c':
>          libxl_defbool_set(&r_info.colo, true);
> +        break;
> +    case 'p':
> +        libxl_defbool_set(&r_info.userspace_colo_proxy, true);
>      }
>  
>      domid = find_domain(argv[optind]);
> @@ -8931,9 +8934,17 @@ int main_remus(int argc, char **argv)
>      /* Defaults */
>      libxl_defbool_setdefault(&r_info.blackhole, false);
>      libxl_defbool_setdefault(&r_info.colo, false);
> +    libxl_defbool_setdefault(&r_info.userspace_colo_proxy, false);
> +

Hmm... I think setting defaults should be pushed into libxl.

But I think this is issue is orthogonal to this patch, and we can
revisit this later.

>      if (!libxl_defbool_val(r_info.colo) && !r_info.interval)
>          r_info.interval = 200;
>  
> +    if (libxl_defbool_val(r_info.userspace_colo_proxy) &&
> +        !libxl_defbool_val(r_info.colo)) {
> +        perror("option -p must open with -c");

"Option -p must be used in conjunction with -c".

And please use fprintf(stderr,...) here because libxl_defbool_val
doesn't touch errno.


> +        exit(-1);
> +    }
> +
>      if (libxl_defbool_val(r_info.colo)) {
>          if (r_info.interval || libxl_defbool_val(r_info.blackhole) ||
>              !libxl_defbool_is_default(r_info.netbuf) ||
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 588d5d9..a124059 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -506,7 +506,8 @@ struct cmd_spec cmd_table[] = {
>        "-n                      Disable network output buffering. Works only in unsafe mode.\n"
>        "-d                      Disable disk replication. Works only in unsafe mode.\n"
>        "-c                      Enable COLO HA. It is conflict with -i and -b, and memory\n"
> -      "                        checkpoint must be disabled"
> +      "                        checkpoint must be disabled.\n"
> +      "-p                      Enable COLO userspace proxy."

"Use".

Wei.

>      },
>  #endif
>      { "devd",
> -- 
> 2.7.4
> 
> 
>
Zhang Chen Feb. 6, 2017, 6:01 a.m. UTC | #2
On 01/28/2017 01:05 AM, Wei Liu wrote:
> On Thu, Jan 26, 2017 at 02:36:04PM +0800, Zhang Chen wrote:
>> Add remus '-p' to enable userspace colo proxy(in qemu).
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   docs/man/xl.pod.1.in          |  4 ++++
>>   tools/libxl/libxl_colo.h      |  5 +++++
>>   tools/libxl/libxl_colo_save.c |  2 ++
>>   tools/libxl/libxl_types.idl   | 17 +++++++++--------
>>   tools/libxl/xl_cmdimpl.c      | 13 ++++++++++++-
>>   tools/libxl/xl_cmdtable.c     |  3 ++-
>>   6 files changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
>> index 09c1faa..b5fb7c1 100644
>> --- a/docs/man/xl.pod.1.in
>> +++ b/docs/man/xl.pod.1.in
>> @@ -553,6 +553,10 @@ Disable disk replication. Requires enabling unsafe mode.
>>   Enable COLO HA. This conflicts with B<-i> and B<-b>, and memory
>>   checkpoint compression must be disabled.
>>   
>> +=item B<-p>
>> +
>> +Enable userspace COLO Proxy. Must open with B<-c>.
>> +
> Use userspace COLO Proxy. This option must be used in conjunction with B<-c>.

OK.

>
>>   =back
>>   
>>   =item B<pause> I<domain-id>
>> diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
>> index 682275c..4746d8c 100644
>> --- a/tools/libxl/libxl_colo.h
>> +++ b/tools/libxl/libxl_colo.h
>> @@ -64,6 +64,11 @@ struct libxl__colo_proxy_state {
>>   
>>       int sock_fd;
>>       int index;
>> +    /*
>> +     * Private, True means use userspace colo proxy
>> +     *          False means use kernel colo proxy.
>> +     */
>> +    bool is_userspace_proxy;
>>   };
>>   
>>   struct libxl__colo_save_state {
>> diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
>> index 6277770..eb8336c 100644
>> --- a/tools/libxl/libxl_colo_save.c
>> +++ b/tools/libxl/libxl_colo_save.c
>> @@ -101,6 +101,8 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
>>       css->qdisk_setuped = false;
>>       css->qdisk_used = false;
>>       libxl__ev_child_init(&css->child);
>> +    css->cps.is_userspace_proxy =
>> +        libxl_defbool_val(dss->remus->userspace_colo_proxy);
>>   
>>       if (dss->remus->netbufscript)
>>           css->colo_proxy_script = libxl__strdup(gc, dss->remus->netbufscript);
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index a612d1f..1bd2057 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -844,14 +844,15 @@ libxl_sched_credit2_params = Struct("sched_credit2_params", [
>>       ], dispose_fn=None)
>>   
>>   libxl_domain_remus_info = Struct("domain_remus_info",[
>> -    ("interval",     integer),
>> -    ("allow_unsafe", libxl_defbool),
>> -    ("blackhole",    libxl_defbool),
>> -    ("compression",  libxl_defbool),
>> -    ("netbuf",       libxl_defbool),
>> -    ("netbufscript", string),
>> -    ("diskbuf",      libxl_defbool),
>> -    ("colo",         libxl_defbool)
>> +    ("interval",             integer),
>> +    ("allow_unsafe",         libxl_defbool),
>> +    ("blackhole",            libxl_defbool),
>> +    ("compression",          libxl_defbool),
>> +    ("netbuf",               libxl_defbool),
>> +    ("netbufscript",         string),
>> +    ("diskbuf",              libxl_defbool),
>> +    ("colo",                 libxl_defbool),
>> +    ("userspace_colo_proxy", libxl_defbool)
> Please add a LIBXL_HAVE macro in libxl.h.

I will add #define LIBXL_HAVE_USERSPACE_COLO_PROXY 1 in libxl.h
In next version.

>
>>       ])
>>   
>>   libxl_event_type = Enumeration("event_type", [
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 7e8a8ae..905c5f6 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -8893,7 +8893,7 @@ int main_remus(int argc, char **argv)
>>   
>>       memset(&r_info, 0, sizeof(libxl_domain_remus_info));
>>   
>> -    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ec", NULL, "remus", 2) {
>> +    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ecp", NULL, "remus", 2) {
>>       case 'i':
>>           r_info.interval = atoi(optarg);
>>           break;
>> @@ -8923,6 +8923,9 @@ int main_remus(int argc, char **argv)
>>           break;
>>       case 'c':
>>           libxl_defbool_set(&r_info.colo, true);
>> +        break;
>> +    case 'p':
>> +        libxl_defbool_set(&r_info.userspace_colo_proxy, true);
>>       }
>>   
>>       domid = find_domain(argv[optind]);
>> @@ -8931,9 +8934,17 @@ int main_remus(int argc, char **argv)
>>       /* Defaults */
>>       libxl_defbool_setdefault(&r_info.blackhole, false);
>>       libxl_defbool_setdefault(&r_info.colo, false);
>> +    libxl_defbool_setdefault(&r_info.userspace_colo_proxy, false);
>> +
> Hmm... I think setting defaults should be pushed into libxl.
>
> But I think this is issue is orthogonal to this patch, and we can
> revisit this later.
>
>>       if (!libxl_defbool_val(r_info.colo) && !r_info.interval)
>>           r_info.interval = 200;
>>   
>> +    if (libxl_defbool_val(r_info.userspace_colo_proxy) &&
>> +        !libxl_defbool_val(r_info.colo)) {
>> +        perror("option -p must open with -c");
> "Option -p must be used in conjunction with -c".
>
> And please use fprintf(stderr,...) here because libxl_defbool_val
> doesn't touch errno.

OK, I will fix this in next version.

>
>> +        exit(-1);
>> +    }
>> +
>>       if (libxl_defbool_val(r_info.colo)) {
>>           if (r_info.interval || libxl_defbool_val(r_info.blackhole) ||
>>               !libxl_defbool_is_default(r_info.netbuf) ||
>> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
>> index 588d5d9..a124059 100644
>> --- a/tools/libxl/xl_cmdtable.c
>> +++ b/tools/libxl/xl_cmdtable.c
>> @@ -506,7 +506,8 @@ struct cmd_spec cmd_table[] = {
>>         "-n                      Disable network output buffering. Works only in unsafe mode.\n"
>>         "-d                      Disable disk replication. Works only in unsafe mode.\n"
>>         "-c                      Enable COLO HA. It is conflict with -i and -b, and memory\n"
>> -      "                        checkpoint must be disabled"
>> +      "                        checkpoint must be disabled.\n"
>> +      "-p                      Enable COLO userspace proxy."
> "Use".

OK.

Thanks
Zhang Chen

>
> Wei.
>
>>       },
>>   #endif
>>       { "devd",
>> -- 
>> 2.7.4
>>
>>
>>
>
> .
>
diff mbox

Patch

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 09c1faa..b5fb7c1 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -553,6 +553,10 @@  Disable disk replication. Requires enabling unsafe mode.
 Enable COLO HA. This conflicts with B<-i> and B<-b>, and memory
 checkpoint compression must be disabled.
 
+=item B<-p>
+
+Enable userspace COLO Proxy. Must open with B<-c>.
+
 =back
 
 =item B<pause> I<domain-id>
diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
index 682275c..4746d8c 100644
--- a/tools/libxl/libxl_colo.h
+++ b/tools/libxl/libxl_colo.h
@@ -64,6 +64,11 @@  struct libxl__colo_proxy_state {
 
     int sock_fd;
     int index;
+    /*
+     * Private, True means use userspace colo proxy
+     *          False means use kernel colo proxy.
+     */
+    bool is_userspace_proxy;
 };
 
 struct libxl__colo_save_state {
diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
index 6277770..eb8336c 100644
--- a/tools/libxl/libxl_colo_save.c
+++ b/tools/libxl/libxl_colo_save.c
@@ -101,6 +101,8 @@  void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
     css->qdisk_setuped = false;
     css->qdisk_used = false;
     libxl__ev_child_init(&css->child);
+    css->cps.is_userspace_proxy =
+        libxl_defbool_val(dss->remus->userspace_colo_proxy);
 
     if (dss->remus->netbufscript)
         css->colo_proxy_script = libxl__strdup(gc, dss->remus->netbufscript);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a612d1f..1bd2057 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -844,14 +844,15 @@  libxl_sched_credit2_params = Struct("sched_credit2_params", [
     ], dispose_fn=None)
 
 libxl_domain_remus_info = Struct("domain_remus_info",[
-    ("interval",     integer),
-    ("allow_unsafe", libxl_defbool),
-    ("blackhole",    libxl_defbool),
-    ("compression",  libxl_defbool),
-    ("netbuf",       libxl_defbool),
-    ("netbufscript", string),
-    ("diskbuf",      libxl_defbool),
-    ("colo",         libxl_defbool)
+    ("interval",             integer),
+    ("allow_unsafe",         libxl_defbool),
+    ("blackhole",            libxl_defbool),
+    ("compression",          libxl_defbool),
+    ("netbuf",               libxl_defbool),
+    ("netbufscript",         string),
+    ("diskbuf",              libxl_defbool),
+    ("colo",                 libxl_defbool),
+    ("userspace_colo_proxy", libxl_defbool)
     ])
 
 libxl_event_type = Enumeration("event_type", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7e8a8ae..905c5f6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8893,7 +8893,7 @@  int main_remus(int argc, char **argv)
 
     memset(&r_info, 0, sizeof(libxl_domain_remus_info));
 
-    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ec", NULL, "remus", 2) {
+    SWITCH_FOREACH_OPT(opt, "Fbundi:s:N:ecp", NULL, "remus", 2) {
     case 'i':
         r_info.interval = atoi(optarg);
         break;
@@ -8923,6 +8923,9 @@  int main_remus(int argc, char **argv)
         break;
     case 'c':
         libxl_defbool_set(&r_info.colo, true);
+        break;
+    case 'p':
+        libxl_defbool_set(&r_info.userspace_colo_proxy, true);
     }
 
     domid = find_domain(argv[optind]);
@@ -8931,9 +8934,17 @@  int main_remus(int argc, char **argv)
     /* Defaults */
     libxl_defbool_setdefault(&r_info.blackhole, false);
     libxl_defbool_setdefault(&r_info.colo, false);
+    libxl_defbool_setdefault(&r_info.userspace_colo_proxy, false);
+
     if (!libxl_defbool_val(r_info.colo) && !r_info.interval)
         r_info.interval = 200;
 
+    if (libxl_defbool_val(r_info.userspace_colo_proxy) &&
+        !libxl_defbool_val(r_info.colo)) {
+        perror("option -p must open with -c");
+        exit(-1);
+    }
+
     if (libxl_defbool_val(r_info.colo)) {
         if (r_info.interval || libxl_defbool_val(r_info.blackhole) ||
             !libxl_defbool_is_default(r_info.netbuf) ||
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 588d5d9..a124059 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -506,7 +506,8 @@  struct cmd_spec cmd_table[] = {
       "-n                      Disable network output buffering. Works only in unsafe mode.\n"
       "-d                      Disable disk replication. Works only in unsafe mode.\n"
       "-c                      Enable COLO HA. It is conflict with -i and -b, and memory\n"
-      "                        checkpoint must be disabled"
+      "                        checkpoint must be disabled.\n"
+      "-p                      Enable COLO userspace proxy."
     },
 #endif
     { "devd",