Re: [PATCH v2 5/8] tools: dynamically attach/detach CQM service for a guest

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

On Thu, Nov 21, 2013 at 03:20:41PM +0800, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
> 
> Add two new xl instruction for attaching and detaching CQM services
> for a running guest.
> 
> Above "qos_res" is the monitoring resource type, and current supported
> type is "cqm".
> 
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  tools/libxc/xc_domain.c   |   18 ++++++++++
>  tools/libxc/xenctrl.h     |    2 ++
>  tools/libxl/Makefile      |    3 +-
>  tools/libxl/libxl.h       |    3 ++
>  tools/libxl/libxl_pqos.c  |   88 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/xl.h          |    2 ++
>  tools/libxl/xl_cmdimpl.c  |   36 +++++++++++++++++++
>  tools/libxl/xl_cmdtable.c |   10 ++++++
>  8 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_pqos.c
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 1ccafc5..85c2d4d 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -1776,6 +1776,24 @@ int xc_domain_set_max_evtchn(xc_interface *xch, uint32_t domid,
>      return do_domctl(xch, &domctl);
>  }
>  
> +int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags)
> +{
> +    DECLARE_DOMCTL;
> +    domctl.cmd = XEN_DOMCTL_attach_pqos;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.qos_res.flags = flags;
> +    return do_domctl(xch, &domctl);
> +}
> +
> +int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags)
> +{
> +    DECLARE_DOMCTL;
> +    domctl.cmd = XEN_DOMCTL_detach_pqos;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.qos_res.flags = flags;
> +    return do_domctl(xch, &domctl);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 4ac6b8a..a57f147 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -2395,4 +2395,6 @@ int xc_kexec_load(xc_interface *xch, uint8_t type, uint16_t arch,
>   */
>  int xc_kexec_unload(xc_interface *xch, int type);
>  
> +int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags);
> +int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags);
>  #endif /* XENCTRL_H */
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index cf214bb..35f0b97 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -74,7 +74,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
>  			libxl_internal.o libxl_utils.o libxl_uuid.o \
>  			libxl_json.o libxl_aoutils.o libxl_numa.o \
>  			libxl_save_callout.o _libxl_save_msgs_callout.o \
> -			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
> +			libxl_qmp.o libxl_event.o libxl_fork.o libxl_pqos.o \
> +			$(LIBXL_OBJS-y)
>  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
>  
>  $(LIBXL_OBJS): CFLAGS += $(CFLAGS_LIBXL) -include $(XEN_ROOT)/tools/config.h
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index c7dceda..a9a506f 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -1051,6 +1051,9 @@ int libxl_flask_getenforce(libxl_ctx *ctx);
>  int libxl_flask_setenforce(libxl_ctx *ctx, int mode);
>  int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
>  
> +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
> +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
> +
>  /* misc */
>  
>  /* Each of these sets or clears the flag according to whether the
> diff --git a/tools/libxl/libxl_pqos.c b/tools/libxl/libxl_pqos.c
> new file mode 100644
> index 0000000..e92b94b
> --- /dev/null
> +++ b/tools/libxl/libxl_pqos.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright (C) 2013      Intel Corporation
> + * Author Jiongxi Li <jiongxi.li@intel.com>
> + * Author Dongxiao Xu <dongxiao.xu@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include "libxl_internal.h"
> +
> +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
> +{
> +    int rc;
> +    int flags = 0;
> +
> +    if (!strcmp(qosres, "cqm"))
> +        flags |= XEN_DOMCTL_ADF_pqos_cqm;
> +    else {
> +        rc = -EINVAL;
> +        fprintf(stderr, "Invalid QoS resource type! "
> +                        "Supported types are: \"cqm\".\n");

There are these nice LIBXL__LOG macros. Could you use those instead please?

(And also in the other patch that touches libxl)

> +        return rc;
> +    }
> +
> +    rc = xc_domain_pqos_attach(ctx->xch, domid, flags);
> +    if (rc < 0) {
> +        fprintf(stderr, "Failed to attach CQM service to domain %d. ", domid);
> +        if (errno == ENODEV)
> +            fprintf(stderr, "CQM is not supported in this system.\n");
> +        else if (errno == EBUSY)
> +            fprintf(stderr, "There is no free CQM RMID available.\n");
> +        else if (errno == EINVAL)
> +            fprintf(stderr, "Domain %d is already attached with CQM service.\n", domid);
> +        else if (errno == ESRCH)
> +            fprintf(stderr, "Is Domain %d a valid domain?\n", domid);
> +        else
> +            fprintf(stderr, "errno: %d\n", errno);

This could be made in a:

static const char * const msg[] = {
	[ENODEV] = "Failed to attach CQM service to domain %d!",
	...
}
and then you just iterate over the msg[] until you 

> +    }
> +
> +    return rc;
> +}
> +
> +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
> +{
> +    int rc;
> +    int flags = 0;
> +
> +    if (!strcmp(qosres, "cqm"))

strncmp!

> +        flags |= XEN_DOMCTL_ADF_pqos_cqm;
> +    else {
> +        rc = -EINVAL;
> +        fprintf(stderr, "Invalid QoS resource type! "
> +                        "Supported types are: \"cqm\".\n");
> +        return rc;
> +    }
> +
> +    rc = xc_domain_pqos_detach(ctx->xch, domid, flags);
> +    if (rc < 0) {
> +        fprintf(stderr, "Failed to detach CQM service to domain %d. ", domid);
> +        if (errno == ENODEV)
> +            fprintf(stderr, "CQM is not supported in this system.\n");
> +        else if (errno == EINVAL)
> +            fprintf(stderr, "Domain %d is not attached with CQM service.\n", domid);
> +        else if (errno == ESRCH)
> +            fprintf(stderr, "Is Domain %d a valid domain?\n", domid);
> +        else
> +            fprintf(stderr, "errno: %d\n", errno);

Again, the thing I mentioned above.
> +    }
> +
> +    return rc;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index e005c39..78738b9 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -105,6 +105,8 @@ int main_getenforce(int argc, char **argv);
>  int main_setenforce(int argc, char **argv);
>  int main_loadpolicy(int argc, char **argv);
>  int main_remus(int argc, char **argv);
> +int main_pqosattach(int argc, char **argv);
> +int main_pqosdetach(int argc, char **argv);
>  
>  void help(const char *command);
>  
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 84a604f..1a12e8f 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7195,6 +7195,42 @@ int main_remus(int argc, char **argv)
>      return -ERROR_FAIL;
>  }
>  
> +int main_pqosattach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt, rc;
> +    const char *qosres = NULL;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-attach", 2) {
> +        /* No options */
> +    }
> +
> +    qosres = argv[optind];
> +    domid = find_domain(argv[optind + 1]);
> +
> +    rc = libxl_pqos_attach(ctx, domid, qosres);
> +
> +    return rc;
> +}
> +
> +int main_pqosdetach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt, rc;
> +    const char *qosres = NULL;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-detach", 2) {
> +        /* No options */
> +    }
> +
> +    qosres = argv[optind];
> +    domid = find_domain(argv[optind + 1]);
> +
> +    rc = libxl_pqos_detach(ctx, domid, qosres);
> +
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 326a660..02a2572 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -488,6 +488,16 @@ struct cmd_spec cmd_table[] = {
>        "                        of the domain."
>  
>      },
> +    { "pqos-attach",
> +      &main_pqosattach, 0, 1,
> +      "Allocate and map qos resource",
> +      "<Resource> <Domain>",
> +    },
> +    { "pqos-detach",
> +      &main_pqosdetach, 0, 1,
> +      "Reliquish qos resource",
> +      "<Resource> <Domain>",
> +    },
>  };
>  
>  int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel