Re: [PATCH for-4.5 v20 07/12] xl/remus: change bool to defbool

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Sep 25, 2014 at 02:16:19PM +0800, Yang Hongyang wrote:
> Use defbool instead of bool for boolean flags in remus_info struct.

While that change by itself looks OK, the change in 'libxl_types.idl'
break the ABI.

Could you say a bit of why that is OK? As in, would there had
been in the past any users of this ABI that now would have issues with this?

Also, how important is this patch? Does it have to go in or
can it be dropped from the patchset?

> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  tools/libxl/libxl.c         | 3 +++
>  tools/libxl/libxl_dom.c     | 2 +-
>  tools/libxl/libxl_types.idl | 4 ++--
>  tools/libxl/xl_cmdimpl.c    | 9 ++++-----
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 79b508f..9e0a800 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -804,6 +804,9 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
>          goto out;
>      }
>  
> +    libxl_defbool_setdefault(&info->blackhole, false);
> +    libxl_defbool_setdefault(&info->compression, true);
> +
>      GCNEW(dss);
>      dss->ao = ao;
>      dss->callback = remus_failover_cb;
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index e9d29b5..d63ae1b 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1809,7 +1809,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>  
>      if (r_info != NULL) {
>          dss->interval = r_info->interval;
> -        if (r_info->compression)
> +        if (libxl_defbool_val(r_info->compression))
>              dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>      }
>  
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index da4c52d..16e374f 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -611,8 +611,8 @@ libxl_sched_credit_params = Struct("sched_credit_params", [
>  
>  libxl_domain_remus_info = Struct("domain_remus_info",[
>      ("interval",     integer),
> -    ("blackhole",    bool),
> -    ("compression",  bool),
> +    ("blackhole",    libxl_defbool),
> +    ("compression",  libxl_defbool),
>      ])
>  
>  libxl_event_type = Enumeration("event_type", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d205f96..e9e8900 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7495,18 +7495,17 @@ int main_remus(int argc, char **argv)
>      memset(&r_info, 0, sizeof(libxl_domain_remus_info));
>      /* Defaults */
>      r_info.interval = 200;
> -    r_info.blackhole = 0;
> -    r_info.compression = 1;
> +    libxl_defbool_setdefault(&r_info.blackhole, false);
>  
>      SWITCH_FOREACH_OPT(opt, "bui:s:e", NULL, "remus", 2) {
>      case 'i':
>          r_info.interval = atoi(optarg);
>          break;
>      case 'b':
> -        r_info.blackhole = 1;
> +        libxl_defbool_set(&r_info.blackhole, true);
>          break;
>      case 'u':
> -        r_info.compression = 0;
> +        libxl_defbool_set(&r_info.compression, false);
>          break;
>      case 's':
>          ssh_command = optarg;
> @@ -7519,7 +7518,7 @@ int main_remus(int argc, char **argv)
>      domid = find_domain(argv[optind]);
>      host = argv[optind + 1];
>  
> -    if (r_info.blackhole) {
> +    if (libxl_defbool_val(r_info.blackhole)) {
>          send_fd = open("/dev/null", O_RDWR, 0644);
>          if (send_fd < 0) {
>              perror("failed to open /dev/null");
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel