All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] xenstore: make it easier to run xenstore in a domain
@ 2015-12-11 15:47 Juergen Gross
  2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
                   ` (8 more replies)
  0 siblings, 9 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Xen supports to run xenstore in a dedicated domain. It is, however, a
setup which isn't easy to configure. Today flask is required for full
functionality and the resulting xenstore domain is not configurable in
the same way as the xenstore daemon.

This patch series enables running a xenstore domain without flask. The
tool needed to start that domain is added to the installed binaries
and it is modified to pass arbitrary options to the xenstore domain.

I didn't include a configuration option for starting xenstore as an
own domain instead of the daemon, as this will require some major
tweaking of especially the systemd configuration files. Support for
this will be added later. The main issue is that running xenstore in
a domain requires the xenstore sockets not to be created. I don't
think this is possible with the current scheme of letting systemd
create those sockets.

Juergen Gross (9):
  xen: add xenstore domain flag to hypervisor
  libxc: support new xenstore domain flag in libxc
  xenstore: install init-xenstore-domain via make install
  xenstore: add error messages to init-xenstore-domain
  xenstore: modify init-xenstore-domain parameter syntax
  xenstore: don't start xenstore domain if already one is active
  xenstore: add init-xenstore-domain parameter to specify cmdline
  xenstore: write xenstore domain data to xenstore
  xenstore: when running in mini-os use printk for diagnostic messages

 tools/libxc/include/xenctrl.h         |   2 +-
 tools/libxc/xc_domain.c               |  17 +--
 tools/xenstore/Makefile               |   3 +
 tools/xenstore/init-xenstore-domain.c | 223 ++++++++++++++++++++++++++++------
 tools/xenstore/utils.c                |   6 +
 xen/common/domain.c                   |   6 +
 xen/common/domctl.c                   |  14 ++-
 xen/include/public/domctl.h           |   6 +
 xen/include/xen/sched.h               |   5 +
 xen/include/xsm/dummy.h               |   6 +
 xen/include/xsm/xsm.h                 |   1 +
 11 files changed, 241 insertions(+), 48 deletions(-)

-- 
2.6.2

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

* [PATCH 1/9] xen: add xenstore domain flag to hypervisor
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-11 15:54   ` Fwd: " Juergen Gross
  2015-12-15 22:18   ` Daniel De Graaf
  2015-12-11 15:47 ` [PATCH 2/9] libxc: support new xenstore domain flag in libxc Juergen Gross
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross, Keir Fraser, Andrew Cooper, David Vrabel,
	Jan Beulich, Tim Deegan, Daniel De Graaf

In order to be able to have full support of a xenstore domain in Xen
add a "Xenstore-domain" flag to the hypervisor. This flag must be
specified at domain creation time and is returned by
XEN_DOMCTL_getdomaininfo.

It will allow the domain to retrieve domain information by issuing the
XEN_DOMCTL_getdomaininfo itself in order to be able to check for
domains having been destroyed. At the same time this flag will inhibit
the domain to be migrated, as this wouldn't be a very wise thing to do.

In case of a later support of a rebootable Dom0 this flag will allow to
recognize a xenstore domain already being present to connect to.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>A
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/common/domain.c         |  6 ++++++
 xen/common/domctl.c         | 14 +++++++++-----
 xen/include/public/domctl.h |  6 ++++++
 xen/include/xen/sched.h     |  5 +++++
 xen/include/xsm/dummy.h     |  6 ++++++
 xen/include/xsm/xsm.h       |  1 +
 6 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index f56b7ff..ac24cfd 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -318,6 +318,12 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
         hardware_domain = d;
     }
 
+    if ( domcr_flags & DOMCRF_xs_domain )
+    {
+        d->is_xenstore = 1;
+        d->disable_migrate = 1;
+    }
+
     rangeset_domain_initialise(d);
     init_status |= INIT_rangeset;
 
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 46b967e..380d326 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -183,10 +183,11 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
     info->cpu_time = cpu_time;
 
     info->flags = (info->nr_online_vcpus ? flags : 0) |
-        ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying    : 0) |
-        (d->is_shut_down                ? XEN_DOMINF_shutdown : 0) |
-        (d->controller_pause_count > 0  ? XEN_DOMINF_paused   : 0) |
-        (d->debugger_attached           ? XEN_DOMINF_debugged : 0) |
+        ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying     : 0) |
+        (d->is_shut_down                ? XEN_DOMINF_shutdown  : 0) |
+        (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
+        (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
+        (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
     switch ( d->guest_type )
@@ -551,7 +552,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
                | XEN_DOMCTL_CDF_pvh_guest
                | XEN_DOMCTL_CDF_hap
                | XEN_DOMCTL_CDF_s3_integrity
-               | XEN_DOMCTL_CDF_oos_off)) )
+               | XEN_DOMCTL_CDF_oos_off
+               | XEN_DOMCTL_CDF_xs_domain)) )
             break;
 
         dom = op->domain;
@@ -593,6 +595,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             domcr_flags |= DOMCRF_s3_integrity;
         if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off )
             domcr_flags |= DOMCRF_oos_off;
+        if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_xs_domain )
+            domcr_flags |= DOMCRF_xs_domain;
 
         d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref,
                           &op->u.createdomain.config);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 7a56b3f..2d8076c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -63,6 +63,9 @@ struct xen_domctl_createdomain {
  /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
 #define _XEN_DOMCTL_CDF_pvh_guest     4
 #define XEN_DOMCTL_CDF_pvh_guest      (1U<<_XEN_DOMCTL_CDF_pvh_guest)
+ /* Is this a xenstore domain? */
+#define _XEN_DOMCTL_CDF_xs_domain     5
+#define XEN_DOMCTL_CDF_xs_domain      (1U<<_XEN_DOMCTL_CDF_xs_domain)
     uint32_t flags;
     struct xen_arch_domainconfig config;
 };
@@ -97,6 +100,9 @@ struct xen_domctl_getdomaininfo {
 /* domain is PVH */
 #define _XEN_DOMINF_pvh_guest 7
 #define XEN_DOMINF_pvh_guest  (1U<<_XEN_DOMINF_pvh_guest)
+/* domain is a xenstore domain */
+#define _XEN_DOMINF_xs_domain 8
+#define XEN_DOMINF_xs_domain  (1U<<_XEN_DOMINF_xs_domain)
  /* XEN_DOMINF_shutdown guest-supplied code.  */
 #define XEN_DOMINF_shutdownmask 255
 #define XEN_DOMINF_shutdownshift 16
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3729b0f..5b18bba 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -374,6 +374,8 @@ struct domain
     bool_t           auto_node_affinity;
     /* Is this guest fully privileged (aka dom0)? */
     bool_t           is_privileged;
+    /* Is this a xenstore domain (not dom0)? */
+    bool_t           is_xenstore;
     /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
     bool_t           is_pinned;
     /* Non-migratable and non-restoreable? */
@@ -533,6 +535,9 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
  /* DOMCRF_pvh: Create PV domain in HVM container. */
 #define _DOMCRF_pvh             5
 #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
+ /* DOMCRF_xs_domain: xenstore domain */
+#define _DOMCRF_xs_domain       6
+#define DOMCRF_xs_domain        (1U<<_DOMCRF_xs_domain)
 
 /*
  * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index e43f2a1..a07c4c6 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -71,6 +71,10 @@ static always_inline int xsm_default_action(
         if ( src->is_privileged )
             return 0;
         return -EPERM;
+    case XSM_XS_PRIV:
+        if ( src->is_xenstore || src->is_privileged )
+            return 0;
+        return -EPERM;
     default:
         LINKER_BUG_ON(1);
         return -EPERM;
@@ -123,6 +127,8 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct domain *d, int cmd)
     case XEN_DOMCTL_bind_pt_irq:
     case XEN_DOMCTL_unbind_pt_irq:
         return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+    case XEN_DOMCTL_getdomaininfo:
+        return xsm_default_action(XSM_XS_PRIV, current->domain, d);
     default:
         return xsm_default_action(XSM_PRIV, current->domain, d);
     }
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index f48cf60..01329b8 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -35,6 +35,7 @@ enum xsm_default {
     XSM_DM_PRIV,  /* Device model can perform on its target domain */
     XSM_TARGET,   /* Can perform on self or your target domain */
     XSM_PRIV,     /* Privileged - normally restricted to dom0 */
+    XSM_XS_PRIV,  /* Xenstore domain can obtain domain info */
     XSM_OTHER     /* Something more complex */
 };
 typedef enum xsm_default xsm_default_t;
-- 
2.6.2

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

* [PATCH 2/9] libxc: support new xenstore domain flag in libxc
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
  2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-11 15:47 ` [PATCH 3/9] xenstore: install init-xenstore-domain via make install Juergen Gross
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Support the xenstore domain flag for obtaining domain info.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxc/include/xenctrl.h |  2 +-
 tools/libxc/xc_domain.c       | 17 +++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 01a6dda..45d8ff6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -456,7 +456,7 @@ typedef struct xc_dominfo {
     uint32_t      ssidref;
     unsigned int  dying:1, crashed:1, shutdown:1,
                   paused:1, blocked:1, running:1,
-                  hvm:1, debugged:1, pvh:1;
+                  hvm:1, debugged:1, pvh:1, xs_domain:1;
     unsigned int  shutdown_reason; /* only meaningful if shutdown==1 */
     unsigned long nr_pages; /* current number, not maximum */
     unsigned long nr_outstanding_pages;
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 96506d5..46bca92 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -366,14 +366,15 @@ int xc_domain_getinfo(xc_interface *xch,
             break;
         info->domid      = (uint16_t)domctl.domain;
 
-        info->dying    = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
-        info->shutdown = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_shutdown);
-        info->paused   = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_paused);
-        info->blocked  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_blocked);
-        info->running  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_running);
-        info->hvm      = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hvm_guest);
-        info->debugged = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_debugged);
-        info->pvh      = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_pvh_guest);
+        info->dying     = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
+        info->shutdown  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_shutdown);
+        info->paused    = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_paused);
+        info->blocked   = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_blocked);
+        info->running   = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_running);
+        info->hvm       = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hvm_guest);
+        info->debugged  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_debugged);
+        info->pvh       = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_pvh_guest);
+        info->xs_domain = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_xs_domain);
 
         info->shutdown_reason =
             (domctl.u.getdomaininfo.flags>>XEN_DOMINF_shutdownshift) &
-- 
2.6.2

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

* [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
  2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
  2015-12-11 15:47 ` [PATCH 2/9] libxc: support new xenstore domain flag in libxc Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:16   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 4/9] xenstore: add error messages to init-xenstore-domain Juergen Gross
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

The program init-xenstore-domain to start a xenstore domain instead
of the xenstored daemon is built, but not installed. Change that.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 1b4a494..8c8b116 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -153,6 +153,9 @@ endif
 	$(INSTALL_DATA) include/compat/xs_lib.h $(DESTDIR)$(includedir)/xenstore-compat/xs_lib.h
 	ln -sf xenstore-compat/xs.h  $(DESTDIR)$(includedir)/xs.h
 	ln -sf xenstore-compat/xs_lib.h $(DESTDIR)$(includedir)/xs_lib.h
+ifeq ($(CONFIG_Linux),y)
+	$(INSTALL_PROG) init-xenstore-domain $(DESTDIR)$(LIBEXEC_BIN)
+endif
 
 .PHONY: clients-install
 clients-install: clients
-- 
2.6.2

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

* [PATCH 4/9] xenstore: add error messages to init-xenstore-domain
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (2 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 3/9] xenstore: install init-xenstore-domain via make install Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:20   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax Juergen Gross
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Additional add some diagnostic messages to the program to have an idea
why it failed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/init-xenstore-domain.c | 109 +++++++++++++++++++++++++++-------
 1 file changed, 87 insertions(+), 22 deletions(-)

diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-xenstore-domain.c
index 297afe5..c9963c6 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -10,6 +10,7 @@
 #include <xc_dom.h>
 #include <xenstore.h>
 #include <xen/sys/xenbus_dev.h>
+#include <xen-xsm/flask/flask.h>
 
 static uint32_t domid = -1;
 
@@ -24,58 +25,111 @@ static int build(xc_interface *xch, int argc, char** argv)
 	int limit_kb = (maxmem + 1)*1024;
 
 	xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
-	if (xs_fd == -1) return -1;
+	if (xs_fd == -1) {
+		fprintf(stderr, "Could not open /dev/xen/xenbus_backend\n");
+		return -1;
+	}
 
 	rv = xc_flask_context_to_sid(xch, argv[3], strlen(argv[3]), &ssid);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_flask_context_to_sid failed\n");
+		goto err;
+	}
 	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_create failed\n");
+		goto err;
+	}
 	rv = xc_domain_max_vcpus(xch, domid, 1);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_max_vcpus failed\n");
+		goto err;
+	}
 	rv = xc_domain_setmaxmem(xch, domid, limit_kb);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_setmaxmem failed\n");
+		goto err;
+	}
 	rv = xc_domain_set_memmap_limit(xch, domid, limit_kb);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_set_memmap_limit failed\n");
+		goto err;
+	}
 
 	rv = ioctl(xs_fd, IOCTL_XENBUS_BACKEND_SETUP, domid);
-	if (rv < 0) goto err;
+	if (rv < 0) {
+		fprintf(stderr, "Xenbus setup ioctl failed\n");
+		goto err;
+	}
 	snprintf(cmdline, 512, "--event %d --internal-db", rv);
 
 	dom = xc_dom_allocate(xch, cmdline, NULL);
 	rv = xc_dom_kernel_file(dom, argv[1]);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_kernel_file failed\n");
+		goto err;
+	}
 
 	if (argc > 4) {
 		rv = xc_dom_ramdisk_file(dom, argv[4]);
-		if (rv) goto err;
+		if (rv) {
+			fprintf(stderr, "xc_dom_ramdisk_file failed\n");
+			goto err;
+		}
 	}
 
 	rv = xc_dom_boot_xen_init(dom, xch, domid);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_boot_xen_init failed\n");
+		goto err;
+	}
 	rv = xc_dom_parse_image(dom);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_parse_image failed\n");
+		goto err;
+	}
 	rv = xc_dom_mem_init(dom, maxmem);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_mem_init failed\n");
+		goto err;
+	}
 	rv = xc_dom_boot_mem_init(dom);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_boot_mem_init failed\n");
+		goto err;
+	}
 	rv = xc_dom_build_image(dom);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_build_image failed\n");
+		goto err;
+	}
 	rv = xc_dom_boot_image(dom);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_dom_boot_image failed\n");
+		goto err;
+	}
 
 	xc_dom_release(dom);
 	dom = NULL;
 
 	rv = xc_domain_set_virq_handler(xch, domid, VIRQ_DOM_EXC);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_set_virq_handler failed\n");
+		goto err;
+	}
 	rv = xc_domain_unpause(xch, domid);
-	if (rv) goto err;
+	if (rv) {
+		fprintf(stderr, "xc_domain_unpause failed\n");
+		goto err;
+	}
 
 	return 0;
 
 err:
 	if (dom)
 		xc_dom_release(dom);
+	if (domid >= 0)
+		xc_domain_destroy(xch, domid);
 	close(xs_fd);
 	return rv;
 }
@@ -88,18 +142,24 @@ int main(int argc, char** argv)
 	int rv, fd;
 
 	if (argc < 4 || argc > 5) {
-		printf("Use: %s <xenstore-kernel> <memory_mb> <flask-label> [<ramdisk-file>]\n", argv[0]);
+		fprintf(stderr,
+			"Use: %s <xenstore-kernel> <memory_mb> <flask-label> [<ramdisk-file>]\n",
+			argv[0]);
 		return 2;
 	}
 
 	xch = xc_interface_open(NULL, NULL, 0);
-	if (!xch) return 1;
+	if (!xch) {
+		fprintf(stderr, "xc_interface_open() failed\n");
+		return 1;
+	}
 
 	rv = build(xch, argc, argv);
 
 	xc_interface_close(xch);
 
-	if (rv) return 1;
+	if (rv)
+		return 1;
 
 	xsh = xs_open(0);
 	rv = snprintf(buf, 16, "%d", domid);
@@ -107,13 +167,18 @@ int main(int argc, char** argv)
 	xs_daemon_close(xsh);
 
 	fd = creat("/var/run/xenstored.pid", 0666);
-	if (fd < 0)
+	if (fd < 0) {
+		fprintf(stderr, "Creating /var/run/xenstored.pid failed\n");
 		return 3;
+	}
 	rv = snprintf(buf, 16, "domid:%d\n", domid);
 	rv = write(fd, buf, rv);
 	close(fd);
-	if (rv < 0)
+	if (rv < 0) {
+		fprintf(stderr,
+			"Writing domid to /var/run/xenstored.pid failed\n");
 		return 3;
+	}
 
 	return 0;
 }
-- 
2.6.2

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

* [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (3 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 4/9] xenstore: add error messages to init-xenstore-domain Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:22   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 6/9] xenstore: don't start xenstore domain if already one is active Juergen Gross
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

init-xenstore-domain takes only positional parameters today. Change
this to a more flexible parameter syntax allowing to specify additional
options or to omit some.

Today the supported usage is:

init-xenstore-domain <xenstore-kernel> <memory_mb> <flask-label>
                     [<ramdisk-file>]

Modify this to:

init-xenstore-domain --kernel <xenstore-kernel>
                     --memory <memory_mb>
                     [--flask <flask-label>]
                     [--ramdisk <ramdisk-file>]

The flask label is made optional in order to support xenstore domains
without the need of a flask enabled hypervisor.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/init-xenstore-domain.c | 79 ++++++++++++++++++++++++++++-------
 1 file changed, 63 insertions(+), 16 deletions(-)

diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-xenstore-domain.c
index c9963c6..068887c 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -4,6 +4,7 @@
 #include <string.h>
 #include <stdint.h>
 #include <stdlib.h>
+#include <getopt.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <xenctrl.h>
@@ -13,16 +14,42 @@
 #include <xen-xsm/flask/flask.h>
 
 static uint32_t domid = -1;
+static char *kernel;
+static char *ramdisk;
+static char *flask;
+static int memory;
+
+static struct option options[] = {
+	{ "kernel", 1, NULL, 'k' },
+	{ "memory", 1, NULL, 'm' },
+	{ "flask", 1, NULL, 'f' },
+	{ "ramdisk", 1, NULL, 'r' },
+	{ NULL, 0, NULL, 0 }
+};
+
+static void usage(void)
+{
+	fprintf(stderr,
+"Usage:\n"
+"\n"
+"init-xenstore-domain <options>\n"
+"\n"
+"where options may include:\n"
+"\n"
+"  --kernel <xenstore-kernel>  kernel file of the xenstore domain, mandatory\n"
+"  --memory <memory size>      size of the domain in MB, mandatory\n"
+"  --flask <flask-label>       optional flask label of the domain\n"
+"  --ramdisk <ramdisk-file>    optional ramdisk file for the domain\n");
+}
 
-static int build(xc_interface *xch, int argc, char** argv)
+static int build(xc_interface *xch)
 {
 	char cmdline[512];
 	uint32_t ssid;
 	xen_domain_handle_t handle = { 0 };
 	int rv, xs_fd;
 	struct xc_dom_image *dom = NULL;
-	int maxmem = atoi(argv[2]);
-	int limit_kb = (maxmem + 1)*1024;
+	int limit_kb = (memory + 1)*1024;
 
 	xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
 	if (xs_fd == -1) {
@@ -30,10 +57,14 @@ static int build(xc_interface *xch, int argc, char** argv)
 		return -1;
 	}
 
-	rv = xc_flask_context_to_sid(xch, argv[3], strlen(argv[3]), &ssid);
-	if (rv) {
-		fprintf(stderr, "xc_flask_context_to_sid failed\n");
-		goto err;
+	if (flask) {
+		rv = xc_flask_context_to_sid(xch, flask, strlen(flask), &ssid);
+		if (rv) {
+			fprintf(stderr, "xc_flask_context_to_sid failed\n");
+			goto err;
+		}
+	} else {
+		ssid = SECINITSID_DOMU;
 	}
 	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
 	if (rv) {
@@ -64,14 +95,14 @@ static int build(xc_interface *xch, int argc, char** argv)
 	snprintf(cmdline, 512, "--event %d --internal-db", rv);
 
 	dom = xc_dom_allocate(xch, cmdline, NULL);
-	rv = xc_dom_kernel_file(dom, argv[1]);
+	rv = xc_dom_kernel_file(dom, kernel);
 	if (rv) {
 		fprintf(stderr, "xc_dom_kernel_file failed\n");
 		goto err;
 	}
 
-	if (argc > 4) {
-		rv = xc_dom_ramdisk_file(dom, argv[4]);
+	if (ramdisk) {
+		rv = xc_dom_ramdisk_file(dom, ramdisk);
 		if (rv) {
 			fprintf(stderr, "xc_dom_ramdisk_file failed\n");
 			goto err;
@@ -88,7 +119,7 @@ static int build(xc_interface *xch, int argc, char** argv)
 		fprintf(stderr, "xc_dom_parse_image failed\n");
 		goto err;
 	}
-	rv = xc_dom_mem_init(dom, maxmem);
+	rv = xc_dom_mem_init(dom, memory);
 	if (rv) {
 		fprintf(stderr, "xc_dom_mem_init failed\n");
 		goto err;
@@ -136,15 +167,31 @@ err:
 
 int main(int argc, char** argv)
 {
+	int opt;
 	xc_interface *xch;
 	struct xs_handle *xsh;
 	char buf[16];
 	int rv, fd;
 
-	if (argc < 4 || argc > 5) {
-		fprintf(stderr,
-			"Use: %s <xenstore-kernel> <memory_mb> <flask-label> [<ramdisk-file>]\n",
-			argv[0]);
+	while ((opt = getopt_long(argc, argv, "", options, NULL)) != -1) {
+		switch (opt) {
+		case 'k':
+			kernel = optarg;
+			break;
+		case 'm':
+			memory = strtol(optarg, NULL, 10);
+			break;
+		case 'f':
+			flask = optarg;
+			break;
+		case 'r':
+			ramdisk = optarg;
+			break;
+		}
+	}
+
+	if (optind != argc) {
+		usage();
 		return 2;
 	}
 
@@ -154,7 +201,7 @@ int main(int argc, char** argv)
 		return 1;
 	}
 
-	rv = build(xch, argc, argv);
+	rv = build(xch);
 
 	xc_interface_close(xch);
 
-- 
2.6.2

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

* [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (4 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:23   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline Juergen Gross
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Don't start a new xenstore domain in case one is already detected to
be running.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/init-xenstore-domain.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-xenstore-domain.c
index 068887c..0ca7eed 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -66,7 +66,8 @@ static int build(xc_interface *xch)
 	} else {
 		ssid = SECINITSID_DOMU;
 	}
-	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
+	rv = xc_domain_create(xch, ssid, handle, XEN_DOMCTL_CDF_xs_domain,
+			      &domid, NULL);
 	if (rv) {
 		fprintf(stderr, "xc_domain_create failed\n");
 		goto err;
@@ -165,6 +166,21 @@ err:
 	return rv;
 }
 
+static int check_domain(xc_interface *xch)
+{
+	xc_dominfo_t info;
+	uint32_t dom;
+
+	dom = 0;
+	while (xc_domain_getinfo(xch, dom, 1, &info) == 1) {
+		if (info.xs_domain)
+			return 1;
+		dom = info.domid + 1;
+	}
+
+	return 0;
+}
+
 int main(int argc, char** argv)
 {
 	int opt;
@@ -201,7 +217,12 @@ int main(int argc, char** argv)
 		return 1;
 	}
 
-	rv = build(xch);
+	rv = check_domain(xch);
+
+	if (!rv)
+		rv = build(xch);
+	else
+		fprintf(stderr, "xenstore domain already present.\n");
 
 	xc_interface_close(xch);
 
-- 
2.6.2

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

* [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (5 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 6/9] xenstore: don't start xenstore domain if already one is active Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:24   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 8/9] xenstore: write xenstore domain data to xenstore Juergen Gross
  2015-12-11 15:47 ` [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages Juergen Gross
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Add a parameter to init-xenstore-domain for support of arbitrary
parameters for the xenstore domain.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/init-xenstore-domain.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-xenstore-domain.c
index 0ca7eed..eedcf32 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -17,6 +17,7 @@ static uint32_t domid = -1;
 static char *kernel;
 static char *ramdisk;
 static char *flask;
+static char *param;
 static int memory;
 
 static struct option options[] = {
@@ -24,6 +25,7 @@ static struct option options[] = {
 	{ "memory", 1, NULL, 'm' },
 	{ "flask", 1, NULL, 'f' },
 	{ "ramdisk", 1, NULL, 'r' },
+	{ "param", 1, NULL, 'p' },
 	{ NULL, 0, NULL, 0 }
 };
 
@@ -36,10 +38,11 @@ static void usage(void)
 "\n"
 "where options may include:\n"
 "\n"
-"  --kernel <xenstore-kernel>  kernel file of the xenstore domain, mandatory\n"
-"  --memory <memory size>      size of the domain in MB, mandatory\n"
-"  --flask <flask-label>       optional flask label of the domain\n"
-"  --ramdisk <ramdisk-file>    optional ramdisk file for the domain\n");
+"  --kernel <xenstore-kernel> kernel file of the xenstore domain, mandatory\n"
+"  --memory <memory size>     size of the domain in MB, mandatory\n"
+"  --flask <flask-label>      optional flask label of the domain\n"
+"  --ramdisk <ramdisk-file>   optional ramdisk file for the domain\n"
+"  --param <cmdline>          optional additional parameters for the domain\n");
 }
 
 static int build(xc_interface *xch)
@@ -93,7 +96,12 @@ static int build(xc_interface *xch)
 		fprintf(stderr, "Xenbus setup ioctl failed\n");
 		goto err;
 	}
-	snprintf(cmdline, 512, "--event %d --internal-db", rv);
+
+	if (param)
+		snprintf(cmdline, 512, "--event %d --internal-db %s", rv,
+			 param);
+	else
+		snprintf(cmdline, 512, "--event %d --internal-db", rv);
 
 	dom = xc_dom_allocate(xch, cmdline, NULL);
 	rv = xc_dom_kernel_file(dom, kernel);
@@ -203,6 +211,9 @@ int main(int argc, char** argv)
 		case 'r':
 			ramdisk = optarg;
 			break;
+		case 'p':
+			param = optarg;
+			break;
 		}
 	}
 
-- 
2.6.2

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

* [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (6 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:26   ` Ian Campbell
  2015-12-11 15:47 ` [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages Juergen Gross
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

After starting the xenstore domain write the basic data (domid and
name) to the xenstore.

Add a new option to init-xenstore-domain to be able to specify the
domain's name.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/init-xenstore-domain.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-xenstore-domain.c
index eedcf32..2a8a417 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -18,6 +18,7 @@ static char *kernel;
 static char *ramdisk;
 static char *flask;
 static char *param;
+static char *name = "Xenstore";
 static int memory;
 
 static struct option options[] = {
@@ -26,6 +27,7 @@ static struct option options[] = {
 	{ "flask", 1, NULL, 'f' },
 	{ "ramdisk", 1, NULL, 'r' },
 	{ "param", 1, NULL, 'p' },
+	{ "name", 1, NULL, 'n' },
 	{ NULL, 0, NULL, 0 }
 };
 
@@ -42,7 +44,8 @@ static void usage(void)
 "  --memory <memory size>     size of the domain in MB, mandatory\n"
 "  --flask <flask-label>      optional flask label of the domain\n"
 "  --ramdisk <ramdisk-file>   optional ramdisk file for the domain\n"
-"  --param <cmdline>          optional additional parameters for the domain\n");
+"  --param <cmdline>          optional additional parameters for the domain\n"
+"  --name <name>              name of the domain (default: Xenstore)\n");
 }
 
 static int build(xc_interface *xch)
@@ -195,6 +198,7 @@ int main(int argc, char** argv)
 	xc_interface *xch;
 	struct xs_handle *xsh;
 	char buf[16];
+	char path[64];
 	int rv, fd;
 
 	while ((opt = getopt_long(argc, argv, "", options, NULL)) != -1) {
@@ -214,6 +218,9 @@ int main(int argc, char** argv)
 		case 'p':
 			param = optarg;
 			break;
+		case 'n':
+			name = optarg;
+			break;
 		}
 	}
 
@@ -243,6 +250,10 @@ int main(int argc, char** argv)
 	xsh = xs_open(0);
 	rv = snprintf(buf, 16, "%d", domid);
 	xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv);
+	snprintf(path, 64, "/local/domain/%d/domid", domid);
+	xs_write(xsh, XBT_NULL, path, buf, rv);
+	snprintf(path, 64, "/local/domain/%d/name", domid);
+	xs_write(xsh, XBT_NULL, path, name, strlen(name));
 	xs_daemon_close(xsh);
 
 	fd = creat("/var/run/xenstored.pid", 0666);
-- 
2.6.2

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

* [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
                   ` (7 preceding siblings ...)
  2015-12-11 15:47 ` [PATCH 8/9] xenstore: write xenstore domain data to xenstore Juergen Gross
@ 2015-12-11 15:47 ` Juergen Gross
  2015-12-15 12:31   ` Ian Campbell
  8 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:47 UTC (permalink / raw)
  To: xen-devel, Ian.Campbell, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Juergen Gross

Xenstore messages for diagnostic purposes are lost today in case
xenstore is running under mini-os instead in a dom0 daemon.

Use printk of mini-os in this situation.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/utils.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/xenstore/utils.c b/tools/xenstore/utils.c
index a1ac125..ea8a5e8 100644
--- a/tools/xenstore/utils.c
+++ b/tools/xenstore/utils.c
@@ -10,6 +10,11 @@
 #include <signal.h>
 #include "utils.h"
 
+#ifdef __MINIOS__
+void printk(const char *fmt, ...);
+
+void (*xprintf)(const char *fmt, ...) = printk;
+#else
 static void default_xprintf(const char *fmt, ...)
 {
 	va_list args;
@@ -21,6 +26,7 @@ static void default_xprintf(const char *fmt, ...)
 }
 
 void (*xprintf)(const char *fmt, ...) = default_xprintf;
+#endif
 
 void barf(const char *fmt, ...)
 {
-- 
2.6.2

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

* Fwd: [PATCH 1/9] xen: add xenstore domain flag to hypervisor
  2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
@ 2015-12-11 15:54   ` Juergen Gross
  2015-12-15 22:18   ` Daniel De Graaf
  1 sibling, 0 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-11 15:54 UTC (permalink / raw)
  To: Tim Deegan, xen-devel

Sorry Tim,

your mail address has gained an additional character in the original
mail. Forwarded the mail just for you convenience.


Juergen


-------- Forwarded Message --------
Subject: [Xen-devel] [PATCH 1/9] xen: add xenstore domain flag to hypervisor
Date: Fri, 11 Dec 2015 16:47:33 +0100
From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xen.org, Ian.Campbell@citrix.com,
ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com,
wei.liu2@citrix.com
CC: Juergen Gross <jgross@suse.com>, Keir Fraser <keir@xen.org>, Andrew
Cooper <andrew.cooper3@citrix.com>, David Vrabel
<david.vrabel@citrix.com>, Jan Beulich <jbeulich@suse.com>, Tim Deegan
<tim@xen.orgA>, Daniel De Graaf <dgdegra@tycho.nsa.gov>

In order to be able to have full support of a xenstore domain in Xen
add a "Xenstore-domain" flag to the hypervisor. This flag must be
specified at domain creation time and is returned by
XEN_DOMCTL_getdomaininfo.

It will allow the domain to retrieve domain information by issuing the
XEN_DOMCTL_getdomaininfo itself in order to be able to check for
domains having been destroyed. At the same time this flag will inhibit
the domain to be migrated, as this wouldn't be a very wise thing to do.

In case of a later support of a rebootable Dom0 this flag will allow to
recognize a xenstore domain already being present to connect to.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>A
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/common/domain.c         |  6 ++++++
 xen/common/domctl.c         | 14 +++++++++-----
 xen/include/public/domctl.h |  6 ++++++
 xen/include/xen/sched.h     |  5 +++++
 xen/include/xsm/dummy.h     |  6 ++++++
 xen/include/xsm/xsm.h       |  1 +
 6 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index f56b7ff..ac24cfd 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -318,6 +318,12 @@ struct domain *domain_create(domid_t domid,
unsigned int domcr_flags,
         hardware_domain = d;
     }

+    if ( domcr_flags & DOMCRF_xs_domain )
+    {
+        d->is_xenstore = 1;
+        d->disable_migrate = 1;
+    }
+
     rangeset_domain_initialise(d);
     init_status |= INIT_rangeset;

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 46b967e..380d326 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -183,10 +183,11 @@ void getdomaininfo(struct domain *d, struct
xen_domctl_getdomaininfo *info)
     info->cpu_time = cpu_time;

     info->flags = (info->nr_online_vcpus ? flags : 0) |
-        ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying    : 0) |
-        (d->is_shut_down                ? XEN_DOMINF_shutdown : 0) |
-        (d->controller_pause_count > 0  ? XEN_DOMINF_paused   : 0) |
-        (d->debugger_attached           ? XEN_DOMINF_debugged : 0) |
+        ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying     : 0) |
+        (d->is_shut_down                ? XEN_DOMINF_shutdown  : 0) |
+        (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
+        (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
+        (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;

     switch ( d->guest_type )
@@ -551,7 +552,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
                | XEN_DOMCTL_CDF_pvh_guest
                | XEN_DOMCTL_CDF_hap
                | XEN_DOMCTL_CDF_s3_integrity
-               | XEN_DOMCTL_CDF_oos_off)) )
+               | XEN_DOMCTL_CDF_oos_off
+               | XEN_DOMCTL_CDF_xs_domain)) )
             break;

         dom = op->domain;
@@ -593,6 +595,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
             domcr_flags |= DOMCRF_s3_integrity;
         if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off )
             domcr_flags |= DOMCRF_oos_off;
+        if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_xs_domain )
+            domcr_flags |= DOMCRF_xs_domain;

         d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref,
                           &op->u.createdomain.config);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 7a56b3f..2d8076c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -63,6 +63,9 @@ struct xen_domctl_createdomain {
  /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
 #define _XEN_DOMCTL_CDF_pvh_guest     4
 #define XEN_DOMCTL_CDF_pvh_guest      (1U<<_XEN_DOMCTL_CDF_pvh_guest)
+ /* Is this a xenstore domain? */
+#define _XEN_DOMCTL_CDF_xs_domain     5
+#define XEN_DOMCTL_CDF_xs_domain      (1U<<_XEN_DOMCTL_CDF_xs_domain)
     uint32_t flags;
     struct xen_arch_domainconfig config;
 };
@@ -97,6 +100,9 @@ struct xen_domctl_getdomaininfo {
 /* domain is PVH */
 #define _XEN_DOMINF_pvh_guest 7
 #define XEN_DOMINF_pvh_guest  (1U<<_XEN_DOMINF_pvh_guest)
+/* domain is a xenstore domain */
+#define _XEN_DOMINF_xs_domain 8
+#define XEN_DOMINF_xs_domain  (1U<<_XEN_DOMINF_xs_domain)
  /* XEN_DOMINF_shutdown guest-supplied code.  */
 #define XEN_DOMINF_shutdownmask 255
 #define XEN_DOMINF_shutdownshift 16
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3729b0f..5b18bba 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -374,6 +374,8 @@ struct domain
     bool_t           auto_node_affinity;
     /* Is this guest fully privileged (aka dom0)? */
     bool_t           is_privileged;
+    /* Is this a xenstore domain (not dom0)? */
+    bool_t           is_xenstore;
     /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
     bool_t           is_pinned;
     /* Non-migratable and non-restoreable? */
@@ -533,6 +535,9 @@ struct domain *domain_create(domid_t domid, unsigned
int domcr_flags,
  /* DOMCRF_pvh: Create PV domain in HVM container. */
 #define _DOMCRF_pvh             5
 #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
+ /* DOMCRF_xs_domain: xenstore domain */
+#define _DOMCRF_xs_domain       6
+#define DOMCRF_xs_domain        (1U<<_DOMCRF_xs_domain)

 /*
  * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index e43f2a1..a07c4c6 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -71,6 +71,10 @@ static always_inline int xsm_default_action(
         if ( src->is_privileged )
             return 0;
         return -EPERM;
+    case XSM_XS_PRIV:
+        if ( src->is_xenstore || src->is_privileged )
+            return 0;
+        return -EPERM;
     default:
         LINKER_BUG_ON(1);
         return -EPERM;
@@ -123,6 +127,8 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG
struct domain *d, int cmd)
     case XEN_DOMCTL_bind_pt_irq:
     case XEN_DOMCTL_unbind_pt_irq:
         return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+    case XEN_DOMCTL_getdomaininfo:
+        return xsm_default_action(XSM_XS_PRIV, current->domain, d);
     default:
         return xsm_default_action(XSM_PRIV, current->domain, d);
     }
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index f48cf60..01329b8 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -35,6 +35,7 @@ enum xsm_default {
     XSM_DM_PRIV,  /* Device model can perform on its target domain */
     XSM_TARGET,   /* Can perform on self or your target domain */
     XSM_PRIV,     /* Privileged - normally restricted to dom0 */
+    XSM_XS_PRIV,  /* Xenstore domain can obtain domain info */
     XSM_OTHER     /* Something more complex */
 };
 typedef enum xsm_default xsm_default_t;
-- 
2.6.2


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

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-11 15:47 ` [PATCH 3/9] xenstore: install init-xenstore-domain via make install Juergen Gross
@ 2015-12-15 12:16   ` Ian Campbell
  2015-12-15 12:19     ` Juergen Gross
  2015-12-15 21:41     ` Daniel De Graaf
  0 siblings, 2 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:16 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> The program init-xenstore-domain to start a xenstore domain instead
> of the xenstored daemon is built, but not installed. Change that.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

CCing Daniel too since he has an interest in this mode of operation. 

I appreciate the CONFIG_Linux thing is pre-existing, and stems from being
unable to build stubdoms on other platforms, but should we consider
building these supporting tools anyway, after all a user might get a
suitable stubdom via some other means?

> ---
>  tools/xenstore/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> index 1b4a494..8c8b116 100644
> --- a/tools/xenstore/Makefile
> +++ b/tools/xenstore/Makefile
> @@ -153,6 +153,9 @@ endif
>  	$(INSTALL_DATA) include/compat/xs_lib.h
> $(DESTDIR)$(includedir)/xenstore-compat/xs_lib.h
>  	ln -sf xenstore-compat/xs.h  $(DESTDIR)$(includedir)/xs.h
>  	ln -sf xenstore-compat/xs_lib.h $(DESTDIR)$(includedir)/xs_lib.h
> +ifeq ($(CONFIG_Linux),y)
> +	$(INSTALL_PROG) init-xenstore-domain $(DESTDIR)$(LIBEXEC_BIN)
> +endif
>  
>  .PHONY: clients-install
>  clients-install: clients

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

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-15 12:16   ` Ian Campbell
@ 2015-12-15 12:19     ` Juergen Gross
  2015-12-15 12:31       ` Ian Campbell
  2015-12-15 21:41     ` Daniel De Graaf
  1 sibling, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:19 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On 15/12/15 13:16, Ian Campbell wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> The program init-xenstore-domain to start a xenstore domain instead
>> of the xenstored daemon is built, but not installed. Change that.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> CCing Daniel too since he has an interest in this mode of operation. 
> 
> I appreciate the CONFIG_Linux thing is pre-existing, and stems from being
> unable to build stubdoms on other platforms, but should we consider
> building these supporting tools anyway, after all a user might get a
> suitable stubdom via some other means?

In case there are no objections, I can remove the CONFIG_Linux
dependency.


Juergen

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

* Re: [PATCH 4/9] xenstore: add error messages to init-xenstore-domain
  2015-12-11 15:47 ` [PATCH 4/9] xenstore: add error messages to init-xenstore-domain Juergen Gross
@ 2015-12-15 12:20   ` Ian Campbell
  2015-12-15 21:54     ` Daniel De Graaf
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:20 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

Adding Daniel again.

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> Additional add some diagnostic messages to the program to have an idea
> why it failed.
[...]
>  err:
>  	if (dom)
>  		xc_dom_release(dom);
> +	if (domid >= 0)
> +		xc_domain_destroy(xch, domid);

This important looking bug fix should't be buried in this otherwise quite
mechanical commit.

The rest looks reasonable, so with the above moved to a separate commit
this one would be:
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

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

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

* Re: [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax
  2015-12-11 15:47 ` [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax Juergen Gross
@ 2015-12-15 12:22   ` Ian Campbell
  2015-12-15 21:49     ` Daniel De Graaf
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:22 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

+Daniel
On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> init-xenstore-domain takes only positional parameters today. Change
> this to a more flexible parameter syntax allowing to specify additional
> options or to omit some.
> 
> Today the supported usage is:
> 
> init-xenstore-domain <xenstore-kernel> <memory_mb> <flask-label>
>                      [<ramdisk-file>]
> 
> Modify this to:
> 
> init-xenstore-domain --kernel <xenstore-kernel>
>                      --memory <memory_mb>
>                      [--flask <flask-label>]
>                      [--ramdisk <ramdisk-file>]
> 
> The flask label is made optional in order to support xenstore domains
> without the need of a flask enabled hypervisor.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Looks fine to me

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I'd like to see Daniel opinion of all the changes to init-xenstore-domain.c 
in this series though.

> ---
>  tools/xenstore/init-xenstore-domain.c | 79 ++++++++++++++++++++++++++++-
> ------
>  1 file changed, 63 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-
> xenstore-domain.c
> index c9963c6..068887c 100644
> --- a/tools/xenstore/init-xenstore-domain.c
> +++ b/tools/xenstore/init-xenstore-domain.c
> @@ -4,6 +4,7 @@
>  #include <string.h>
>  #include <stdint.h>
>  #include <stdlib.h>
> +#include <getopt.h>
>  #include <sys/ioctl.h>
>  #include <sys/mman.h>
>  #include <xenctrl.h>
> @@ -13,16 +14,42 @@
>  #include <xen-xsm/flask/flask.h>
>  
>  static uint32_t domid = -1;
> +static char *kernel;
> +static char *ramdisk;
> +static char *flask;
> +static int memory;
> +
> +static struct option options[] = {
> +	{ "kernel", 1, NULL, 'k' },
> +	{ "memory", 1, NULL, 'm' },
> +	{ "flask", 1, NULL, 'f' },
> +	{ "ramdisk", 1, NULL, 'r' },
> +	{ NULL, 0, NULL, 0 }
> +};
> +
> +static void usage(void)
> +{
> +	fprintf(stderr,
> +"Usage:\n"
> +"\n"
> +"init-xenstore-domain <options>\n"
> +"\n"
> +"where options may include:\n"
> +"\n"
> +"  --kernel <xenstore-kernel>  kernel file of the xenstore domain,
> mandatory\n"
> +"  --memory <memory size>      size of the domain in MB, mandatory\n"
> +"  --flask <flask-label>       optional flask label of the domain\n"
> +"  --ramdisk <ramdisk-file>    optional ramdisk file for the domain\n");
> +}
>  
> -static int build(xc_interface *xch, int argc, char** argv)
> +static int build(xc_interface *xch)
>  {
>  	char cmdline[512];
>  	uint32_t ssid;
>  	xen_domain_handle_t handle = { 0 };
>  	int rv, xs_fd;
>  	struct xc_dom_image *dom = NULL;
> -	int maxmem = atoi(argv[2]);
> -	int limit_kb = (maxmem + 1)*1024;
> +	int limit_kb = (memory + 1)*1024;
>  
>  	xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
>  	if (xs_fd == -1) {
> @@ -30,10 +57,14 @@ static int build(xc_interface *xch, int argc, char**
> argv)
>  		return -1;
>  	}
>  
> -	rv = xc_flask_context_to_sid(xch, argv[3], strlen(argv[3]),
> &ssid);
> -	if (rv) {
> -		fprintf(stderr, "xc_flask_context_to_sid failed\n");
> -		goto err;
> +	if (flask) {
> +		rv = xc_flask_context_to_sid(xch, flask, strlen(flask),
> &ssid);
> +		if (rv) {
> +			fprintf(stderr, "xc_flask_context_to_sid
> failed\n");
> +			goto err;
> +		}
> +	} else {
> +		ssid = SECINITSID_DOMU;
>  	}
>  	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
>  	if (rv) {
> @@ -64,14 +95,14 @@ static int build(xc_interface *xch, int argc, char**
> argv)
>  	snprintf(cmdline, 512, "--event %d --internal-db", rv);
>  
>  	dom = xc_dom_allocate(xch, cmdline, NULL);
> -	rv = xc_dom_kernel_file(dom, argv[1]);
> +	rv = xc_dom_kernel_file(dom, kernel);
>  	if (rv) {
>  		fprintf(stderr, "xc_dom_kernel_file failed\n");
>  		goto err;
>  	}
>  
> -	if (argc > 4) {
> -		rv = xc_dom_ramdisk_file(dom, argv[4]);
> +	if (ramdisk) {
> +		rv = xc_dom_ramdisk_file(dom, ramdisk);
>  		if (rv) {
>  			fprintf(stderr, "xc_dom_ramdisk_file failed\n");
>  			goto err;
> @@ -88,7 +119,7 @@ static int build(xc_interface *xch, int argc, char**
> argv)
>  		fprintf(stderr, "xc_dom_parse_image failed\n");
>  		goto err;
>  	}
> -	rv = xc_dom_mem_init(dom, maxmem);
> +	rv = xc_dom_mem_init(dom, memory);
>  	if (rv) {
>  		fprintf(stderr, "xc_dom_mem_init failed\n");
>  		goto err;
> @@ -136,15 +167,31 @@ err:
>  
>  int main(int argc, char** argv)
>  {
> +	int opt;
>  	xc_interface *xch;
>  	struct xs_handle *xsh;
>  	char buf[16];
>  	int rv, fd;
>  
> -	if (argc < 4 || argc > 5) {
> -		fprintf(stderr,
> -			"Use: %s <xenstore-kernel> <memory_mb> <flask-
> label> [<ramdisk-file>]\n",
> -			argv[0]);
> +	while ((opt = getopt_long(argc, argv, "", options, NULL)) != -1)
> {
> +		switch (opt) {
> +		case 'k':
> +			kernel = optarg;
> +			break;
> +		case 'm':
> +			memory = strtol(optarg, NULL, 10);
> +			break;
> +		case 'f':
> +			flask = optarg;
> +			break;
> +		case 'r':
> +			ramdisk = optarg;
> +			break;
> +		}
> +	}
> +
> +	if (optind != argc) {
> +		usage();
>  		return 2;
>  	}
>  
> @@ -154,7 +201,7 @@ int main(int argc, char** argv)
>  		return 1;
>  	}
>  
> -	rv = build(xch, argc, argv);
> +	rv = build(xch);
>  
>  	xc_interface_close(xch);
>  

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

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-11 15:47 ` [PATCH 6/9] xenstore: don't start xenstore domain if already one is active Juergen Gross
@ 2015-12-15 12:23   ` Ian Campbell
  2015-12-15 12:28     ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:23 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> Don't start a new xenstore domain in case one is already detected to
> be running.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenstore/init-xenstore-domain.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-
> xenstore-domain.c
> index 068887c..0ca7eed 100644
> --- a/tools/xenstore/init-xenstore-domain.c
> +++ b/tools/xenstore/init-xenstore-domain.c
> @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
>  	} else {
>  		ssid = SECINITSID_DOMU;
>  	}
> -	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
> +	rv = xc_domain_create(xch, ssid, handle, XEN_DOMCTL_CDF_xs_domain,
> +			      &domid, NULL);

Doesn't this bit belong earlier on in the series?

>  	if (rv) {
>  		fprintf(stderr, "xc_domain_create failed\n");
>  		goto err;
> @@ -165,6 +166,21 @@ err:
>  	return rv;
>  }
>  
> +static int check_domain(xc_interface *xch)
> +{
> +	xc_dominfo_t info;
> +	uint32_t dom;
> +
> +	dom = 0;
> +	while (xc_domain_getinfo(xch, dom, 1, &info) == 1) {
> +		if (info.xs_domain)
> +			return 1;
> +		dom = info.domid + 1;
> +	}
> +
> +	return 0;
> +}
> +
>  int main(int argc, char** argv)
>  {
>  	int opt;
> @@ -201,7 +217,12 @@ int main(int argc, char** argv)
>  		return 1;
>  	}
>  
> -	rv = build(xch);
> +	rv = check_domain(xch);
> +
> +	if (!rv)
> +		rv = build(xch);
> +	else
> +		fprintf(stderr, "xenstore domain already present.\n");
>  
>  	xc_interface_close(xch);
>  

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

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

* Re: [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline
  2015-12-11 15:47 ` [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline Juergen Gross
@ 2015-12-15 12:24   ` Ian Campbell
  0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:24 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> Add a parameter to init-xenstore-domain for support of arbitrary
> parameters for the xenstore domain.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenstore/init-xenstore-domain.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-
> xenstore-domain.c
> index 0ca7eed..eedcf32 100644
> --- a/tools/xenstore/init-xenstore-domain.c
> +++ b/tools/xenstore/init-xenstore-domain.c
> @@ -17,6 +17,7 @@ static uint32_t domid = -1;
>  static char *kernel;
>  static char *ramdisk;
>  static char *flask;
> +static char *param;
>  static int memory;
>  
>  static struct option options[] = {
> @@ -24,6 +25,7 @@ static struct option options[] = {
>  	{ "memory", 1, NULL, 'm' },
>  	{ "flask", 1, NULL, 'f' },
>  	{ "ramdisk", 1, NULL, 'r' },
> +	{ "param", 1, NULL, 'p' },
>  	{ NULL, 0, NULL, 0 }
>  };
>  
> @@ -36,10 +38,11 @@ static void usage(void)
>  "\n"
>  "where options may include:\n"
>  "\n"
> -"  --kernel   kernel file of the xenstore domain, mandatory\n"
> -"  --memory       size of the domain in MB, mandatory\n"
> -"  --flask        optional flask label of the domain\n"
> -"  --ramdisk     optional ramdisk file for the domain\n");
> +"  --kernel  kernel file of the xenstore domain, mandatory\n"
> +"  --memory      size of the domain in MB, mandatory\n"
> +"  --flask       optional flask label of the domain\n"
> +"  --ramdisk    optional ramdisk file for the domain\n"
> +"  --param <cmdline>          optional additional parameters for the domain\n");

This seems spurious, either get the indent right when you add the original
block or leave it alone here I think.

Otherwise this looks good to me:
Acked-by: Ian Campbell <ian.campbell@citrix.com>

>  }
>  
>  static int build(xc_interface *xch)
> @@ -93,7 +96,12 @@ static int build(xc_interface *xch)
>  		fprintf(stderr, "Xenbus setup ioctl failed\n");
>  		goto err;
>  	}
> -	snprintf(cmdline, 512, "--event %d --internal-db", rv);
> +
> +	if (param)
> +		snprintf(cmdline, 512, "--event %d --internal-db %s",
> rv,
> +			 param);
> +	else
> +		snprintf(cmdline, 512, "--event %d --internal-db", rv);
>  
>  	dom = xc_dom_allocate(xch, cmdline, NULL);
>  	rv = xc_dom_kernel_file(dom, kernel);
> @@ -203,6 +211,9 @@ int main(int argc, char** argv)
>  		case 'r':
>  			ramdisk = optarg;
>  			break;
> +		case 'p':
> +			param = optarg;
> +			break;
>  		}
>  	}
>  

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

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-11 15:47 ` [PATCH 8/9] xenstore: write xenstore domain data to xenstore Juergen Gross
@ 2015-12-15 12:26   ` Ian Campbell
  2015-12-15 12:34     ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:26 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> After starting the xenstore domain write the basic data (domid and
> name) to the xenstore.
> 
> Add a new option to init-xenstore-domain to be able to specify the
> domain's name.

Is all this enough to keep "xl list" etc happy?

Do we need to also create a dummy json object like we do for dom0 (init-
xen-dom0)?

> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenstore/init-xenstore-domain.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-
> xenstore-domain.c
> index eedcf32..2a8a417 100644
> --- a/tools/xenstore/init-xenstore-domain.c
> +++ b/tools/xenstore/init-xenstore-domain.c
> @@ -18,6 +18,7 @@ static char *kernel;
>  static char *ramdisk;
>  static char *flask;
>  static char *param;
> +static char *name = "Xenstore";
>  static int memory;
>  
>  static struct option options[] = {
> @@ -26,6 +27,7 @@ static struct option options[] = {
>  	{ "flask", 1, NULL, 'f' },
>  	{ "ramdisk", 1, NULL, 'r' },
>  	{ "param", 1, NULL, 'p' },
> +	{ "name", 1, NULL, 'n' },
>  	{ NULL, 0, NULL, 0 }
>  };
>  
> @@ -42,7 +44,8 @@ static void usage(void)
>  "  --memory <memory size>     size of the domain in MB, mandatory\n"
>  "  --flask <flask-label>      optional flask label of the domain\n"
>  "  --ramdisk <ramdisk-file>   optional ramdisk file for the domain\n"
> -"  --param <cmdline>          optional additional parameters for the
> domain\n");
> +"  --param <cmdline>          optional additional parameters for the
> domain\n"
> +"  --name <name>              name of the domain (default:
> Xenstore)\n");
>  }
>  
>  static int build(xc_interface *xch)
> @@ -195,6 +198,7 @@ int main(int argc, char** argv)
>  	xc_interface *xch;
>  	struct xs_handle *xsh;
>  	char buf[16];
> +	char path[64];
>  	int rv, fd;
>  
>  	while ((opt = getopt_long(argc, argv, "", options, NULL)) != -1)
> {
> @@ -214,6 +218,9 @@ int main(int argc, char** argv)
>  		case 'p':
>  			param = optarg;
>  			break;
> +		case 'n':
> +			name = optarg;
> +			break;
>  		}
>  	}
>  
> @@ -243,6 +250,10 @@ int main(int argc, char** argv)
>  	xsh = xs_open(0);
>  	rv = snprintf(buf, 16, "%d", domid);
>  	xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv);
> +	snprintf(path, 64, "/local/domain/%d/domid", domid);
> +	xs_write(xsh, XBT_NULL, path, buf, rv);
> +	snprintf(path, 64, "/local/domain/%d/name", domid);
> +	xs_write(xsh, XBT_NULL, path, name, strlen(name));
>  	xs_daemon_close(xsh);
>  
>  	fd = creat("/var/run/xenstored.pid", 0666);

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

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-15 12:23   ` Ian Campbell
@ 2015-12-15 12:28     ` Juergen Gross
  2015-12-15 12:32       ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:28 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On 15/12/15 13:23, Ian Campbell wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> Don't start a new xenstore domain in case one is already detected to
>> be running.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  tools/xenstore/init-xenstore-domain.c | 25 +++++++++++++++++++++++--
>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/xenstore/init-xenstore-domain.c b/tools/xenstore/init-
>> xenstore-domain.c
>> index 068887c..0ca7eed 100644
>> --- a/tools/xenstore/init-xenstore-domain.c
>> +++ b/tools/xenstore/init-xenstore-domain.c
>> @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
>>  	} else {
>>  		ssid = SECINITSID_DOMU;
>>  	}
>> -	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
>> +	rv = xc_domain_create(xch, ssid, handle, XEN_DOMCTL_CDF_xs_domain,
>> +			      &domid, NULL);
> 
> Doesn't this bit belong earlier on in the series?

I can make this patch number 3 of the series, if you like.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-11 15:47 ` [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages Juergen Gross
@ 2015-12-15 12:31   ` Ian Campbell
  2015-12-15 12:47     ` Juergen Gross
  2015-12-15 13:03     ` Samuel Thibault
  0 siblings, 2 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:31 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> Xenstore messages for diagnostic purposes are lost today in case
> xenstore is running under mini-os instead in a dom0 daemon.

Where does vfprintf end up under mini-os? 

> Use printk of mini-os in this situation.

and this now ends up on the console and (for a debug h/v) in the xen dmesg,
is that right?



> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/xenstore/utils.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/xenstore/utils.c b/tools/xenstore/utils.c
> index a1ac125..ea8a5e8 100644
> --- a/tools/xenstore/utils.c
> +++ b/tools/xenstore/utils.c
> @@ -10,6 +10,11 @@
>  #include <signal.h>
>  #include "utils.h"
>  
> +#ifdef __MINIOS__
> +void printk(const char *fmt, ...);
> +
> +void (*xprintf)(const char *fmt, ...) = printk;
> +#else
>  static void default_xprintf(const char *fmt, ...)
>  {
>  	va_list args;
> @@ -21,6 +26,7 @@ static void default_xprintf(const char *fmt, ...)
>  }
>  
>  void (*xprintf)(const char *fmt, ...) = default_xprintf;
> +#endif
>  
>  void barf(const char *fmt, ...)
>  {

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

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-15 12:19     ` Juergen Gross
@ 2015-12-15 12:31       ` Ian Campbell
  0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:31 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Tue, 2015-12-15 at 13:19 +0100, Juergen Gross wrote:
> On 15/12/15 13:16, Ian Campbell wrote:
> > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > The program init-xenstore-domain to start a xenstore domain instead
> > > of the xenstored daemon is built, but not installed. Change that.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > CCing Daniel too since he has an interest in this mode of operation. 
> > 
> > I appreciate the CONFIG_Linux thing is pre-existing, and stems from
> > being
> > unable to build stubdoms on other platforms, but should we consider
> > building these supporting tools anyway, after all a user might get a
> > suitable stubdom via some other means?
> 
> In case there are no objections, I can remove the CONFIG_Linux
> dependency.

I think that would be ok, if you are happy to so so, but please do it in a
separate patch.

Ian.

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-15 12:28     ` Juergen Gross
@ 2015-12-15 12:32       ` Ian Campbell
  2015-12-15 12:40         ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:32 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Tue, 2015-12-15 at 13:28 +0100, Juergen Gross wrote:
> On 15/12/15 13:23, Ian Campbell wrote:
> > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > Don't start a new xenstore domain in case one is already detected to
> > > be running.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > ---
> > >  tools/xenstore/init-xenstore-domain.c | 25 +++++++++++++++++++++++--
> > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/xenstore/init-xenstore-domain.c
> > > b/tools/xenstore/init-
> > > xenstore-domain.c
> > > index 068887c..0ca7eed 100644
> > > --- a/tools/xenstore/init-xenstore-domain.c
> > > +++ b/tools/xenstore/init-xenstore-domain.c
> > > @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
> > >  	} else {
> > >  		ssid = SECINITSID_DOMU;
> > >  	}
> > > -	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
> > > +	rv = xc_domain_create(xch, ssid, handle,
> > > XEN_DOMCTL_CDF_xs_domain,
> > > +			      &domid, NULL);
> > 
> > Doesn't this bit belong earlier on in the series?
> 
> I can make this patch number 3 of the series, if you like.

Not part of the patch which adds the setting?

NB, I think you mean just this hunk rather than reordering this patch?
That's what I was trying to get at at least.

Ian.

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

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 12:26   ` Ian Campbell
@ 2015-12-15 12:34     ` Juergen Gross
  2015-12-15 12:49       ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:34 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 13:26, Ian Campbell wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> After starting the xenstore domain write the basic data (domid and
>> name) to the xenstore.
>>
>> Add a new option to init-xenstore-domain to be able to specify the
>> domain's name.
> 
> Is all this enough to keep "xl list" etc happy?

I hope so. At least I didn't notice any problems.

> Do we need to also create a dummy json object like we do for dom0 (init-
> xen-dom0)?

Good question. Depends on what the dom0 json object is used for.

BTW: I think I've found another detail missing:
tools/hotplug/Linux/xendomains.in might need a tweak to avoid stopping
the xenstore domain.


Juergen

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-15 12:32       ` Ian Campbell
@ 2015-12-15 12:40         ` Juergen Gross
  2015-12-15 12:47           ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:40 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On 15/12/15 13:32, Ian Campbell wrote:
> On Tue, 2015-12-15 at 13:28 +0100, Juergen Gross wrote:
>> On 15/12/15 13:23, Ian Campbell wrote:
>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>> Don't start a new xenstore domain in case one is already detected to
>>>> be running.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>  tools/xenstore/init-xenstore-domain.c | 25 +++++++++++++++++++++++--
>>>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/xenstore/init-xenstore-domain.c
>>>> b/tools/xenstore/init-
>>>> xenstore-domain.c
>>>> index 068887c..0ca7eed 100644
>>>> --- a/tools/xenstore/init-xenstore-domain.c
>>>> +++ b/tools/xenstore/init-xenstore-domain.c
>>>> @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
>>>>  	} else {
>>>>  		ssid = SECINITSID_DOMU;
>>>>  	}
>>>> -	rv = xc_domain_create(xch, ssid, handle, 0, &domid, NULL);
>>>> +	rv = xc_domain_create(xch, ssid, handle,
>>>> XEN_DOMCTL_CDF_xs_domain,
>>>> +			      &domid, NULL);
>>>
>>> Doesn't this bit belong earlier on in the series?
>>
>> I can make this patch number 3 of the series, if you like.
> 
> Not part of the patch which adds the setting?

Hmm, do you really think so? Isn't the normal setup of a patch series
to split adding a new feature and using it to different patches?

> NB, I think you mean just this hunk rather than reordering this patch?
> That's what I was trying to get at at least.

Why? With specifying the xenstore flag I can make use of it being set
for a xenstore domain in the same patch. I can split it, if you really
like, but this split would be less obvious than the one between this
patch and patch 2, IMO.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 12:31   ` Ian Campbell
@ 2015-12-15 12:47     ` Juergen Gross
  2015-12-15 12:52       ` Ian Campbell
  2015-12-15 13:03     ` Samuel Thibault
  1 sibling, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:47 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 13:31, Ian Campbell wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> Xenstore messages for diagnostic purposes are lost today in case
>> xenstore is running under mini-os instead in a dom0 daemon.
> 
> Where does vfprintf end up under mini-os?

TBH: I just couldn't find it. I tried to get some diagnostics when
I started this work and couldn't find any place where the messages
would occur.

>> Use printk of mini-os in this situation.
> 
> and this now ends up on the console and (for a debug h/v) in the xen dmesg,
> is that right?

The console of the xenstore domain seems to be a little bit special, as
there is no component up to now writing the information needed to
connect xenconsoled to the xenstore domain. This could be the reason I
wasn't able to see any message printed via vfprintf. :-(

So in the end it will be part of xen dmesg. And this is where it should
be, as such messages will be needed in case of xenstore domain not
working properly. And I think it's console can be accessed by any means
only in case of a working xenstore. :-)


Juergen

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-15 12:40         ` Juergen Gross
@ 2015-12-15 12:47           ` Ian Campbell
  2015-12-15 12:49             ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:47 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Tue, 2015-12-15 at 13:40 +0100, Juergen Gross wrote:
> On 15/12/15 13:32, Ian Campbell wrote:
> > On Tue, 2015-12-15 at 13:28 +0100, Juergen Gross wrote:
> > > On 15/12/15 13:23, Ian Campbell wrote:
> > > > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > > > Don't start a new xenstore domain in case one is already detected
> > > > > to
> > > > > be running.
> > > > > 
> > > > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > > > ---
> > > > >  tools/xenstore/init-xenstore-domain.c | 25
> > > > > +++++++++++++++++++++++--
> > > > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/tools/xenstore/init-xenstore-domain.c
> > > > > b/tools/xenstore/init-
> > > > > xenstore-domain.c
> > > > > index 068887c..0ca7eed 100644
> > > > > --- a/tools/xenstore/init-xenstore-domain.c
> > > > > +++ b/tools/xenstore/init-xenstore-domain.c
> > > > > @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
> > > > >  	} else {
> > > > >  		ssid = SECINITSID_DOMU;
> > > > >  	}
> > > > > -	rv = xc_domain_create(xch, ssid, handle, 0, &domid,
> > > > > NULL);
> > > > > +	rv = xc_domain_create(xch, ssid, handle,
> > > > > XEN_DOMCTL_CDF_xs_domain,
> > > > > +			      &domid, NULL);
> > > > 
> > > > Doesn't this bit belong earlier on in the series?
> > > 
> > > I can make this patch number 3 of the series, if you like.
> > 
> > Not part of the patch which adds the setting?
> 
> Hmm, do you really think so? Isn't the normal setup of a patch series
> to split adding a new feature and using it to different patches?

Yes, but the title of this patch is "don't start xenstore if..." not "mark
the xenstore domain as being such".

IOW I think a better commit log would be a valid alternative to splitting.

> > NB, I think you mean just this hunk rather than reordering this patch?
> > That's what I was trying to get at at least.
> 
> Why? With specifying the xenstore flag I can make use of it being set
> for a xenstore domain in the same patch. I can split it, if you really
> like, but this split would be less obvious than the one between this
> patch and patch 2, IMO.



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

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 12:34     ` Juergen Gross
@ 2015-12-15 12:49       ` Ian Campbell
  2015-12-15 12:53         ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:49 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On Tue, 2015-12-15 at 13:34 +0100, Juergen Gross wrote:
> On 15/12/15 13:26, Ian Campbell wrote:
> > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > After starting the xenstore domain write the basic data (domid and
> > > name) to the xenstore.
> > > 
> > > Add a new option to init-xenstore-domain to be able to specify the
> > > domain's name.
> > 
> > Is all this enough to keep "xl list" etc happy?
> 
> I hope so. At least I didn't notice any problems.

Great. Please could you add "This makes the domain appear correctly in xl
list" or something similar.

> > Do we need to also create a dummy json object like we do for dom0
> > (init-
> > xen-dom0)?
> 
> Good question. Depends on what the dom0 json object is used for.

Mostly for various types of hotplug (irrelevant here I guess?) and reboot
(also irrelevant?).

Maybe also "xl list -v" or some other "look more closely" xl commands?

mem-set too perhaps?

Ian.

> BTW: I think I've found another detail missing:
> tools/hotplug/Linux/xendomains.in might need a tweak to avoid stopping
> the xenstore domain.
> 
> 
> Juergen

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

* Re: [PATCH 6/9] xenstore: don't start xenstore domain if already one is active
  2015-12-15 12:47           ` Ian Campbell
@ 2015-12-15 12:49             ` Juergen Gross
  0 siblings, 0 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:49 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On 15/12/15 13:47, Ian Campbell wrote:
> On Tue, 2015-12-15 at 13:40 +0100, Juergen Gross wrote:
>> On 15/12/15 13:32, Ian Campbell wrote:
>>> On Tue, 2015-12-15 at 13:28 +0100, Juergen Gross wrote:
>>>> On 15/12/15 13:23, Ian Campbell wrote:
>>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>>> Don't start a new xenstore domain in case one is already detected
>>>>>> to
>>>>>> be running.
>>>>>>
>>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>>> ---
>>>>>>  tools/xenstore/init-xenstore-domain.c | 25
>>>>>> +++++++++++++++++++++++--
>>>>>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/xenstore/init-xenstore-domain.c
>>>>>> b/tools/xenstore/init-
>>>>>> xenstore-domain.c
>>>>>> index 068887c..0ca7eed 100644
>>>>>> --- a/tools/xenstore/init-xenstore-domain.c
>>>>>> +++ b/tools/xenstore/init-xenstore-domain.c
>>>>>> @@ -66,7 +66,8 @@ static int build(xc_interface *xch)
>>>>>>  	} else {
>>>>>>  		ssid = SECINITSID_DOMU;
>>>>>>  	}
>>>>>> -	rv = xc_domain_create(xch, ssid, handle, 0, &domid,
>>>>>> NULL);
>>>>>> +	rv = xc_domain_create(xch, ssid, handle,
>>>>>> XEN_DOMCTL_CDF_xs_domain,
>>>>>> +			      &domid, NULL);
>>>>>
>>>>> Doesn't this bit belong earlier on in the series?
>>>>
>>>> I can make this patch number 3 of the series, if you like.
>>>
>>> Not part of the patch which adds the setting?
>>
>> Hmm, do you really think so? Isn't the normal setup of a patch series
>> to split adding a new feature and using it to different patches?
> 
> Yes, but the title of this patch is "don't start xenstore if..." not "mark
> the xenstore domain as being such".
> 
> IOW I think a better commit log would be a valid alternative to splitting.

Yes, this makes sense.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 12:47     ` Juergen Gross
@ 2015-12-15 12:52       ` Ian Campbell
  2015-12-15 12:55         ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 12:52 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
> On 15/12/15 13:31, Ian Campbell wrote:
> > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > Xenstore messages for diagnostic purposes are lost today in case
> > > xenstore is running under mini-os instead in a dom0 daemon.
> > 
> > Where does vfprintf end up under mini-os?
> 
> TBH: I just couldn't find it. I tried to get some diagnostics when
> I started this work and couldn't find any place where the messages
> would occur.
> 
> > > Use printk of mini-os in this situation.
> > 
> > and this now ends up on the console and (for a debug h/v) in the xen
> > dmesg,
> > is that right?
> 
> The console of the xenstore domain seems to be a little bit special, as
> there is no component up to now writing the information needed to
> connect xenconsoled to the xenstore domain. This could be the reason I
> wasn't able to see any message printed via vfprintf. :-(

That sounds probable.

> So in the end it will be part of xen dmesg.

Only for a debug=y Xen, not for a release build IIRC, unless you added some
other privilege along with your new flag which I missed.

>  And this is where it should
> be, as such messages will be needed in case of xenstore domain not
> working properly. And I think it's console can be accessed by any means
> only in case of a working xenstore. :-)

An alternative would be init-xenstore-domain to daemonize and take on
respoibility for consuming the console ring and throwing it into a file?

Ian.

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 12:49       ` Ian Campbell
@ 2015-12-15 12:53         ` Juergen Gross
  2015-12-15 13:19           ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:53 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 13:49, Ian Campbell wrote:
> On Tue, 2015-12-15 at 13:34 +0100, Juergen Gross wrote:
>> On 15/12/15 13:26, Ian Campbell wrote:
>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>> After starting the xenstore domain write the basic data (domid and
>>>> name) to the xenstore.
>>>>
>>>> Add a new option to init-xenstore-domain to be able to specify the
>>>> domain's name.
>>>
>>> Is all this enough to keep "xl list" etc happy?
>>
>> I hope so. At least I didn't notice any problems.
> 
> Great. Please could you add "This makes the domain appear correctly in xl
> list" or something similar.

Sure.

>>> Do we need to also create a dummy json object like we do for dom0
>>> (init-
>>> xen-dom0)?
>>
>> Good question. Depends on what the dom0 json object is used for.
> 
> Mostly for various types of hotplug (irrelevant here I guess?) and reboot
> (also irrelevant?).

I think so (the "irrelevant" part).

> Maybe also "xl list -v" or some other "look more closely" xl commands?

Hmm, I'll have a closer look.

> mem-set too perhaps?

Without mini-os supporting ballooning or memory hotplug up to now:
I don't think so. :-)


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 12:52       ` Ian Campbell
@ 2015-12-15 12:55         ` Juergen Gross
  2015-12-15 14:06           ` Andrew Cooper
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 12:55 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 13:52, Ian Campbell wrote:
> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>> On 15/12/15 13:31, Ian Campbell wrote:
>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>
>>> Where does vfprintf end up under mini-os?
>>
>> TBH: I just couldn't find it. I tried to get some diagnostics when
>> I started this work and couldn't find any place where the messages
>> would occur.
>>
>>>> Use printk of mini-os in this situation.
>>>
>>> and this now ends up on the console and (for a debug h/v) in the xen
>>> dmesg,
>>> is that right?
>>
>> The console of the xenstore domain seems to be a little bit special, as
>> there is no component up to now writing the information needed to
>> connect xenconsoled to the xenstore domain. This could be the reason I
>> wasn't able to see any message printed via vfprintf. :-(
> 
> That sounds probable.
> 
>> So in the end it will be part of xen dmesg.
> 
> Only for a debug=y Xen, not for a release build IIRC, unless you added some
> other privilege along with your new flag which I missed.

No, this is correct.

>>  And this is where it should
>> be, as such messages will be needed in case of xenstore domain not
>> working properly. And I think it's console can be accessed by any means
>> only in case of a working xenstore. :-)
> 
> An alternative would be init-xenstore-domain to daemonize and take on
> respoibility for consuming the console ring and throwing it into a file?

Interesting idea. I'll try this.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 12:31   ` Ian Campbell
  2015-12-15 12:47     ` Juergen Gross
@ 2015-12-15 13:03     ` Samuel Thibault
  1 sibling, 0 replies; 49+ messages in thread
From: Samuel Thibault @ 2015-12-15 13:03 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Juergen Gross, wei.liu2, stefano.stabellini, ian.jackson, xen-devel

Ian Campbell, on Tue 15 Dec 2015 12:31:05 +0000, wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > Xenstore messages for diagnostic purposes are lost today in case
> > xenstore is running under mini-os instead in a dom0 daemon.
> 
> Where does vfprintf end up under mini-os? 

In the domain console.

> > Use printk of mini-os in this situation.
> 
> and this now ends up on the console and (for a debug h/v) in the xen dmesg,
> is that right?

Only in xen dmesg, not on the console.

Samuel

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 12:53         ` Juergen Gross
@ 2015-12-15 13:19           ` Ian Campbell
  2015-12-15 13:30             ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-15 13:19 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
> 
> > mem-set too perhaps?
> 
> Without mini-os supporting ballooning or memory hotplug up to now:
> I don't think so. :-)

Sure, but you'd get strange errors from mem-set rather than a try and fail,
probably.

Ian.


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

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 13:19           ` Ian Campbell
@ 2015-12-15 13:30             ` Juergen Gross
  2015-12-17  8:26               ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 13:30 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 14:19, Ian Campbell wrote:
> On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
>>  
>>> mem-set too perhaps?
>>
>> Without mini-os supporting ballooning or memory hotplug up to now:
>> I don't think so. :-)
> 
> Sure, but you'd get strange errors from mem-set rather than a try and fail,
> probably.

I'll try it.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 12:55         ` Juergen Gross
@ 2015-12-15 14:06           ` Andrew Cooper
  2015-12-15 14:57             ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Andrew Cooper @ 2015-12-15 14:06 UTC (permalink / raw)
  To: Juergen Gross, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 12:55, Juergen Gross wrote:
> On 15/12/15 13:52, Ian Campbell wrote:
>> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>>> On 15/12/15 13:31, Ian Campbell wrote:
>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>> Where does vfprintf end up under mini-os?
>>> TBH: I just couldn't find it. I tried to get some diagnostics when
>>> I started this work and couldn't find any place where the messages
>>> would occur.
>>>
>>>>> Use printk of mini-os in this situation.
>>>> and this now ends up on the console and (for a debug h/v) in the xen
>>>> dmesg,
>>>> is that right?
>>> The console of the xenstore domain seems to be a little bit special, as
>>> there is no component up to now writing the information needed to
>>> connect xenconsoled to the xenstore domain. This could be the reason I
>>> wasn't able to see any message printed via vfprintf. :-(
>> That sounds probable.
>>
>>> So in the end it will be part of xen dmesg.
>> Only for a debug=y Xen, not for a release build IIRC, unless you added some
>> other privilege along with your new flag which I missed.
> No, this is correct.
>
>>>  And this is where it should
>>> be, as such messages will be needed in case of xenstore domain not
>>> working properly. And I think it's console can be accessed by any means
>>> only in case of a working xenstore. :-)
>> An alternative would be init-xenstore-domain to daemonize and take on
>> respoibility for consuming the console ring and throwing it into a file?
> Interesting idea. I'll try this.

If the stubdomain could handle buffering of its ring for a while,
xenconsoled could just be used.  Alternatively, have xenconsoled able to
cope without xenstored while the xenstored stubdomain bootstraps itself.

IMO, it would be better to have just a single consoled, rather than
having a stub running alongside.  A stub would necessarily need some
negotiation with the main xenconsoled so they don't both map the console
ring and play the part of consumer.

~Andrew

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 14:06           ` Andrew Cooper
@ 2015-12-15 14:57             ` Juergen Gross
  2015-12-15 15:01               ` Andrew Cooper
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 14:57 UTC (permalink / raw)
  To: Andrew Cooper, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 15:06, Andrew Cooper wrote:
> On 15/12/15 12:55, Juergen Gross wrote:
>> On 15/12/15 13:52, Ian Campbell wrote:
>>> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>>>> On 15/12/15 13:31, Ian Campbell wrote:
>>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>>> Where does vfprintf end up under mini-os?
>>>> TBH: I just couldn't find it. I tried to get some diagnostics when
>>>> I started this work and couldn't find any place where the messages
>>>> would occur.
>>>>
>>>>>> Use printk of mini-os in this situation.
>>>>> and this now ends up on the console and (for a debug h/v) in the xen
>>>>> dmesg,
>>>>> is that right?
>>>> The console of the xenstore domain seems to be a little bit special, as
>>>> there is no component up to now writing the information needed to
>>>> connect xenconsoled to the xenstore domain. This could be the reason I
>>>> wasn't able to see any message printed via vfprintf. :-(
>>> That sounds probable.
>>>
>>>> So in the end it will be part of xen dmesg.
>>> Only for a debug=y Xen, not for a release build IIRC, unless you added some
>>> other privilege along with your new flag which I missed.
>> No, this is correct.
>>
>>>>  And this is where it should
>>>> be, as such messages will be needed in case of xenstore domain not
>>>> working properly. And I think it's console can be accessed by any means
>>>> only in case of a working xenstore. :-)
>>> An alternative would be init-xenstore-domain to daemonize and take on
>>> respoibility for consuming the console ring and throwing it into a file?
>> Interesting idea. I'll try this.
> 
> If the stubdomain could handle buffering of its ring for a while,
> xenconsoled could just be used.  Alternatively, have xenconsoled able to
> cope without xenstored while the xenstored stubdomain bootstraps itself.

This is a hen-and-egg problem.

Today xenconsoled relies on xenstore for connecting to the console
frontends. I think letting xenconsoled run without xenstore would
require quite some work in xenconsoled.

I suspect mini-os tries to initialize it's console frontend before
activating it's main thread and thus xenstore. So we would have to
re-init the xenstore domain's console after xenstore is capable to
add entries. I'm not sure the xenstore frontend of mini-os is
capable to talk to the backend directly instead via xenbus.

> IMO, it would be better to have just a single consoled, rather than
> having a stub running alongside.  A stub would necessarily need some
> negotiation with the main xenconsoled so they don't both map the console
> ring and play the part of consumer.

I think as long as the xenstore domain's console frontend isn't
creating xenstore entries the risk should be zero. I haven't checked
whether the information needed to get access to the console ring is
available without xenstore.

Looking at the alternatives I think trying to use xenconsoled just
normally via xenstore is the best solution. So we need a way to
handle the mini-os console frontend initialization, which should be
doable.


Juergen

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 14:57             ` Juergen Gross
@ 2015-12-15 15:01               ` Andrew Cooper
  2015-12-15 15:44                 ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Andrew Cooper @ 2015-12-15 15:01 UTC (permalink / raw)
  To: Juergen Gross, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 14:57, Juergen Gross wrote:
> On 15/12/15 15:06, Andrew Cooper wrote:
>> On 15/12/15 12:55, Juergen Gross wrote:
>>> On 15/12/15 13:52, Ian Campbell wrote:
>>>> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>>>>> On 15/12/15 13:31, Ian Campbell wrote:
>>>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>>>> Where does vfprintf end up under mini-os?
>>>>> TBH: I just couldn't find it. I tried to get some diagnostics when
>>>>> I started this work and couldn't find any place where the messages
>>>>> would occur.
>>>>>
>>>>>>> Use printk of mini-os in this situation.
>>>>>> and this now ends up on the console and (for a debug h/v) in the xen
>>>>>> dmesg,
>>>>>> is that right?
>>>>> The console of the xenstore domain seems to be a little bit special, as
>>>>> there is no component up to now writing the information needed to
>>>>> connect xenconsoled to the xenstore domain. This could be the reason I
>>>>> wasn't able to see any message printed via vfprintf. :-(
>>>> That sounds probable.
>>>>
>>>>> So in the end it will be part of xen dmesg.
>>>> Only for a debug=y Xen, not for a release build IIRC, unless you added some
>>>> other privilege along with your new flag which I missed.
>>> No, this is correct.
>>>
>>>>>  And this is where it should
>>>>> be, as such messages will be needed in case of xenstore domain not
>>>>> working properly. And I think it's console can be accessed by any means
>>>>> only in case of a working xenstore. :-)
>>>> An alternative would be init-xenstore-domain to daemonize and take on
>>>> respoibility for consuming the console ring and throwing it into a file?
>>> Interesting idea. I'll try this.
>> If the stubdomain could handle buffering of its ring for a while,
>> xenconsoled could just be used.  Alternatively, have xenconsoled able to
>> cope without xenstored while the xenstored stubdomain bootstraps itself.
> This is a hen-and-egg problem.
>
> Today xenconsoled relies on xenstore for connecting to the console
> frontends. I think letting xenconsoled run without xenstore would
> require quite some work in xenconsoled.
>
> I suspect mini-os tries to initialize it's console frontend before
> activating it's main thread and thus xenstore. So we would have to
> re-init the xenstore domain's console after xenstore is capable to
> add entries. I'm not sure the xenstore frontend of mini-os is
> capable to talk to the backend directly instead via xenbus.
>
>> IMO, it would be better to have just a single consoled, rather than
>> having a stub running alongside.  A stub would necessarily need some
>> negotiation with the main xenconsoled so they don't both map the console
>> ring and play the part of consumer.
> I think as long as the xenstore domain's console frontend isn't
> creating xenstore entries the risk should be zero. I haven't checked
> whether the information needed to get access to the console ring is
> available without xenstore.
>
> Looking at the alternatives I think trying to use xenconsoled just
> normally via xenstore is the best solution. So we need a way to
> handle the mini-os console frontend initialization, which should be
> doable.

All console information (from the domains point of view) is provided in
the shared info page for PV, or via HVM Params for HVM.

The toolstack is responsible for stashing the same information in
xenstore so xenconsoled can connect.  (This is somewhat problematic as
the two can get out of sync and nothing notices.)

~Andrew

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 15:01               ` Andrew Cooper
@ 2015-12-15 15:44                 ` Juergen Gross
  2015-12-17 16:38                   ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-15 15:44 UTC (permalink / raw)
  To: Andrew Cooper, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 16:01, Andrew Cooper wrote:
> On 15/12/15 14:57, Juergen Gross wrote:
>> On 15/12/15 15:06, Andrew Cooper wrote:
>>> On 15/12/15 12:55, Juergen Gross wrote:
>>>> On 15/12/15 13:52, Ian Campbell wrote:
>>>>> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>>>>>> On 15/12/15 13:31, Ian Campbell wrote:
>>>>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>>>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>>>>> Where does vfprintf end up under mini-os?
>>>>>> TBH: I just couldn't find it. I tried to get some diagnostics when
>>>>>> I started this work and couldn't find any place where the messages
>>>>>> would occur.
>>>>>>
>>>>>>>> Use printk of mini-os in this situation.
>>>>>>> and this now ends up on the console and (for a debug h/v) in the xen
>>>>>>> dmesg,
>>>>>>> is that right?
>>>>>> The console of the xenstore domain seems to be a little bit special, as
>>>>>> there is no component up to now writing the information needed to
>>>>>> connect xenconsoled to the xenstore domain. This could be the reason I
>>>>>> wasn't able to see any message printed via vfprintf. :-(
>>>>> That sounds probable.
>>>>>
>>>>>> So in the end it will be part of xen dmesg.
>>>>> Only for a debug=y Xen, not for a release build IIRC, unless you added some
>>>>> other privilege along with your new flag which I missed.
>>>> No, this is correct.
>>>>
>>>>>>  And this is where it should
>>>>>> be, as such messages will be needed in case of xenstore domain not
>>>>>> working properly. And I think it's console can be accessed by any means
>>>>>> only in case of a working xenstore. :-)
>>>>> An alternative would be init-xenstore-domain to daemonize and take on
>>>>> respoibility for consuming the console ring and throwing it into a file?
>>>> Interesting idea. I'll try this.
>>> If the stubdomain could handle buffering of its ring for a while,
>>> xenconsoled could just be used.  Alternatively, have xenconsoled able to
>>> cope without xenstored while the xenstored stubdomain bootstraps itself.
>> This is a hen-and-egg problem.
>>
>> Today xenconsoled relies on xenstore for connecting to the console
>> frontends. I think letting xenconsoled run without xenstore would
>> require quite some work in xenconsoled.
>>
>> I suspect mini-os tries to initialize it's console frontend before
>> activating it's main thread and thus xenstore. So we would have to
>> re-init the xenstore domain's console after xenstore is capable to
>> add entries. I'm not sure the xenstore frontend of mini-os is
>> capable to talk to the backend directly instead via xenbus.
>>
>>> IMO, it would be better to have just a single consoled, rather than
>>> having a stub running alongside.  A stub would necessarily need some
>>> negotiation with the main xenconsoled so they don't both map the console
>>> ring and play the part of consumer.
>> I think as long as the xenstore domain's console frontend isn't
>> creating xenstore entries the risk should be zero. I haven't checked
>> whether the information needed to get access to the console ring is
>> available without xenstore.
>>
>> Looking at the alternatives I think trying to use xenconsoled just
>> normally via xenstore is the best solution. So we need a way to
>> handle the mini-os console frontend initialization, which should be
>> doable.
> 
> All console information (from the domains point of view) is provided in
> the shared info page for PV, or via HVM Params for HVM.

Aah, of course. This information is put by the domain builder into the
start_info page. So it is available to init-xenstore-domain which could
then write the corresponding xenstore entries.

> The toolstack is responsible for stashing the same information in
> xenstore so xenconsoled can connect.  (This is somewhat problematic as
> the two can get out of sync and nothing notices.)

Toolstack would be init-xenstore-domain in this case. Doing this would
mean xenconsoled is just working, I guess. I'll have a try then.


Juergen

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-15 12:16   ` Ian Campbell
  2015-12-15 12:19     ` Juergen Gross
@ 2015-12-15 21:41     ` Daniel De Graaf
  2015-12-16  6:27       ` Juergen Gross
  1 sibling, 1 reply; 49+ messages in thread
From: Daniel De Graaf @ 2015-12-15 21:41 UTC (permalink / raw)
  To: Ian Campbell, Juergen Gross, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 07:16, Ian Campbell wrote:
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> The program init-xenstore-domain to start a xenstore domain instead
>> of the xenstored daemon is built, but not installed. Change that.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> CCing Daniel too since he has an interest in this mode of operation.
>
> I appreciate the CONFIG_Linux thing is pre-existing, and stems from being
> unable to build stubdoms on other platforms, but should we consider
> building these supporting tools anyway, after all a user might get a
> suitable stubdom via some other means?

Yes, this is actually rather easy to do: compile the stubdom on Linux and
just copy the .gz over; starting a stubdom doesn't actually rely on using
the tool.

The reason this is linux-specific is that the IOCTL_XENBUS_BACKEND_SETUP
call on /dev/xen/xenbus_backend is (afaik) only implemented in Linux. If
this changes, or if it compiles anyway, then I see no problem.

-- 
Daniel De Graaf
National Security Agency

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

* Re: [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax
  2015-12-15 12:22   ` Ian Campbell
@ 2015-12-15 21:49     ` Daniel De Graaf
  0 siblings, 0 replies; 49+ messages in thread
From: Daniel De Graaf @ 2015-12-15 21:49 UTC (permalink / raw)
  To: Ian Campbell, Juergen Gross, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 07:22, Ian Campbell wrote:
> +Daniel
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> init-xenstore-domain takes only positional parameters today. Change
>> this to a more flexible parameter syntax allowing to specify additional
>> options or to omit some.
>>
>> Today the supported usage is:
>>
>> init-xenstore-domain <xenstore-kernel> <memory_mb> <flask-label>
>>                       [<ramdisk-file>]
>>
>> Modify this to:
>>
>> init-xenstore-domain --kernel <xenstore-kernel>
>>                       --memory <memory_mb>
>>                       [--flask <flask-label>]
>>                       [--ramdisk <ramdisk-file>]
>>
>> The flask label is made optional in order to support xenstore domains
>> without the need of a flask enabled hypervisor.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>
> Looks fine to me
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> I'd like to see Daniel opinion of all the changes to init-xenstore-domain.c
> in this series though.

This and the other command line changes in #7 look good, and make the
rather opaque invocation of the tool a bit nicer to read.

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

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

* Re: [PATCH 4/9] xenstore: add error messages to init-xenstore-domain
  2015-12-15 12:20   ` Ian Campbell
@ 2015-12-15 21:54     ` Daniel De Graaf
  0 siblings, 0 replies; 49+ messages in thread
From: Daniel De Graaf @ 2015-12-15 21:54 UTC (permalink / raw)
  To: Ian Campbell, Juergen Gross, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 07:20, Ian Campbell wrote:
> Adding Daniel again.
>
> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>> Additional add some diagnostic messages to the program to have an idea
>> why it failed.
> [...]
>>   err:
>>   	if (dom)
>>   		xc_dom_release(dom);
>> +	if (domid >= 0)
>> +		xc_domain_destroy(xch, domid);
>
> This important looking bug fix should't be buried in this otherwise quite
> mechanical commit.
>
> The rest looks reasonable, so with the above moved to a separate commit
> this one would be:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Ian.

Both parts of the fix look good, split or together (but I agree that
splitting is better for the history).  Either way:

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

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

* Re: [PATCH 1/9] xen: add xenstore domain flag to hypervisor
  2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
  2015-12-11 15:54   ` Fwd: " Juergen Gross
@ 2015-12-15 22:18   ` Daniel De Graaf
  1 sibling, 0 replies; 49+ messages in thread
From: Daniel De Graaf @ 2015-12-15 22:18 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, Ian.Campbell, ian.jackson,
	stefano.stabellini, wei.liu2
  Cc: Tim Deegan, Keir Fraser, David Vrabel, Jan Beulich, Andrew Cooper

On 11/12/15 10:47, Juergen Gross wrote:
> In order to be able to have full support of a xenstore domain in Xen
> add a "Xenstore-domain" flag to the hypervisor. This flag must be
> specified at domain creation time and is returned by
> XEN_DOMCTL_getdomaininfo.
>
> It will allow the domain to retrieve domain information by issuing the
> XEN_DOMCTL_getdomaininfo itself in order to be able to check for
> domains having been destroyed. At the same time this flag will inhibit
> the domain to be migrated, as this wouldn't be a very wise thing to do.
>
> In case of a later support of a rebootable Dom0 this flag will allow to
> recognize a xenstore domain already being present to connect to.
>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

The XSM hook changes look good.  If XSM_XS_PRIV is ever used for a
permission other than getdomaininfo, the comment in xsm.h will be
inaccurate, but I'm not sure if that will ever be needed.

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

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-15 21:41     ` Daniel De Graaf
@ 2015-12-16  6:27       ` Juergen Gross
  2015-12-16 10:01         ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-16  6:27 UTC (permalink / raw)
  To: Daniel De Graaf, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 22:41, Daniel De Graaf wrote:
> On 15/12/15 07:16, Ian Campbell wrote:
>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>> The program init-xenstore-domain to start a xenstore domain instead
>>> of the xenstored daemon is built, but not installed. Change that.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>
>> CCing Daniel too since he has an interest in this mode of operation.
>>
>> I appreciate the CONFIG_Linux thing is pre-existing, and stems from being
>> unable to build stubdoms on other platforms, but should we consider
>> building these supporting tools anyway, after all a user might get a
>> suitable stubdom via some other means?
> 
> Yes, this is actually rather easy to do: compile the stubdom on Linux and
> just copy the .gz over; starting a stubdom doesn't actually rely on using
> the tool.
> 
> The reason this is linux-specific is that the IOCTL_XENBUS_BACKEND_SETUP
> call on /dev/xen/xenbus_backend is (afaik) only implemented in Linux. If
> this changes, or if it compiles anyway, then I see no problem.

Aah, thanks for this information.

So currently the required dom0 driver functionality for xenstore domain
is available on Linux only.

Basically I think it makes no sense to have a tool built and installed
which can just say "sorry, I won't work" on the target system. I'll
just leave the patch as it is. Ian?


Juergen

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

* Re: [PATCH 3/9] xenstore: install init-xenstore-domain via make install
  2015-12-16  6:27       ` Juergen Gross
@ 2015-12-16 10:01         ` Ian Campbell
  0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-16 10:01 UTC (permalink / raw)
  To: Juergen Gross, Daniel De Graaf, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On Wed, 2015-12-16 at 07:27 +0100, Juergen Gross wrote:
> On 15/12/15 22:41, Daniel De Graaf wrote:
> > On 15/12/15 07:16, Ian Campbell wrote:
> > > On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
> > > > The program init-xenstore-domain to start a xenstore domain instead
> > > > of the xenstored daemon is built, but not installed. Change that.
> > > > 
> > > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > 
> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > 
> > > CCing Daniel too since he has an interest in this mode of operation.
> > > 
> > > I appreciate the CONFIG_Linux thing is pre-existing, and stems from
> > > being
> > > unable to build stubdoms on other platforms, but should we consider
> > > building these supporting tools anyway, after all a user might get a
> > > suitable stubdom via some other means?
> > 
> > Yes, this is actually rather easy to do: compile the stubdom on Linux
> > and
> > just copy the .gz over; starting a stubdom doesn't actually rely on
> > using
> > the tool.
> > 
> > The reason this is linux-specific is that the
> > IOCTL_XENBUS_BACKEND_SETUP
> > call on /dev/xen/xenbus_backend is (afaik) only implemented in Linux.
> > If
> > this changes, or if it compiles anyway, then I see no problem.
> 
> Aah, thanks for this information.
> 
> So currently the required dom0 driver functionality for xenstore domain
> is available on Linux only.
> 
> Basically I think it makes no sense to have a tool built and installed
> which can just say "sorry, I won't work" on the target system. I'll
> just leave the patch as it is. Ian?

Yep, given that ioctl is Linux only I agree.

Ian.

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-15 13:30             ` Juergen Gross
@ 2015-12-17  8:26               ` Juergen Gross
  2015-12-17 10:01                 ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-17  8:26 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2

On 15/12/15 14:30, Juergen Gross wrote:
> On 15/12/15 14:19, Ian Campbell wrote:
>> On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
>>>  
>>>> mem-set too perhaps?
>>>
>>> Without mini-os supporting ballooning or memory hotplug up to now:
>>> I don't think so. :-)
>>
>> Sure, but you'd get strange errors from mem-set rather than a try and fail,
>> probably.
> 
> I'll try it.

I tried both "xl list -l" and "xl mem-set". Seems as if the json object
would be a good idea.

I guess this will require using libxl and I should move
init-xenstore-domain to tools/libxl in order to avoid dependency loops.

In the long run I think it would be a good idea to split the libraries
from the tools using them. So all libraries under tools/libs and the
programs (xl, init-xenstore-domain, xenstore-ls, ...) under tools/foo.
Otherwise all programs might tend to migrate to tools/libxl as this
is the last directory built containing a library. And having to select
the program's directory by the libraries it is using is a little bit
strange.

I think this would be a natural next step after Ian's libxc split has
been applied at least partially by introducing tools/libs.


Juergen

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-17  8:26               ` Juergen Gross
@ 2015-12-17 10:01                 ` Ian Campbell
  2015-12-17 10:08                   ` Juergen Gross
  0 siblings, 1 reply; 49+ messages in thread
From: Ian Campbell @ 2015-12-17 10:01 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Thu, 2015-12-17 at 09:26 +0100, Juergen Gross wrote:
> On 15/12/15 14:30, Juergen Gross wrote:
> > On 15/12/15 14:19, Ian Campbell wrote:
> > > On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
> > > >  
> > > > > mem-set too perhaps?
> > > > 
> > > > Without mini-os supporting ballooning or memory hotplug up to now:
> > > > I don't think so. :-)
> > > 
> > > Sure, but you'd get strange errors from mem-set rather than a try and
> > > fail,
> > > probably.
> > 
> > I'll try it.
> 
> I tried both "xl list -l" and "xl mem-set". Seems as if the json object
> would be a good idea.
> 
> I guess this will require using libxl and I should move
> init-xenstore-domain to tools/libxl in order to avoid dependency loops.
> 
> In the long run I think it would be a good idea to split the libraries
> from the tools using them. So all libraries under tools/libs and the
> programs (xl, init-xenstore-domain, xenstore-ls, ...) under tools/foo.
> Otherwise all programs might tend to migrate to tools/libxl as this
> is the last directory built containing a library. And having to select
> the program's directory by the libraries it is using is a little bit
> strange.
> 
> I think this would be a natural next step after Ian's libxc split has
> been applied at least partially by introducing tools/libs.

Yes, I have a few followup activities planned for after the initial set:

   A. adding libxendevicemodel et al to remove the final unstable APIs from
      QEMU.
   B. refactoring/splitting tools/libxc so that libxenctrl and libxenguest are
      actually separate. Which would probably be an opportune moment to move
      to tools/libs/x{c,g}
   C. moving libxl to tools/libs/xl and xl to tools/xl (or something)

(A is independent, but C depends on B for build order reasons)

However I'm not sure how long it is going to take to get through that list.
In the meantime it does seem that it would make sense to move init-
xenstore-domain alongside xen-init-dom0 in tools/libxl.

We might even consider merging init-xenstore-domain into xen-init-dom0 as
an optional behaviour. xen-init-dom0 would then be more like xen-init-host
in functionality but renaming would be unnecessary churn IMHO. I'm not
terribly sure this is a good idea...

You could also consider moving both xen-init-dom0 and init-xenstore-domain
to a new subdirectory of tools (tools/init?) which is built after the libxl
dir. That woudln't conflict with step C above.

Ian.



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

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-17 10:01                 ` Ian Campbell
@ 2015-12-17 10:08                   ` Juergen Gross
  2015-12-17 10:16                     ` Ian Campbell
  0 siblings, 1 reply; 49+ messages in thread
From: Juergen Gross @ 2015-12-17 10:08 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On 17/12/15 11:01, Ian Campbell wrote:
> On Thu, 2015-12-17 at 09:26 +0100, Juergen Gross wrote:
>> On 15/12/15 14:30, Juergen Gross wrote:
>>> On 15/12/15 14:19, Ian Campbell wrote:
>>>> On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
>>>>>  
>>>>>> mem-set too perhaps?
>>>>>
>>>>> Without mini-os supporting ballooning or memory hotplug up to now:
>>>>> I don't think so. :-)
>>>>
>>>> Sure, but you'd get strange errors from mem-set rather than a try and
>>>> fail,
>>>> probably.
>>>
>>> I'll try it.
>>
>> I tried both "xl list -l" and "xl mem-set". Seems as if the json object
>> would be a good idea.
>>
>> I guess this will require using libxl and I should move
>> init-xenstore-domain to tools/libxl in order to avoid dependency loops.
>>
>> In the long run I think it would be a good idea to split the libraries
>> from the tools using them. So all libraries under tools/libs and the
>> programs (xl, init-xenstore-domain, xenstore-ls, ...) under tools/foo.
>> Otherwise all programs might tend to migrate to tools/libxl as this
>> is the last directory built containing a library. And having to select
>> the program's directory by the libraries it is using is a little bit
>> strange.
>>
>> I think this would be a natural next step after Ian's libxc split has
>> been applied at least partially by introducing tools/libs.
> 
> Yes, I have a few followup activities planned for after the initial set:
> 
>    A. adding libxendevicemodel et al to remove the final unstable APIs from
>       QEMU.
>    B. refactoring/splitting tools/libxc so that libxenctrl and libxenguest are
>       actually separate. Which would probably be an opportune moment to move
>       to tools/libs/x{c,g}
>    C. moving libxl to tools/libs/xl and xl to tools/xl (or something)
> 
> (A is independent, but C depends on B for build order reasons)
> 
> However I'm not sure how long it is going to take to get through that list.
> In the meantime it does seem that it would make sense to move init-
> xenstore-domain alongside xen-init-dom0 in tools/libxl.
> 
> We might even consider merging init-xenstore-domain into xen-init-dom0 as
> an optional behaviour. xen-init-dom0 would then be more like xen-init-host
> in functionality but renaming would be unnecessary churn IMHO. I'm not
> terribly sure this is a good idea...
> 
> You could also consider moving both xen-init-dom0 and init-xenstore-domain
> to a new subdirectory of tools (tools/init?) which is built after the libxl
> dir. That woudln't conflict with step C above.

I like the last variant most. This way there won't be any later
adjustments to the libs work needed. What about naming the subdirectory
tools/helpers? This would allow to gather more such programs in there
without sacrificing the directory name or having to create other
directories with just a few tools in there.


Juergen

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

* Re: [PATCH 8/9] xenstore: write xenstore domain data to xenstore
  2015-12-17 10:08                   ` Juergen Gross
@ 2015-12-17 10:16                     ` Ian Campbell
  0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-12-17 10:16 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, ian.jackson, stefano.stabellini, wei.liu2
  Cc: Daniel De Graaf

On Thu, 2015-12-17 at 11:08 +0100, Juergen Gross wrote:
> On 17/12/15 11:01, Ian Campbell wrote:
> > On Thu, 2015-12-17 at 09:26 +0100, Juergen Gross wrote:
> > > On 15/12/15 14:30, Juergen Gross wrote:
> > > > On 15/12/15 14:19, Ian Campbell wrote:
> > > > > On Tue, 2015-12-15 at 13:53 +0100, Juergen Gross wrote:
> > > > > >  
> > > > > > > mem-set too perhaps?
> > > > > > 
> > > > > > Without mini-os supporting ballooning or memory hotplug up to
> > > > > > now:
> > > > > > I don't think so. :-)
> > > > > 
> > > > > Sure, but you'd get strange errors from mem-set rather than a try
> > > > > and
> > > > > fail,
> > > > > probably.
> > > > 
> > > > I'll try it.
> > > 
> > > I tried both "xl list -l" and "xl mem-set". Seems as if the json
> > > object
> > > would be a good idea.
> > > 
> > > I guess this will require using libxl and I should move
> > > init-xenstore-domain to tools/libxl in order to avoid dependency
> > > loops.
> > > 
> > > In the long run I think it would be a good idea to split the
> > > libraries
> > > from the tools using them. So all libraries under tools/libs and the
> > > programs (xl, init-xenstore-domain, xenstore-ls, ...) under
> > > tools/foo.
> > > Otherwise all programs might tend to migrate to tools/libxl as this
> > > is the last directory built containing a library. And having to
> > > select
> > > the program's directory by the libraries it is using is a little bit
> > > strange.
> > > 
> > > I think this would be a natural next step after Ian's libxc split has
> > > been applied at least partially by introducing tools/libs.
> > 
> > Yes, I have a few followup activities planned for after the initial
> > set:
> > 
> >    A. adding libxendevicemodel et al to remove the final unstable APIs
> > from
> >       QEMU.
> >    B. refactoring/splitting tools/libxc so that libxenctrl and
> > libxenguest are
> >       actually separate. Which would probably be an opportune moment to
> > move
> >       to tools/libs/x{c,g}
> >    C. moving libxl to tools/libs/xl and xl to tools/xl (or something)
> > 
> > (A is independent, but C depends on B for build order reasons)
> > 
> > However I'm not sure how long it is going to take to get through that
> > list.
> > In the meantime it does seem that it would make sense to move init-
> > xenstore-domain alongside xen-init-dom0 in tools/libxl.
> > 
> > We might even consider merging init-xenstore-domain into xen-init-dom0
> > as
> > an optional behaviour. xen-init-dom0 would then be more like xen-init-
> > host
> > in functionality but renaming would be unnecessary churn IMHO. I'm not
> > terribly sure this is a good idea...
> > 
> > You could also consider moving both xen-init-dom0 and init-xenstore-
> > domain
> > to a new subdirectory of tools (tools/init?) which is built after the
> > libxl
> > dir. That woudln't conflict with step C above.
> 
> I like the last variant most. This way there won't be any later
> adjustments to the libs work needed. What about naming the subdirectory
> tools/helpers? This would allow to gather more such programs in there
> without sacrificing the directory name or having to create other
> directories with just a few tools in there.

I don't really mind, although directory names aren't exactly in short
supply IMHO and git mv is always an option later. Do as you like I think.

Ian.


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

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

* Re: [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages
  2015-12-15 15:44                 ` Juergen Gross
@ 2015-12-17 16:38                   ` Juergen Gross
  0 siblings, 0 replies; 49+ messages in thread
From: Juergen Gross @ 2015-12-17 16:38 UTC (permalink / raw)
  To: Andrew Cooper, Ian Campbell, xen-devel, ian.jackson,
	stefano.stabellini, wei.liu2

On 15/12/15 16:44, Juergen Gross wrote:
> On 15/12/15 16:01, Andrew Cooper wrote:
>> On 15/12/15 14:57, Juergen Gross wrote:
>>> On 15/12/15 15:06, Andrew Cooper wrote:
>>>> On 15/12/15 12:55, Juergen Gross wrote:
>>>>> On 15/12/15 13:52, Ian Campbell wrote:
>>>>>> On Tue, 2015-12-15 at 13:47 +0100, Juergen Gross wrote:
>>>>>>> On 15/12/15 13:31, Ian Campbell wrote:
>>>>>>>> On Fri, 2015-12-11 at 16:47 +0100, Juergen Gross wrote:
>>>>>>>>> Xenstore messages for diagnostic purposes are lost today in case
>>>>>>>>> xenstore is running under mini-os instead in a dom0 daemon.
>>>>>>>> Where does vfprintf end up under mini-os?
>>>>>>> TBH: I just couldn't find it. I tried to get some diagnostics when
>>>>>>> I started this work and couldn't find any place where the messages
>>>>>>> would occur.
>>>>>>>
>>>>>>>>> Use printk of mini-os in this situation.
>>>>>>>> and this now ends up on the console and (for a debug h/v) in the xen
>>>>>>>> dmesg,
>>>>>>>> is that right?
>>>>>>> The console of the xenstore domain seems to be a little bit special, as
>>>>>>> there is no component up to now writing the information needed to
>>>>>>> connect xenconsoled to the xenstore domain. This could be the reason I
>>>>>>> wasn't able to see any message printed via vfprintf. :-(
>>>>>> That sounds probable.
>>>>>>
>>>>>>> So in the end it will be part of xen dmesg.
>>>>>> Only for a debug=y Xen, not for a release build IIRC, unless you added some
>>>>>> other privilege along with your new flag which I missed.
>>>>> No, this is correct.
>>>>>
>>>>>>>  And this is where it should
>>>>>>> be, as such messages will be needed in case of xenstore domain not
>>>>>>> working properly. And I think it's console can be accessed by any means
>>>>>>> only in case of a working xenstore. :-)
>>>>>> An alternative would be init-xenstore-domain to daemonize and take on
>>>>>> respoibility for consuming the console ring and throwing it into a file?
>>>>> Interesting idea. I'll try this.
>>>> If the stubdomain could handle buffering of its ring for a while,
>>>> xenconsoled could just be used.  Alternatively, have xenconsoled able to
>>>> cope without xenstored while the xenstored stubdomain bootstraps itself.
>>> This is a hen-and-egg problem.
>>>
>>> Today xenconsoled relies on xenstore for connecting to the console
>>> frontends. I think letting xenconsoled run without xenstore would
>>> require quite some work in xenconsoled.
>>>
>>> I suspect mini-os tries to initialize it's console frontend before
>>> activating it's main thread and thus xenstore. So we would have to
>>> re-init the xenstore domain's console after xenstore is capable to
>>> add entries. I'm not sure the xenstore frontend of mini-os is
>>> capable to talk to the backend directly instead via xenbus.
>>>
>>>> IMO, it would be better to have just a single consoled, rather than
>>>> having a stub running alongside.  A stub would necessarily need some
>>>> negotiation with the main xenconsoled so they don't both map the console
>>>> ring and play the part of consumer.
>>> I think as long as the xenstore domain's console frontend isn't
>>> creating xenstore entries the risk should be zero. I haven't checked
>>> whether the information needed to get access to the console ring is
>>> available without xenstore.
>>>
>>> Looking at the alternatives I think trying to use xenconsoled just
>>> normally via xenstore is the best solution. So we need a way to
>>> handle the mini-os console frontend initialization, which should be
>>> doable.
>>
>> All console information (from the domains point of view) is provided in
>> the shared info page for PV, or via HVM Params for HVM.
> 
> Aah, of course. This information is put by the domain builder into the
> start_info page. So it is available to init-xenstore-domain which could
> then write the corresponding xenstore entries.
> 
>> The toolstack is responsible for stashing the same information in
>> xenstore so xenconsoled can connect.  (This is somewhat problematic as
>> the two can get out of sync and nothing notices.)
> 
> Toolstack would be init-xenstore-domain in this case. Doing this would
> mean xenconsoled is just working, I guess. I'll have a try then.

This is still a problem: xenstore stubdom is configured to work without
all the frontends. And I think this is due to the fact that the console
frontend will be initialized before the xenstore is up in that domain.
And it is really the frontend which is responsible to write the ring-ref
and port information to xenstore (see init_consfront() in mini-os's
console/xenbus.c).

I think I'll just drop this patch and we can think about the console
problem later.


Juergen

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

end of thread, other threads:[~2015-12-17 16:38 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-11 15:47 [PATCH 0/9] xenstore: make it easier to run xenstore in a domain Juergen Gross
2015-12-11 15:47 ` [PATCH 1/9] xen: add xenstore domain flag to hypervisor Juergen Gross
2015-12-11 15:54   ` Fwd: " Juergen Gross
2015-12-15 22:18   ` Daniel De Graaf
2015-12-11 15:47 ` [PATCH 2/9] libxc: support new xenstore domain flag in libxc Juergen Gross
2015-12-11 15:47 ` [PATCH 3/9] xenstore: install init-xenstore-domain via make install Juergen Gross
2015-12-15 12:16   ` Ian Campbell
2015-12-15 12:19     ` Juergen Gross
2015-12-15 12:31       ` Ian Campbell
2015-12-15 21:41     ` Daniel De Graaf
2015-12-16  6:27       ` Juergen Gross
2015-12-16 10:01         ` Ian Campbell
2015-12-11 15:47 ` [PATCH 4/9] xenstore: add error messages to init-xenstore-domain Juergen Gross
2015-12-15 12:20   ` Ian Campbell
2015-12-15 21:54     ` Daniel De Graaf
2015-12-11 15:47 ` [PATCH 5/9] xenstore: modify init-xenstore-domain parameter syntax Juergen Gross
2015-12-15 12:22   ` Ian Campbell
2015-12-15 21:49     ` Daniel De Graaf
2015-12-11 15:47 ` [PATCH 6/9] xenstore: don't start xenstore domain if already one is active Juergen Gross
2015-12-15 12:23   ` Ian Campbell
2015-12-15 12:28     ` Juergen Gross
2015-12-15 12:32       ` Ian Campbell
2015-12-15 12:40         ` Juergen Gross
2015-12-15 12:47           ` Ian Campbell
2015-12-15 12:49             ` Juergen Gross
2015-12-11 15:47 ` [PATCH 7/9] xenstore: add init-xenstore-domain parameter to specify cmdline Juergen Gross
2015-12-15 12:24   ` Ian Campbell
2015-12-11 15:47 ` [PATCH 8/9] xenstore: write xenstore domain data to xenstore Juergen Gross
2015-12-15 12:26   ` Ian Campbell
2015-12-15 12:34     ` Juergen Gross
2015-12-15 12:49       ` Ian Campbell
2015-12-15 12:53         ` Juergen Gross
2015-12-15 13:19           ` Ian Campbell
2015-12-15 13:30             ` Juergen Gross
2015-12-17  8:26               ` Juergen Gross
2015-12-17 10:01                 ` Ian Campbell
2015-12-17 10:08                   ` Juergen Gross
2015-12-17 10:16                     ` Ian Campbell
2015-12-11 15:47 ` [PATCH 9/9] xenstore: when running in mini-os use printk for diagnostic messages Juergen Gross
2015-12-15 12:31   ` Ian Campbell
2015-12-15 12:47     ` Juergen Gross
2015-12-15 12:52       ` Ian Campbell
2015-12-15 12:55         ` Juergen Gross
2015-12-15 14:06           ` Andrew Cooper
2015-12-15 14:57             ` Juergen Gross
2015-12-15 15:01               ` Andrew Cooper
2015-12-15 15:44                 ` Juergen Gross
2015-12-17 16:38                   ` Juergen Gross
2015-12-15 13:03     ` Samuel Thibault

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.