xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Allow runtime adjustment to log level thresholds
@ 2016-07-04 15:13 Wei Liu
  2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

Previously Jan posted [0]. The downside of that approach is that we need to
rework log level number space to make it suitable for stable API (safe against
addition and removal of numbers).

This version changes the interface to use buffers that contain string
representation of log level. User space libraries won't care about what is
inside. The parsing and interpretation is up to the hypervisor.

Wei.

[0] <56D9C80702000078000D9910@prv-mh.provo.novell.com>

Wei Liu (5):
  xen/console: consolidate log levels to an array
  xen/console: allow log level threshold adjustments
  libxc: wrapper for log level sysctl
  libxl: introduce APIs to get and set log level
  xl: new loglvl command

 docs/man/xl.pod.1.in                |  22 +++
 tools/flask/policy/modules/dom0.te  |   2 +-
 tools/libxc/include/xenctrl.h       |   6 +
 tools/libxc/xc_misc.c               | 143 ++++++++++++++++
 tools/libxl/libxl.c                 |  40 +++++
 tools/libxl/libxl.h                 |  11 ++
 tools/libxl/xl.h                    |   1 +
 tools/libxl/xl_cmdimpl.c            |  49 ++++++
 tools/libxl/xl_cmdtable.c           |   6 +
 xen/common/sysctl.c                 |   5 +
 xen/drivers/char/console.c          | 314 +++++++++++++++++++++++++++++++++++-
 xen/include/public/sysctl.h         |  41 +++++
 xen/include/xen/console.h           |   2 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 15 files changed, 639 insertions(+), 8 deletions(-)

-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/5] xen/console: consolidate log levels to an array
  2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
@ 2016-07-04 15:13 ` Wei Liu
  2016-07-04 15:28   ` Wei Liu
  2016-07-07 10:39   ` Jan Beulich
  2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu, Jan Beulich

It cleaner than open-coding strings and numbers. The array will also
become handy later when we need to refactor things a bit.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/char/console.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 650035d..6620a1c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -139,6 +139,20 @@ custom_param("guest_loglvl", parse_guest_loglvl);
 
 static atomic_t print_everything = ATOMIC_INIT(0);
 
+struct log_level {
+    const char *str;
+    unsigned int num;
+};
+
+static struct log_level __initdata log_levels[] = {
+    { "none",    0 },
+    { "error",   1 },
+    { "warning", 2 },
+    { "info",    3 },
+    { "debug",   4 },
+    { "all",     4 },
+};
+
 #define ___parse_loglvl(s, ps, lvlstr, lvlnum)          \
     if ( !strncmp((s), (lvlstr), strlen(lvlstr)) ) {    \
         *(ps) = (s) + strlen(lvlstr);                   \
@@ -147,12 +161,11 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 
 static int __init __parse_loglvl(char *s, char **ps)
 {
-    ___parse_loglvl(s, ps, "none",    0);
-    ___parse_loglvl(s, ps, "error",   1);
-    ___parse_loglvl(s, ps, "warning", 2);
-    ___parse_loglvl(s, ps, "info",    3);
-    ___parse_loglvl(s, ps, "debug",   4);
-    ___parse_loglvl(s, ps, "all",     4);
+    unsigned int i;
+
+    for ( i = 0; i < ARRAY_SIZE(log_levels); i++ )
+        ___parse_loglvl(s, ps, log_levels[i].str, log_levels[i].num);
+
     return 2; /* sane fallback */
 }
 
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/5] xen/console: allow log level threshold adjustments
  2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
  2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
@ 2016-07-04 15:13 ` Wei Liu
  2016-07-05 17:52   ` Daniel De Graaf
                     ` (2 more replies)
  2016-07-04 15:13 ` [PATCH v2 3/5] libxc: wrapper for log level sysctl Wei Liu
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Daniel De GRaaf, Wei Liu, Jan Beulich

... from serial console and via sysctl so that one doesn't always need
to reboot to see more / less messages.

Note that upper thresholds driven from the serial console are sticky,
i.e. while they get adjusted upwards when the lower threshold would
otherwise end up above the upper one, they don't get adjusted when
reducing the lower one. Full flexibility is available only via the
sysctl interface.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Rework the sysctl interface to pass in / out strings directly. Provide
some helper functions to transform from log level numbers to strings and
vice verse. Lower and upper bounds are checked. Add XSM hook.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Daniel De GRaaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/flask/policy/modules/dom0.te  |   2 +-
 xen/common/sysctl.c                 |   5 +
 xen/drivers/char/console.c          | 291 +++++++++++++++++++++++++++++++++++-
 xen/include/public/sysctl.h         |  41 +++++
 xen/include/xen/console.h           |   2 +
 xen/xsm/flask/hooks.c               |   3 +
 xen/xsm/flask/policy/access_vectors |   2 +
 7 files changed, 343 insertions(+), 3 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index 2d982d9..4e45e10 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -15,7 +15,7 @@ allow dom0_t xen_t:xen {
 };
 allow dom0_t xen_t:xen2 {
 	resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
-	get_cpu_levelling_caps get_cpu_featureset livepatch_op
+	get_cpu_levelling_caps get_cpu_featureset livepatch_op loglvl
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 55f2077..cf8fa68 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             copyback = 1;
         break;
 
+    case XEN_SYSCTL_loglvl_op:
+        ret = console_loglvl_op(&op->u.loglvl);
+        copyback = 1;
+        break;
+
     default:
         ret = arch_do_sysctl(op, u_sysctl);
         copyback = 0;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 6620a1c..df847c4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -144,7 +144,7 @@ struct log_level {
     unsigned int num;
 };
 
-static struct log_level __initdata log_levels[] = {
+static struct log_level log_levels[] = {
     { "none",    0 },
     { "error",   1 },
     { "warning", 2 },
@@ -152,6 +152,9 @@ static struct log_level __initdata log_levels[] = {
     { "debug",   4 },
     { "all",     4 },
 };
+#define LOG_LEVEL_MIN 0
+#define LOG_LEVEL_MAX 4
+#define LOG_LEVEL_STRLEN_MAX 7  /* "warning" is the longest one */
 
 #define ___parse_loglvl(s, ps, lvlstr, lvlnum)          \
     if ( !strncmp((s), (lvlstr), strlen(lvlstr)) ) {    \
@@ -188,7 +191,41 @@ static void __init parse_guest_loglvl(char *s)
     _parse_loglvl(s, &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh);
 }
 
-static char * __init loglvl_str(int lvl)
+/*
+ * A variant used to parse log level during runtime. Returns -1 if it
+ * fails to parse.
+ */
+static int parse_log_level(const char *s)
+{
+    unsigned int i;
+
+    for ( i = 0; i < ARRAY_SIZE(log_levels); i++ )
+    {
+        if ( !strcmp(log_levels[i].str, s) )
+            return log_levels[i].num;
+    }
+
+    return -1;
+}
+
+static const char *loglvl_to_str(int lvl)
+{
+    unsigned int i;
+
+    if ( lvl < LOG_LEVEL_MIN || lvl > LOG_LEVEL_MAX )
+        return "???";
+
+    /* Multiple log levels can use the same number. Return the most
+     * comprehensive log level string.
+     */
+    for ( i = ARRAY_SIZE(log_levels) - 1; i >= 0; i-- )
+    {
+        if ( log_levels[i].num == lvl )
+            return log_levels[i].str;
+    }
+}
+
+static char *loglvl_str(int lvl)
 {
     switch ( lvl )
     {
@@ -201,6 +238,250 @@ static char * __init loglvl_str(int lvl)
     return "???";
 }
 
+static int *__read_mostly upper_thresh_adj = &xenlog_upper_thresh;
+static int *__read_mostly lower_thresh_adj = &xenlog_lower_thresh;
+static const char *__read_mostly thresh_adj = "standard";
+
+static void do_toggle_guest(unsigned char key, struct cpu_user_regs *regs)
+{
+    if ( upper_thresh_adj == &xenlog_upper_thresh )
+    {
+        upper_thresh_adj = &xenlog_guest_upper_thresh;
+        lower_thresh_adj = &xenlog_guest_lower_thresh;
+        thresh_adj = "guest";
+    }
+    else
+    {
+        upper_thresh_adj = &xenlog_upper_thresh;
+        lower_thresh_adj = &xenlog_lower_thresh;
+        thresh_adj = "standard";
+    }
+    printk("'%c' pressed -> %s log level adjustments enabled\n",
+           key, thresh_adj);
+}
+
+static void do_adj_thresh(unsigned char key)
+{
+    if ( *upper_thresh_adj < *lower_thresh_adj )
+        *upper_thresh_adj = *lower_thresh_adj;
+    printk("'%c' pressed -> %s log level: %s (rate limited %s)\n",
+           key, thresh_adj, loglvl_str(*lower_thresh_adj),
+           loglvl_str(*upper_thresh_adj));
+}
+
+static void do_inc_thresh(unsigned char key, struct cpu_user_regs *regs)
+{
+    if ( *lower_thresh_adj == LOG_LEVEL_MAX )
+        return;
+    ++*lower_thresh_adj;
+    do_adj_thresh(key);
+}
+
+static void do_dec_thresh(unsigned char key, struct cpu_user_regs *regs)
+{
+    if ( *lower_thresh_adj == LOG_LEVEL_MIN )
+        return;
+    --*lower_thresh_adj;
+    do_adj_thresh(key);
+}
+
+static void __putstr(const char *);
+static void printk_start_of_line(const char *);
+
+static void do_loglvl_op(const int lower_thresh, const int upper_thresh,
+                         int *lower, int *upper, const char *which)
+{
+    if ( lower_thresh < 0 && upper_thresh < 0 )
+        return;
+
+    if ( lower_thresh >= 0 )
+        *lower = lower_thresh;
+
+    if ( upper_thresh >= 0 )
+        *upper = upper_thresh;
+
+    if ( *upper < *lower )
+    {
+        if ( upper_thresh < 0 )
+            *upper = *lower;
+        else
+            *lower = *upper;
+    }
+
+    if ( printk_ratelimit() )
+    {
+        spin_lock_irq(&console_lock);
+        printk_start_of_line("(XEN) ");
+        __putstr(which);
+        __putstr(" log level: ");
+        __putstr(loglvl_str(*lower));
+        __putstr(" (rate limited ");
+        __putstr(loglvl_str(*upper));
+        __putstr(")\n");
+        spin_unlock_irq(&console_lock);
+    }
+}
+
+int console_loglvl_op(struct xen_sysctl_loglvl_op *op)
+{
+    int ret;
+
+    switch ( op->cmd )
+    {
+    default:
+        ret = -EOPNOTSUPP;
+        break;
+
+    case XEN_SYSCTL_LOGLVL_set:
+    {
+        char *buf;
+        unsigned int buf_size = 0;
+        int host_lower_thresh, host_upper_thresh;
+        int guest_lower_thresh, guest_upper_thresh;
+
+        buf_size = op->host.lower_thresh_len;
+        if ( buf_size < op->host.upper_thresh_len )
+            buf_size = op->host.upper_thresh_len;
+        if ( buf_size < op->guest.lower_thresh_len )
+            buf_size = op->guest.lower_thresh_len;
+        if ( buf_size < op->guest.upper_thresh_len )
+            buf_size = op->guest.upper_thresh_len;
+
+        buf = xmalloc_array(char, buf_size);
+        if ( !buf )
+        {
+            ret = -ENOMEM;
+            goto set_done;
+        }
+
+#define parse(hg, lu)                                                   \
+        hg##_##lu##_thresh = -1;                                        \
+        if ( op->hg.lu ##_thresh_len )                                  \
+        {                                                               \
+            if ( copy_from_guest(buf, op->hg.lu ##_thresh,              \
+                                 op->hg.lu ##_thresh_len) )             \
+            {                                                           \
+                ret = -EFAULT;                                          \
+                goto set_done;                                          \
+            }                                                           \
+                                                                        \
+            buf[buf_size-1] = 0;                                        \
+                                                                        \
+            ret = parse_log_level(buf);                                 \
+            if ( ret == -1 )                                            \
+            {                                                           \
+                ret = -EINVAL;                                          \
+                goto set_done;                                          \
+            }                                                           \
+                                                                        \
+            hg##_##lu##_thresh = ret;                                   \
+        }
+
+        parse(host,  lower);
+        parse(host,  upper);
+        parse(guest, lower);
+        parse(guest, upper);
+
+#undef parse
+
+        if ( (host_lower_thresh >= 0 && host_upper_thresh >= 0 &&
+              host_lower_thresh > host_upper_thresh) ||
+             (guest_lower_thresh >= 0 && guest_upper_thresh >= 0 &&
+              guest_lower_thresh > guest_upper_thresh) )
+        {
+            ret = -EINVAL;
+            goto set_done;
+        }
+
+        do_loglvl_op(host_lower_thresh, host_upper_thresh,
+                     &xenlog_lower_thresh, &xenlog_upper_thresh,
+                     "standard");
+
+        do_loglvl_op(guest_lower_thresh, guest_upper_thresh,
+                     &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh,
+                     "guest");
+
+        ret = 0;
+    set_done:
+        xfree(buf);
+        break;
+    }
+    case XEN_SYSCTL_LOGLVL_get:
+    {
+        unsigned int host_lower_thresh_len =
+            strlen(loglvl_to_str(xenlog_lower_thresh)) + 1;
+        unsigned int host_upper_thresh_len =
+            strlen(loglvl_to_str(xenlog_upper_thresh)) + 1;
+        unsigned int guest_lower_thresh_len =
+            strlen(loglvl_to_str(xenlog_guest_lower_thresh)) + 1;
+        unsigned int guest_upper_thresh_len =
+            strlen(loglvl_to_str(xenlog_guest_upper_thresh)) + 1;
+        char scratch[LOG_LEVEL_STRLEN_MAX+1];
+
+        if ( (op->host.lower_thresh_len &&
+              op->host.lower_thresh_len < host_lower_thresh_len) ||
+             (op->host.upper_thresh_len &&
+              op->host.upper_thresh_len < host_upper_thresh_len) ||
+             (op->guest.lower_thresh_len &&
+              op->guest.lower_thresh_len < guest_lower_thresh_len) ||
+             (op->guest.upper_thresh_len
+              && op->guest.upper_thresh_len < guest_upper_thresh_len)
+            )
+        {
+            ret = -ENOBUFS;
+            goto get_done;
+        }
+
+        ret = -EFAULT;
+
+        if ( op->host.lower_thresh_len )
+        {
+            snprintf(scratch, sizeof(scratch), "%s",
+                     loglvl_to_str(xenlog_lower_thresh));
+            if ( copy_to_guest(op->host.lower_thresh, scratch,
+                               strlen(scratch)+1) )
+                goto get_done;
+        }
+
+        if ( op->host.upper_thresh_len )
+        {
+            snprintf(scratch, sizeof(scratch), "%s",
+                     loglvl_to_str(xenlog_upper_thresh));
+            if ( copy_to_guest(op->host.upper_thresh, scratch,
+                               strlen(scratch)+1) )
+                goto get_done;
+        }
+
+        if ( op->guest.lower_thresh_len )
+        {
+            snprintf(scratch, sizeof(scratch), "%s",
+                     loglvl_to_str(xenlog_guest_lower_thresh));
+            if ( copy_to_guest(op->guest.lower_thresh, scratch,
+                               strlen(scratch)+1) )
+                goto get_done;
+        }
+
+        if ( op->guest.upper_thresh_len )
+        {
+            snprintf(scratch, sizeof(scratch), "%s",
+                     loglvl_to_str(xenlog_guest_upper_thresh));
+            if ( copy_to_guest(op->guest.upper_thresh, scratch,
+                               strlen(scratch)+1) )
+                goto get_done;
+        }
+
+        ret = 0;
+    get_done:
+        op->host.lower_thresh_len = host_lower_thresh_len;
+        op->host.upper_thresh_len = host_upper_thresh_len;
+        op->guest.lower_thresh_len = guest_lower_thresh_len;
+        op->guest.upper_thresh_len = guest_upper_thresh_len;
+        break;
+    }
+    }
+
+    return ret;
+}
 /*
  * ********************************************************
  * *************** ACCESS TO CONSOLE RING *****************
@@ -829,6 +1110,12 @@ void __init console_endboot(void)
 
     register_keyhandler('w', dump_console_ring_key,
                         "synchronously dump console ring buffer (dmesg)", 0);
+    register_irq_keyhandler('+', &do_inc_thresh,
+                            "increase log level threshold", 0);
+    register_irq_keyhandler('-', &do_dec_thresh,
+                            "decrease log level threshold", 0);
+    register_irq_keyhandler('G', &do_toggle_guest,
+                            "toggle host/guest log level adjustment", 0);
 
     /* Serial input is directed to DOM0 by default. */
     switch_serial_input();
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 8197c14..124a393 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1031,6 +1031,45 @@ struct xen_sysctl_livepatch_op {
 typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
 
+/* XEN_SYSCTL_loglvl_op */
+#define XEN_SYSCTL_LOGLVL_get 0
+#define XEN_SYSCTL_LOGLVL_set 1
+struct xen_sysctl_loglvl_op {
+    uint32_t cmd;        /* XEN_SYSCTL_LOGLVL_* */
+    struct xen_sysctl_loglvl_thresh {
+        /*
+         * IN/OUT variables.
+         *
+         * SET command:
+         *
+         * {lower,upper}_thresh: string representation of lower or
+         * upper threshold of log level.
+         *
+         * {lower,upper}_thresh_len: len == 0 indicates toolstack is
+         * not interested in setting the threshold. len should always
+         * count the trailing 0.
+         *
+         * GET command:
+         *
+         * {lower,upper}_thresh: points to buffer for hypervisor to
+         * fill in.
+         *
+         * {lower,upper}_thresh_len: len == 0 indicates toolstack is
+         * not interested in getting the threshold. For any len != 0,
+         * if len is too small, hypervisor returns -ENOBUFS.  The
+         * actual length of the string representation of threshold
+         * (including trailing 0) is always copied back to to these
+         * fields.
+         */
+        XEN_GUEST_HANDLE_64(char) lower_thresh;
+        XEN_GUEST_HANDLE_64(char) upper_thresh;
+        uint32_t lower_thresh_len;
+        uint32_t upper_thresh_len;
+    } host, guest;
+};
+typedef struct xen_sysctl_loglvl_op xen_sysctl_loglvl_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_loglvl_op_t);
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -1059,6 +1098,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_get_cpu_levelling_caps        25
 #define XEN_SYSCTL_get_cpu_featureset            26
 #define XEN_SYSCTL_livepatch_op                  27
+#define XEN_SYSCTL_loglvl_op                     28
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -1087,6 +1127,7 @@ struct xen_sysctl {
         struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
         struct xen_sysctl_cpu_featureset    cpu_featureset;
         struct xen_sysctl_livepatch_op      livepatch;
+        struct xen_sysctl_loglvl_op         loglvl;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index ea06fd8..ac57f78 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -12,6 +12,8 @@
 
 struct xen_sysctl_readconsole;
 long read_console_ring(struct xen_sysctl_readconsole *op);
+struct xen_sysctl_loglvl_op;
+int console_loglvl_op(struct xen_sysctl_loglvl_op *);
 
 void console_init_preirq(void);
 void console_init_ring(void);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 2692a6f..de6aaa8 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -823,6 +823,9 @@ static int flask_sysctl(int cmd)
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__LIVEPATCH_OP, NULL);
 
+    case XEN_SYSCTL_loglvl_op:
+        return domain_has_xen(current->domain, XEN2__LOGLVL);
+
     default:
         return avc_unknown_permission("sysctl", cmd);
     }
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 49c9a9e..f9d37a2 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -99,6 +99,8 @@ class xen2
     get_cpu_featureset
 # XEN_SYSCTL_livepatch_op
     livepatch_op
+# XEN_SYSCTL_loglvl_op
+    loglvl
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] libxc: wrapper for log level sysctl
  2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
  2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
  2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
@ 2016-07-04 15:13 ` Wei Liu
  2016-07-06 11:11   ` Ian Jackson
  2016-07-04 15:13 ` [PATCH v2 4/5] libxl: introduce APIs to get and set log level Wei Liu
  2016-07-04 15:13 ` [PATCH v2 5/5] xl: new loglvl command Wei Liu
  4 siblings, 1 reply; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Wei Liu, Jan Beulich

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
---
 tools/libxc/include/xenctrl.h |   6 ++
 tools/libxc/xc_misc.c         | 143 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4a85b4a..059c7b4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1179,6 +1179,12 @@ int xc_readconsolering(xc_interface *xch,
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
 
+int xc_get_log_level(xc_interface *xch, bool guest,
+                     char *lower_thresh, unsigned int *lower_thresh_len,
+                     char *upper_thresh, unsigned int *upper_thresh_len);
+int xc_set_log_level(xc_interface *xch, bool guest,
+                     char *lower_thresh, char *upper_thresh);
+
 typedef xen_sysctl_physinfo_t xc_physinfo_t;
 typedef xen_sysctl_cputopo_t xc_cputopo_t;
 typedef xen_sysctl_numainfo_t xc_numainfo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 06e90de..1dbbe9c 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -187,6 +187,149 @@ int xc_send_debug_keys(xc_interface *xch, char *keys)
     return ret;
 }
 
+int xc_get_log_level(xc_interface *xch, bool guest,
+                     char *lower_thresh, unsigned int *lower_thresh_len,
+                     char *upper_thresh, unsigned int *upper_thresh_len)
+{
+    int ret;
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BOUNCE(lower_thresh, *lower_thresh_len,
+                             XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+    DECLARE_HYPERCALL_BOUNCE(upper_thresh, *upper_thresh_len,
+                             XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+
+    if ( (ret = xc_hypercall_bounce_pre(xch, lower_thresh)) )
+        goto out;
+    if ( (ret = xc_hypercall_bounce_pre(xch, upper_thresh)) )
+        goto out;
+
+    sysctl.cmd = XEN_SYSCTL_loglvl_op;
+    sysctl.u.loglvl.cmd = XEN_SYSCTL_LOGLVL_get;
+
+    if ( guest )
+    {
+        set_xen_guest_handle(sysctl.u.loglvl.host.lower_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.host.lower_thresh_len = 0;
+        set_xen_guest_handle(sysctl.u.loglvl.host.upper_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.host.upper_thresh_len = 0;
+
+        set_xen_guest_handle(sysctl.u.loglvl.guest.lower_thresh,
+                             lower_thresh);
+        sysctl.u.loglvl.guest.lower_thresh_len = *lower_thresh_len;
+        set_xen_guest_handle(sysctl.u.loglvl.guest.upper_thresh,
+                             upper_thresh);
+        sysctl.u.loglvl.guest.upper_thresh_len = *upper_thresh_len;
+    }
+    else
+    {
+        set_xen_guest_handle(sysctl.u.loglvl.host.lower_thresh,
+                             lower_thresh);
+        sysctl.u.loglvl.host.lower_thresh_len = *lower_thresh_len;
+        set_xen_guest_handle(sysctl.u.loglvl.host.upper_thresh,
+                             upper_thresh);
+        sysctl.u.loglvl.host.upper_thresh_len = *upper_thresh_len;
+
+        set_xen_guest_handle(sysctl.u.loglvl.guest.lower_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.guest.lower_thresh_len = 0;
+        set_xen_guest_handle(sysctl.u.loglvl.guest.upper_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.guest.upper_thresh_len = 0;
+    }
+
+    ret = do_sysctl(xch, &sysctl);
+
+    if ( guest )
+    {
+        *lower_thresh_len = sysctl.u.loglvl.guest.lower_thresh_len;
+        *upper_thresh_len = sysctl.u.loglvl.guest.upper_thresh_len;
+    }
+    else
+    {
+        *lower_thresh_len = sysctl.u.loglvl.host.lower_thresh_len;
+        *upper_thresh_len = sysctl.u.loglvl.host.upper_thresh_len;
+    }
+
+out:
+    xc_hypercall_bounce_post(xch, lower_thresh);
+    xc_hypercall_bounce_post(xch, upper_thresh);
+    return ret;
+}
+
+int xc_set_log_level(xc_interface *xch, bool guest,
+                     char *lower_thresh, char *upper_thresh)
+{
+    int ret;
+    unsigned int lower_thresh_len = 0, upper_thresh_len = 0;
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BOUNCE(lower_thresh, 0 /* later */,
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    DECLARE_HYPERCALL_BOUNCE(upper_thresh, 0 /* later */,
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if (!lower_thresh && !upper_thresh)
+        return 0;
+
+    sysctl.cmd = XEN_SYSCTL_loglvl_op;
+    sysctl.u.loglvl.cmd = XEN_SYSCTL_LOGLVL_set;
+
+    if (lower_thresh) {
+        lower_thresh_len = strlen(lower_thresh) + 1;
+        HYPERCALL_BOUNCE_SET_SIZE(lower_thresh, lower_thresh_len);
+    }
+
+    if (upper_thresh) {
+        upper_thresh_len = strlen(upper_thresh) + 1;
+        HYPERCALL_BOUNCE_SET_SIZE(upper_thresh, upper_thresh_len);
+    }
+
+    if ( (ret = xc_hypercall_bounce_pre(xch, lower_thresh)) )
+        goto out;
+    if ( (ret = xc_hypercall_bounce_pre(xch, upper_thresh)) )
+        goto out;
+
+    if ( guest )
+    {
+        set_xen_guest_handle(sysctl.u.loglvl.host.lower_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.host.lower_thresh_len = 0;
+        set_xen_guest_handle(sysctl.u.loglvl.host.upper_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.host.upper_thresh_len = 0;
+        set_xen_guest_handle(sysctl.u.loglvl.guest.lower_thresh,
+                             lower_thresh);
+        sysctl.u.loglvl.guest.lower_thresh_len = lower_thresh_len;
+        set_xen_guest_handle(sysctl.u.loglvl.guest.upper_thresh,
+                             upper_thresh);
+        sysctl.u.loglvl.guest.upper_thresh_len = upper_thresh_len;
+    }
+    else
+    {
+        set_xen_guest_handle(sysctl.u.loglvl.host.lower_thresh,
+                             lower_thresh);
+        sysctl.u.loglvl.host.lower_thresh_len = lower_thresh_len;
+        set_xen_guest_handle(sysctl.u.loglvl.host.upper_thresh,
+                             upper_thresh);
+        sysctl.u.loglvl.host.upper_thresh_len = upper_thresh_len;
+
+        set_xen_guest_handle(sysctl.u.loglvl.guest.lower_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.guest.lower_thresh_len = 0;
+        set_xen_guest_handle(sysctl.u.loglvl.guest.upper_thresh,
+                             HYPERCALL_BUFFER_NULL);
+        sysctl.u.loglvl.guest.upper_thresh_len = 0;
+    }
+
+    ret = do_sysctl(xch, &sysctl);
+
+out:
+    xc_hypercall_bounce_post(xch, lower_thresh);
+    xc_hypercall_bounce_post(xch, upper_thresh);
+    return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
                 xc_physinfo_t *put_info)
 {
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/5] libxl: introduce APIs to get and set log level
  2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
                   ` (2 preceding siblings ...)
  2016-07-04 15:13 ` [PATCH v2 3/5] libxc: wrapper for log level sysctl Wei Liu
@ 2016-07-04 15:13 ` Wei Liu
  2016-07-06 11:15   ` Ian Jackson
  2016-07-04 15:13 ` [PATCH v2 5/5] xl: new loglvl command Wei Liu
  4 siblings, 1 reply; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Wei Liu, Jan Beulich

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
---
 tools/libxl/libxl.c | 40 ++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h | 11 +++++++++++
 2 files changed, 51 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..ff70af7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6599,6 +6599,46 @@ int libxl_send_debug_keys(libxl_ctx *ctx, char *keys)
     return 0;
 }
 
+int libxl_set_log_level(libxl_ctx *ctx, bool guest,
+                        char *lower_thresh, char *upper_thresh)
+{
+    int rc, ret;
+    GC_INIT(ctx);
+
+    ret = xc_set_log_level(ctx->xch, guest, lower_thresh, upper_thresh);
+    if (ret) {
+        LOGE(ERROR, "unable to set log level");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    GC_FREE;
+    return rc;
+}
+
+int libxl_get_log_level(libxl_ctx *ctx, bool guest,
+                        char *lower_thresh, unsigned int *lower_thresh_bufsize,
+                        char *upper_thresh, unsigned int *upper_thresh_bufsize)
+{
+    int rc, ret;
+    GC_INIT(ctx);
+
+    ret = xc_get_log_level(ctx->xch, guest, lower_thresh, lower_thresh_bufsize,
+                           upper_thresh, upper_thresh_bufsize);
+    if (ret) {
+        LOGE(ERROR, "unable to get log level");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    GC_FREE;
+    return rc;
+}
+
 libxl_xen_console_reader *
     libxl_xen_console_read_start(libxl_ctx *ctx, int clear)
 {
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2c0f868..c460d11 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,12 @@
  * the same $(XEN_VERSION) (e.g. throughout a major release).
  */
 
+/* LIBXL_HAVE_GET_SET_LOG_LEVEL
+ *
+ * If this is defined libxl has a pair of APIs to get and set log levels
+ */
+#define LIBXL_HAVE_GET_SET_LOG_LEVEL 1
+
 /* LIBXL_HAVE_VNUMA
  *
  * If this is defined the type libxl_vnode_info exists, and a
@@ -1951,6 +1957,11 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
+int libxl_set_log_level(libxl_ctx *ctx, bool guest,
+                        char *lower_thresh, char *upper_thresh);
+int libxl_get_log_level(libxl_ctx *ctx, bool guest,
+                        char *lower_thresh, unsigned int *lower_thresh_bufsize,
+                        char *upper_thresh, unsigned int *upper_thresh_bufsize);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/5] xl: new loglvl command
  2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
                   ` (3 preceding siblings ...)
  2016-07-04 15:13 ` [PATCH v2 4/5] libxl: introduce APIs to get and set log level Wei Liu
@ 2016-07-04 15:13 ` Wei Liu
  4 siblings, 0 replies; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Wei Liu, Jan Beulich

Introduce a new command to dynamically change log level thresholds.
Provide adequate documentation.

Based on a patch by Jan Beulich.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 docs/man/xl.pod.1.in      | 22 +++++++++++++++++++++
 tools/libxl/xl.h          |  1 +
 tools/libxl/xl_cmdimpl.c  | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/xl_cmdtable.c |  6 ++++++
 4 files changed, 78 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index c1e6b7f..9abd06a 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -810,6 +810,28 @@ Pass VNC password to vncviewer via stdin.
 Send debug I<keys> to Xen. It is the same as pressing the Xen
 "conswitch" (Ctrl-A by default) three times and then pressing "keys".
 
+=item B<loglvl> [B<-g>] [B<-s [lower_threshold][/upper_threshold]>]
+
+Get and set the log level thresholds.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-g>, B<--guest>
+
+Select guest log levels instead of host wide log levels.
+
+=item B<-s [lower_threshold][/upper_threshold]> B<--set=[lower_threshold][/upper_threshold]>
+
+Set the lower threshold to B<lower_threshold> and optionally set upper
+threshold to B<upper_threshold>.
+
+If this option is not specified, the command prints the current lower
+threshold and upper threshold.
+
+=back
+
 =item B<dmesg> [B<-c>]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index e601ca1..4bf16ee 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -80,6 +80,7 @@ int main_rename(int argc, char **argv);
 int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
+int main_loglvl(int argc, char **argv);
 int main_dmesg(int argc, char **argv);
 int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6459eec..b01bdaa 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7160,6 +7160,55 @@ int main_debug_keys(int argc, char **argv)
     return EXIT_SUCCESS;
 }
 
+int main_loglvl(int argc, char **argv)
+{
+    int opt;
+    bool guest = false, set = false;
+    char *lower_thresh = NULL, *upper_thresh = NULL, *delim = NULL;
+    char lower_thresh_out[16], upper_thresh_out[16];
+    unsigned int llen, ulen;
+    int rc;
+    static const struct option opts[] = {
+        {"guest", 0, 0, 'g'},
+        {"set", 0, 0, 's'},
+        COMMON_LONG_OPTS
+    };
+
+    SWITCH_FOREACH_OPT(opt, "gs:", opts, "loglvl", 0) {
+    case 'g':
+        guest = true;
+        break;
+    case 's':
+        delim = strstr(optarg, "/");
+
+        if (delim) {
+            if (delim != optarg)
+                lower_thresh = optarg;
+            *delim = 0;
+            upper_thresh = ++delim;
+        } else {
+            lower_thresh = optarg;
+        }
+
+        set = true;
+        break;
+    }
+
+    if (set)
+        rc = libxl_set_log_level(ctx, guest, lower_thresh, upper_thresh);
+    else {
+        llen = sizeof(lower_thresh_out);
+        ulen = sizeof(upper_thresh_out);
+        rc = libxl_get_log_level(ctx, guest, lower_thresh_out, &llen,
+                                 upper_thresh_out, &ulen);
+        if (!rc)
+            printf("%s log levels: %s/%s\n", guest ? "guest" : "host",
+                   lower_thresh_out, upper_thresh_out);
+    }
+
+    return rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
 int main_dmesg(int argc, char **argv)
 {
     unsigned int clear = 0;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index bf69ffb..3f3e996 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -583,6 +583,12 @@ struct cmd_spec cmd_table[] = {
       "List information about all USB controllers and devices for a domain",
       "<Domain>",
     },
+    { "loglvl",
+      &main_loglvl, 0, 1,
+      "[-g] [-s=[LOWER]/[UPPER]]",
+      "-g,                 --guest                 act on guest log level\n"
+      "-s [LOWER][/UPPER], --set=[LOWER][/UPPER]   set new log level\n"
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] xen/console: consolidate log levels to an array
  2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
@ 2016-07-04 15:28   ` Wei Liu
  2016-07-07 10:39   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Wei Liu @ 2016-07-04 15:28 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu, Jan Beulich

On Mon, Jul 04, 2016 at 04:13:22PM +0100, Wei Liu wrote:
> It cleaner than open-coding strings and numbers. The array will also
> become handy later when we need to refactor things a bit.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/drivers/char/console.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 650035d..6620a1c 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -139,6 +139,20 @@ custom_param("guest_loglvl", parse_guest_loglvl);
>  
>  static atomic_t print_everything = ATOMIC_INIT(0);
>  
> +struct log_level {
> +    const char *str;
> +    unsigned int num;

This should be int instead of unsigned int.

Wei.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] xen/console: allow log level threshold adjustments
  2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
@ 2016-07-05 17:52   ` Daniel De Graaf
  2016-07-06 11:14   ` Ian Jackson
  2016-07-07 11:51   ` Jan Beulich
  2 siblings, 0 replies; 13+ messages in thread
From: Daniel De Graaf @ 2016-07-05 17:52 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Ian Jackson, Jan Beulich

On 07/04/2016 11:13 AM, Wei Liu wrote:
> ... from serial console and via sysctl so that one doesn't always need
> to reboot to see more / less messages.
>
> Note that upper thresholds driven from the serial console are sticky,
> i.e. while they get adjusted upwards when the lower threshold would
> otherwise end up above the upper one, they don't get adjusted when
> reducing the lower one. Full flexibility is available only via the
> sysctl interface.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Rework the sysctl interface to pass in / out strings directly. Provide
> some helper functions to transform from log level numbers to strings and
> vice verse. Lower and upper bounds are checked. Add XSM hook.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] libxc: wrapper for log level sysctl
  2016-07-04 15:13 ` [PATCH v2 3/5] libxc: wrapper for log level sysctl Wei Liu
@ 2016-07-06 11:11   ` Ian Jackson
  0 siblings, 0 replies; 13+ messages in thread
From: Ian Jackson @ 2016-07-06 11:11 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Jan Beulich

Wei Liu writes ("[PATCH v2 3/5] libxc: wrapper for log level sysctl"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

I have no objection to this new wrapper but the actual code is very
tedious.  Is this really the least repetitive way of writing this ?

Ian.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] xen/console: allow log level threshold adjustments
  2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
  2016-07-05 17:52   ` Daniel De Graaf
@ 2016-07-06 11:14   ` Ian Jackson
  2016-07-07 11:51   ` Jan Beulich
  2 siblings, 0 replies; 13+ messages in thread
From: Ian Jackson @ 2016-07-06 11:14 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Daniel De GRaaf, Jan Beulich

Wei Liu writes ("[PATCH v2 2/5] xen/console: allow log level threshold adjustments"):
> ... from serial console and via sysctl so that one doesn't always need
> to reboot to see more / less messages.
> 
> Note that upper thresholds driven from the serial console are sticky,
> i.e. while they get adjusted upwards when the lower threshold would
> otherwise end up above the upper one, they don't get adjusted when
> reducing the lower one. Full flexibility is available only via the
> sysctl interface.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Rework the sysctl interface to pass in / out strings directly. Provide
> some helper functions to transform from log level numbers to strings and
> vice verse. Lower and upper bounds are checked. Add XSM hook.

Is this really the best way to do this ?  This is an awful lot of
hypervisor code.  Perhaps instead there should be a way to request the
table of level strings, and userspace could do the conversion ?

Or, given that this is a sysctl, so does not need a stable ABI, the
table of loglevel strings could be provided in the .h file in the form
of a suitable lifted list #define, and at runtime be implicit in the
ABI version.

Ian.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 4/5] libxl: introduce APIs to get and set log level
  2016-07-04 15:13 ` [PATCH v2 4/5] libxl: introduce APIs to get and set log level Wei Liu
@ 2016-07-06 11:15   ` Ian Jackson
  0 siblings, 0 replies; 13+ messages in thread
From: Ian Jackson @ 2016-07-06 11:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Jan Beulich

Wei Liu writes ("[PATCH v2 4/5] libxl: introduce APIs to get and set log level"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
...
> +int libxl_set_log_level(libxl_ctx *ctx, bool guest,
> +                        char *lower_thresh, char *upper_thresh)

I think the log level names should be exposed as a list to the
caller.  It might be better to allow the caller to specify an integer.

Ian.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] xen/console: consolidate log levels to an array
  2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
  2016-07-04 15:28   ` Wei Liu
@ 2016-07-07 10:39   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-07-07 10:39 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel

>>> On 04.07.16 at 17:13, <wei.liu2@citrix.com> wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -139,6 +139,20 @@ custom_param("guest_loglvl", parse_guest_loglvl);
>  
>  static atomic_t print_everything = ATOMIC_INIT(0);
>  
> +struct log_level {
> +    const char *str;
> +    unsigned int num;
> +};
> +
> +static struct log_level __initdata log_levels[] = {

const ... __initconstrel

But the question really is, as Ian had already indicated, whether this
can't be solved with less hypervisor changes (in the second patch).
And jftr - I did consider passing strings as part of the hypercall
arguments, but I generally dislike non-human interfaces taking human
readable strings.

Jan


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] xen/console: allow log level threshold adjustments
  2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
  2016-07-05 17:52   ` Daniel De Graaf
  2016-07-06 11:14   ` Ian Jackson
@ 2016-07-07 11:51   ` Jan Beulich
  2 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-07-07 11:51 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, DanielDe GRaaf, Ian Jackson

>>> On 04.07.16 at 17:13, <wei.liu2@citrix.com> wrote:

In case this (or a variant thereof) is to be used despite the earlier
voiced concerns, a couple of mechanical comments:

> +static const char *loglvl_to_str(int lvl)
> +{
> +    unsigned int i;
> +
> +    if ( lvl < LOG_LEVEL_MIN || lvl > LOG_LEVEL_MAX )
> +        return "???";
> +
> +    /* Multiple log levels can use the same number. Return the most
> +     * comprehensive log level string.
> +     */

Comment style.

> +    for ( i = ARRAY_SIZE(log_levels) - 1; i >= 0; i-- )

Possibly infinite loop - i is unsigned and hence always non-negative.

> +int console_loglvl_op(struct xen_sysctl_loglvl_op *op)
> +{
> +    int ret;
> +
> +    switch ( op->cmd )
> +    {
> +    default:
> +        ret = -EOPNOTSUPP;
> +        break;
> +
> +    case XEN_SYSCTL_LOGLVL_set:
> +    {
> +        char *buf;
> +        unsigned int buf_size = 0;

Pointless initializer.

> +        int host_lower_thresh, host_upper_thresh;
> +        int guest_lower_thresh, guest_upper_thresh;
> +
> +        buf_size = op->host.lower_thresh_len;
> +        if ( buf_size < op->host.upper_thresh_len )
> +            buf_size = op->host.upper_thresh_len;
> +        if ( buf_size < op->guest.lower_thresh_len )
> +            buf_size = op->guest.lower_thresh_len;
> +        if ( buf_size < op->guest.upper_thresh_len )
> +            buf_size = op->guest.upper_thresh_len;
> +
> +        buf = xmalloc_array(char, buf_size);
> +        if ( !buf )
> +        {
> +            ret = -ENOMEM;
> +            goto set_done;
> +        }
> +
> +#define parse(hg, lu)                                                   \
> +        hg##_##lu##_thresh = -1;                                        \
> +        if ( op->hg.lu ##_thresh_len )                                  \
> +        {                                                               \
> +            if ( copy_from_guest(buf, op->hg.lu ##_thresh,              \
> +                                 op->hg.lu ##_thresh_len) )             \
> +            {                                                           \
> +                ret = -EFAULT;                                          \
> +                goto set_done;                                          \
> +            }                                                           \
> +                                                                        \
> +            buf[buf_size-1] = 0;                                        \
> +                                                                        \
> +            ret = parse_log_level(buf);                                 \
> +            if ( ret == -1 )                                            \
> +            {                                                           \
> +                ret = -EINVAL;                                          \
> +                goto set_done;                                          \
> +            }                                                           \
> +                                                                        \
> +            hg##_##lu##_thresh = ret;                                   \
> +        }
> +
> +        parse(host,  lower);
> +        parse(host,  upper);
> +        parse(guest, lower);
> +        parse(guest, upper);
> +
> +#undef parse
> +
> +        if ( (host_lower_thresh >= 0 && host_upper_thresh >= 0 &&
> +              host_lower_thresh > host_upper_thresh) ||
> +             (guest_lower_thresh >= 0 && guest_upper_thresh >= 0 &&
> +              guest_lower_thresh > guest_upper_thresh) )
> +        {
> +            ret = -EINVAL;
> +            goto set_done;
> +        }
> +
> +        do_loglvl_op(host_lower_thresh, host_upper_thresh,
> +                     &xenlog_lower_thresh, &xenlog_upper_thresh,
> +                     "standard");
> +
> +        do_loglvl_op(guest_lower_thresh, guest_upper_thresh,
> +                     &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh,
> +                     "guest");
> +
> +        ret = 0;
> +    set_done:
> +        xfree(buf);
> +        break;
> +    }
> +    case XEN_SYSCTL_LOGLVL_get:

Blank line ahead of the case label please.

> +    {
> +        unsigned int host_lower_thresh_len =
> +            strlen(loglvl_to_str(xenlog_lower_thresh)) + 1;
> +        unsigned int host_upper_thresh_len =
> +            strlen(loglvl_to_str(xenlog_upper_thresh)) + 1;
> +        unsigned int guest_lower_thresh_len =
> +            strlen(loglvl_to_str(xenlog_guest_lower_thresh)) + 1;
> +        unsigned int guest_upper_thresh_len =
> +            strlen(loglvl_to_str(xenlog_guest_upper_thresh)) + 1;
> +        char scratch[LOG_LEVEL_STRLEN_MAX+1];
> +
> +        if ( (op->host.lower_thresh_len &&
> +              op->host.lower_thresh_len < host_lower_thresh_len) ||
> +             (op->host.upper_thresh_len &&
> +              op->host.upper_thresh_len < host_upper_thresh_len) ||
> +             (op->guest.lower_thresh_len &&
> +              op->guest.lower_thresh_len < guest_lower_thresh_len) ||
> +             (op->guest.upper_thresh_len
> +              && op->guest.upper_thresh_len < guest_upper_thresh_len)
> +            )
> +        {
> +            ret = -ENOBUFS;
> +            goto get_done;
> +        }
> +
> +        ret = -EFAULT;
> +
> +        if ( op->host.lower_thresh_len )
> +        {
> +            snprintf(scratch, sizeof(scratch), "%s",
> +                     loglvl_to_str(xenlog_lower_thresh));
> +            if ( copy_to_guest(op->host.lower_thresh, scratch,
> +                               strlen(scratch)+1) )
> +                goto get_done;

Why the indirection through snprintf()?

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1031,6 +1031,45 @@ struct xen_sysctl_livepatch_op {
>  typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
>  
> +/* XEN_SYSCTL_loglvl_op */
> +#define XEN_SYSCTL_LOGLVL_get 0
> +#define XEN_SYSCTL_LOGLVL_set 1
> +struct xen_sysctl_loglvl_op {
> +    uint32_t cmd;        /* XEN_SYSCTL_LOGLVL_* */
> +    struct xen_sysctl_loglvl_thresh {

Explicit padding please ahead of this struct.

Jan

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-07-07 11:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 15:13 [PATCH v2 0/5] Allow runtime adjustment to log level thresholds Wei Liu
2016-07-04 15:13 ` [PATCH v2 1/5] xen/console: consolidate log levels to an array Wei Liu
2016-07-04 15:28   ` Wei Liu
2016-07-07 10:39   ` Jan Beulich
2016-07-04 15:13 ` [PATCH v2 2/5] xen/console: allow log level threshold adjustments Wei Liu
2016-07-05 17:52   ` Daniel De Graaf
2016-07-06 11:14   ` Ian Jackson
2016-07-07 11:51   ` Jan Beulich
2016-07-04 15:13 ` [PATCH v2 3/5] libxc: wrapper for log level sysctl Wei Liu
2016-07-06 11:11   ` Ian Jackson
2016-07-04 15:13 ` [PATCH v2 4/5] libxl: introduce APIs to get and set log level Wei Liu
2016-07-06 11:15   ` Ian Jackson
2016-07-04 15:13 ` [PATCH v2 5/5] xl: new loglvl command Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).