Xen devel - Sep 2013 - [PATCH RFC 00/12] libxl: add driver domain backend daemon

This patch series introduces support to launch device backends from
driver domains using libxl, removing the need to setup udev on driver
domains.

The daemon currently supports vbds, vifs and qdisks as backends.

Patches from 1 to 11 focus on getting libxl to work on driver domains,
and add several helper functions that will be used to build the
infraestructure needed for libxl to work on domains different than
Dom0.

The last patch (12) adds a new xl command, "devd", that can be used to
launch a daemon that will listen to changes in the xenstore "backend"
directory for the domain and perform the necessary actions to attach
the devices.

This patch writes Dom0 domid on xenstore (like it's done for other
guests), and adds a libxl helper function to fetch that domid from
xenstore.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/hotplug/Linux/init.d/xencommons |    1 +
 tools/libxl/libxl.c                   |   17 +++++++++++++++++
 tools/libxl/libxl_internal.h          |    3 +++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/xencommons
b/tools/hotplug/Linux/init.d/xencommons
index a2e633b..43e09bc 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -110,6 +110,7 @@ do_start () {
 
 		echo Setting domain 0 name...
 		${BINDIR}/xenstore-write "/local/domain/0/name"
"Domain-0"
+		${BINDIR}/xenstore-write "/local/domain/0/domid" "0"
 	fi
 
 	echo Starting xenconsoled...
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1bce4bb..5417b48 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1688,6 +1688,23 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid,
int autopass)
     return ERROR_FAIL;
 }
 
+int libxl__get_domid(libxl__gc *gc, uint32_t *domid)
+{
+    int rc;
+    const char *xs_domid;
+
+    rc = libxl__xs_read_checked(gc, XBT_NULL, DOMID_XS_PATH, &xs_domid);
+    if (rc || !xs_domid) {
+        rc = rc ? rc : ERROR_FAIL;
+        goto out;
+    }
+
+    *domid = atoi(xs_domid);
+
+out:
+    return rc;
+}
+
 /******************************************************************************/
 
 /* generic callback for devices that only need to set ao_complete */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f051d91..3b74726 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -101,6 +101,7 @@
 #define STUBDOM_SPECIAL_CONSOLES 3
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DISABLE_UDEV_PATH "libxl/disable_udev"
+#define DOMID_XS_PATH "domid"
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
@@ -978,6 +979,8 @@ _hidden const char *libxl__device_nic_devname(libxl__gc *gc,
                                               uint32_t devid,
                                               libxl_nic_type type);
 
+_hidden int libxl__get_domid(libxl__gc *gc, uint32_t *domid);
+
 /*
  * libxl__ev_devstate - waits a given time for a device to
  * reach a given state.  Follows the libxl_ev_* conventions.
-- 
1.7.7.5 (Apple Git-26)


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

The info fetched by libxl_domain_info is not used anywere in the
function.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_device.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 16a92a4..2971dd3 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -715,16 +715,8 @@ void libxl__wait_device_connection(libxl__egc *egc,
libxl__ao_device *aodev)
     STATE_AO_GC(aodev->ao);
     char *be_path = libxl__device_backend_path(gc, aodev->dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
-    libxl_dominfo info;
-    uint32_t domid = aodev->dev->domid;
     int rc = 0;
 
-    libxl_dominfo_init(&info);
-    rc = libxl_domain_info(CTX, &info, domid);
-    if (rc) {
-        LOG(ERROR, "unable to get info for domain %d", domid);
-        goto out;
-    }
     if (QEMU_BACKEND(aodev->dev)) {
         /*
          * If Qemu is not running, there's no point in waiting for
@@ -746,12 +738,10 @@ void libxl__wait_device_connection(libxl__egc *egc,
libxl__ao_device *aodev)
         goto out;
     }
 
-    libxl_dominfo_dispose(&info);
     return;
 
 out:
     aodev->rc = rc;
-    libxl_dominfo_dispose(&info);
     device_hotplug_done(egc, aodev);
     return;
 }
-- 
1.7.7.5 (Apple Git-26)


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

libxl currently refuses to execute hotplug scripts if the backend
domid of a device is different than LIBXL_TOOLSTACK_DOMID. This will
prevent libxl from executing hotplug scripts when running on a domain
different than LIBXL_TOOLSTACK_DOMID, we should instead check if
backend_domid is different than current domid.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_device.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 2971dd3..48acc92 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -908,12 +908,15 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
     int rc = 0;
     int hotplug;
     pid_t pid;
+    uint32_t domid;
 
     /*
      * If device is attached from a driver domain don't try to execute
      * hotplug scripts
      */
-    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID)
+    rc = libxl__get_domid(gc, &domid);
+    if (rc) goto out;
+    if (aodev->dev->backend_domid != domid)
         goto out;
 
     /* Check if we have to execute hotplug scripts for this device
-- 
1.7.7.5 (Apple Git-26)


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

If libxl is executed inside a guest domain it needs write access to
the local libxl xenstore dir (/local/<domid>/libxl) to store internal
data. This also applies to Qemu which needs a
/local/<domid>/device-model xenstore directory.

This patch creates the mentioned directories for each guest launched
from libxl.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_create.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0c32d0b..aac19b5 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -498,6 +498,18 @@ retry_transaction:
     libxl__xs_mkdir(gc, t,
                     libxl__sprintf(gc, "%s/data", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
+    /*
+     * Create a local "libxl" directory for each guest, since we
might want
+     * to use libxl from inside the guest
+     */
+    libxl__xs_mkdir(gc, t, GCSPRINTF("%s/libxl", dom_path), rwperm,
+                    ARRAY_SIZE(rwperm));
+    /*
+     * Create a local "device-model" directory for each guest, since
we
+     * might want to use Qemu from inside the guest
+     */
+    libxl__xs_mkdir(gc, t, GCSPRINTF("%s/device-model", dom_path),
rwperm,
+                    ARRAY_SIZE(rwperm));
     if (info->type == LIBXL_DOMAIN_TYPE_HVM)
         libxl__xs_mkdir(gc, t,
             libxl__sprintf(gc, "%s/hvmloader/generation-id-address",
dom_path),
-- 
1.7.7.5 (Apple Git-26)


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

If the backend of a device is running on a driver domain, it should be
the driver domain itself the one to clean the backend path once the
device has been successfully disconnected.

And the opposite way, a domain different than LIBXL_TOOLSTACK_DOMID
should not try to remove the frontend paths of a device.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_device.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 48acc92..082bd2a 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -562,6 +562,10 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device
*dev)
     const char *tapdisk_params;
     xs_transaction_t t = 0;
     int rc;
+    uint32_t domid;
+
+    rc = libxl__get_domid(gc, &domid);
+    if (rc) goto out;
 
     for (;;) {
         rc = libxl__xs_transaction_start(gc, &t);
@@ -571,8 +575,10 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device
*dev)
         rc = libxl__xs_read_checked(gc, t, tapdisk_path, &tapdisk_params);
         if (rc) goto out;
 
-        libxl__xs_path_cleanup(gc, t, fe_path);
-        libxl__xs_path_cleanup(gc, t, be_path);
+        if (domid == LIBXL_TOOLSTACK_DOMID)
+            libxl__xs_path_cleanup(gc, t, fe_path);
+        if (dev->backend_domid == domid)
+            libxl__xs_path_cleanup(gc, t, be_path);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
-- 
1.7.7.5 (Apple Git-26)


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

The backend path should be fully owned by the domain where it resides.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_device.c   |   45 +++++++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_internal.h |    2 +
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 082bd2a..f39b7b1 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -83,6 +83,47 @@ out:
     return rc;
 }
 
+int libxl__create_backend_path(libxl__gc *gc, xs_transaction_t t,
+                               libxl__device *device)
+{
+    char *dom_backend_path;
+    char *be_path = libxl__device_backend_path(gc, device);
+    char *p;
+    struct xs_permissions backend_dir_perms[2];
+    struct xs_permissions backend_path_perms[1];
+    int rc;
+
+    backend_path_perms[0].id = device->backend_domid;
+    backend_path_perms[0].perms = XS_PERM_NONE;
+
+    backend_dir_perms[0].id = device->backend_domid;
+    backend_dir_perms[0].perms = XS_PERM_NONE;
+    backend_dir_perms[1].id = device->domid;
+    backend_dir_perms[1].perms = XS_PERM_READ;
+
+    rc = libxl__xs_rm_checked(gc, t, be_path);
+    if (rc) goto error;
+    if (!libxl__xs_mkdir(gc, t, be_path, backend_dir_perms,
+        ARRAY_SIZE(backend_dir_perms)))
+        goto error;
+
+    dom_backend_path = GCSPRINTF("%s/backend",
+                            libxl__xs_get_dompath(gc,
device->backend_domid));
+    while (strcmp(be_path, dom_backend_path) != 0) {
+        p = strrchr(be_path, '/');
+        if (!p) goto error;
+        *p = '\0';
+
+        xs_set_permissions(CTX->xsh, t, be_path, backend_path_perms,
+                           ARRAY_SIZE(backend_path_perms));
+    }
+
+    return 0;
+
+error:
+    return ERROR_FAIL;
+}
+
 int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
         libxl__device *device, char **bents, char **fents, char **ro_fents)
 {
@@ -135,9 +176,7 @@ retry_transaction:
     }
 
     if (bents) {
-        xs_rm(ctx->xsh, t, backend_path);
-        xs_mkdir(ctx->xsh, t, backend_path);
-        xs_set_permissions(ctx->xsh, t, backend_path, backend_perms,
ARRAY_SIZE(backend_perms));
+        libxl__create_backend_path(gc, t, device);
         xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/frontend",
backend_path), frontend_path, strlen(frontend_path));
         libxl__xs_writev(gc, t, backend_path, bents);
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3b74726..1746b7d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -929,6 +929,8 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc,
xs_transaction_t t,
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
+_hidden int libxl__create_backend_path(libxl__gc *gc, xs_transaction_t t,
+                                       libxl__device *device);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int
*major, int *minor);
 _hidden int libxl__device_disk_dev_number(const char *virtpath,
-- 
1.7.7.5 (Apple Git-26)


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

When Qemu is launched from a driver domain to act as a PV disk
backend we can make sure that Qemu is running before detaching
devices, so there's no need for the bodge there.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_device.c |   71 ++++++++++++++++++++++++++++++++-----------
 1 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index f39b7b1..751a9e4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -801,30 +801,38 @@ void libxl__initiate_device_remove(libxl__egc *egc,
     char *online_path = GCSPRINTF("%s/online", be_path);
     const char *state;
     libxl_dominfo info;
-    uint32_t domid = aodev->dev->domid;
+    uint32_t my_domid, domid = aodev->dev->domid;
     int rc = 0;
 
-    libxl_dominfo_init(&info);
-    rc = libxl_domain_info(CTX, &info, domid);
+    rc = libxl__get_domid(gc, &my_domid);
     if (rc) {
-        LOG(ERROR, "unable to get info for domain %d", domid);
+        LOG(ERROR, "unable to get my domid");
         goto out;
     }
-    if (QEMU_BACKEND(aodev->dev) &&
-        (info.paused || info.dying || info.shutdown)) {
-        /*
-         * TODO: 4.2 Bodge due to QEMU, see comment on top of
-         * libxl__initiate_device_remove in libxl_internal.h
-         */
-        rc = libxl__ev_time_register_rel(gc, &aodev->timeout,
-                                         device_qemu_timeout,
-                                         LIBXL_QEMU_BODGE_TIMEOUT * 1000);
+
+    if (my_domid == LIBXL_TOOLSTACK_DOMID) {
+        libxl_dominfo_init(&info);
+        rc = libxl_domain_info(CTX, &info, domid);
         if (rc) {
-            LOG(ERROR, "unable to register timeout for Qemu device
%s",
-                       be_path);
+            LOG(ERROR, "unable to get info for domain %d", domid);
             goto out;
         }
-        return;
+        if (QEMU_BACKEND(aodev->dev) &&
+            (info.paused || info.dying || info.shutdown)) {
+            /*
+             * TODO: 4.2 Bodge due to QEMU, see comment on top of
+             * libxl__initiate_device_remove in libxl_internal.h
+             */
+            rc = libxl__ev_time_register_rel(gc, &aodev->timeout,
+                                             device_qemu_timeout,
+                                             LIBXL_QEMU_BODGE_TIMEOUT * 1000);
+            if (rc) {
+                LOG(ERROR, "unable to register timeout for Qemu device
%s",
+                           be_path);
+                goto out;
+            }
+            return;
+        }
     }
 
     for (;;) {
@@ -893,19 +901,46 @@ static void device_qemu_timeout(libxl__egc *egc,
libxl__ev_time *ev,
     STATE_AO_GC(aodev->ao);
     char *be_path = libxl__device_backend_path(gc, aodev->dev);
     char *state_path = GCSPRINTF("%s/state", be_path);
+    const char *xs_state;
+    xs_transaction_t t = 0;
     int rc = 0;
 
     libxl__ev_time_deregister(gc, &aodev->timeout);
 
-    rc = libxl__xs_write_checked(gc, XBT_NULL, state_path, "6");
-    if (rc) goto out;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) {
+            LOG(ERROR, "unable to start transaction");
+            goto out;
+        }
+
+        /*
+         * Check that the state path exists and is actually different than
+         * 6 before unconditionally setting it. If Qemu runs on a driver
+         * domain it is possible that the driver domain has already cleaned
+         * the backend path if the device has reached state 6.
+         */
+        rc = libxl__xs_read_checked(gc, XBT_NULL, state_path, &xs_state);
+        if (rc) goto out;
+
+        if (xs_state && atoi(xs_state) != XenbusStateClosed) {
+            rc = libxl__xs_write_checked(gc, XBT_NULL, state_path,
"6");
+            if (rc) goto out;
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
 
     device_hotplug(egc, aodev);
     return;
 
 out:
+    libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
     device_hotplug_done(egc, aodev);
+    return;
 }
 
 static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
-- 
1.7.7.5 (Apple Git-26)


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

In libxl__need_xenpv_qemu check that the backend domain of the Qdisk
device is Dom0 before launching a Qemu instance in the toolstack
domain.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_dm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4035b6d..ddc7367 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1368,7 +1368,8 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
 
     if (nr_disks > 0) {
         for (i = 0; i < nr_disks; i++) {
-            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK) {
+            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK &&
+                disks[i].backend_domid == LIBXL_TOOLSTACK_DOMID) {
                 ret = 1;
                 goto out;
             }
-- 
1.7.7.5 (Apple Git-26)


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

Current Qemu launch functions in libxl require the usage of data
structures only avaialbe on domain creation. All this information is
not need in order to launch a Qemu instance to serve Qdisk backends,
so introduce a new simplified helper that can be used to launch
Qemu/Qdisk, that will be used to launch Qdisk in driver domains.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_dm.c       |  149 +++++++++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_internal.h |   17 +++++
 2 files changed, 157 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ddc7367..01eb903 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1308,18 +1308,123 @@ static void device_model_spawn_outcome(libxl__egc *egc,
     dmss->callback(egc, dmss, rc);
 }
 
-int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
+/* callbacks passed in libxl__spawn_spawn for Qdisk attach */
+static void qdisk_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                          const char *xsdata);
+static void qdisk_startup_failed(libxl__egc *egc,
+                                 libxl__spawn_state *spawn);
+static void qdisk_detached(libxl__egc *egc,
+                           libxl__spawn_state *spawn);
+
+void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__spawn_qdisk_state *sqs)
 {
-    char *pid;
-    int ret;
+    STATE_AO_GC(sqs->spawn.ao);
+    flexarray_t *dm_args;
+    char **args;
+    char *logfile = NULL;
+    const char *dm;
+    int logfile_w, null, rc;
+    uint32_t domid = sqs->guest_domid;
+
+    /* Always use qemu-xen as device model */
+    dm = qemu_xen_path(gc);
+
+    dm_args = flexarray_make(gc, 15, 1);
+    flexarray_vappend(dm_args, dm, "-xen-domid",
+                      GCSPRINTF("%d", domid), NULL);
+    flexarray_append(dm_args, "-xen-attach");
+    flexarray_vappend(dm_args, "-name",
+                      GCSPRINTF("domain-%u", domid), NULL);
+    flexarray_append(dm_args, "-nographic");
+    flexarray_vappend(dm_args, "-M", "xenpv", NULL);
+    flexarray_vappend(dm_args, "-monitor", "/dev/null",
NULL);
+    flexarray_vappend(dm_args, "-serial", "/dev/null",
NULL);
+    flexarray_vappend(dm_args, "-parallel", "/dev/null",
NULL);
+    flexarray_append(dm_args, NULL);
+    args = (char **) flexarray_contents(dm_args);
 
-    pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"/local/domain/%d/image/device-model-pid", domid));
-    if (!pid || !atoi(pid)) {
-        LOG(ERROR, "could not find device-model's pid for dom
%u", domid);
-        ret = ERROR_FAIL;
+    libxl_create_logfile(CTX, GCSPRINTF("qdisk-%u", domid),
&logfile);
+    logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
+    if (logfile_w < 0) {
+        free(logfile);
+        rc = logfile_w;
+        goto error;
+    }
+    free(logfile);
+    null = open("/dev/null", O_RDONLY);
+
+    sqs->spawn.what = GCSPRINTF("domain %u Qdisk backend", domid);
+    sqs->spawn.xspath = GCSPRINTF("device-model/%u/state", domid);
+    sqs->spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+    /*
+     * We cannot save Qemu pid anywhere in the xenstore guest dir,
+     * because we will call this from unprivileged driver domains,
+     * so save it in the current domain libxl private dir.
+     */
+    sqs->spawn.pidpath = GCSPRINTF("libxl/%u/qdisk-backend-pid",
domid);
+    sqs->spawn.midproc_cb = libxl__spawn_record_pid;
+    sqs->spawn.confirm_cb = qdisk_confirm;
+    sqs->spawn.failure_cb = qdisk_startup_failed;
+    sqs->spawn.detached_cb = qdisk_detached;
+    rc = libxl__spawn_spawn(egc, &sqs->spawn);
+    if (rc < 0)
+        goto error;
+    if (!rc) { /* inner child */
+        setsid();
+        libxl__exec(gc, null, logfile_w, logfile_w, dm, args, NULL);
+    }
+
+    return;
+
+error:
+    assert(rc);
+    sqs->callback(egc, sqs, rc);
+    return;
+}
+
+static void qdisk_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                          const char *xsdata)
+{
+    STATE_AO_GC(spawn->ao);
+
+    if (!xsdata)
+        return;
+
+    if (strcmp(xsdata, "running"))
+        return;
+
+    libxl__spawn_initiate_detach(gc, spawn);
+}
+
+static void qdisk_startup_failed(libxl__egc *egc,
+                                 libxl__spawn_state *spawn)
+{
+    libxl__spawn_qdisk_state *sqs = CONTAINER_OF(spawn, *sqs, spawn);
+    sqs->callback(egc, sqs, ERROR_FAIL);
+}
+
+static void qdisk_detached(libxl__egc *egc,
+                           libxl__spawn_state *spawn)
+{
+    libxl__spawn_qdisk_state *sqs = CONTAINER_OF(spawn, *sqs, spawn);
+    sqs->callback(egc, sqs, 0);
+}
+
+/* Generic function to signal a Qemu instance to exit */
+static int kill_device_model(libxl__gc *gc, const char *xs_path_pid)
+{
+    const char *xs_pid;
+    int ret, pid;
+
+    ret = libxl__xs_read_checked(gc, XBT_NULL, xs_path_pid, &xs_pid);
+    if (ret || !xs_pid) {
+        LOG(ERROR, "unable to find device model pid in %s",
xs_path_pid);
+        ret = ret ? : ERROR_FAIL;
         goto out;
     }
-    ret = kill(atoi(pid), SIGHUP);
+    pid = atoi(xs_pid);
+
+    ret = kill(pid, SIGHUP);
     if (ret < 0 && errno == ESRCH) {
         LOG(ERROR, "Device Model already exited");
         ret = 0;
@@ -1327,7 +1432,7 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t
domid)
         LOG(DEBUG, "Device Model signaled");
         ret = 0;
     } else {
-        LOGE(ERROR, "failed to kill Device Model [%d]", atoi(pid));
+        LOGE(ERROR, "failed to kill Device Model [%d]", pid);
         ret = ERROR_FAIL;
         goto out;
     }
@@ -1336,6 +1441,32 @@ out:
     return ret;
 }
 
+/* Helper to destroy a Qdisk backend */
+int libxl__destroy_qdisk_backend(libxl__gc *gc, uint32_t domid)
+{
+    char *pid_path;
+    int rc;
+
+    pid_path = GCSPRINTF("libxl/%u/qdisk-backend-pid", domid);
+
+    rc = kill_device_model(gc, pid_path);
+    if (rc)
+        goto out;
+
+    libxl__xs_rm_checked(gc, XBT_NULL, pid_path);
+    libxl__xs_rm_checked(gc, XBT_NULL,
+                         GCSPRINTF("device-model/%u", domid));
+
+out:
+    return rc;
+}
+
+int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
+{
+    return kill_device_model(gc,
+                GCSPRINTF("/local/domain/%d/image/device-model-pid",
domid));
+}
+
 int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1746b7d..fe53b80 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2502,6 +2502,23 @@ _hidden void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state*);
 
 _hidden char *libxl__stub_dm_name(libxl__gc *gc, const char * guest_name);
 
+/* Qdisk backend launch helpers */
+
+typedef struct libxl__spawn_qdisk_state libxl__spawn_qdisk_state;
+
+typedef void libxl__qdisk_spawn_cb(libxl__egc *egc, libxl__spawn_qdisk_state*,
+                                   int rc /* if !0, error was logged */);
+
+struct libxl__spawn_qdisk_state {
+    uint32_t guest_domid;
+    libxl__spawn_state spawn;
+    libxl__qdisk_spawn_cb *callback;
+};
+
+_hidden void libxl__spawn_qdisk_backend(libxl__egc *egc,
+                                        libxl__spawn_qdisk_state *sqs);
+_hidden int libxl__destroy_qdisk_backend(libxl__gc *gc, uint32_t domid);
+
 /*----- Domain creation -----*/
 
 typedef struct libxl__domain_create_state libxl__domain_create_state;
-- 
1.7.7.5 (Apple Git-26)


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

Move the daemonizer code from create_domain into it's own function
that can be called from other places different than create_domain.
This will be used to daemonize the driver domain backend handler.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |  102 ++++++++++++++++++++++++++--------------------
 1 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 884f050..553ba70 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -370,6 +370,58 @@ out:
     if (ferror(stdout) || fflush(stdout)) { perror("stdout");
exit(-1); }
 }
 
+static int do_daemonize(char *name)
+{
+    char *fullname;
+    pid_t child1, got_child;
+    int nullfd, ret = 0;
+    int status = 0;
+
+    child1 = xl_fork(child_waitdaemon);
+    if (child1) {
+        printf("Daemon running with PID %d\n", child1);
+
+        for (;;) {
+            got_child = xl_waitpid(child_waitdaemon, &status, 0);
+            if (got_child == child1) break;
+            assert(got_child == -1);
+            perror("failed to wait for daemonizing child");
+            ret = ERROR_FAIL;
+            goto out;
+        }
+        if (status) {
+            libxl_report_child_exitstatus(ctx, XTL_ERROR,
+                       "daemonizing child", child1, status);
+            ret = ERROR_FAIL;
+            goto out;
+        }
+        ret = 1;
+        goto out;
+    }
+
+    postfork();
+
+    ret = libxl_create_logfile(ctx, name, &fullname);
+    if (ret) {
+        LOG("failed to open logfile %s:
%s",fullname,strerror(errno));
+        exit(-1);
+    }
+
+    CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND,
+                               0644) )<0);
+    free(fullname);
+
+    CHK_ERRNO(( nullfd = open("/dev/null", O_RDONLY) )<0);
+    dup2(nullfd, 0);
+    dup2(logfile, 1);
+    dup2(logfile, 2);
+
+    CHK_ERRNO(daemon(0, 1) < 0);
+
+out:
+    return ret;
+}
+
 static int parse_action_on_shutdown(const char *buf, libxl_action_on_shutdown
*a)
 {
     int i;
@@ -1859,7 +1911,6 @@ static uint32_t create_domain(struct domain_create
*dom_info)
     void *config_data = 0;
     int config_len = 0;
     int restore_fd = -1;
-    int status = 0;
     const libxl_asyncprogress_how *autoconnect_console_how;
     struct save_file_header hdr;
 
@@ -2091,55 +2142,18 @@ start:
         vncviewer(domid, vncautopass);
 
     if (need_daemon) {
-        char *fullname, *name;
-        pid_t child1, got_child;
-        int nullfd;
-
-        child1 = xl_fork(child_waitdaemon);
-        if (child1) {
-            printf("Daemon running with PID %d\n", child1);
-
-            for (;;) {
-                got_child = xl_waitpid(child_waitdaemon, &status, 0);
-                if (got_child == child1) break;
-                assert(got_child == -1);
-                perror("failed to wait for daemonizing child");
-                ret = ERROR_FAIL;
-                goto out;
-            }
-            if (status) {
-                libxl_report_child_exitstatus(ctx, XTL_ERROR,
-                           "daemonizing child", child1, status);
-                ret = ERROR_FAIL;
-                goto out;
-            }
-            ret = domid;
-            goto out;
-        }
-
-        postfork();
+        char *name;
 
         if (asprintf(&name, "xl-%s", d_config.c_info.name) <
0) {
             LOG("Failed to allocate memory in asprintf");
             exit(1);
         }
-        rc = libxl_create_logfile(ctx, name, &fullname);
-        if (rc) {
-            LOG("failed to open logfile %s:
%s",fullname,strerror(errno));
-            exit(-1);
-        }
-
-        CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND,
-                                   0644) )<0);
-        free(fullname);
+        ret = do_daemonize(name);
         free(name);
-
-        CHK_ERRNO(( nullfd = open("/dev/null", O_RDONLY) )<0);
-        dup2(nullfd, 0);
-        dup2(logfile, 1);
-        dup2(logfile, 2);
-
-        CHK_ERRNO(daemon(0, 1) < 0);
+        if (ret) {
+            ret = (ret == 1) ? domid : ret;
+            goto out;
+        }
         need_daemon = 0;
     }
     LOG("Waiting for domain %s (domid %d) to die [pid %ld]",
-- 
1.7.7.5 (Apple Git-26)


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

When running libxl from a driver domain there's no xenstore pid file
(because xenstore is not running on the driver domain). Also, at that
point in libxl initialization there's no way to know wether libxl is
running on a domain different than Dom0, so just revert the change in
order to allow libxl to work on driver domains.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c          |    7 -------
 tools/libxl/libxl_internal.h |    1 -
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5417b48..3337518 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -25,7 +25,6 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
                     unsigned flags, xentoollog_logger * lg)
 {
     libxl_ctx *ctx = NULL;
-    struct stat stat_buf;
     int rc;
 
     if (version != LIBXL_VERSION) { rc = ERROR_VERSION; goto out; }
@@ -82,12 +81,6 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
     rc = libxl__poller_init(ctx, &ctx->poller_app);
     if (rc) goto out;
 
-    if ( stat(XENSTORE_PID_FILE, &stat_buf) != 0 ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Is xenstore daemon
running?\n"
-                     "failed to stat %s", XENSTORE_PID_FILE);
-        rc = ERROR_FAIL; goto out;
-    }
-
     ctx->xch = xc_interface_open(lg,lg,0);
     if (!ctx->xch) {
         LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fe53b80..d2b0171 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -378,7 +378,6 @@ typedef struct {
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
 #define AUTO_PHP_SLOT          0x100
-#define XENSTORE_PID_FILE      "/var/run/xenstored.pid"
 
 #define PROC_PCI_NUM_RESOURCES 7
 #define PCI_BAR_IO             0x01
-- 
1.7.7.5 (Apple Git-26)


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

Add the necessary logic in libxl to allow it to act as a listener for
launching backends in a driver domain, replacing udev (like we already
do on Dom0). This new functionality is acomplished by watching the
domain backend path (/local/domain/<domid>/backend) and reacting to
device creation/destruction.

The way to launch this listener daemon is from xl, using the newly
introduced "devd" command. The command will daemonize by default,
using "xldevd.log" as it's logfile. Optionally the user can force
the
execution of the listener in the foreground by passing the "-F"
option to the devd command.

Current backends handled by this daemon include Qdisk, vbd and vif
device types.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
In order to use Qdisk on a driver domain the following patch has to be
applied to Qemu:

http://marc.info/?l=qemu-devel&m=137953390608727&w=2
---
I'm quite sure memory management is not done correctly, after each
call to backend_watch_callback the garbage collector should free all
the references, but I think this is not happening, and I would
like someone with experience on libxl memory management (IanJ) to
comment on possible ways to deal with that.
---
 tools/libxl/libxl.c       |  309 +++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h       |    8 ++
 tools/libxl/xl.h          |    1 +
 tools/libxl/xl_cmdimpl.c  |   24 ++++
 tools/libxl/xl_cmdtable.c |    6 +
 5 files changed, 348 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3337518..dd9cab6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3517,6 +3517,315 @@ DEFINE_DEVICE_ADD(vtpm)
 
 /******************************************************************************/
 
+/*
+ * Data structures used to track devices handled by driver domains
+ */
+
+/*
+ * Structure that describes a device handled by a driver domain
+ */
+typedef struct libxl__ddomain_device {
+    libxl__device *dev;
+    LIBXL_SLIST_ENTRY(struct libxl__ddomain_device) next;
+} libxl__ddomain_device;
+
+/*
+ * Structure that describes a domain and it's associated devices
+ */
+typedef struct libxl__ddomain_guest {
+    uint32_t domid;
+    int num_vifs, num_vbds, num_qdisks;
+    LIBXL_SLIST_HEAD(, struct libxl__ddomain_device) devices;
+    LIBXL_SLIST_ENTRY(struct libxl__ddomain_guest) next;
+} libxl__ddomain_guest;
+
+/*
+ * Main structure used by a driver domain to keep track of devices
+ * currently in use
+ */
+typedef struct {
+    libxl__ao *ao;
+    libxl__ev_xswatch watch;
+    LIBXL_SLIST_HEAD(, struct libxl__ddomain_guest) guests;
+} libxl__ddomain;
+
+static libxl__ddomain_guest *search_for_guest(libxl__ddomain *ddomain,
+                                               uint32_t domid)
+{
+    libxl__ddomain_guest *dguest;
+
+    LIBXL_SLIST_FOREACH(dguest, &ddomain->guests, next) {
+        if (dguest->domid == domid)
+            return dguest;
+    }
+    return NULL;
+}
+
+static libxl__ddomain_device *search_for_device(libxl__ddomain_guest *dguest,
+                                                libxl__device *dev)
+{
+    libxl__ddomain_device *ddev;
+
+    LIBXL_SLIST_FOREACH(ddev, &dguest->devices, next) {
+        if (memcmp(ddev->dev, dev, sizeof(*dev)) == 0)
+            return ddev;
+    }
+    return NULL;
+}
+
+static void device_complete(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+
+    if (aodev->action == LIBXL__DEVICE_ACTION_REMOVE)
+        free(aodev->dev);
+
+    LOG(DEBUG, "device %s %s %s",
+               libxl__device_backend_path(gc, aodev->dev),
+               libxl__device_action_to_string(aodev->action),
+               aodev->rc ? "failed" : "succeed");
+
+    return;
+}
+
+static void qdisk_spawn_outcome(libxl__egc *egc, libxl__spawn_qdisk_state *sqs,
+                                int rc)
+{
+    STATE_AO_GC(sqs->spawn.ao);
+
+    LOG(DEBUG, "qdisk backend spawn for domain %u %s",
sqs->guest_domid,
+               rc ? "failed" : "succeed");
+}
+
+static int add_device(libxl__egc *egc, libxl__ddomain *ddomain,
+                      libxl__ddomain_guest *dguest,
+                      libxl__ddomain_device *ddev)
+{
+    STATE_AO_GC(ddomain->ao);
+    libxl__device *dev = ddev->dev;
+    libxl__ao_device *aodev;
+    libxl__spawn_qdisk_state *sqs;
+
+    switch(dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VBD:
+    case LIBXL__DEVICE_KIND_VIF:
+        if (dev->backend_kind == LIBXL__DEVICE_KIND_VBD)
dguest->num_vbds++;
+        if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF)
dguest->num_vifs++;
+
+        GCNEW(aodev);
+        libxl__prepare_ao_device(ao, aodev);
+        aodev->dev = dev;
+        aodev->action = LIBXL__DEVICE_ACTION_ADD;
+        aodev->callback = device_complete;
+        libxl__wait_device_connection(egc, aodev);
+
+        break;
+    case LIBXL__DEVICE_KIND_QDISK:
+        if (dguest->num_qdisks == 0) {
+            GCNEW(sqs);
+            sqs->guest_domid = dev->domid;
+            sqs->spawn.ao = ao;
+            sqs->callback = qdisk_spawn_outcome;
+
+            libxl__spawn_qdisk_backend(egc, sqs);
+        }
+        dguest->num_qdisks++;
+
+        break;
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+static int remove_device(libxl__egc *egc, libxl__ddomain *ddomain,
+                         libxl__ddomain_guest *dguest,
+                         libxl__ddomain_device *ddev)
+{
+    STATE_AO_GC(ddomain->ao);
+    libxl__device *dev = ddev->dev;
+    libxl__ao_device *aodev;
+    int rc;
+
+    switch(ddev->dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VBD:
+    case LIBXL__DEVICE_KIND_VIF:
+        if (dev->backend_kind == LIBXL__DEVICE_KIND_VBD)
dguest->num_vbds--;
+        if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF)
dguest->num_vifs--;
+
+        GCNEW(aodev);
+        libxl__prepare_ao_device(ao, aodev);
+        aodev->dev = dev;
+        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
+        aodev->callback = device_complete;
+        libxl__initiate_device_remove(egc, aodev);
+        break;
+    case LIBXL__DEVICE_KIND_QDISK:
+        if (--dguest->num_qdisks == 0) {
+            rc = libxl__destroy_qdisk_backend(gc, dev->domid);
+            if (rc)
+                goto out;
+        }
+        libxl__device_destroy(gc, dev);
+        free(dev);
+        break;
+    default:
+        break;
+    }
+
+out:
+    return rc;
+}
+
+static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
+                                   const char *watch_path,
+                                   const char *event_path)
+{
+    libxl__ddomain *ddomain = CONTAINER_OF(watch, *ddomain, watch);
+    STATE_AO_GC(ddomain->ao);
+    char *p, *path;
+    const char *sstate;
+    int state, rc, num_devs;
+    libxl__device *dev = NULL;
+    libxl__ddomain_device *ddev = NULL;
+    libxl__ddomain_guest *dguest = NULL;
+
+    /* Check if event_path ends with "state" and truncate it */
+    if (strlen(event_path) < strlen("state"))
+        goto skip;
+
+    path = libxl__strdup(gc, event_path);
+    p = path + strlen(path) - strlen("state") - 1;
+    if (*p != '/')
+        goto skip;
+    *p = '\0';
+    p++;
+    if (strcmp(p, "state") != 0)
+        goto skip;
+
+    /* Check if the state is 1 (XenbusStateInitialising) or greater */
+    rc = libxl__xs_read_checked(gc, XBT_NULL, event_path, &sstate);
+    if (rc || !sstate)
+        goto skip;
+    state = atoi(sstate);
+
+    dev = libxl__zalloc(NOGC, sizeof(*dev));
+    rc = libxl__parse_backend_path(gc, path, dev);
+    if (rc)
+        goto skip;
+
+    dguest = search_for_guest(ddomain, dev->domid);
+    if (dguest == NULL && state == XenbusStateClosed) {
+        /*
+         * Spurious state change, device has already been disconnected
+         * or never attached.
+         */
+        goto skip;
+    }
+    if (dguest == NULL) {
+        /* Create a new guest struct and initialize it */
+        dguest = libxl__zalloc(NOGC, sizeof(*dguest));
+        dguest->domid = dev->domid;
+        LIBXL_SLIST_INIT(&dguest->devices);
+        LIBXL_SLIST_INSERT_HEAD(&ddomain->guests, dguest, next);
+        LOG(DEBUG, "added domain %u to the list of active guests",
+                   dguest->domid);
+    }
+    ddev = search_for_device(dguest, dev);
+    if (ddev == NULL && state == XenbusStateClosed) {
+        /*
+         * Spurious state change, device has already been disconnected
+         * or never attached.
+         */
+        goto skip;
+    } else if (ddev == NULL) {
+        /*
+         * New device addition, allocate a struct to hold it and add it
+         * to the list of active devices for a given guest.
+         */
+        ddev = libxl__zalloc(NOGC, sizeof(*ddev));
+        ddev->dev = dev;
+        LIBXL_SLIST_INSERT_HEAD(&dguest->devices, ddev, next);
+        LOG(DEBUG, "added device %s to the list of active devices",
path);
+        add_device(egc, ddomain, dguest, ddev);
+    } else if (state == XenbusStateClosed) {
+        /*
+         * Removal of an active device, remove it from the list and
+         * free it's data structures if they are no longer needed.
+         *
+         * The free of the associated libxl__device is left to the
+         * helper remove_device function.
+         */
+        LIBXL_SLIST_REMOVE(&dguest->devices, ddev,
libxl__ddomain_device,
+                           next);
+        LOG(DEBUG, "removed device %s from the list of active
devices", path);
+        remove_device(egc, ddomain, dguest, ddev);
+
+        free(ddev);
+        /* If this was the last device in the domain, remove it from the list
*/
+        num_devs = dguest->num_vifs + dguest->num_vbds +
dguest->num_qdisks;
+        if (num_devs == 0) {
+            LIBXL_SLIST_REMOVE(&ddomain->guests, dguest,
libxl__ddomain_guest,
+                               next);
+            LOG(DEBUG, "removed domain %u from the list of active
guests",
+                       dguest->domid);
+            /* Clear any leftovers in libxl/<domid> */
+            libxl__xs_rm_checked(gc, XBT_NULL,
+                                 GCSPRINTF("libxl/%u",
dguest->domid));
+            free(dguest);
+        }
+    }
+
+    return;
+
+skip:
+    free(dev);
+    free(ddev);
+    free(dguest);
+    return;
+}
+
+/* Handler of events for device driver domains */
+int libxl_device_events_handler(libxl_ctx *ctx,
+                                const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, 0, ao_how);
+    int rc;
+    uint32_t domid;
+    libxl__ddomain ddomain;
+    char *be_path;
+
+    ddomain.ao = ao;
+    LIBXL_SLIST_INIT(&ddomain.guests);
+
+    rc = libxl__get_domid(gc, &domid);
+    if (rc) {
+        LOG(ERROR, "unable to get domain id");
+        goto out;
+    }
+
+    rc = libxl__xs_write_checked(gc, XBT_NULL, DISABLE_UDEV_PATH,
"1");
+    if (rc) {
+        LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
+        goto out;
+    }
+
+    /*
+     * We use absolute paths because we want xswatch to also return
+     * absolute paths that can be parsed by libxl__parse_backend_path.
+     */
+    be_path = GCSPRINTF("/local/domain/%u/backend", domid);
+    rc = libxl__ev_xswatch_register(gc, &ddomain.watch,
backend_watch_callback,
+                                    be_path);
+
+out:
+    GC_FREE;
+    return rc ? : AO_INPROGRESS;
+}
+
+/******************************************************************************/
+
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index be19bf5..933bb94 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -867,6 +867,14 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx,
uint32_t domid,
                                         int *num);
 
 /*
+ * Function that drives the main loop that checks for device actions and
+ * launches the callbacks passed by the user.
+ */
+int libxl_device_events_handler(libxl_ctx *ctx,
+                                const libxl_asyncop_how *ao_how)
+                                LIBXL_EXTERNAL_CALLERS_ONLY;
+
+/*
  * Functions related to making devices assignable -- that is, bound to
  * the pciback driver, ready to be given to a guest via
  * libxl_pci_device_add.
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index e72a7d2..2dfcf19 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -105,6 +105,7 @@ int main_getenforce(int argc, char **argv);
 int main_setenforce(int argc, char **argv);
 int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
+int main_devd(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 553ba70..cebfe3a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7158,6 +7158,30 @@ int main_remus(int argc, char **argv)
     return -ERROR_FAIL;
 }
 
+int main_devd(int argc, char **argv)
+{
+    int ret = 0, opt = 0, daemonize = 1;
+
+    SWITCH_FOREACH_OPT(opt, "F", NULL, "devd", 0) {
+    case 'F':
+        daemonize = 0;
+        break;
+    }
+
+    if (daemonize) {
+        ret = do_daemonize("xldevd");
+        if (ret) {
+            ret = (ret == 1) ? 0 : ret;
+            goto out;
+        }
+    }
+
+    libxl_device_events_handler(ctx, 0);
+
+out:
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 326a660..7709206 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -488,6 +488,12 @@ struct cmd_spec cmd_table[] = {
       "                        of the domain."
 
     },
+    { "devd",
+      &main_devd, 0, 1,
+      "Daemon that listens for devices and launches backends",
+      "[options]",
+      "-F                      Run in the foreground",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
-- 
1.7.7.5 (Apple Git-26)


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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> If libxl is executed inside a guest domain it needs write access to
> the local libxl xenstore dir (/local/<domid>/libxl) to store internal
> data.
These sorts of changes need a patch against docs/misc/xenstore-paths.txt
too.
>  This also applies to Qemu which needs a
> /local/<domid>/device-model xenstore directory.
Is this a new requirement or a separate preexisting issue? How have we
lived without it?
> This patch creates the mentioned directories for each guest launched
> from libxl.
I don't really like leaking toolstack "internals" into the normal
guest
namespace. Can we add an option to specify "this is a toolstack
domain"
and key off that?

I also wonder if this shouldn't be /libxl/<domid> at the top level.
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_create.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 0c32d0b..aac19b5 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -498,6 +498,18 @@ retry_transaction:
>      libxl__xs_mkdir(gc, t,
>                      libxl__sprintf(gc, "%s/data", dom_path),
>                      rwperm, ARRAY_SIZE(rwperm));
> +    /*
> +     * Create a local "libxl" directory for each guest, since we
might want
> +     * to use libxl from inside the guest
> +     */
> +    libxl__xs_mkdir(gc, t, GCSPRINTF("%s/libxl", dom_path),
rwperm,
> +                    ARRAY_SIZE(rwperm));
> +    /*
> +     * Create a local "device-model" directory for each guest,
since we
> +     * might want to use Qemu from inside the guest
> +     */
> +    libxl__xs_mkdir(gc, t, GCSPRINTF("%s/device-model",
dom_path), rwperm,
> +                    ARRAY_SIZE(rwperm));
>      if (info->type == LIBXL_DOMAIN_TYPE_HVM)
>          libxl__xs_mkdir(gc, t,
>              libxl__sprintf(gc,
"%s/hvmloader/generation-id-address", dom_path),


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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> This patch writes Dom0 domid on xenstore (like it's done for other
> guests), and adds a libxl helper function to fetch that domid from
> xenstore.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>



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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> The info fetched by libxl_domain_info is not used anywere in the
> function.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_device.c |   10 ----------
>  1 files changed, 0 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 16a92a4..2971dd3 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -715,16 +715,8 @@ void libxl__wait_device_connection(libxl__egc *egc,
libxl__ao_device *aodev)
>      STATE_AO_GC(aodev->ao);
>      char *be_path = libxl__device_backend_path(gc, aodev->dev);
>      char *state_path = libxl__sprintf(gc, "%s/state", be_path);
> -    libxl_dominfo info;
> -    uint32_t domid = aodev->dev->domid;
>      int rc = 0;
>  
> -    libxl_dominfo_init(&info);
> -    rc = libxl_domain_info(CTX, &info, domid);
> -    if (rc) {
> -        LOG(ERROR, "unable to get info for domain %d", domid);
> -        goto out;
> -    }
>      if (QEMU_BACKEND(aodev->dev)) {
>          /*
>           * If Qemu is not running, there's no point in waiting for
> @@ -746,12 +738,10 @@ void libxl__wait_device_connection(libxl__egc *egc,
libxl__ao_device *aodev)
>          goto out;
>      }
>  
> -    libxl_dominfo_dispose(&info);
>      return;
>  
>  out:
>      aodev->rc = rc;
> -    libxl_dominfo_dispose(&info);
>      device_hotplug_done(egc, aodev);
>      return;
>  }


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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> libxl currently refuses to execute hotplug scripts if the backend
> domid of a device is different than LIBXL_TOOLSTACK_DOMID. This will
> prevent libxl from executing hotplug scripts when running on a domain
> different than LIBXL_TOOLSTACK_DOMID, we should instead check if
> backend_domid is different than current domid.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although as I read the rest of the series I may wonder if this should'be
be a parameter which comes in i.e. handle_hotplug_events_for_domid.
We'll see.
> ---
>  tools/libxl/libxl_device.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 2971dd3..48acc92 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -908,12 +908,15 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
>      int rc = 0;
>      int hotplug;
>      pid_t pid;
> +    uint32_t domid;
>  
>      /*
>       * If device is attached from a driver domain don't try to execute
>       * hotplug scripts
>       */
> -    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID)
> +    rc = libxl__get_domid(gc, &domid);
> +    if (rc) goto out;
> +    if (aodev->dev->backend_domid != domid)
>          goto out;
>  
>      /* Check if we have to execute hotplug scripts for this device


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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> If the backend of a device is running on a driver domain, it should be
> the driver domain itself the one to clean the backend path once the
> device has been successfully disconnected.
> 
> And the opposite way, a domain different than LIBXL_TOOLSTACK_DOMID
> should not try to remove the frontend paths of a device.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_device.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 48acc92..082bd2a 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -562,6 +562,10 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device
*dev)
>      const char *tapdisk_params;
>      xs_transaction_t t = 0;
>      int rc;
> +    uint32_t domid;
> +
> +    rc = libxl__get_domid(gc, &domid);
> +    if (rc) goto out;
>  
>      for (;;) {
>          rc = libxl__xs_transaction_start(gc, &t);
> @@ -571,8 +575,10 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device
*dev)
>          rc = libxl__xs_read_checked(gc, t, tapdisk_path,
&tapdisk_params);
>          if (rc) goto out;
>  
> -        libxl__xs_path_cleanup(gc, t, fe_path);
> -        libxl__xs_path_cleanup(gc, t, be_path);
> +        if (domid == LIBXL_TOOLSTACK_DOMID)
> +            libxl__xs_path_cleanup(gc, t, fe_path);
> +        if (dev->backend_domid == domid)
> +            libxl__xs_path_cleanup(gc, t, be_path);
>  
>          rc = libxl__xs_transaction_commit(gc, &t);
>          if (!rc) break;


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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> The backend path should be fully owned by the domain where it resides.
I can see why /local/domain/<domid>backends/<type>/<id> would
need to be
owned by the backend dom, but why
do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?

The backend should be writing only to the one directory associated with
the device I think.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_device.c   |   45
+++++++++++++++++++++++++++++++++++++++--
>  tools/libxl/libxl_internal.h |    2 +
>  2 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 082bd2a..f39b7b1 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -83,6 +83,47 @@ out:
>      return rc;
>  }
>  
> +int libxl__create_backend_path(libxl__gc *gc, xs_transaction_t t,
> +                               libxl__device *device)
> +{
> +    char *dom_backend_path;
> +    char *be_path = libxl__device_backend_path(gc, device);
> +    char *p;
> +    struct xs_permissions backend_dir_perms[2];
> +    struct xs_permissions backend_path_perms[1];
> +    int rc;
> +
> +    backend_path_perms[0].id = device->backend_domid;
> +    backend_path_perms[0].perms = XS_PERM_NONE;
> +
> +    backend_dir_perms[0].id = device->backend_domid;
> +    backend_dir_perms[0].perms = XS_PERM_NONE;
> +    backend_dir_perms[1].id = device->domid;
> +    backend_dir_perms[1].perms = XS_PERM_READ;
> +
> +    rc = libxl__xs_rm_checked(gc, t, be_path);
> +    if (rc) goto error;
> +    if (!libxl__xs_mkdir(gc, t, be_path, backend_dir_perms,
> +        ARRAY_SIZE(backend_dir_perms)))
> +        goto error;
> +
> +    dom_backend_path = GCSPRINTF("%s/backend",
> +                            libxl__xs_get_dompath(gc,
device->backend_domid));
> +    while (strcmp(be_path, dom_backend_path) != 0) {
> +        p = strrchr(be_path, '/');
> +        if (!p) goto error;
> +        *p = '\0';
> +
> +        xs_set_permissions(CTX->xsh, t, be_path, backend_path_perms,
> +                           ARRAY_SIZE(backend_path_perms));
> +    }
> +
> +    return 0;
> +
> +error:
> +    return ERROR_FAIL;
> +}
> +
>  int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
>          libxl__device *device, char **bents, char **fents, char
**ro_fents)
>  {
> @@ -135,9 +176,7 @@ retry_transaction:
>      }
>  
>      if (bents) {
> -        xs_rm(ctx->xsh, t, backend_path);
> -        xs_mkdir(ctx->xsh, t, backend_path);
> -        xs_set_permissions(ctx->xsh, t, backend_path, backend_perms,
ARRAY_SIZE(backend_perms));
> +        libxl__create_backend_path(gc, t, device);
>          xs_write(ctx->xsh, t, libxl__sprintf(gc,
"%s/frontend", backend_path), frontend_path, strlen(frontend_path));
>          libxl__xs_writev(gc, t, backend_path, bents);
>      }
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 3b74726..1746b7d 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -929,6 +929,8 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc
*gc, xs_transaction_t t,
>  _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend
backend);
>  _hidden char *libxl__device_disk_string_of_format(libxl_disk_format
format);
>  _hidden int libxl__device_disk_set_backend(libxl__gc*,
libxl_device_disk*);
> +_hidden int libxl__create_backend_path(libxl__gc *gc, xs_transaction_t t,
> +                                       libxl__device *device);
>  
>  _hidden int libxl__device_physdisk_major_minor(const char *physpath, int
*major, int *minor);
>  _hidden int libxl__device_disk_dev_number(const char *virtpath,


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

On 25/09/13 15:57, Ian Campbell wrote:
> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>> The backend path should be fully owned by the domain where it resides.
> 
> I can see why /local/domain/<domid>backends/<type>/<id>
would need to be
> owned by the backend dom, but why
> do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
already owned by the driver domain, the problem is that if the driver
domain has to perform the clean up of this path it should be able to
fully remove it, otherwise we are leaving empty directories around in
xenstore (backend/<type>/<guest_domid> and so).

And performing the clean up from the toolstack domain is not that easy,
we will have to add a way to signal the toolstack domain that the driver
domain has finished the disconnection and the directory can be cleaned.

On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné
wrote:
> On 25/09/13 15:57, Ian Campbell wrote:
> > On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
> >> The backend path should be fully owned by the domain where it
resides.
> > 
> > I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
> > owned by the backend dom, but why
> > do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
> 
> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
> already owned by the driver domain, the problem is that if the driver
> domain has to perform the clean up of this path it should be able to
> fully remove it, otherwise we are leaving empty directories around in
> xenstore (backend/<type>/<guest_domid> and so).
> 
> And performing the clean up from the toolstack domain is not that easy,
> we will have to add a way to signal the toolstack domain that the driver
> domain has finished the disconnection and the directory can be cleaned.
Don't we need that anyway so that the toolstack domain can synchronise
the end of the asynchronous op which is the remove?

Ian




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

On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> In libxl__need_xenpv_qemu check that the backend domain of the Qdisk
> device is Dom0 before launching a Qemu instance in the toolstack
> domain.
Should the check not be disks[i].backend == domid from the function you
added in patch #1?
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_dm.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 4035b6d..ddc7367 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1368,7 +1368,8 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
>  
>      if (nr_disks > 0) {
>          for (i = 0; i < nr_disks; i++) {
> -            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK) {
> +            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK &&
> +                disks[i].backend_domid == LIBXL_TOOLSTACK_DOMID) {
>                  ret = 1;
>                  goto out;
>              }


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

On 25/09/13 17:00, Ian Campbell wrote:
> On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné wrote:
>> On 25/09/13 15:57, Ian Campbell wrote:
>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>>>> The backend path should be fully owned by the domain where it
resides.
>>>
>>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
>>> owned by the backend dom, but why
>>> do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
>>
>> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
>> already owned by the driver domain, the problem is that if the driver
>> domain has to perform the clean up of this path it should be able to
>> fully remove it, otherwise we are leaving empty directories around in
>> xenstore (backend/<type>/<guest_domid> and so).
>>
>> And performing the clean up from the toolstack domain is not that easy,
>> we will have to add a way to signal the toolstack domain that the
driver
>> domain has finished the disconnection and the directory can be cleaned.
> 
> Don't we need that anyway so that the toolstack domain can synchronise
> the end of the asynchronous op which is the remove?
Not really IMHO, the toolstack waits for the device frontend to
disconnect and then it removes the frontend xenstore entries. Then (or
in parallel) the domain that's handling the backend should deal with the
disconnection of the backend and perform any necessary actions and clean
up the backend xenstore entries.


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

On 25/09/13 17:06, Ian Campbell wrote:
> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>> In libxl__need_xenpv_qemu check that the backend domain of the Qdisk
>> device is Dom0 before launching a Qemu instance in the toolstack
>> domain.
> 
> Should the check not be disks[i].backend == domid from the function you
> added in patch #1?
Yes it could be, and it''s probably easier to understand. I did it this
way because the only user of this function is the toolstack domain,
driver domains don''t make use of it.

On Wed, 2013-09-25 at 17:10 +0200, Roger Pau Monné
wrote:
> On 25/09/13 17:00, Ian Campbell wrote:
> > On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné wrote:
> >> On 25/09/13 15:57, Ian Campbell wrote:
> >>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
> >>>> The backend path should be fully owned by the domain where
it resides.
> >>>
> >>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
> >>> owned by the backend dom, but why
> >>> do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
> >>
> >> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
> >> already owned by the driver domain, the problem is that if the
driver
> >> domain has to perform the clean up of this path it should be able
to
> >> fully remove it, otherwise we are leaving empty directories around
in
> >> xenstore (backend/<type>/<guest_domid> and so).
> >>
> >> And performing the clean up from the toolstack domain is not that
easy,
> >> we will have to add a way to signal the toolstack domain that the
driver
> >> domain has finished the disconnection and the directory can be
cleaned.
> > 
> > Don't we need that anyway so that the toolstack domain can
synchronise
> > the end of the asynchronous op which is the remove?
> 
> Not really IMHO, the toolstack waits for the device frontend to
> disconnect and then it removes the frontend xenstore entries. Then (or
> in parallel) the domain that's handling the backend should deal with
the
> disconnection of the backend and perform any necessary actions and clean
> up the backend xenstore entries.
So e.g. xl destroy can potentially return while the backend is still
closing down? That would mean it might fail to reboot the guest, because
the block device in the driver domain is still busy.

Ian.


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

On 25/09/13 17:18, Ian Campbell wrote:
> On Wed, 2013-09-25 at 17:10 +0200, Roger Pau Monné wrote:
>> On 25/09/13 17:00, Ian Campbell wrote:
>>> On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné wrote:
>>>> On 25/09/13 15:57, Ian Campbell wrote:
>>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>>>>>> The backend path should be fully owned by the domain
where it resides.
>>>>>
>>>>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
>>>>> owned by the backend dom, but why
>>>>> do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
>>>>
>>>> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
>>>> already owned by the driver domain, the problem is that if the
driver
>>>> domain has to perform the clean up of this path it should be
able to
>>>> fully remove it, otherwise we are leaving empty directories
around in
>>>> xenstore (backend/<type>/<guest_domid> and so).
>>>>
>>>> And performing the clean up from the toolstack domain is not
that easy,
>>>> we will have to add a way to signal the toolstack domain that
the driver
>>>> domain has finished the disconnection and the directory can be
cleaned.
>>>
>>> Don't we need that anyway so that the toolstack domain can
synchronise
>>> the end of the asynchronous op which is the remove?
>>
>> Not really IMHO, the toolstack waits for the device frontend to
>> disconnect and then it removes the frontend xenstore entries. Then (or
>> in parallel) the domain that's handling the backend should deal
with the
>> disconnection of the backend and perform any necessary actions and
clean
>> up the backend xenstore entries.
> 
> So e.g. xl destroy can potentially return while the backend is still
> closing down? That would mean it might fail to reboot the guest, because
> the block device in the driver domain is still busy.
True, since we don't synchronize with the toolstack there's a small
window were a new domain could be created before the driver domain
hasn't finished unplugging. This is a problem already with current udev
implementation, but I think we can solve this by adding a watch in the
toolstack that waits for the backend path to be destroyed (when the
backend is on a driver domain, of course).

I will expand this patch to include the above mentioned check.


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

On Thu, 2013-09-26 at 11:20 +0200, Roger Pau Monné
wrote:
> On 25/09/13 17:18, Ian Campbell wrote:
> > On Wed, 2013-09-25 at 17:10 +0200, Roger Pau Monné wrote:
> >> On 25/09/13 17:00, Ian Campbell wrote:
> >>> On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné wrote:
> >>>> On 25/09/13 15:57, Ian Campbell wrote:
> >>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> >>>>>> The backend path should be fully owned by the
domain where it resides.
> >>>>>
> >>>>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
> >>>>> owned by the backend dom, but why
> >>>>> do /local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
> >>>>
> >>>> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
> >>>> already owned by the driver domain, the problem is that if
the driver
> >>>> domain has to perform the clean up of this path it should
be able to
> >>>> fully remove it, otherwise we are leaving empty
directories around in
> >>>> xenstore (backend/<type>/<guest_domid> and
so).
> >>>>
> >>>> And performing the clean up from the toolstack domain is
not that easy,
> >>>> we will have to add a way to signal the toolstack domain
that the driver
> >>>> domain has finished the disconnection and the directory
can be cleaned.
> >>>
> >>> Don't we need that anyway so that the toolstack domain can
synchronise
> >>> the end of the asynchronous op which is the remove?
> >>
> >> Not really IMHO, the toolstack waits for the device frontend to
> >> disconnect and then it removes the frontend xenstore entries. Then
(or
> >> in parallel) the domain that's handling the backend should
deal with the
> >> disconnection of the backend and perform any necessary actions and
clean
> >> up the backend xenstore entries.
> > 
> > So e.g. xl destroy can potentially return while the backend is still
> > closing down? That would mean it might fail to reboot the guest,
because
> > the block device in the driver domain is still busy.
> 
> True, since we don't synchronize with the toolstack there's a small
> window were a new domain could be created before the driver domain
> hasn't finished unplugging. This is a problem already with current udev
> implementation, but I think we can solve this by adding a watch in the
> toolstack that waits for the backend path to be destroyed (when the
> backend is on a driver domain, of course).
> 
> I will expand this patch to include the above mentioned check.
Please do.

Can you also enumerate the intended permissions for each bit
of
/local/domain/<domid>/backends/<type>/<guest_domid>/<id>

I think you are changing the perms of "backends" onwards, but is that
really necessary? The backend should be cleaning up <id>, which needs
write perms for <guest_domid> but could the toolstack not do the rest?

Ian.


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

On 25/09/13 15:48, Ian Campbell wrote:
> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>> If libxl is executed inside a guest domain it needs write access to
>> the local libxl xenstore dir (/local/<domid>/libxl) to store
internal
>> data.
> 
> These sorts of changes need a patch against docs/misc/xenstore-paths.txt
> too.
Ack.
> 
>>  This also applies to Qemu which needs a
>> /local/<domid>/device-model xenstore directory.
> 
> Is this a new requirement or a separate preexisting issue? How have we
> lived without it?
This entry is so far only created on Dom0 (because it''s the only domain
that launches Qemu, and is created by Qemu if needed), but when running
on a guest, the guest doesn''t have permissions to write on
/local/<domid>/, so we have to create the directory in the toolstack
domain and assign write permissions for the guest.

This path is hardcoded in Qemu, so it''s quite difficult to change it,
that''s why I''ve opted for creating it on domain creation.
> 
>> This patch creates the mentioned directories for each guest launched
>> from libxl.
> 
> I don''t really like leaking toolstack "internals" into
the normal guest
> namespace. Can we add an option to specify "this is a toolstack
domain"
> and key off that?
OK, something like driver_domain=1|0
> I also wonder if this shouldn''t be /libxl/<domid> at the top
level.
I don''t have a strong opinion on this, but I generally prefer to keep
all the xenstore related keys for a domain inside /libxl/<domid>.

On 26/09/13 11:24, Ian Campbell wrote:
> On Thu, 2013-09-26 at 11:20 +0200, Roger Pau Monné wrote:
>> On 25/09/13 17:18, Ian Campbell wrote:
>>> On Wed, 2013-09-25 at 17:10 +0200, Roger Pau Monné wrote:
>>>> On 25/09/13 17:00, Ian Campbell wrote:
>>>>> On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné wrote:
>>>>>> On 25/09/13 15:57, Ian Campbell wrote:
>>>>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
>>>>>>>> The backend path should be fully owned by the
domain where it resides.
>>>>>>>
>>>>>>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
>>>>>>> owned by the backend dom, but why
>>>>>>> do
/local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
>>>>>>
>>>>>> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
>>>>>> already owned by the driver domain, the problem is that
if the driver
>>>>>> domain has to perform the clean up of this path it
should be able to
>>>>>> fully remove it, otherwise we are leaving empty
directories around in
>>>>>> xenstore (backend/<type>/<guest_domid> and
so).
>>>>>>
>>>>>> And performing the clean up from the toolstack domain
is not that easy,
>>>>>> we will have to add a way to signal the toolstack
domain that the driver
>>>>>> domain has finished the disconnection and the directory
can be cleaned.
>>>>>
>>>>> Don't we need that anyway so that the toolstack domain
can synchronise
>>>>> the end of the asynchronous op which is the remove?
>>>>
>>>> Not really IMHO, the toolstack waits for the device frontend to
>>>> disconnect and then it removes the frontend xenstore entries.
Then (or
>>>> in parallel) the domain that's handling the backend should
deal with the
>>>> disconnection of the backend and perform any necessary actions
and clean
>>>> up the backend xenstore entries.
>>>
>>> So e.g. xl destroy can potentially return while the backend is
still
>>> closing down? That would mean it might fail to reboot the guest,
because
>>> the block device in the driver domain is still busy.
>>
>> True, since we don't synchronize with the toolstack there's a
small
>> window were a new domain could be created before the driver domain
>> hasn't finished unplugging. This is a problem already with current
udev
>> implementation, but I think we can solve this by adding a watch in the
>> toolstack that waits for the backend path to be destroyed (when the
>> backend is on a driver domain, of course).
>>
>> I will expand this patch to include the above mentioned check.
> 
> Please do.
> 
> Can you also enumerate the intended permissions for each bit
> of
/local/domain/<domid>/backends/<type>/<guest_domid>/<id>
> 
> I think you are changing the perms of "backends" onwards, but is
that
> really necessary? The backend should be cleaning up <id>, which needs
> write perms for <guest_domid> but could the toolstack not do the
rest?
Yes, since I'm going to change the patch to syncronize with the
toolstack domain, it can do the rest of the cleaning on behalf of the
guest (the guest will only remove
/local/domain/<domid>/backends/<type>/<guest_domid>/<id>
and there won't
be a need to change any permissions).


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

On Thu, 2013-09-26 at 11:27 +0200, Roger Pau Monné
wrote:
> On 25/09/13 15:48, Ian Campbell wrote:
> > On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
> >> If libxl is executed inside a guest domain it needs write access
to
> >> the local libxl xenstore dir (/local/<domid>/libxl) to store
internal
> >> data.
> > 
> > These sorts of changes need a patch against
docs/misc/xenstore-paths.txt
> > too.
> 
> Ack.
> 
> > 
> >>  This also applies to Qemu which needs a
> >> /local/<domid>/device-model xenstore directory.
> > 
> > Is this a new requirement or a separate preexisting issue? How have we
> > lived without it?
> 
> This entry is so far only created on Dom0 (because it's the only domain
> that launches Qemu, and is created by Qemu if needed), but when running
> on a guest, the guest doesn't have permissions to write on
> /local/<domid>/, so we have to create the directory in the toolstack
> domain and assign write permissions for the guest.
> 
> This path is hardcoded in Qemu, so it's quite difficult to change it,
> that's why I've opted for creating it on domain creation.
OK :-/
> >> This patch creates the mentioned directories for each guest
launched
> >> from libxl.
> > 
> > I don't really like leaking toolstack "internals" into
the normal guest
> > namespace. Can we add an option to specify "this is a toolstack
domain"
> > and key off that?
> 
> OK, something like driver_domain=1|0
Something like that, yes.
> > I also wonder if this shouldn't be /libxl/<domid> at the top
level.
> 
> I don't have a strong opinion on this, but I generally prefer to keep
> all the xenstore related keys for a domain inside /libxl/<domid>.
Are you agreeing with me or did you mean /local/(domain/)<domid> ?



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

On Thu, 2013-09-26 at 11:30 +0200, Roger Pau Monné
wrote:
> On 26/09/13 11:24, Ian Campbell wrote:
> > On Thu, 2013-09-26 at 11:20 +0200, Roger Pau Monné wrote:
> >> On 25/09/13 17:18, Ian Campbell wrote:
> >>> On Wed, 2013-09-25 at 17:10 +0200, Roger Pau Monné wrote:
> >>>> On 25/09/13 17:00, Ian Campbell wrote:
> >>>>> On Wed, 2013-09-25 at 16:02 +0200, Roger Pau Monné
wrote:
> >>>>>> On 25/09/13 15:57, Ian Campbell wrote:
> >>>>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau
Monne wrote:
> >>>>>>>> The backend path should be fully owned by
the domain where it resides.
> >>>>>>>
> >>>>>>> I can see why
/local/domain/<domid>backends/<type>/<id> would need to be
> >>>>>>> owned by the backend dom, but why
> >>>>>>> do
/local/domain/<domid>backends/<type>/,
/local/domain/<domid>backends/, etc need to be?
> >>>>>>
> >>>>>> The path
/local/domain/<domid>backends/<type>/<guest_domid>/<id>
is
> >>>>>> already owned by the driver domain, the problem is
that if the driver
> >>>>>> domain has to perform the clean up of this path it
should be able to
> >>>>>> fully remove it, otherwise we are leaving empty
directories around in
> >>>>>> xenstore (backend/<type>/<guest_domid>
and so).
> >>>>>>
> >>>>>> And performing the clean up from the toolstack
domain is not that easy,
> >>>>>> we will have to add a way to signal the toolstack
domain that the driver
> >>>>>> domain has finished the disconnection and the
directory can be cleaned.
> >>>>>
> >>>>> Don't we need that anyway so that the toolstack
domain can synchronise
> >>>>> the end of the asynchronous op which is the remove?
> >>>>
> >>>> Not really IMHO, the toolstack waits for the device
frontend to
> >>>> disconnect and then it removes the frontend xenstore
entries. Then (or
> >>>> in parallel) the domain that's handling the backend
should deal with the
> >>>> disconnection of the backend and perform any necessary
actions and clean
> >>>> up the backend xenstore entries.
> >>>
> >>> So e.g. xl destroy can potentially return while the backend is
still
> >>> closing down? That would mean it might fail to reboot the
guest, because
> >>> the block device in the driver domain is still busy.
> >>
> >> True, since we don't synchronize with the toolstack
there's a small
> >> window were a new domain could be created before the driver domain
> >> hasn't finished unplugging. This is a problem already with
current udev
> >> implementation, but I think we can solve this by adding a watch in
the
> >> toolstack that waits for the backend path to be destroyed (when
the
> >> backend is on a driver domain, of course).
> >>
> >> I will expand this patch to include the above mentioned check.
> > 
> > Please do.
> > 
> > Can you also enumerate the intended permissions for each bit
> > of
/local/domain/<domid>/backends/<type>/<guest_domid>/<id>
> > 
> > I think you are changing the perms of "backends" onwards,
but is that
> > really necessary? The backend should be cleaning up <id>, which
needs
> > write perms for <guest_domid> but could the toolstack not do the
rest?
> 
> Yes, since I'm going to change the patch to syncronize with the
> toolstack domain, it can do the rest of the cleaning on behalf of the
> guest (the guest will only remove
>
/local/domain/<domid>/backends/<type>/<guest_domid>/<id>
and there won't
> be a need to change any permissions).
Excellent!


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

On 26/09/13 11:39, Ian Campbell wrote:
> On Thu, 2013-09-26 at 11:27 +0200, Roger Pau Monné wrote:
>> On 25/09/13 15:48, Ian Campbell wrote:
>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>>>> If libxl is executed inside a guest domain it needs write
access to
>>>> the local libxl xenstore dir (/local/<domid>/libxl) to
store internal
>>>> data.
>>>
>>> These sorts of changes need a patch against
docs/misc/xenstore-paths.txt
>>> too.
>>
>> Ack.
>>
>>>
>>>>  This also applies to Qemu which needs a
>>>> /local/<domid>/device-model xenstore directory.
>>>
>>> Is this a new requirement or a separate preexisting issue? How have
we
>>> lived without it?
>>
>> This entry is so far only created on Dom0 (because it's the only
domain
>> that launches Qemu, and is created by Qemu if needed), but when running
>> on a guest, the guest doesn't have permissions to write on
>> /local/<domid>/, so we have to create the directory in the
toolstack
>> domain and assign write permissions for the guest.
>>
>> This path is hardcoded in Qemu, so it's quite difficult to change
it,
>> that's why I've opted for creating it on domain creation.
> 
> OK :-/
> 
>>>> This patch creates the mentioned directories for each guest
launched
>>>> from libxl.
>>>
>>> I don't really like leaking toolstack "internals"
into the normal guest
>>> namespace. Can we add an option to specify "this is a
toolstack domain"
>>> and key off that?
>>
>> OK, something like driver_domain=1|0
> 
> Something like that, yes.
> 
>>> I also wonder if this shouldn't be /libxl/<domid> at the
top level.
>>
>> I don't have a strong opinion on this, but I generally prefer to
keep
>> all the xenstore related keys for a domain inside /libxl/<domid>.
> 
> Are you agreeing with me or did you mean /local/(domain/)<domid> ?
I would prefer to place it in /local/domain/<domid>/libxl/ because I get
the feeling it's more contained, but frankly I don't have a strong
opinion on this, if you prefer to use /libxl/<domid> I can go for it.

However keep in mind that we will have to create the
/local/domain/<domid>/libxl dir anyway, because we have to write the
hotplug disable key there (/local/domain/<domid>/libxl/disable_udev).


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

On Thu, 2013-09-26 at 11:57 +0200, Roger Pau Monné
wrote:
> On 26/09/13 11:39, Ian Campbell wrote:
> > On Thu, 2013-09-26 at 11:27 +0200, Roger Pau Monné wrote:
> >> On 25/09/13 15:48, Ian Campbell wrote:
> >>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
> >>>> If libxl is executed inside a guest domain it needs write
access to
> >>>> the local libxl xenstore dir (/local/<domid>/libxl)
to store internal
> >>>> data.
> >>>
> >>> These sorts of changes need a patch against
docs/misc/xenstore-paths.txt
> >>> too.
> >>
> >> Ack.
> >>
> >>>
> >>>>  This also applies to Qemu which needs a
> >>>> /local/<domid>/device-model xenstore directory.
> >>>
> >>> Is this a new requirement or a separate preexisting issue? How
have we
> >>> lived without it?
> >>
> >> This entry is so far only created on Dom0 (because it's the
only domain
> >> that launches Qemu, and is created by Qemu if needed), but when
running
> >> on a guest, the guest doesn't have permissions to write on
> >> /local/<domid>/, so we have to create the directory in the
toolstack
> >> domain and assign write permissions for the guest.
> >>
> >> This path is hardcoded in Qemu, so it's quite difficult to
change it,
> >> that's why I've opted for creating it on domain creation.
> > 
> > OK :-/
> > 
> >>>> This patch creates the mentioned directories for each
guest launched
> >>>> from libxl.
> >>>
> >>> I don't really like leaking toolstack
"internals" into the normal guest
> >>> namespace. Can we add an option to specify "this is a
toolstack domain"
> >>> and key off that?
> >>
> >> OK, something like driver_domain=1|0
> > 
> > Something like that, yes.
> > 
> >>> I also wonder if this shouldn't be /libxl/<domid> at
the top level.
> >>
> >> I don't have a strong opinion on this, but I generally prefer
to keep
> >> all the xenstore related keys for a domain inside
/libxl/<domid>.
> > 
> > Are you agreeing with me or did you mean /local/(domain/)<domid>
?
> 
> I would prefer to place it in /local/domain/<domid>/libxl/ because I
get
> the feeling it's more contained, but frankly I don't have a strong
> opinion on this, if you prefer to use /libxl/<domid> I can go for it.
> 
> However keep in mind that we will have to create the
> /local/domain/<domid>/libxl dir anyway, because we have to write the
> hotplug disable key there (/local/domain/<domid>/libxl/disable_udev).
That's a toolstack "internal" key isn't it, which we can move?
Or is it?



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

On 26/09/13 12:15, Ian Campbell wrote:
> On Thu, 2013-09-26 at 11:57 +0200, Roger Pau Monné wrote:
>> On 26/09/13 11:39, Ian Campbell wrote:
>>> On Thu, 2013-09-26 at 11:27 +0200, Roger Pau Monné wrote:
>>>> On 25/09/13 15:48, Ian Campbell wrote:
>>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne wrote:
>>>>>> If libxl is executed inside a guest domain it needs
write access to
>>>>>> the local libxl xenstore dir
(/local/<domid>/libxl) to store internal
>>>>>> data.
>>>>>
>>>>> These sorts of changes need a patch against
docs/misc/xenstore-paths.txt
>>>>> too.
>>>>
>>>> Ack.
>>>>
>>>>>
>>>>>>  This also applies to Qemu which needs a
>>>>>> /local/<domid>/device-model xenstore directory.
>>>>>
>>>>> Is this a new requirement or a separate preexisting issue?
How have we
>>>>> lived without it?
>>>>
>>>> This entry is so far only created on Dom0 (because it's the
only domain
>>>> that launches Qemu, and is created by Qemu if needed), but when
running
>>>> on a guest, the guest doesn't have permissions to write on
>>>> /local/<domid>/, so we have to create the directory in
the toolstack
>>>> domain and assign write permissions for the guest.
>>>>
>>>> This path is hardcoded in Qemu, so it's quite difficult to
change it,
>>>> that's why I've opted for creating it on domain
creation.
>>>
>>> OK :-/
>>>
>>>>>> This patch creates the mentioned directories for each
guest launched
>>>>>> from libxl.
>>>>>
>>>>> I don't really like leaking toolstack
"internals" into the normal guest
>>>>> namespace. Can we add an option to specify "this is a
toolstack domain"
>>>>> and key off that?
>>>>
>>>> OK, something like driver_domain=1|0
>>>
>>> Something like that, yes.
>>>
>>>>> I also wonder if this shouldn't be /libxl/<domid>
at the top level.
>>>>
>>>> I don't have a strong opinion on this, but I generally
prefer to keep
>>>> all the xenstore related keys for a domain inside
/libxl/<domid>.
>>>
>>> Are you agreeing with me or did you mean
/local/(domain/)<domid> ?
>>
>> I would prefer to place it in /local/domain/<domid>/libxl/
because I get
>> the feeling it's more contained, but frankly I don't have a
strong
>> opinion on this, if you prefer to use /libxl/<domid> I can go for
it.
>>
>> However keep in mind that we will have to create the
>> /local/domain/<domid>/libxl dir anyway, because we have to write
the
>> hotplug disable key there
(/local/domain/<domid>/libxl/disable_udev).
> 
> That's a toolstack "internal" key isn't it, which we can
move? Or is it?
Yes, we can move it with some modifications to the hotplug scripts, but
we will still need to create a directory like /libxl/<domid>/, or
something similar where the guest has write permissions, but I don't see
how this is any better than /local/domain/<domid>/libxl/.


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

On Thu, 2013-09-26 at 12:20 +0200, Roger Pau Monné
wrote:
> On 26/09/13 12:15, Ian Campbell wrote:
> > On Thu, 2013-09-26 at 11:57 +0200, Roger Pau Monné wrote:
> >> On 26/09/13 11:39, Ian Campbell wrote:
> >>> On Thu, 2013-09-26 at 11:27 +0200, Roger Pau Monné wrote:
> >>>> On 25/09/13 15:48, Ian Campbell wrote:
> >>>>> On Mon, 2013-09-23 at 12:30 +0200, Roger Pau Monne
wrote:
> >>>>>> If libxl is executed inside a guest domain it
needs write access to
> >>>>>> the local libxl xenstore dir
(/local/<domid>/libxl) to store internal
> >>>>>> data.
> >>>>>
> >>>>> These sorts of changes need a patch against
docs/misc/xenstore-paths.txt
> >>>>> too.
> >>>>
> >>>> Ack.
> >>>>
> >>>>>
> >>>>>>  This also applies to Qemu which needs a
> >>>>>> /local/<domid>/device-model xenstore
directory.
> >>>>>
> >>>>> Is this a new requirement or a separate preexisting
issue? How have we
> >>>>> lived without it?
> >>>>
> >>>> This entry is so far only created on Dom0 (because
it's the only domain
> >>>> that launches Qemu, and is created by Qemu if needed), but
when running
> >>>> on a guest, the guest doesn't have permissions to
write on
> >>>> /local/<domid>/, so we have to create the directory
in the toolstack
> >>>> domain and assign write permissions for the guest.
> >>>>
> >>>> This path is hardcoded in Qemu, so it's quite
difficult to change it,
> >>>> that's why I've opted for creating it on domain
creation.
> >>>
> >>> OK :-/
> >>>
> >>>>>> This patch creates the mentioned directories for
each guest launched
> >>>>>> from libxl.
> >>>>>
> >>>>> I don't really like leaking toolstack
"internals" into the normal guest
> >>>>> namespace. Can we add an option to specify "this
is a toolstack domain"
> >>>>> and key off that?
> >>>>
> >>>> OK, something like driver_domain=1|0
> >>>
> >>> Something like that, yes.
> >>>
> >>>>> I also wonder if this shouldn't be
/libxl/<domid> at the top level.
> >>>>
> >>>> I don't have a strong opinion on this, but I generally
prefer to keep
> >>>> all the xenstore related keys for a domain inside
/libxl/<domid>.
> >>>
> >>> Are you agreeing with me or did you mean
/local/(domain/)<domid> ?
> >>
> >> I would prefer to place it in /local/domain/<domid>/libxl/
because I get
> >> the feeling it's more contained, but frankly I don't have
a strong
> >> opinion on this, if you prefer to use /libxl/<domid> I can
go for it.
> >>
> >> However keep in mind that we will have to create the
> >> /local/domain/<domid>/libxl dir anyway, because we have to
write the
> >> hotplug disable key there
(/local/domain/<domid>/libxl/disable_udev).
> > 
> > That's a toolstack "internal" key isn't it, which we
can move? Or is it?
> 
> Yes, we can move it with some modifications to the hotplug scripts, but
> we will still need to create a directory like /libxl/<domid>/, or
> something similar where the guest has write permissions, but I don't
see
> how this is any better than /local/domain/<domid>/libxl/.
I suppose as long as the path is clearly documented as for being for
code implementing dissagregated toolstack functionality (driver domains)
only then there's not much distinction. I thought the /libxl path was a
bit more "self enforcing/documenting" is all by being outside the
regular path.

Ian.


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