[PATCH v2 0/2] add xl command to get hypervisor .config

[PATCH v2 0/2] add xl command to get hypervisor .config

[Juergen Gross]

Add "xl get-hypervisor-config" printing the .config used to build the
currently running hypervisor.

Juergen Gross (2):
  xen: add interface for obtaining .config from hypervisor
  tools: add new xl command get-hypervisor-config

 .gitignore                          |  2 ++
 docs/man/xl.1.pod.in                |  5 +++++
 tools/flask/policy/modules/dom0.te  |  2 +-
 tools/libxc/include/xenctrl.h       |  8 +++++++
 tools/libxc/include/xenguest.h      | 13 ++++++++++++
 tools/libxc/xc_misc.c               | 42 +++++++++++++++++++++++++++++++++++++
 tools/libxc/xg_private.h            |  4 ----
 tools/libxl/libxl.c                 | 34 ++++++++++++++++++++++++++++++
 tools/libxl/libxl.h                 |  8 +++++++
 tools/xl/xl.h                       |  1 +
 tools/xl/xl_cmdtable.c              |  5 +++++
 tools/xl/xl_misc.c                  | 20 ++++++++++++++++++
 xen/common/Makefile                 |  7 +++++++
 xen/common/sysctl.c                 | 17 +++++++++++++++
 xen/include/public/sysctl.h         | 18 +++++++++++++++-
 xen/include/xen/kernel.h            |  3 +++
 xen/tools/Makefile                  |  9 ++++++--
 xen/tools/bin2c.c                   | 28 +++++++++++++++++++++++++
 xen/xsm/flask/hooks.c               |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 20 files changed, 223 insertions(+), 8 deletions(-)
 create mode 100644 xen/tools/bin2c.c

-- 
2.16.4


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

[PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross]

Add a sysctl interface for obtaining the .config file used to build
the hypervisor. The mechanism is inspired by the Linux kernel's one.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)
---
V2:
- bump sysctl interface version
- check pad to be zero (Wei Liu)
- only copy data if buffer is large enough (Wei Liu)
- add .gitignore entry at correct position (Wei Liu)
- make xen_config_data_sz const (Jan Beulich)
---
 .gitignore                          |  2 ++
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/common/Makefile                 |  7 +++++++
 xen/common/sysctl.c                 | 17 +++++++++++++++++
 xen/include/public/sysctl.h         | 18 +++++++++++++++++-
 xen/include/xen/kernel.h            |  3 +++
 xen/tools/Makefile                  |  9 +++++++--
 xen/tools/bin2c.c                   | 28 ++++++++++++++++++++++++++++
 xen/xsm/flask/hooks.c               |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 10 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 xen/tools/bin2c.c

diff --git a/.gitignore b/.gitignore
index 26bc583f74..b433bce092 100644
--- a/.gitignore
+++ b/.gitignore
@@ -309,6 +309,7 @@ xen/arch/*/efi/boot.c
 xen/arch/*/efi/compat.c
 xen/arch/*/efi/efi.h
 xen/arch/*/efi/runtime.c
+xen/common/config_data.c
 xen/include/headers*.chk
 xen/include/asm
 xen/include/asm-*/asm-offsets.h
@@ -326,6 +327,7 @@ xen/test/livepatch/xen_bye_world.livepatch
 xen/test/livepatch/xen_hello_world.livepatch
 xen/test/livepatch/xen_nop.livepatch
 xen/test/livepatch/xen_replace_world.livepatch
+xen/tools/bin2c
 xen/tools/kconfig/.tmp_gtkcheck
 xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index a347d664f8..b776e9f307 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
 	resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
 	get_cpu_levelling_caps get_cpu_featureset livepatch_op
-	coverage_op set_parameter
+	coverage_op set_parameter get_config
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/Makefile b/xen/common/Makefile
index bca48e6e22..7d98dad478 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_ARGO) += argo.o
 obj-y += bitmap.o
 obj-y += bsearch.o
+obj-y += config_data.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-y += cpupool.o
@@ -84,3 +85,9 @@ subdir-$(CONFIG_UBSAN) += ubsan
 
 subdir-$(CONFIG_NEEDS_LIBELF) += libelf
 subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
+
+config_data.c: ../.config
+	( echo "const char xen_config_data[] ="; \
+	  cat $< | gzip | ../tools/bin2c; \
+	  echo ";"; \
+	  echo "const unsigned int xen_config_data_sz = sizeof(xen_config_data) - 1;" ) > $@
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..7d4329882d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -13,6 +13,7 @@
 #include <xen/domain.h>
 #include <xen/event.h>
 #include <xen/domain_page.h>
+#include <xen/kernel.h>
 #include <xen/tmem.h>
 #include <xen/trace.h>
 #include <xen/console.h>
@@ -502,6 +503,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         break;
     }
 
+    case XEN_SYSCTL_get_config:
+    {
+        if ( op->u.get_config.pad )
+        {
+            ret = -EINVAL;
+            break;
+        }
+        if ( xen_config_data_sz <= op->u.get_config.size &&
+             copy_to_guest(op->u.get_config.buffer, xen_config_data,
+                           xen_config_data_sz) )
+            ret = -EFAULT;
+        op->u.get_config.size = xen_config_data_sz;
+
+        break;
+    }
+
     default:
         ret = arch_do_sysctl(op, u_sysctl);
         copyback = 0;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c49b4dcc99..6139321971 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013
 
 /*
  * Read console content from Xen buffer ring.
@@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
 #endif
 
+/*
+ * XEN_SYSCTL_get_config
+ *
+ * Return gzip-ed .config file
+ */
+struct xen_sysctl_get_config {
+    XEN_GUEST_HANDLE_64(char) buffer;   /* IN: pointer to buffer. */
+    uint32_t size;                      /* IN: size of buffer. */
+                                        /* OUT: size of config data. */
+    uint32_t pad;                       /* IN: MUST be zero. */
+};
+typedef struct xen_sysctl_get_config xen_sysctl_get_config_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_get_config_t);
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -1130,6 +1144,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_livepatch_op                  27
 #define XEN_SYSCTL_set_parameter                 28
 #define XEN_SYSCTL_get_cpu_policy                29
+#define XEN_SYSCTL_get_config                    30
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -1162,6 +1177,7 @@ struct xen_sysctl {
 #if defined(__i386__) || defined(__x86_64__)
         struct xen_sysctl_cpu_policy        cpu_policy;
 #endif
+        struct xen_sysctl_get_config        get_config;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64da9f..043a401659 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -100,5 +100,8 @@ extern enum system_state {
 
 bool_t is_active_kernel_text(unsigned long addr);
 
+extern const char xen_config_data[];
+extern unsigned int xen_config_data_sz;
+
 #endif /* _LINUX_KERNEL_H */
 
diff --git a/xen/tools/Makefile b/xen/tools/Makefile
index e940939d61..cd2bbbf647 100644
--- a/xen/tools/Makefile
+++ b/xen/tools/Makefile
@@ -1,13 +1,18 @@
 
 include $(XEN_ROOT)/Config.mk
 
+PROGS = symbols bin2c
+
 .PHONY: default
 default:
-	$(MAKE) symbols
+	$(MAKE) $(PROGS)
 
 .PHONY: clean
 clean:
-	rm -f *.o symbols
+	rm -f *.o $(PROGS)
 
 symbols: symbols.c
 	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<
+
+bin2c: bin2c.c
+	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<
diff --git a/xen/tools/bin2c.c b/xen/tools/bin2c.c
new file mode 100644
index 0000000000..c332399b70
--- /dev/null
+++ b/xen/tools/bin2c.c
@@ -0,0 +1,28 @@
+/*
+ * Unloved program to convert a binary on stdin to a C include on stdout
+ *
+ * Jan 1999 Matt Mackall <mpm@selenic.com>
+ *
+ * This software may be used and distributed according to the terms
+ * of the GNU General Public License, incorporated herein by reference.
+ */
+
+#include <stdio.h>
+
+int main(int argc, char *argv[])
+{
+	int ch, total = 0;
+
+	do {
+		printf("\t\"");
+		while ((ch = getchar()) != EOF) {
+			total++;
+			printf("\\x%02x", ch);
+			if (total % 16 == 0)
+				break;
+		}
+		printf("\"\n");
+	} while (ch != EOF);
+
+	return 0;
+}
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 3d00c747f6..1f3fa6ea56 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -830,6 +830,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_set_parameter:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__SET_PARAMETER, NULL);
+    case XEN_SYSCTL_get_config:
+        return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+                                    XEN2__GET_CONFIG, NULL);
 
     default:
         return avc_unknown_permission("sysctl", cmd);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index e00448b776..d560fdc463 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -103,6 +103,8 @@ class xen2
     coverage_op
 # XEN_SYSCTL_set_parameter
     set_parameter
+# XEN_SYSCTL_get_config
+    get_config
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.16.4


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

[PATCH v2 2/2] tools: add new xl command get-hypervisor-config

[Juergen Gross]

Add   new subcommand "get-hypervisor-config" to xl config to print the
hypervisor .config file.

To be able to reuse already existing decompressing code in libxenguest
xc_inflate_buffer() has to be moved to libxenguest.h.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- rename subcommand to get-hypervisor-config (Wei Liu)
- use goto style error handling (Wei Liu)
---
 docs/man/xl.1.pod.in           |  5 +++++
 tools/libxc/include/xenctrl.h  |  8 ++++++++
 tools/libxc/include/xenguest.h | 13 +++++++++++++
 tools/libxc/xc_misc.c          | 42 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxc/xg_private.h       |  4 ----
 tools/libxl/libxl.c            | 34 ++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h            |  8 ++++++++
 tools/xl/xl.h                  |  1 +
 tools/xl/xl_cmdtable.c         |  5 +++++
 tools/xl/xl_misc.c             | 20 ++++++++++++++++++++
 10 files changed, 136 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index 4310fcd818..9d20958e91 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -844,6 +844,11 @@ Clears Xen's message buffer.
 
 =back
 
+=item B<get-hypervisor-config>
+
+Print the software configuration file (.config) used to build the
+hypervisor.
+
 =item B<info> [I<OPTIONS>]
 
 Print information about the Xen host in I<name : value> format.  When
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a3628e56bb..c6a203e1a4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2632,6 +2632,14 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
 int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
                          xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
 
+/*
+ * Get gzip-ed .config from hypervisor.
+ * *buffer must be free()-ed by caller.
+ * data size is returned in `size`.
+ * Returns 0 on success.
+ */
+int xc_get_config(xc_interface *xch, char **buffer, unsigned long *size);
+
 /* Compat shims */
 #include "xenctrl_compat.h"
 
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index b4b2e19619..76e87ea97c 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -310,4 +310,17 @@ xen_pfn_t *xc_map_m2p(xc_interface *xch,
                       unsigned long max_mfn,
                       int prot,
                       unsigned long *mfn0);
+
+/**
+ * Decompress a gzip-ed stream.
+ * @parm xch a handle to an open hypervisor interface
+ * @parm in_buf buffer holding the gzip-ed data
+ * @parm in_size size in bytes of the gzip-ed data
+ * @parm out_size where to store the gunzip-ed data length
+ * @return new allocated buffer holding the gunzip-ed data
+ */
+char *xc_inflate_buffer(xc_interface *xch,
+                        const char *in_buf,
+                        unsigned long in_size,
+                        unsigned long *out_size);
 #endif /* XENGUEST_H */
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 5e6714ae2b..83d259e46e 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -888,6 +888,48 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout)
     return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout);
 }
 
+int xc_get_config(xc_interface *xch, char **buffer, unsigned long *size)
+{
+    int rc;
+    DECLARE_SYSCTL;
+    DECLARE_HYPERCALL_BUFFER(char, buf);
+
+    sysctl.cmd = XEN_SYSCTL_get_config;
+    sysctl.u.get_config.size = 0;
+    set_xen_guest_handle(sysctl.u.get_config.buffer, HYPERCALL_BUFFER_NULL);
+    rc = do_sysctl(xch, &sysctl);
+    if ( rc )
+        return rc;
+
+    *size = sysctl.u.get_config.size;
+    buf = xc_hypercall_buffer_alloc(xch, buf, *size);
+    if ( !buf )
+    {
+        errno = ENOMEM;
+        return -1;
+    }
+
+    sysctl.cmd = XEN_SYSCTL_get_config;
+    sysctl.u.get_config.size = *size;
+    set_xen_guest_handle(sysctl.u.get_config.buffer, buf);
+    rc = do_sysctl(xch, &sysctl);
+
+    if ( rc )
+        goto out;
+
+    *buffer = calloc(1, *size);
+    if ( !*buffer )
+    {
+        errno = ENOMEM;
+        goto out;
+    }
+    memmove(*buffer, buf, *size);
+
+out:
+    xc_hypercall_buffer_free(xch, buf);
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h
index f0a4b2c616..ca85e10737 100644
--- a/tools/libxc/xg_private.h
+++ b/tools/libxc/xg_private.h
@@ -43,10 +43,6 @@
 
 char *xc_read_image(xc_interface *xch,
                     const char *filename, unsigned long *size);
-char *xc_inflate_buffer(xc_interface *xch,
-                        const char *in_buf,
-                        unsigned long in_size,
-                        unsigned long *out_size);
 
 unsigned long csum_page (void * page);
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..e363371811 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -669,6 +669,40 @@ int libxl_set_parameters(libxl_ctx *ctx, char *params)
     return 0;
 }
 
+int libxl_get_hypervisor_config(libxl_ctx *ctx, char **buffer)
+{
+    int ret;
+    unsigned long gz_size, out_size;
+    char *gz_buffer;
+    GC_INIT(ctx);
+
+    ret = xc_get_config(ctx->xch, &gz_buffer, &gz_size);
+    if (ret < 0) {
+        LOGEV(ERROR, ret, "getting config");
+        ret = ERROR_FAIL;
+        goto out;
+    }
+
+    *buffer = xc_inflate_buffer(ctx->xch, gz_buffer, gz_size, &out_size);
+
+    free(gz_buffer);
+
+    if (!*buffer) {
+        LOGE(ERROR, "decompressing config data failed");
+        ret = ERROR_FAIL;
+        goto out;
+    }
+
+    *buffer = libxl__realloc(NOGC, *buffer, out_size + 1);
+    (*buffer)[out_size] = 0;
+
+    ret = 0;
+
+out:
+    GC_FREE;
+    return ret;
+}
+
 static int fd_set_flags(libxl_ctx *ctx, int fd,
                         int fcntlgetop, int fcntlsetop, const char *fl,
                         int flagmask, int set_p)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a38e5cdba2..0d48ac0ae3 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1149,6 +1149,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_SET_PARAMETERS 1
 
+/*
+ * LIBXL_HAVE_GET_HYPERVISOR_CONFIG
+ *
+ * If this is defined getting hypervisor config is supported.
+ */
+#define LIBXL_HAVE_GET_HYPERVISOR_CONFIG 1
+
 /*
  * LIBXL_HAVE_PV_SHIM
  *
@@ -2307,6 +2314,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
 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_parameters(libxl_ctx *ctx, char *params);
+int libxl_get_hypervisor_config(libxl_ctx *ctx, char **buffer);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index cf4202bc89..cc537b07b9 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -156,6 +156,7 @@ int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
 int main_set_parameters(int argc, char **argv);
+int main_get_hypervisor_config(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/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 89716badcb..53b1f1f922 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -320,6 +320,11 @@ struct cmd_spec cmd_table[] = {
       "Set hypervisor parameters",
       "<Params>",
     },
+    { "get-hypervisor-config",
+      &main_get_hypervisor_config, 0, 0,
+      "Get hypervisor build config",
+      "",
+    },
     { "dmesg",
       &main_dmesg, 0, 0,
       "Read and/or clear dmesg buffer",
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index dcf940a6d4..00dd51e935 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -175,6 +175,26 @@ int main_set_parameters(int argc, char **argv)
     return EXIT_SUCCESS;
 }
 
+int main_get_hypervisor_config(int argc, char **argv)
+{
+    int opt;
+    char *conf;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "get-hypervisor-config", 0) {
+        /* No options */
+    }
+
+    if (libxl_get_hypervisor_config(ctx, &conf)) {
+        fprintf(stderr, "cannot get config\n");
+        return EXIT_FAILURE;
+    }
+
+    printf("%s\n", conf);
+    free(conf);
+
+    return EXIT_SUCCESS;
+}
+
 int main_devd(int argc, char **argv)
 {
     int ret = 0, opt = 0, daemonize = 1;
-- 
2.16.4


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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Wei Liu]

On Thu, Mar 14, 2019 at 12:59:36PM +0100, Juergen Gross wrote:
> Add a sysctl interface for obtaining the .config file used to build
> the hypervisor. The mechanism is inspired by the Linux kernel's one.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 2/2] tools: add new xl command get-hypervisor-config

[Wei Liu]

On Thu, Mar 14, 2019 at 12:59:37PM +0100, Juergen Gross wrote:
> Add   new subcommand "get-hypervisor-config" to xl config to print the
> hypervisor .config file.
> 
> To be able to reuse already existing decompressing code in libxenguest
> xc_inflate_buffer() has to be moved to libxenguest.h.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Jan Beulich]

>>> On 14.03.19 at 12:59, <jgross@suse.com> wrote:
> Add a sysctl interface for obtaining the .config file used to build
> the hypervisor. The mechanism is inspired by the Linux kernel's one.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)
> ---
> V2:
> - bump sysctl interface version

Judging from the style you use for other entries here this wasn't
a result of review feedback. Why did you do this? The change is
a plain addition to the interface, which doesn't require caller
awareness. My R-b does not hold with this in place.

Jan



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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross]

On 15/03/2019 14:57, Jan Beulich wrote:
>>>> On 14.03.19 at 12:59, <jgross@suse.com> wrote:
>> Add a sysctl interface for obtaining the .config file used to build
>> the hypervisor. The mechanism is inspired by the Linux kernel's one.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)
>> ---
>> V2:
>> - bump sysctl interface version
> 
> Judging from the style you use for other entries here this wasn't
> a result of review feedback. Why did you do this? The change is

V1 was posted before 4.12 was branched away, so bumping would have
clearly be wrong at that time.

> a plain addition to the interface, which doesn't require caller
> awareness. My R-b does not hold with this in place.

Ah, I wasn't aware of this rule. I'll remove the bumping in case this
can't be done when committing.


Juergen

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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Jan Beulich]

>>> On 15.03.19 at 15:01, <jgross@suse.com> wrote:
> On 15/03/2019 14:57, Jan Beulich wrote:
>>>>> On 14.03.19 at 12:59, <jgross@suse.com> wrote:
>>> Add a sysctl interface for obtaining the .config file used to build
>>> the hypervisor. The mechanism is inspired by the Linux kernel's one.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)
>>> ---
>>> V2:
>>> - bump sysctl interface version
>> 
>> Judging from the style you use for other entries here this wasn't
>> a result of review feedback. Why did you do this? The change is
> 
> V1 was posted before 4.12 was branched away, so bumping would have
> clearly be wrong at that time.
> 
>> a plain addition to the interface, which doesn't require caller
>> awareness. My R-b does not hold with this in place.
> 
> Ah, I wasn't aware of this rule. I'll remove the bumping in case this
> can't be done when committing.

It's pretty straightforward to drop this while committing.

Jan



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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Andrew Cooper]

On 14/03/2019 11:59, Juergen Gross wrote:
> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>  #endif
>  
> +/*
> + * XEN_SYSCTL_get_config
> + *
> + * Return gzip-ed .config file

I don't think it is wise to bake gzip into the API.  These days, there
really are better options than gzip both in terms of compression ratio
and decompress speed.

I'd suggest just saying "file may be compressed" and leave it up to the
packagers build system and `xl` choices to agree on which compression
(if any) to use.

~Andrew

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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross]

On 15/03/2019 16:55, Andrew Cooper wrote:
> On 14/03/2019 11:59, Juergen Gross wrote:
>> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>>  #endif
>>  
>> +/*
>> + * XEN_SYSCTL_get_config
>> + *
>> + * Return gzip-ed .config file
> 
> I don't think it is wise to bake gzip into the API.  These days, there
> really are better options than gzip both in terms of compression ratio
> and decompress speed.
> 
> I'd suggest just saying "file may be compressed" and leave it up to the
> packagers build system and `xl` choices to agree on which compression
> (if any) to use.

I am not convinced this is a good idea. I think the data format should
be specified.

And TBH: the difference in size for a rather "normal" .config is about
50 bytes (I tried gzip, xz and bzip2 with gzip producing the smallest
result).


Juergen

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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Daniel De Graaf]

On 3/14/19 7:59 AM, Juergen Gross wrote:
> Add a sysctl interface for obtaining the .config file used to build
> the hypervisor. The mechanism is inspired by the Linux kernel's one.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com> (apart from XSM changes)

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



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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Wei Liu]

On Fri, Mar 15, 2019 at 05:29:28PM +0100, Juergen Gross wrote:
> On 15/03/2019 16:55, Andrew Cooper wrote:
> > On 14/03/2019 11:59, Juergen Gross wrote:
> >> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
> >>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
> >>  #endif
> >>  
> >> +/*
> >> + * XEN_SYSCTL_get_config
> >> + *
> >> + * Return gzip-ed .config file
> > 
> > I don't think it is wise to bake gzip into the API.  These days, there
> > really are better options than gzip both in terms of compression ratio
> > and decompress speed.
> > 
> > I'd suggest just saying "file may be compressed" and leave it up to the
> > packagers build system and `xl` choices to agree on which compression
> > (if any) to use.
> 
> I am not convinced this is a good idea. I think the data format should
> be specified.
> 
> And TBH: the difference in size for a rather "normal" .config is about
> 50 bytes (I tried gzip, xz and bzip2 with gzip producing the smallest
> result).

Has this been resolved?

I don't really have an opinion on this.

Wei.

> 
> 
> Juergen

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

Re: [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross]

On 04/04/2019 15:27, Wei Liu wrote:
> On Fri, Mar 15, 2019 at 05:29:28PM +0100, Juergen Gross wrote:
>> On 15/03/2019 16:55, Andrew Cooper wrote:
>>> On 14/03/2019 11:59, Juergen Gross wrote:
>>>> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
>>>>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>>>>  #endif
>>>>  
>>>> +/*
>>>> + * XEN_SYSCTL_get_config
>>>> + *
>>>> + * Return gzip-ed .config file
>>>
>>> I don't think it is wise to bake gzip into the API.  These days, there
>>> really are better options than gzip both in terms of compression ratio
>>> and decompress speed.
>>>
>>> I'd suggest just saying "file may be compressed" and leave it up to the
>>> packagers build system and `xl` choices to agree on which compression
>>> (if any) to use.
>>
>> I am not convinced this is a good idea. I think the data format should
>> be specified.
>>
>> And TBH: the difference in size for a rather "normal" .config is about
>> 50 bytes (I tried gzip, xz and bzip2 with gzip producing the smallest
>> result).
> 
> Has this been resolved?
> 
> I don't really have an opinion on this.

I got no reply for that answer.


Juergen


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

Re: [Xen-devel] [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[George Dunlap]

On 4/4/19 2:35 PM, Juergen Gross wrote:
> On 04/04/2019 15:27, Wei Liu wrote:
>> On Fri, Mar 15, 2019 at 05:29:28PM +0100, Juergen Gross wrote:
>>> On 15/03/2019 16:55, Andrew Cooper wrote:
>>>> On 14/03/2019 11:59, Juergen Gross wrote:
>>>>> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>>>>>  #endif
>>>>>  
>>>>> +/*
>>>>> + * XEN_SYSCTL_get_config
>>>>> + *
>>>>> + * Return gzip-ed .config file
>>>>
>>>> I don't think it is wise to bake gzip into the API.  These days, there
>>>> really are better options than gzip both in terms of compression ratio
>>>> and decompress speed.
>>>>
>>>> I'd suggest just saying "file may be compressed" and leave it up to the
>>>> packagers build system and `xl` choices to agree on which compression
>>>> (if any) to use.
>>>
>>> I am not convinced this is a good idea. I think the data format should
>>> be specified.
>>>
>>> And TBH: the difference in size for a rather "normal" .config is about
>>> 50 bytes (I tried gzip, xz and bzip2 with gzip producing the smallest
>>> result).
>>
>> Has this been resolved?
>>
>> I don't really have an opinion on this.
> 
> I got no reply for that answer.

You don't need a reply; if a suggestion is made, and you explain why you
don't like it, then it's up to the other person to unabiguously Nack it
if they don't like it; silence is interpreted as consent.

To move this forward, I'd suggest re-posting it, adding a comment about
the choice of compression algorithm in the commit message.

 -George

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

Re: [Xen-devel] [PATCH v2 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross]

On 05.08.19 17:07, George Dunlap wrote:
> On 4/4/19 2:35 PM, Juergen Gross wrote:
>> On 04/04/2019 15:27, Wei Liu wrote:
>>> On Fri, Mar 15, 2019 at 05:29:28PM +0100, Juergen Gross wrote:
>>>> On 15/03/2019 16:55, Andrew Cooper wrote:
>>>>> On 14/03/2019 11:59, Juergen Gross wrote:
>>>>>> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
>>>>>>   DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>>>>>>   #endif
>>>>>>   
>>>>>> +/*
>>>>>> + * XEN_SYSCTL_get_config
>>>>>> + *
>>>>>> + * Return gzip-ed .config file
>>>>>
>>>>> I don't think it is wise to bake gzip into the API.  These days, there
>>>>> really are better options than gzip both in terms of compression ratio
>>>>> and decompress speed.
>>>>>
>>>>> I'd suggest just saying "file may be compressed" and leave it up to the
>>>>> packagers build system and `xl` choices to agree on which compression
>>>>> (if any) to use.
>>>>
>>>> I am not convinced this is a good idea. I think the data format should
>>>> be specified.
>>>>
>>>> And TBH: the difference in size for a rather "normal" .config is about
>>>> 50 bytes (I tried gzip, xz and bzip2 with gzip producing the smallest
>>>> result).
>>>
>>> Has this been resolved?
>>>
>>> I don't really have an opinion on this.
>>
>> I got no reply for that answer.
> 
> You don't need a reply; if a suggestion is made, and you explain why you
> don't like it, then it's up to the other person to unabiguously Nack it
> if they don't like it; silence is interpreted as consent.
> 
> To move this forward, I'd suggest re-posting it, adding a comment about
> the choice of compression algorithm in the commit message.

At the summit we had a discussion about a simple name-value store in the
hypervisor for e.g. communicating mitigation settings or runtime param
settings to at least dom0. The .config used for building Xen was agreed
to be another candidate for that interface.

I wanted to have a look at that when I'm finished with other urgent work
I'm actually doing.


Juergen


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