* [PATCH 0/5] libxl: call hotplug scripts from libxl
@ 2012-04-16 15:06 Roger Pau Monne
2012-04-16 15:06 ` [PATCH 1/5] libxl: allow libxl__exec to take a parameter containing the env variables Roger Pau Monne
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel
This series removes the use of udev rules to call hotplug scripts when using
libxl. Scripts are directly called from the toolstack at the necessary points,
making use of the new event library and it's fork support.
[PATCH 1/5] libxl: allow libxl__exec to take a parameter containing
Small change to libxl__exec, so we can pass an array of env variables.
[PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
Much of the meat is in this patch and the following one. Most significant
changes in this patch, (apart from the introduction of the hotplug functions),
is the addition of a new xenstore entry in each backend directory, that tells
the hotplug script wheter it should be executed from udev or form the toolstack.
[PATCH 3/5] libxl: call hotplug scripts from libxl for vif
Another xenstore backend entry is added on this patch for each nic device,
called "type", it stores the type of the interface (currently only VIF or
IOEMU), so that the plug and unplug operations know with what they are dealing.
[PATCH 4/5] libxl: add "downscript=no" to Qemu call
[PATCH 5/5] libxl: clean xenstore console directories recursively on
Two minor bugfixes that I've found while working on this hotplug series.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/5] libxl: allow libxl__exec to take a parameter containing the env variables
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
@ 2012-04-16 15:06 ` Roger Pau Monne
2012-04-16 15:06 ` [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd Roger Pau Monne
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Add another parameter to libxl__exec call that contains the
environment variables to use when performing the execvp call.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_bootloader.c | 4 ++--
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_exec.c | 7 ++++++-
tools/libxl/libxl_internal.h | 13 ++++++++++++-
5 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 59992b6..16ebef3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1253,7 +1253,7 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass)
args[2] = "-autopass";
}
- libxl__exec(autopass_fd, -1, -1, args[0], args);
+ libxl__exec(autopass_fd, -1, -1, args[0], args, NULL);
abort();
x_fail:
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 2774062..7120dad 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -113,12 +113,12 @@ static int open_xenconsoled_pty(int *master, int *slave, char *slave_path, size_
static pid_t fork_exec_bootloader(int *master, const char *arg0, char **args)
{
struct termios termattr;
+ char *env[] = {"TERM", "vt100", NULL};
pid_t pid = forkpty(master, NULL, NULL, NULL);
if (pid == -1)
return -1;
else if (pid == 0) {
- setenv("TERM", "vt100", 1);
- libxl__exec(-1, -1, -1, arg0, args);
+ libxl__exec(-1, -1, -1, arg0, args, env);
return -1;
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1261499..30908d1 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -991,7 +991,7 @@ retry_transaction:
goto out_close;
if (!rc) { /* inner child */
setsid();
- libxl__exec(null, logfile_w, logfile_w, dm, args);
+ libxl__exec(null, logfile_w, logfile_w, dm, args, NULL);
}
rc = 0;
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index b10e79f..be9e407 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -72,7 +72,7 @@ static void check_open_fds(const char *what)
}
void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
- char **args)
+ char **args, char **env)
/* call this in the child */
{
if (stdinfd != -1)
@@ -95,6 +95,11 @@ void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
/* in case our caller set it to IGN. subprocesses are entitled
* to assume they got DFL. */
+ if (env != NULL) {
+ for (int i = 0; env[i] != NULL && env[i+1] != NULL; i += 2) {
+ setenv(env[i], env[i+1], 1);
+ }
+ }
execvp(arg0, args);
fprintf(stderr, "libxl: cannot execute %s: %s\n", arg0, strerror(errno));
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d486af2..2181774 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -949,8 +949,19 @@ _hidden int libxl__spawn_check(libxl__gc *gc,
/* low-level stuff, for synchronous subprocesses etc. */
+/*
+ * env should be passed using the following format,
+ *
+ * env[0]: name of env variable
+ * env[1]: value of env variable
+ *
+ * So it efectively becomes something like:
+ * export env[0]=env[1]
+ *
+ * The last entry of the array always has to be NULL.
+ */
_hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
- const char *arg0, char **args); // logs errors, never returns
+ const char *arg0, char **args, char **env); // logs errors, never returns
/* from xl_create */
_hidden int libxl__domain_make(libxl__gc *gc,
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
2012-04-16 15:06 ` [PATCH 1/5] libxl: allow libxl__exec to take a parameter containing the env variables Roger Pau Monne
@ 2012-04-16 15:06 ` Roger Pau Monne
2012-04-16 16:44 ` Ian Campbell
2012-04-16 15:06 ` [PATCH 3/5] libxl: call hotplug scripts from libxl for vif Roger Pau Monne
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
This is a rather big change, that adds the necessary machinery to perform
hotplug script calling from libxl for Linux. This patch launches the necessary
scripts to attach and detach PHY and TAP backend types (Qemu doesn't use hotplug
scripts).
Here's a list of the major changes introduced:
* libxl_device_disk_add makes use of the new event library and takes the
optional parameter libxl_asyncop_how, to choose how the operation has to be
issued (sync or async).
* libxl_device_disk_add waits for backend to switch to state 2 (XenbusInitWait)
and then launches the hotplug script.
* libxl__devices_destroy no longer calls libxl__device_destroy, and instead
calls libxl__initiate_device_remove, so we can disconnect the device and
execute the necessary hotplug scripts instead of just deleting the backend
entries. So libxl__devices_destroy now uses the event library, and so does
libxl_domain_destroy.
* Since libxl__devices_destroy calls multiple times
libxl__initiate_device_remove, this function now returns a different value
regarding the actions taken (if an event was added or not). The user has to
call libxl__ao_complete after using this function if necessary.
* The internal API for hotplug scripts has been added, which consists of one
function; libxl__device_hotplug that takes the device and the action to
execute.
* Linux hotplug scripts are called by setting the necessary env variables to
emulate udev rules, so there's no need to change them (although a rework to
pass this as parameters insted of env variables would be suitable, so both
NetBSD and Linux hotplug scripts take the same parameters).
* Added a check in xen-hotplug-common.sh, so scripts are only executed from
udev when using xend. xl adds an execute_udev=n to xenstore device backend
and passes XL_CALL=y to the env of the script call, and udev calls check that
execute_udev is not set and XL_CALL is empty also.
I've also modified the python binding for pyxl_domain_destroy, that now take the
ao_how parameter, but I'm not really sure if it's done correctly, let's just say
that it compiles.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
tools/libxl/Makefile | 3 +-
tools/libxl/libxl.c | 104 ++++++++++++++++++----
tools/libxl/libxl.h | 7 +-
tools/libxl/libxl_create.c | 4 +-
tools/libxl/libxl_device.c | 140 +++++++++++++++++++++++++++--
tools/libxl/libxl_dm.c | 4 +-
tools/libxl/libxl_hotplug.c | 67 ++++++++++++++
tools/libxl/libxl_internal.h | 43 +++++++++-
tools/libxl/libxl_linux.c | 117 ++++++++++++++++++++++++
tools/libxl/xl_cmdimpl.c | 14 ++--
tools/python/xen/lowlevel/xl/xl.c | 5 +-
12 files changed, 474 insertions(+), 40 deletions(-)
create mode 100644 tools/libxl/libxl_hotplug.c
diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
index 8f6557d..dc3e867 100644
--- a/tools/hotplug/Linux/xen-hotplug-common.sh
+++ b/tools/hotplug/Linux/xen-hotplug-common.sh
@@ -15,6 +15,12 @@
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
#
+# Hack to prevent the execution of hotplug scripts form udev if the domain
+# has ben launched from libxl
+execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
+if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
+ exit 0
+fi
dir=$(dirname "$0")
. "$dir/hotplugpath.sh"
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index b30d11f..ac8b810 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -53,7 +53,8 @@ LIBXL_LIBS += -lyajl
LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
libxl_internal.o libxl_utils.o libxl_uuid.o libxl_json.o \
- libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+ libxl_qmp.o libxl_event.o libxl_fork.o libxl_hotplug.o \
+ $(LIBXL_OBJS-y)
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 16ebef3..fb4fbf2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1062,13 +1062,15 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
GC_FREE;
}
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
+ AO_CREATE(ctx, domid, ao_how);
char *dom_path;
char *vm_path;
char *pid;
int rc, dm_present;
+ int rc_ao = 0;
rc = libxl_domain_info(ctx, NULL, domid);
switch(rc) {
@@ -1110,7 +1112,8 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
libxl__qmp_cleanup(gc, domid);
}
- if (libxl__devices_destroy(gc, domid) < 0)
+ rc_ao = libxl__devices_destroy(egc, ao, domid);
+ if (rc_ao < 0)
LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
"libxl__devices_destroy failed for %d", domid);
@@ -1133,9 +1136,10 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
goto out;
}
rc = 0;
+
out:
- GC_FREE;
- return rc;
+ if (rc_ao) return AO_ABORT(rc_ao);
+ return AO_INPROGRESS;
}
int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type)
@@ -1313,9 +1317,11 @@ static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_disk *disk,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
+ AO_CREATE(ctx, domid, ao_how);
flexarray_t *front;
flexarray_t *back;
char *dev;
@@ -1330,7 +1336,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
rc = ERROR_NOMEM;
goto out;
}
- back = flexarray_make(16, 1);
+ back = flexarray_make(20, 1);
if (!back) {
rc = ERROR_NOMEM;
goto out_free;
@@ -1361,6 +1367,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
flexarray_append(back, "params");
flexarray_append(back, dev);
+ flexarray_append(back, "script");
+ flexarray_append(back, libxl__sprintf(gc, "%s/%s",
+ libxl_xen_script_dir_path(),
+ "block"));
+
assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
break;
case LIBXL_DISK_BACKEND_TAP:
@@ -1374,6 +1385,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
libxl__device_disk_string_of_format(disk->format),
disk->pdev_path));
+ flexarray_append(back, "script");
+ flexarray_append(back, libxl__sprintf(gc, "%s/%s",
+ libxl_xen_script_dir_path(),
+ "blktap"));
+
/* now create a phy device to export the device to the guest */
goto do_backend_phy;
case LIBXL_DISK_BACKEND_QDISK:
@@ -1406,6 +1422,9 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
flexarray_append(back, disk->readwrite ? "w" : "r");
flexarray_append(back, "device-type");
flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+ /* compatibility addon to keep udev rules */
+ flexarray_append(back, "execute_udev");
+ flexarray_append(back, "n");
flexarray_append(front, "backend-id");
flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
@@ -1420,14 +1439,23 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count));
+ if (disk->backend == LIBXL_DISK_BACKEND_PHY ||
+ disk->backend == LIBXL_DISK_BACKEND_TAP) {
+ rc = libxl__initiate_device_add(egc, ao, &device);
+ if (rc) goto out_free;
+ } else {
+ /* no need to execute hotplug scripts */
+ libxl__ao_complete(egc, ao, 0);
+ }
+
rc = 0;
out_free:
flexarray_free(back);
flexarray_free(front);
out:
- GC_FREE;
- return rc;
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
@@ -1442,7 +1470,18 @@ int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
if (rc != 0) goto out;
rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ switch(rc) {
+ case 1:
+ /* event added */
+ break;
+ case 0:
+ /* no event added */
+ libxl__ao_complete(egc, ao, 0);
+ break;
+ default:
+ /* error */
+ goto out;
+ }
return AO_INPROGRESS;
@@ -1666,11 +1705,11 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
ret = 0;
libxl_device_disk_remove(ctx, domid, disks + i, 0);
- libxl_device_disk_add(ctx, domid, disk);
+ libxl_device_disk_add(ctx, domid, disk, 0);
stubdomid = libxl_get_stubdom_id(ctx, domid);
if (stubdomid) {
libxl_device_disk_remove(ctx, stubdomid, disks + i, 0);
- libxl_device_disk_add(ctx, stubdomid, disk);
+ libxl_device_disk_add(ctx, stubdomid, disk, 0);
}
out:
for (i = 0; i < num; i++)
@@ -1909,7 +1948,18 @@ int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
if (rc != 0) goto out;
rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ switch(rc) {
+ case 1:
+ /* event added */
+ break;
+ case 0:
+ /* no event added */
+ libxl__ao_complete(egc, ao, 0);
+ break;
+ default:
+ /* error */
+ goto out;
+ }
return AO_INPROGRESS;
@@ -2256,7 +2306,18 @@ int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
if (rc != 0) goto out;
rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ switch(rc) {
+ case 1:
+ /* event added */
+ break;
+ case 0:
+ /* no event added */
+ libxl__ao_complete(egc, ao, 0);
+ break;
+ default:
+ /* error */
+ goto out;
+ }
return AO_INPROGRESS;
@@ -2389,7 +2450,18 @@ int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
if (rc != 0) goto out;
rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ switch(rc) {
+ case 1:
+ /* event added */
+ break;
+ case 0:
+ /* no event added */
+ libxl__ao_complete(egc, ao, 0);
+ break;
+ default:
+ /* error */
+ goto out;
+ }
return AO_INPROGRESS;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 50bae2f..b7347be 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -394,7 +394,8 @@ int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
+ const libxl_asyncop_how *ao_how);
int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
/* get max. number of cpus supported by hypervisor */
@@ -523,7 +524,9 @@ void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
*/
/* Disks */
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_disk *disk,
+ const libxl_asyncop_how *ao_how);
int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3675afe..de598ad 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -598,7 +598,7 @@ static int do_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
store_libxl_entry(gc, domid, &d_config->b_info);
for (i = 0; i < d_config->num_disks; i++) {
- ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i]);
+ ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i], 0);
if (ret) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
"cannot add disk %d to domain: %d", i, ret);
@@ -721,7 +721,7 @@ static int do_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
error_out:
if (domid)
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
return ret;
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c7e057d..eb94bd2 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -356,26 +356,95 @@ int libxl__device_disk_dev_number(const char *virtpath, int *pdisk,
return -1;
}
+static int libxl__xs_path_cleanup(libxl__gc *gc, char *path)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ unsigned int nb = 0;
+ char *last;
+
+ if (!path)
+ return 0;
+
+ xs_rm(ctx->xsh, XBT_NULL, path);
+
+ for (last = strrchr(path, '/'); last != NULL; last = strrchr(path, '/')) {
+ *last = '\0';
+ if (!libxl__xs_directory(gc, XBT_NULL, path, &nb))
+ continue;
+ if (nb == 0) {
+ xs_rm(ctx->xsh, XBT_NULL, path);
+ }
+ }
+ return 0;
+}
typedef struct {
libxl__ao *ao;
libxl__ev_devstate ds;
+ libxl__device *dev;
} libxl__ao_device_remove;
+typedef struct {
+ libxl__ao *ao;
+ libxl__ev_devstate ds;
+ libxl__device *dev;
+} libxl__ao_device_add;
+
static void device_remove_cleanup(libxl__gc *gc,
libxl__ao_device_remove *aorm) {
if (!aorm) return;
libxl__ev_devstate_cancel(gc, &aorm->ds);
}
+static void device_add_cleanup(libxl__gc *gc,
+ libxl__ao_device_add *aorm) {
+ if (!aorm) return;
+ libxl__ev_devstate_cancel(gc, &aorm->ds);
+}
+
static void device_remove_callback(libxl__egc *egc, libxl__ev_devstate *ds,
int rc) {
libxl__ao_device_remove *aorm = CONTAINER_OF(ds, *aorm, ds);
libxl__gc *gc = &aorm->ao->gc;
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ rc = libxl__device_hotplug(gc, aorm->dev, DISCONNECT);
+ if (rc)
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute hotplug script for"
+ " device %"PRIu32, aorm->dev->devid);
+ libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc, aorm->dev));
+ libxl__xs_path_cleanup(gc, libxl__device_backend_path(gc, aorm->dev));
libxl__ao_complete(egc, aorm->ao, rc);
device_remove_cleanup(gc, aorm);
}
+static void device_add_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+ int rc)
+{
+ libxl__ao_device_add *aorm = CONTAINER_OF(ds, *aorm, ds);
+ libxl__gc *gc = &aorm->ao->gc;
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ rc = libxl__device_hotplug(gc, aorm->dev, CONNECT);
+ if (rc)
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute hotplug script for"
+ " device %"PRIu32, aorm->dev->devid);
+ libxl__ao_complete(egc, aorm->ao, rc);
+ if (aorm)
+ libxl__ev_devstate_cancel(gc, &aorm->ds);
+ return;
+}
+
+/*
+ * Return codes:
+ * 1 Success and event(s) HAVE BEEN added
+ * 0 Success and NO events where added
+ * < 0 Error
+ *
+ * Since this function can be called several times, and several events
+ * can be added to the same context, the user has to call
+ * libxl__ao_complete when necessary (if no events are added).
+ */
int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
libxl__device *dev)
{
@@ -392,7 +461,6 @@ int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
goto out_ok;
if (atoi(state) != 4) {
libxl__device_destroy_tapdisk(gc, be_path);
- xs_rm(ctx->xsh, XBT_NULL, be_path);
goto out_ok;
}
@@ -413,6 +481,7 @@ retry_transaction:
aorm = libxl__zalloc(gc, sizeof(*aorm));
aorm->ao = ao;
+ aorm->dev = dev;
libxl__ev_devstate_init(&aorm->ds);
rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_remove_callback,
@@ -420,7 +489,7 @@ retry_transaction:
LIBXL_DESTROY_TIMEOUT * 1000);
if (rc) goto out_fail;
- return 0;
+ return 1;
out_fail:
assert(rc);
@@ -428,8 +497,51 @@ retry_transaction:
return rc;
out_ok:
- libxl__ao_complete(egc, ao, 0);
+ rc = libxl__device_hotplug(gc, dev, DISCONNECT);
+ libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc, dev));
+ libxl__xs_path_cleanup(gc, be_path);
+ return 0;
+}
+
+int libxl__initiate_device_add(libxl__egc *egc, libxl__ao *ao,
+ libxl__device *dev)
+{
+ AO_GC;
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+ char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+ int rc = 0;
+ libxl__ao_device_add *aorm = 0;
+
+ if (atoi(state) == XenbusStateInitWait)
+ goto out_ok;
+
+ aorm = libxl__zalloc(gc, sizeof(*aorm));
+ aorm->ao = ao;
+ aorm->dev = dev;
+ libxl__ev_devstate_init(&aorm->ds);
+
+ rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_add_callback,
+ state_path, XenbusStateInitWait,
+ LIBXL_INIT_TIMEOUT * 1000);
+ if (rc) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to initialize device %"
+ PRIu32, dev->devid);
+ libxl__ev_devstate_cancel(gc, &aorm->ds);
+ goto out_fail;
+ }
+
return 0;
+
+out_fail:
+ assert(rc);
+ device_add_cleanup(gc, aorm);
+ return rc;
+out_ok:
+ rc = libxl__device_hotplug(gc, dev, CONNECT);
+ libxl__ao_complete(egc, ao, 0);
+ return rc;
}
int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
@@ -446,13 +558,14 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
return 0;
}
-int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
+int libxl__devices_destroy(libxl__egc *egc, libxl__ao *ao, uint32_t domid)
{
+ AO_GC;
libxl_ctx *ctx = libxl__gc_owner(gc);
char *path;
unsigned int num_kinds, num_devs;
char **kinds = NULL, **devs = NULL;
- int i, j;
+ int i, j, events = 0, rc;
libxl__device dev;
libxl__device_kind kind;
@@ -482,11 +595,24 @@ int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
dev.domid = domid;
dev.kind = kind;
dev.devid = atoi(devs[j]);
-
- libxl__device_destroy(gc, &dev);
+ rc = libxl__initiate_device_remove(egc, ao, &dev);
+ switch(rc) {
+ case 1:
+ events++;
+ break;
+ case 0:
+ /* no events added */
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Could not remove device "
+ "%s", path);
+ }
}
}
}
+ /* Check if any events where added */
+ if (!events)
+ libxl__ao_complete(egc, ao, 0);
/* console 0 frontend directory is not under /local/domain/<domid>/device */
path = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 30908d1..2d51a7f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -790,7 +790,7 @@ retry_transaction:
goto retry_transaction;
for (i = 0; i < dm_config.num_disks; i++) {
- ret = libxl_device_disk_add(ctx, dm_domid, &dm_config.disks[i]);
+ ret = libxl_device_disk_add(ctx, dm_domid, &dm_config.disks[i], 0);
if (ret)
goto out_free;
}
@@ -1044,7 +1044,7 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
goto out;
}
LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom, domid=%d", stubdomid);
- ret = libxl_domain_destroy(ctx, stubdomid);
+ ret = libxl_domain_destroy(ctx, stubdomid, 0);
if (ret)
goto out;
diff --git a/tools/libxl/libxl_hotplug.c b/tools/libxl/libxl_hotplug.c
new file mode 100644
index 0000000..2eace0c
--- /dev/null
+++ b/tools/libxl/libxl_hotplug.c
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2012
+ * Author Roger Pau Monne <roger.pau@citrix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+/*
+ * Generic hotplug helpers
+ *
+ * This helpers are both used by Linux and NetBSD hotplug script
+ * dispatcher functions.
+ */
+
+static void hotplug_exited(libxl__egc *egc, libxl__ev_child *child,
+ pid_t pid, int status)
+{
+ libxl__hotplug_state *hs = CONTAINER_OF(child, *hs, child);
+ EGC_GC;
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, hs->rc ? LIBXL__LOG_ERROR
+ : LIBXL__LOG_WARNING,
+ "hotplug child", pid, status);
+ }
+}
+
+int libxl__hotplug_exec(libxl__gc *gc, char *arg0, char **args, char **env)
+{
+ int rc = 0;
+ pid_t pid = -1;
+ libxl__hotplug_state *hs;
+
+ hs = libxl__zalloc(gc, sizeof(*hs));
+
+ pid = libxl__ev_child_fork(gc, &hs->child, hotplug_exited);
+ if (pid == -1) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (!pid) {
+ /* child */
+ signal(SIGCHLD, SIG_DFL);
+ libxl__exec(-1, -1, -1, arg0, args, env);
+ }
+out:
+ if (libxl__ev_child_inuse(&hs->child)) {
+ hs->rc = rc;
+ /* we will get a callback when the child dies */
+ return 0;
+ }
+
+ assert(rc);
+ return rc;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2181774..a39346c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -70,6 +70,7 @@
#include "_libxl_types_internal_json.h"
#define LIBXL_DESTROY_TIMEOUT 10
+#define LIBXL_INIT_TIMEOUT 10
#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
#define LIBXL_XENCONSOLE_LIMIT 1048576
#define LIBXL_XENCONSOLE_PROTOCOL "vt100"
@@ -455,6 +456,37 @@ _hidden bool libxl__xs_mkdir(libxl__gc *gc, xs_transaction_t t,
_hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
+/*
+ * Hotplug handling
+ *
+ * Hotplug scripts are launched automatically by libxl at guest creation &
+ * shutdown/destroy.
+ */
+
+/* Action to pass to hotplug caller functions */
+typedef enum {
+ CONNECT = 1,
+ DISCONNECT = 2
+} libxl__hotplug_action;
+
+typedef struct libxl__hotplug_state libxl__hotplug_state;
+
+struct libxl__hotplug_state {
+ /* public, result, caller may only read in callback */
+ int rc;
+ /* private for implementation */
+ libxl__ev_child child;
+};
+
+/*
+ * libxl__hotplug_exec is an internal function that should not be used
+ * directly, all hotplug script calls have to implement and use
+ * libxl__device_hotplug.
+ */
+_hidden int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
+ libxl__hotplug_action action);
+_hidden int libxl__hotplug_exec(libxl__gc *gc, char *arg0, char **args,
+ char **env);
/*
* Event generation functions provided by the libxl event core to the
@@ -740,7 +772,8 @@ _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
_hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
libxl__device *dev);
_hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__devices_destroy(libxl__egc *egc, libxl__ao *ao,
+ uint32_t domid);
_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
/*
@@ -763,6 +796,14 @@ _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
_hidden int libxl__initiate_device_remove(libxl__egc*, libxl__ao*,
libxl__device *dev);
+/* Arranges that dev will be added to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done, the ao will be completed. An error
+ * return from libxl__initiate_device_add means that the ao
+ * will _not_ be completed and the caller must do so. */
+_hidden int libxl__initiate_device_add(libxl__egc*, libxl__ao*,
+ libxl__device *dev);
+
/*
* libxl__ev_devstate - waits a given time for a device to
* reach a given state. Follows the libxl_ev_* conventions.
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 925248b..9a9e44a 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -25,3 +25,120 @@ int libxl__try_phy_backend(mode_t st_mode)
return 1;
}
+
+/* Hotplug scripts helpers */
+static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *script;
+ flexarray_t *f_env;
+ int nr = 0;
+
+ script = libxl__xs_read(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/%s", be_path, "script"));
+ if (!script) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
+ be_path);
+ return NULL;
+ }
+
+ f_env = flexarray_make(11, 1);
+ if (!f_env) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "unable to create environment array");
+ return NULL;
+ }
+
+ flexarray_set(f_env, nr++, "script");
+ flexarray_set(f_env, nr++, script);
+ flexarray_set(f_env, nr++, "XENBUS_TYPE");
+ flexarray_set(f_env, nr++, (char *)
+ libxl__device_kind_to_string(dev->backend_kind));
+ flexarray_set(f_env, nr++, "XENBUS_PATH");
+ flexarray_set(f_env, nr++,
+ libxl__sprintf(gc, "backend/%s/%u/%d",
+ libxl__device_kind_to_string(dev->backend_kind),
+ dev->domid, dev->devid));
+ flexarray_set(f_env, nr++, "XENBUS_BASE_PATH");
+ flexarray_set(f_env, nr++, "backend");
+ /* compatibility addon to keep udev rules */
+ flexarray_set(f_env, nr++, "XL_CALL");
+ flexarray_set(f_env, nr++, "y");
+
+ flexarray_set(f_env, nr++, NULL);
+
+ return (char **) flexarray_contents(f_env);
+}
+
+/* Hotplug scripts caller functions */
+
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
+ libxl__hotplug_action action)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *what, *script;
+ char **args, **env;
+ int nr = 0, rc = 0;
+ flexarray_t *f_args;
+
+ script = libxl__xs_read(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/%s", be_path, "script"));
+ if (!script) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
+ be_path);
+ return -1;
+ }
+
+ env = get_hotplug_env(gc, dev);
+ if (!env)
+ return -1;
+
+ f_args = flexarray_make(3, 1);
+ if (!f_args) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments array");
+ return -1;
+ }
+
+ flexarray_set(f_args, nr++, script);
+ flexarray_set(f_args, nr++, action == CONNECT ? "add" : "remove");
+ flexarray_set(f_args, nr++, NULL);
+
+ args = (char **) flexarray_contents(f_args);
+ what = libxl__sprintf(gc, "%s %s", args[0],
+ action == CONNECT ? "connect" : "disconnect");
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+ "Calling hotplug script: %s %s",
+ args[0], args[1]);
+ rc = libxl__hotplug_exec(gc, args[0], args, env);
+ if (rc) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable execute hotplug scripts for "
+ "device %"PRIu32, dev->devid);
+ goto out_free;
+ }
+
+ rc = 0;
+
+out_free:
+ free(env);
+ free(args);
+ return rc;
+}
+
+int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
+ libxl__hotplug_action action)
+{
+ int rc = 0;
+
+ switch (dev->backend_kind) {
+ case LIBXL__DEVICE_KIND_VBD:
+ rc = libxl__hotplug_disk(gc, dev, action);
+ break;
+ default:
+ rc = 0;
+ break;
+ }
+
+ return rc;
+}
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 08815a3..01ff363 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1301,7 +1301,7 @@ static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
/* fall-through */
case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
LOG("Domain %d needs to be cleaned up: destroying the domain", domid);
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
break;
case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
@@ -1862,7 +1862,7 @@ start:
error_out:
release_lock();
if (libxl_domid_valid_guest(domid))
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
out:
if (logfile != 2)
@@ -2339,7 +2339,7 @@ static void destroy_domain(const char *p)
fprintf(stderr, "Cannot destroy privileged domain 0.\n\n");
exit(-1);
}
- rc = libxl_domain_destroy(ctx, domid);
+ rc = libxl_domain_destroy(ctx, domid, 0);
if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); }
}
@@ -2613,7 +2613,7 @@ static int save_domain(const char *p, const char *filename, int checkpoint,
if (checkpoint)
libxl_domain_unpause(ctx, domid);
else
- libxl_domain_destroy(ctx, domid);
+ libxl_domain_destroy(ctx, domid, 0);
exit(0);
}
@@ -2846,7 +2846,7 @@ static void migrate_domain(const char *domain_spec, const char *rune,
}
fprintf(stderr, "migration sender: Target reports successful startup.\n");
- libxl_domain_destroy(ctx, domid); /* bang! */
+ libxl_domain_destroy(ctx, domid, 0); /* bang! */
fprintf(stderr, "Migration successful.\n");
exit(0);
@@ -2964,7 +2964,7 @@ static void migrate_receive(int debug, int daemonize, int monitor)
if (rc) {
fprintf(stderr, "migration target: Failure, destroying our copy.\n");
- rc2 = libxl_domain_destroy(ctx, domid);
+ rc2 = libxl_domain_destroy(ctx, domid, 0);
if (rc2) {
fprintf(stderr, "migration target: Failed to destroy our copy"
" (code %d).\n", rc2);
@@ -5011,7 +5011,7 @@ int main_blockattach(int argc, char **argv)
return 0;
}
- if (libxl_device_disk_add(ctx, fe_domid, &disk)) {
+ if (libxl_device_disk_add(ctx, fe_domid, &disk, 0)) {
fprintf(stderr, "libxl_device_disk_add failed.\n");
}
return 0;
diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index bbbf2a9..a41a720 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -444,9 +444,10 @@ static PyObject *pyxl_domain_reboot(XlObject *self, PyObject *args)
static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args)
{
int domid;
- if ( !PyArg_ParseTuple(args, "i", &domid) )
+ const libxl_asyncop_how *ao_how;
+ if ( !PyArg_ParseTuple(args, "ii", &domid, &ao_how) )
return NULL;
- if ( libxl_domain_destroy(self->ctx, domid) ) {
+ if ( libxl_domain_destroy(self->ctx, domid, ao_how) ) {
PyErr_SetString(xl_error_obj, "cannot destroy domain");
return NULL;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] libxl: call hotplug scripts from libxl for vif
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
2012-04-16 15:06 ` [PATCH 1/5] libxl: allow libxl__exec to take a parameter containing the env variables Roger Pau Monne
2012-04-16 15:06 ` [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd Roger Pau Monne
@ 2012-04-16 15:06 ` Roger Pau Monne
2012-04-16 15:06 ` [PATCH 4/5] libxl: add "downscript=no" to Qemu call Roger Pau Monne
2012-04-16 15:06 ` [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy Roger Pau Monne
4 siblings, 0 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
As the previous change already introduces most of needed machinery to call
hotplug scripts from libxl, this only adds the necessary bits to call this
scripts for vif interfaces also.
libxl_device_nic_add has been changed to make use of the new event
functionality, and the necessary vif hotplug code has been added. No changes
where needed in the teardown part, since it uses exactly the same code
introduced for vbd.
An exception has been introduced for tap devices, since hotplug scripts
should be called after the device model has been created, so the hotplug
call for those is done in do_domain_create after confirmation of the device
model startup. On the other hand, tap devices don't use teardown scripts,
so add another exception for that.
libxl__device_from_nic was moved to libxl_device.c, so it can be used
in libxl_create.c, and libxl__device_from_disk was also moved to mantain
the simmetry.
libxl__initiate_device_remove has been changed a little, to nuke the frontend
xenstore path before trying to perform device teardown, this allows for
unitialized devices to be closed gracefully, specially vif interfaces added to
HVM machines and not used.
PV nic devices are set to LIBXL_NIC_TYPE_VIF, since the default value is
LIBXL_NIC_TYPE_IOEMU regardless of the guest type.
A new gobal option, called disable_vif_scripts has been added to allow the user
decide if he wants to execute the hotplug scripts from xl or from udev. This has
been documented in the xl.conf man page.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
docs/man/xl.conf.pod.5 | 7 ++
tools/libxl/libxl.c | 85 ++++++++------------------
tools/libxl/libxl.h | 3 +-
tools/libxl/libxl_create.c | 22 ++++++-
tools/libxl/libxl_device.c | 77 +++++++++++++++++++++--
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_internal.h | 6 ++
tools/libxl/libxl_linux.c | 138 +++++++++++++++++++++++++++++++++++++++++-
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl.c | 4 +
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 15 ++++-
12 files changed, 289 insertions(+), 72 deletions(-)
diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 8bd45ea..cf2c477 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -55,6 +55,13 @@ default.
Default: C<1>
+=item B<disable_vif_scripts=BOOLEAN>
+
+If enabled xl will not execute nic hotplug scripts itself, and instead
+relegate this task to udev.
+
+Default: C<0>
+
=item B<lockfile="PATH">
Sets the path to the lock file used by xl to serialise certain
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index fb4fbf2..44a54cb 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1277,46 +1277,6 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
return rc;
}
-static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
- libxl_device_disk *disk,
- libxl__device *device)
-{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- int devid;
-
- devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
- if (devid==-1) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
- return ERROR_INVAL;
- }
-
- device->backend_domid = disk->backend_domid;
- device->backend_devid = devid;
-
- switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_TAP:
- device->backend_kind = LIBXL__DEVICE_KIND_VBD;
- break;
- case LIBXL_DISK_BACKEND_QDISK:
- device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
- disk->backend);
- return ERROR_INVAL;
- }
-
- device->domid = domid;
- device->devid = devid;
- device->kind = LIBXL__DEVICE_KIND_VBD;
-
- return 0;
-}
-
int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how)
@@ -1828,23 +1788,10 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
return 0;
}
-static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
- libxl_device_nic *nic,
- libxl__device *device)
-{
- device->backend_devid = nic->devid;
- device->backend_domid = nic->backend_domid;
- device->backend_kind = LIBXL__DEVICE_KIND_VIF;
- device->devid = nic->devid;
- device->domid = domid;
- device->kind = LIBXL__DEVICE_KIND_VIF;
-
- return 0;
-}
-
-int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
+int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
+ AO_CREATE(ctx, domid, ao_how);
flexarray_t *front;
flexarray_t *back;
libxl__device device;
@@ -1859,7 +1806,7 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
rc = ERROR_NOMEM;
goto out;
}
- back = flexarray_make(16, 1);
+ back = flexarray_make(18, 1);
if (!back) {
rc = ERROR_NOMEM;
goto out_free;
@@ -1912,6 +1859,13 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
flexarray_append(back, libxl__strdup(gc, nic->bridge));
flexarray_append(back, "handle");
flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
+ flexarray_append(back, "type");
+ flexarray_append(back, libxl__sprintf(gc, "%d", nic->nictype));
+ /* compatibility addon to keep udev rules */
+ if (!nic->disable_vif_script) {
+ flexarray_append(back, "execute_udev");
+ flexarray_append(back, "n");
+ }
flexarray_append(front, "backend-id");
flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
@@ -1926,14 +1880,25 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
libxl__xs_kvs_of_flexarray(gc, back, back->count),
libxl__xs_kvs_of_flexarray(gc, front, front->count));
- /* FIXME: wait for plug */
+ if (nic->nictype == LIBXL_NIC_TYPE_VIF &&
+ !nic->disable_vif_script) {
+ rc = libxl__initiate_device_add(egc, ao, &device);
+ if (rc) goto out_free;
+ } else {
+ /*
+ * Don't execute hotplug scripts for IOEMU interfaces yet,
+ * we need to launch the device model first.
+ */
+ libxl__ao_complete(egc, ao, 0);
+ }
+
rc = 0;
out_free:
flexarray_free(back);
flexarray_free(front);
out:
- GC_FREE;
- return rc;
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index b7347be..6da6107 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -551,7 +551,8 @@ char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
/* Network Interfaces */
-int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
+ const libxl_asyncop_how *ao_how);
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic,
const libxl_asyncop_how *ao_how);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index de598ad..a1f5731 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -607,7 +607,7 @@ static int do_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
}
}
for (i = 0; i < d_config->num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
+ ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i], 0);
if (ret) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
"cannot add nic %d to domain: %d", i, ret);
@@ -685,6 +685,26 @@ static int do_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
"device model did not start: %d", ret);
goto error_out;
}
+ /*
+ * Execute hotplug scripts for tap devices, this has to be done
+ * after the domain model has been started.
+ */
+ for (i = 0; i < d_config->num_vifs; i++) {
+ if (d_config->vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU &&
+ !d_config->vifs[i].disable_vif_script) {
+ libxl__device device;
+ ret = libxl__device_from_nic(gc, domid, &d_config->vifs[i],
+ &device);
+ if (ret < 0) goto error_out;
+ ret = libxl__device_hotplug(gc, &device, CONNECT);
+ if (ret < 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "unable to launch hotplug script for device: "
+ "%"PRIu32, device.devid);
+ goto error_out;
+ }
+ }
+ }
}
for (i = 0; i < d_config->num_pcidevs; i++)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index eb94bd2..d773d71 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -249,6 +249,60 @@ char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
}
}
+int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__device *device)
+{
+ device->backend_devid = nic->devid;
+ device->backend_domid = nic->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_VIF;
+ device->devid = nic->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_VIF;
+
+ return 0;
+}
+
+int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__device *device)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int devid;
+
+ devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
+ if (devid==-1) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+ " virtual disk identifier %s", disk->vdev);
+ return ERROR_INVAL;
+ }
+
+ device->backend_domid = disk->backend_domid;
+ device->backend_devid = devid;
+
+ switch (disk->backend) {
+ case LIBXL_DISK_BACKEND_PHY:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+ break;
+ case LIBXL_DISK_BACKEND_QDISK:
+ device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
+ disk->backend);
+ return ERROR_INVAL;
+ }
+
+ device->domid = domid;
+ device->devid = devid;
+ device->kind = LIBXL__DEVICE_KIND_VBD;
+
+ return 0;
+}
+
int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
{
struct stat buf;
@@ -450,22 +504,29 @@ int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
{
AO_GC;
libxl_ctx *ctx = libxl__gc_owner(gc);
- xs_transaction_t t;
+ xs_transaction_t t = 0;
char *be_path = libxl__device_backend_path(gc, dev);
char *state_path = libxl__sprintf(gc, "%s/state", be_path);
- char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+ char *state;
int rc = 0;
libxl__ao_device_remove *aorm = 0;
+ /*
+ * Nuke frontend to force backend teardown
+ * It's not pretty, but it's the only way that seems to work for all
+ * types of backends
+ */
+ libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc, dev));
+
+retry_transaction:
+ t = xs_transaction_start(ctx->xsh);
+ state = libxl__xs_read(gc, t, state_path);
if (!state)
goto out_ok;
- if (atoi(state) != 4) {
+ if (atoi(state) == XenbusStateClosed)
libxl__device_destroy_tapdisk(gc, be_path);
goto out_ok;
- }
-retry_transaction:
- t = xs_transaction_start(ctx->xsh);
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
if (!xs_transaction_end(ctx->xsh, t, 0)) {
@@ -476,6 +537,8 @@ retry_transaction:
goto out_fail;
}
}
+ /* mark transaction as ended, to prevent double closing it on out_ok */
+ t = 0;
libxl__device_destroy_tapdisk(gc, be_path);
@@ -497,8 +560,8 @@ retry_transaction:
return rc;
out_ok:
+ if (t) xs_transaction_end(ctx->xsh, t, 0);
rc = libxl__device_hotplug(gc, dev, DISCONNECT);
- libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc, dev));
libxl__xs_path_cleanup(gc, be_path);
return 0;
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2d51a7f..ba5bef7 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -795,7 +795,7 @@ retry_transaction:
goto out_free;
}
for (i = 0; i < dm_config.num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, dm_domid, &dm_config.vifs[i]);
+ ret = libxl_device_nic_add(ctx, dm_domid, &dm_config.vifs[i], 0);
if (ret)
goto out_free;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a39346c..3787217 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -756,6 +756,12 @@ _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__device_from_nic(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__device *device);
+_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk,
+ 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,
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 9a9e44a..474265c 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -27,6 +27,25 @@ int libxl__try_phy_backend(mode_t st_mode)
}
/* Hotplug scripts helpers */
+
+static libxl_nic_type get_nic_type(libxl__gc *gc, libxl__device *dev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *snictype, *be_path;
+
+ be_path = libxl__device_backend_path(gc, dev);
+
+ snictype = libxl__xs_read(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/%s", be_path, "type"));
+ if (!snictype) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read nictype from %s",
+ be_path);
+ return -1;
+ }
+
+ return atoi(snictype);
+}
+
static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -43,7 +62,8 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
return NULL;
}
- f_env = flexarray_make(11, 1);
+ f_env = flexarray_make(15, 1);
+
if (!f_env) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
"unable to create environment array");
@@ -62,6 +82,24 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
dev->domid, dev->devid));
flexarray_set(f_env, nr++, "XENBUS_BASE_PATH");
flexarray_set(f_env, nr++, "backend");
+ if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+ switch (get_nic_type(gc, dev)) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ flexarray_set(f_env, nr++, "INTERFACE");
+ flexarray_set(f_env, nr++,
+ libxl__sprintf(gc, "tap%u.%d",
+ dev->domid, dev->devid));
+ case LIBXL_NIC_TYPE_VIF:
+ flexarray_set(f_env, nr++, "vif");
+ flexarray_set(f_env, nr++,
+ libxl__sprintf(gc, "vif%u.%d",
+ dev->domid, dev->devid));
+ break;
+ default:
+ return NULL;
+ }
+ }
+
/* compatibility addon to keep udev rules */
flexarray_set(f_env, nr++, "XL_CALL");
flexarray_set(f_env, nr++, "y");
@@ -126,6 +164,101 @@ out_free:
return rc;
}
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
+ libxl__hotplug_action action)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *what, *script;
+ char **args, **env;
+ int nr = 0, rc = 0;
+ libxl_nic_type nictype;
+ flexarray_t *vif_args, *tap_args;
+
+ script = libxl__xs_read(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/%s", be_path, "script"));
+ if (!script) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to read script from %s",
+ be_path);
+ return -1;
+ }
+
+ nictype = get_nic_type(gc, dev);
+ if (nictype < 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "error when fetching nic type");
+ return -1;
+ }
+
+ env = get_hotplug_env(gc, dev);
+ if (!env)
+ return -1;
+
+ vif_args = flexarray_make(4, 1);
+ if (!vif_args) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments array");
+ return -1;
+ }
+ tap_args = flexarray_make(4, 1);
+ if (!tap_args) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to create arguments array");
+ return -1;
+ }
+
+ switch(nictype) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ flexarray_set(tap_args, nr++, script);
+ flexarray_set(tap_args, nr++, action == CONNECT ? "add" : "remove");
+ flexarray_set(tap_args, nr++, libxl__sprintf(gc, "type_if=tap"));
+ flexarray_set(tap_args, nr++, NULL);
+ args = (char **) flexarray_contents(tap_args);
+ what = libxl__sprintf(gc, "%s %s", args[0],
+ action == CONNECT ? "connect" : "disconnect");
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+ "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_exec(gc, args[0], args, env);
+ if (rc) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable execute hotplug scripts "
+ "for vif device %"PRIu32,
+ dev->devid);
+ goto out;
+ }
+ free(args);
+ nr = 0;
+ case LIBXL_NIC_TYPE_VIF:
+ flexarray_set(vif_args, nr++, script);
+ flexarray_set(vif_args, nr++, action == CONNECT ? "online" : "offline");
+ flexarray_set(vif_args, nr++, libxl__sprintf(gc, "type_if=vif"));
+ flexarray_set(vif_args, nr++, NULL);
+ args = (char **) flexarray_contents(vif_args);
+ what = libxl__sprintf(gc, "%s %s", args[0],
+ action == CONNECT ? "connect" : "disconnect");
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+ "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_exec(gc, args[0], args, env);
+ if (rc) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable execute hotplug scripts "
+ "for vif device %"PRIu32,
+ dev->devid);
+ goto out;
+ }
+ break;
+ default:
+ /* Unknown network type */
+ LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "unknown network card type with "
+ "id %"PRIu32, dev->devid);
+ return 0;
+ }
+
+ rc = 0;
+
+out:
+ free(env);
+ free(args);
+ return rc;
+}
+
int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
libxl__hotplug_action action)
{
@@ -135,6 +268,9 @@ int libxl__device_hotplug(libxl__gc *gc, libxl__device *dev,
case LIBXL__DEVICE_KIND_VBD:
rc = libxl__hotplug_disk(gc, dev, action);
break;
+ case LIBXL__DEVICE_KIND_VIF:
+ rc = libxl__hotplug_nic(gc, dev, action);
+ break;
default:
rc = 0;
break;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 09089b2..ac3e79f 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -343,6 +343,7 @@ libxl_device_nic = Struct("device_nic", [
("ifname", string),
("script", string),
("nictype", libxl_nic_type),
+ ("disable_vif_script", integer),
])
libxl_device_pci = Struct("device_pci", [
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index a6ffd25..2a705b8 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -35,6 +35,7 @@
xentoollog_logger_stdiostream *logger;
int dryrun_only;
int autoballoon = 1;
+int disable_vif_scripts = 0;
char *lockfile;
char *default_vifscript = NULL;
char *default_bridge = NULL;
@@ -66,6 +67,9 @@ static void parse_global_config(const char *configfile,
if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
autoballoon = l;
+ if (!xlu_cfg_get_long (config, "disable_vif_scripts", &l, 0))
+ disable_vif_scripts = l;
+
if (!xlu_cfg_get_string (config, "lockfile", &buf, 0))
lockfile = strdup(buf);
else {
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 7e258d5..13619e7 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -109,6 +109,7 @@ void postfork(void);
/* global options */
extern int autoballoon;
+extern int disable_vif_scripts;
extern int dryrun_only;
extern char *lockfile;
extern char *default_vifscript;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 01ff363..41230a6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -846,6 +846,19 @@ static void parse_config_data(const char *configfile_filename_report,
nic->script = strdup(default_vifscript);
}
+ /* Set default nic type for PV guests correctly */
+ if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
+ nic->nictype = LIBXL_NIC_TYPE_VIF;
+ }
+
+ /*
+ * Disable nic network script calling, to allow the user
+ * to attach the nic backend from a different domain.
+ */
+ if (disable_vif_scripts) {
+ nic->disable_vif_script = 1;
+ }
+
if (default_bridge) {
free(nic->bridge);
nic->bridge = strdup(default_bridge);
@@ -4901,7 +4914,7 @@ int main_networkattach(int argc, char **argv)
return 1;
}
}
- if (libxl_device_nic_add(ctx, domid, &nic)) {
+ if (libxl_device_nic_add(ctx, domid, &nic, 0)) {
fprintf(stderr, "libxl_device_nic_add failed.\n");
return 1;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] libxl: add "downscript=no" to Qemu call
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
` (2 preceding siblings ...)
2012-04-16 15:06 ` [PATCH 3/5] libxl: call hotplug scripts from libxl for vif Roger Pau Monne
@ 2012-04-16 15:06 ` Roger Pau Monne
2012-04-16 16:48 ` Ian Campbell
2012-04-16 15:06 ` [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy Roger Pau Monne
4 siblings, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Currently we only pass script=no to Qemu, to avoid calling any scripts when
attaching a tap interface, but we should also pass downscript=no to avoid Qemu
trying to execute a script when disconnecting the interface. This prevents the
following harmless error message:
/etc/qemu-ifdown: could not launch network script
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_dm.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ba5bef7..8dc588d 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -216,11 +216,14 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
else
ifname = vifs[i].ifname;
flexarray_vappend(dm_args,
- "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
- vifs[i].devid, smac, vifs[i].model),
- "-net", libxl__sprintf(gc, "tap,vlan=%d,ifname=%s,bridge=%s,script=%s",
- vifs[i].devid, ifname, vifs[i].bridge, libxl_tapif_script(gc)),
- NULL);
+ "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
+ vifs[i].devid, smac, vifs[i].model),
+ "-net", libxl__sprintf(gc,
+ "tap,vlan=%d,ifname=%s,bridge=%s,script=%s,"
+ "downscript=%s",
+ vifs[i].devid, ifname, vifs[i].bridge,
+ libxl_tapif_script(gc), libxl_tapif_script(gc)),
+ NULL);
ioemu_vifs++;
}
}
@@ -462,8 +465,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
vifs[i].devid, smac));
flexarray_append(dm_args, "-netdev");
flexarray_append(dm_args,
- libxl__sprintf(gc, "type=tap,id=net%d,ifname=%s,script=%s",
+ libxl__sprintf(gc, "type=tap,id=net%d,ifname=%s,script=%s,"
+ "downscript=%s",
vifs[i].devid, ifname,
+ libxl_tapif_script(gc),
libxl_tapif_script(gc)));
ioemu_vifs++;
}
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
` (3 preceding siblings ...)
2012-04-16 15:06 ` [PATCH 4/5] libxl: add "downscript=no" to Qemu call Roger Pau Monne
@ 2012-04-16 15:06 ` Roger Pau Monne
2012-04-16 16:50 ` Ian Campbell
4 siblings, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Clean xenstore, to prevent leaving empty directories in the tree, like:
/local/domain/0/backend/console = "" (n0)
/local/domain/0/backend/console/3 = "" (n0)
That was left after a guest was destroyed.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_device.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index d773d71..4161d1bd 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -609,12 +609,11 @@ out_ok:
int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
char *be_path = libxl__device_backend_path(gc, dev);
char *fe_path = libxl__device_frontend_path(gc, dev);
- xs_rm(ctx->xsh, XBT_NULL, be_path);
- xs_rm(ctx->xsh, XBT_NULL, fe_path);
+ libxl__xs_path_cleanup(gc, fe_path);
+ libxl__xs_path_cleanup(gc, be_path);
libxl__device_destroy_tapdisk(gc, be_path);
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
2012-04-16 15:06 ` [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd Roger Pau Monne
@ 2012-04-16 16:44 ` Ian Campbell
2012-04-17 8:54 ` Roger Pau Monne
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-04-16 16:44 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
> This is a rather big change, that adds the necessary machinery to perform
> hotplug script calling from libxl for Linux. This patch launches the necessary
> scripts to attach and detach PHY and TAP backend types (Qemu doesn't use hotplug
> scripts).
>
> Here's a list of the major changes introduced:
>
> * libxl_device_disk_add makes use of the new event library and takes the
> optional parameter libxl_asyncop_how, to choose how the operation has to be
> issued (sync or async).
>
> * libxl_device_disk_add waits for backend to switch to state 2 (XenbusInitWait)
> and then launches the hotplug script.
>
> * libxl__devices_destroy no longer calls libxl__device_destroy, and instead
> calls libxl__initiate_device_remove, so we can disconnect the device and
> execute the necessary hotplug scripts instead of just deleting the backend
> entries. So libxl__devices_destroy now uses the event library, and so does
> libxl_domain_destroy.
>
> * Since libxl__devices_destroy calls multiple times
> libxl__initiate_device_remove, this function now returns a different value
> regarding the actions taken (if an event was added or not). The user has to
> call libxl__ao_complete after using this function if necessary.
>
> * The internal API for hotplug scripts has been added, which consists of one
> function; libxl__device_hotplug that takes the device and the action to
> execute.
>
> * Linux hotplug scripts are called by setting the necessary env variables to
> emulate udev rules, so there's no need to change them (although a rework to
> pass this as parameters insted of env variables would be suitable, so both
> NetBSD and Linux hotplug scripts take the same parameters).
>
> * Added a check in xen-hotplug-common.sh, so scripts are only executed from
> udev when using xend. xl adds an execute_udev=n to xenstore device backend
> and passes XL_CALL=y to the env of the script call, and udev calls check that
> execute_udev is not set and XL_CALL is empty also.
>
> I've also modified the python binding for pyxl_domain_destroy, that now take the
> ao_how parameter, but I'm not really sure if it's done correctly, let's just say
> that it compiles.
>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
> tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
> tools/libxl/Makefile | 3 +-
> tools/libxl/libxl.c | 104 ++++++++++++++++++----
> tools/libxl/libxl.h | 7 +-
> tools/libxl/libxl_create.c | 4 +-
> tools/libxl/libxl_device.c | 140 +++++++++++++++++++++++++++--
> tools/libxl/libxl_dm.c | 4 +-
> tools/libxl/libxl_hotplug.c | 67 ++++++++++++++
> tools/libxl/libxl_internal.h | 43 +++++++++-
> tools/libxl/libxl_linux.c | 117 ++++++++++++++++++++++++
> tools/libxl/xl_cmdimpl.c | 14 ++--
> tools/python/xen/lowlevel/xl/xl.c | 5 +-
> 12 files changed, 474 insertions(+), 40 deletions(-)
> create mode 100644 tools/libxl/libxl_hotplug.c
>
> diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
> index 8f6557d..dc3e867 100644
> --- a/tools/hotplug/Linux/xen-hotplug-common.sh
> +++ b/tools/hotplug/Linux/xen-hotplug-common.sh
> @@ -15,6 +15,12 @@
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> #
>
> +# Hack to prevent the execution of hotplug scripts form udev if the domain
from
> +# has ben launched from libxl
been
> +execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
> +if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
> + exit 0
> +fi
So, am I right that in some future world where we have got rid of xend
and made this stuff work without udev everywhere (e.g. including driver
domains) there is a path to deprecating this snippet without requiring
everyone to go through some sort of transition?
Or are we stuck with this envvar forever? It's not a big deal but if we
can invert it (e.g. by setting ENV{HOTPLUG_GATE}=${XENBUS}/evecute_udev
in xen-backend.rules and ...if -n "${HOTPLUG_GATE}" && xenstore-read
${HOTPLUG_GATE}... etc) then that would be nicer?
> dir=$(dirname "$0")
> . "$dir/hotplugpath.sh"
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 16ebef3..fb4fbf2 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1062,13 +1062,15 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
> GC_FREE;
> }
>
> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
> +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
> + const libxl_asyncop_how *ao_how)
> {
> - GC_INIT(ctx);
> + AO_CREATE(ctx, domid, ao_how);
> char *dom_path;
> char *vm_path;
> char *pid;
> int rc, dm_present;
> + int rc_ao = 0;
>
> rc = libxl_domain_info(ctx, NULL, domid);
> switch(rc) {
> @@ -1110,7 +1112,8 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>
> libxl__qmp_cleanup(gc, domid);
> }
> - if (libxl__devices_destroy(gc, domid) < 0)
> + rc_ao = libxl__devices_destroy(egc, ao, domid);
> + if (rc_ao < 0)
> LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> "libxl__devices_destroy failed for %d", domid);
>
> @@ -1133,9 +1136,10 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
> goto out;
> }
> rc = 0;
> +
> out:
> - GC_FREE;
> - return rc;
> + if (rc_ao) return AO_ABORT(rc_ao);
> + return AO_INPROGRESS;
> }
>
> int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type)
> @@ -1313,9 +1317,11 @@ static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
> return 0;
> }
>
> -int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
> +int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
> + libxl_device_disk *disk,
> + const libxl_asyncop_how *ao_how)
> {
> - GC_INIT(ctx);
> + AO_CREATE(ctx, domid, ao_how);
> flexarray_t *front;
> flexarray_t *back;
> char *dev;
> @@ -1330,7 +1336,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
> rc = ERROR_NOMEM;
> goto out;
> }
> - back = flexarray_make(16, 1);
> + back = flexarray_make(20, 1);
> if (!back) {
> rc = ERROR_NOMEM;
> goto out_free;
> @@ -1361,6 +1367,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
> flexarray_append(back, "params");
> flexarray_append(back, dev);
>
> + flexarray_append(back, "script");
> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
> + libxl_xen_script_dir_path(),
> + "block"));
> +
> assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
> break;
> case LIBXL_DISK_BACKEND_TAP:
> @@ -1374,6 +1385,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
> libxl__device_disk_string_of_format(disk->format),
> disk->pdev_path));
>
> + flexarray_append(back, "script");
> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
> + libxl_xen_script_dir_path(),
> + "blktap"));
> +
> /* now create a phy device to export the device to the guest */
> goto do_backend_phy;
> case LIBXL_DISK_BACKEND_QDISK:
> @@ -1406,6 +1422,9 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
> flexarray_append(back, disk->readwrite ? "w" : "r");
> flexarray_append(back, "device-type");
> flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> + /* compatibility addon to keep udev rules */
> + flexarray_append(back, "execute_udev");
> + flexarray_append(back, "n");
>
> flexarray_append(front, "backend-id");
> flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> @@ -1420,14 +1439,23 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
> libxl__xs_kvs_of_flexarray(gc, back, back->count),
> libxl__xs_kvs_of_flexarray(gc, front, front->count));
>
> + if (disk->backend == LIBXL_DISK_BACKEND_PHY ||
> + disk->backend == LIBXL_DISK_BACKEND_TAP) {
> + rc = libxl__initiate_device_add(egc, ao, &device);
> + if (rc) goto out_free;
> + } else {
> + /* no need to execute hotplug scripts */
> + libxl__ao_complete(egc, ao, 0);
> + }
This would be better as a switch, since it would make it more obvious
that the "else" is actually "case LIBXL_DISK_BACKEND_QDISK" etc.
[...]
> +
> +/*
> + * Return codes:
> + * 1 Success and event(s) HAVE BEEN added
> + * 0 Success and NO events where added
were
(I saw a few of these)
> + * < 0 Error
> + *
> + * Since this function can be called several times, and several events
> + * can be added to the same context, the user has to call
> + * libxl__ao_complete when necessary (if no events are added).
> + */
> int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
> libxl__device *dev)
> {
> @@ -392,7 +461,6 @@ int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
[...]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] libxl: add "downscript=no" to Qemu call
2012-04-16 15:06 ` [PATCH 4/5] libxl: add "downscript=no" to Qemu call Roger Pau Monne
@ 2012-04-16 16:48 ` Ian Campbell
0 siblings, 0 replies; 18+ messages in thread
From: Ian Campbell @ 2012-04-16 16:48 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
> Currently we only pass script=no to Qemu, to avoid calling any scripts when
> attaching a tap interface, but we should also pass downscript=no to avoid Qemu
> trying to execute a script when disconnecting the interface. This prevents the
> following harmless error message:
>
> /etc/qemu-ifdown: could not launch network script
>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
> tools/libxl/libxl_dm.c | 17 +++++++++++------
> 1 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index ba5bef7..8dc588d 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -216,11 +216,14 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
> else
> ifname = vifs[i].ifname;
> flexarray_vappend(dm_args,
> - "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
> - vifs[i].devid, smac, vifs[i].model),
> - "-net", libxl__sprintf(gc, "tap,vlan=%d,ifname=%s,bridge=%s,script=%s",
> - vifs[i].devid, ifname, vifs[i].bridge, libxl_tapif_script(gc)),
> - NULL);
> + "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
> + vifs[i].devid, smac, vifs[i].model),
> + "-net", libxl__sprintf(gc,
> + "tap,vlan=%d,ifname=%s,bridge=%s,script=%s,"
> + "downscript=%s",
> + vifs[i].devid, ifname, vifs[i].bridge,
> + libxl_tapif_script(gc), libxl_tapif_script(gc)),
> + NULL);
I think these should be indented at least a little bit wrt the
flexarray_vappend. Since you've already split the string you might just
need to re-wrap a bit to fit in 80 chars (which I guess is why you
changed this)
> ioemu_vifs++;
> }
> }
> @@ -462,8 +465,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> vifs[i].devid, smac));
> flexarray_append(dm_args, "-netdev");
> flexarray_append(dm_args,
> - libxl__sprintf(gc, "type=tap,id=net%d,ifname=%s,script=%s",
> + libxl__sprintf(gc, "type=tap,id=net%d,ifname=%s,script=%s,"
> + "downscript=%s",
> vifs[i].devid, ifname,
> + libxl_tapif_script(gc),
> libxl_tapif_script(gc)));
The indentation here is now inconsistent.
Other than the whitepace issues:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ioemu_vifs++;
> }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-16 15:06 ` [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy Roger Pau Monne
@ 2012-04-16 16:50 ` Ian Campbell
2012-04-16 17:05 ` Roger Pau Monne
2012-04-17 14:18 ` Roger Pau Monne
0 siblings, 2 replies; 18+ messages in thread
From: Ian Campbell @ 2012-04-16 16:50 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
> Clean xenstore, to prevent leaving empty directories in the tree, like:
>
> /local/domain/0/backend/console = "" (n0)
> /local/domain/0/backend/console/3 = "" (n0)
>
> That was left after a guest was destroyed.
>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
> tools/libxl/libxl_device.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index d773d71..4161d1bd 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -609,12 +609,11 @@ out_ok:
>
> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
> {
> - libxl_ctx *ctx = libxl__gc_owner(gc);
> char *be_path = libxl__device_backend_path(gc, dev);
> char *fe_path = libxl__device_frontend_path(gc, dev);
>
> - xs_rm(ctx->xsh, XBT_NULL, be_path);
> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
> + libxl__xs_path_cleanup(gc, fe_path);
> + libxl__xs_path_cleanup(gc, be_path);
is xs_rm not recursive? At least from the CLI it seems to be
quartz:~# xenstore-write /foo/bar/baz 123
quartz:~# xenstore-ls -f | grep foo
/foo = ""
/foo/bar = ""
/foo/bar/baz = "123"
quartz:~# xenstore-rm /foo
quartz:~# xenstore-ls -f | grep foo
quartz:~#
looking at xenstore_client.c it seems to only call xs_rm() and not do
any traversal itself?
>
> libxl__device_destroy_tapdisk(gc, be_path);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-16 16:50 ` Ian Campbell
@ 2012-04-16 17:05 ` Roger Pau Monne
2012-04-17 14:18 ` Roger Pau Monne
1 sibling, 0 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-16 17:05 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
El 16/04/2012, a las 17:50, Ian Campbell escribió:
> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
>> Clean xenstore, to prevent leaving empty directories in the tree, like:
>>
>> /local/domain/0/backend/console = "" (n0)
>> /local/domain/0/backend/console/3 = "" (n0)
>>
>> That was left after a guest was destroyed.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>> ---
>> tools/libxl/libxl_device.c | 5 ++---
>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index d773d71..4161d1bd 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -609,12 +609,11 @@ out_ok:
>>
>> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
>> {
>> - libxl_ctx *ctx = libxl__gc_owner(gc);
>> char *be_path = libxl__device_backend_path(gc, dev);
>> char *fe_path = libxl__device_frontend_path(gc, dev);
>>
>> - xs_rm(ctx->xsh, XBT_NULL, be_path);
>> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
>> + libxl__xs_path_cleanup(gc, fe_path);
>> + libxl__xs_path_cleanup(gc, be_path);
>
> is xs_rm not recursive? At least from the CLI it seems to be
>
> quartz:~# xenstore-write /foo/bar/baz 123
> quartz:~# xenstore-ls -f | grep foo
> /foo = ""
> /foo/bar = ""
> /foo/bar/baz = "123"
> quartz:~# xenstore-rm /foo
> quartz:~# xenstore-ls -f | grep foo
> quartz:~#
>
> looking at xenstore_client.c it seems to only call xs_rm() and not do
> any traversal itself?
Sure! I had an error probably somewhere else in the code, and changing libxl__xs_path_cleanup to xs_rm didn't make it happen again, so forget about this. I will send an updated version of the series without this stuff (and the appropriate changes).
>
>>
>> libxl__device_destroy_tapdisk(gc, be_path);
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
2012-04-16 16:44 ` Ian Campbell
@ 2012-04-17 8:54 ` Roger Pau Monne
2012-04-17 8:56 ` Ian Campbell
0 siblings, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-17 8:54 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
El 16/04/2012, a las 17:44, Ian Campbell escribió:
> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
>> This is a rather big change, that adds the necessary machinery to perform
>> hotplug script calling from libxl for Linux. This patch launches the necessary
>> scripts to attach and detach PHY and TAP backend types (Qemu doesn't use hotplug
>> scripts).
>>
>> Here's a list of the major changes introduced:
>>
>> * libxl_device_disk_add makes use of the new event library and takes the
>> optional parameter libxl_asyncop_how, to choose how the operation has to be
>> issued (sync or async).
>>
>> * libxl_device_disk_add waits for backend to switch to state 2 (XenbusInitWait)
>> and then launches the hotplug script.
>>
>> * libxl__devices_destroy no longer calls libxl__device_destroy, and instead
>> calls libxl__initiate_device_remove, so we can disconnect the device and
>> execute the necessary hotplug scripts instead of just deleting the backend
>> entries. So libxl__devices_destroy now uses the event library, and so does
>> libxl_domain_destroy.
>>
>> * Since libxl__devices_destroy calls multiple times
>> libxl__initiate_device_remove, this function now returns a different value
>> regarding the actions taken (if an event was added or not). The user has to
>> call libxl__ao_complete after using this function if necessary.
>>
>> * The internal API for hotplug scripts has been added, which consists of one
>> function; libxl__device_hotplug that takes the device and the action to
>> execute.
>>
>> * Linux hotplug scripts are called by setting the necessary env variables to
>> emulate udev rules, so there's no need to change them (although a rework to
>> pass this as parameters insted of env variables would be suitable, so both
>> NetBSD and Linux hotplug scripts take the same parameters).
>>
>> * Added a check in xen-hotplug-common.sh, so scripts are only executed from
>> udev when using xend. xl adds an execute_udev=n to xenstore device backend
>> and passes XL_CALL=y to the env of the script call, and udev calls check that
>> execute_udev is not set and XL_CALL is empty also.
>>
>> I've also modified the python binding for pyxl_domain_destroy, that now take the
>> ao_how parameter, but I'm not really sure if it's done correctly, let's just say
>> that it compiles.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>> ---
>> tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
>> tools/libxl/Makefile | 3 +-
>> tools/libxl/libxl.c | 104 ++++++++++++++++++----
>> tools/libxl/libxl.h | 7 +-
>> tools/libxl/libxl_create.c | 4 +-
>> tools/libxl/libxl_device.c | 140 +++++++++++++++++++++++++++--
>> tools/libxl/libxl_dm.c | 4 +-
>> tools/libxl/libxl_hotplug.c | 67 ++++++++++++++
>> tools/libxl/libxl_internal.h | 43 +++++++++-
>> tools/libxl/libxl_linux.c | 117 ++++++++++++++++++++++++
>> tools/libxl/xl_cmdimpl.c | 14 ++--
>> tools/python/xen/lowlevel/xl/xl.c | 5 +-
>> 12 files changed, 474 insertions(+), 40 deletions(-)
>> create mode 100644 tools/libxl/libxl_hotplug.c
>>
>> diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh b/tools/hotplug/Linux/xen-hotplug-common.sh
>> index 8f6557d..dc3e867 100644
>> --- a/tools/hotplug/Linux/xen-hotplug-common.sh
>> +++ b/tools/hotplug/Linux/xen-hotplug-common.sh
>> @@ -15,6 +15,12 @@
>> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> #
>>
>> +# Hack to prevent the execution of hotplug scripts form udev if the domain
>
> from
>> +# has ben launched from libxl
>
> been
Done
>
>> +execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
>> +if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
>> + exit 0
>> +fi
>
> So, am I right that in some future world where we have got rid of xend
> and made this stuff work without udev everywhere (e.g. including driver
> domains) there is a path to deprecating this snippet without requiring
> everyone to go through some sort of transition?
>
> Or are we stuck with this envvar forever? It's not a big deal but if we
> can invert it (e.g. by setting ENV{HOTPLUG_GATE}=${XENBUS}/evecute_udev
> in xen-backend.rules and ...if -n "${HOTPLUG_GATE}" && xenstore-read
> ${HOTPLUG_GATE}... etc) then that would be nicer?
I will change it to something like:
if [ -z "${HOTPLUG_GATE}" ] && `xenstore-read "$HOTPLUG_GATE" 2>/dev/null` && \
[ -z "$XL_CALL" ]; then
and then set HOTPLUG_GATE from udev rules. I will change the xenstore gate to "disable_udev", which makes more sense, since we will be setting "disable_udev=y" or nothing.
>> dir=$(dirname "$0")
>> . "$dir/hotplugpath.sh"
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 16ebef3..fb4fbf2 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1062,13 +1062,15 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
>> GC_FREE;
>> }
>>
>> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>> +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
>> + const libxl_asyncop_how *ao_how)
>> {
>> - GC_INIT(ctx);
>> + AO_CREATE(ctx, domid, ao_how);
>> char *dom_path;
>> char *vm_path;
>> char *pid;
>> int rc, dm_present;
>> + int rc_ao = 0;
>>
>> rc = libxl_domain_info(ctx, NULL, domid);
>> switch(rc) {
>> @@ -1110,7 +1112,8 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>>
>> libxl__qmp_cleanup(gc, domid);
>> }
>> - if (libxl__devices_destroy(gc, domid) < 0)
>> + rc_ao = libxl__devices_destroy(egc, ao, domid);
>> + if (rc_ao < 0)
>> LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
>> "libxl__devices_destroy failed for %d", domid);
>>
>> @@ -1133,9 +1136,10 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>> goto out;
>> }
>> rc = 0;
>> +
>> out:
>> - GC_FREE;
>> - return rc;
>> + if (rc_ao) return AO_ABORT(rc_ao);
>> + return AO_INPROGRESS;
>> }
>>
>> int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type)
>> @@ -1313,9 +1317,11 @@ static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
>> return 0;
>> }
>>
>> -int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
>> +int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
>> + libxl_device_disk *disk,
>> + const libxl_asyncop_how *ao_how)
>> {
>> - GC_INIT(ctx);
>> + AO_CREATE(ctx, domid, ao_how);
>> flexarray_t *front;
>> flexarray_t *back;
>> char *dev;
>> @@ -1330,7 +1336,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>> rc = ERROR_NOMEM;
>> goto out;
>> }
>> - back = flexarray_make(16, 1);
>> + back = flexarray_make(20, 1);
>> if (!back) {
>> rc = ERROR_NOMEM;
>> goto out_free;
>> @@ -1361,6 +1367,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>> flexarray_append(back, "params");
>> flexarray_append(back, dev);
>>
>> + flexarray_append(back, "script");
>> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
>> + libxl_xen_script_dir_path(),
>> + "block"));
>> +
>> assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
>> break;
>> case LIBXL_DISK_BACKEND_TAP:
>> @@ -1374,6 +1385,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>> libxl__device_disk_string_of_format(disk->format),
>> disk->pdev_path));
>>
>> + flexarray_append(back, "script");
>> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
>> + libxl_xen_script_dir_path(),
>> + "blktap"));
>> +
>> /* now create a phy device to export the device to the guest */
>> goto do_backend_phy;
>> case LIBXL_DISK_BACKEND_QDISK:
>> @@ -1406,6 +1422,9 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>> flexarray_append(back, disk->readwrite ? "w" : "r");
>> flexarray_append(back, "device-type");
>> flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
>> + /* compatibility addon to keep udev rules */
>> + flexarray_append(back, "execute_udev");
>> + flexarray_append(back, "n");
>>
>> flexarray_append(front, "backend-id");
>> flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
>> @@ -1420,14 +1439,23 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>> libxl__xs_kvs_of_flexarray(gc, back, back->count),
>> libxl__xs_kvs_of_flexarray(gc, front, front->count));
>>
>> + if (disk->backend == LIBXL_DISK_BACKEND_PHY ||
>> + disk->backend == LIBXL_DISK_BACKEND_TAP) {
>> + rc = libxl__initiate_device_add(egc, ao, &device);
>> + if (rc) goto out_free;
>> + } else {
>> + /* no need to execute hotplug scripts */
>> + libxl__ao_complete(egc, ao, 0);
>> + }
>
> This would be better as a switch, since it would make it more obvious
> that the "else" is actually "case LIBXL_DISK_BACKEND_QDISK" etc.
>
Yes
> [...]
>> +
>> +/*
>> + * Return codes:
>> + * 1 Success and event(s) HAVE BEEN added
>> + * 0 Success and NO events where added
>
> were
>
> (I saw a few of these)
Will do a grep to see what I can find…
Thanks for the review.
>
>> + * < 0 Error
>> + *
>> + * Since this function can be called several times, and several events
>> + * can be added to the same context, the user has to call
>> + * libxl__ao_complete when necessary (if no events are added).
>> + */
>> int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
>> libxl__device *dev)
>> {
>> @@ -392,7 +461,6 @@ int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
> [...]
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
2012-04-17 8:54 ` Roger Pau Monne
@ 2012-04-17 8:56 ` Ian Campbell
2012-04-17 9:13 ` Roger Pau Monne
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-04-17 8:56 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Tue, 2012-04-17 at 09:54 +0100, Roger Pau Monne wrote:
> El 16/04/2012, a las 17:44, Ian Campbell escribió:
> >> +execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
> >> +if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
> >> + exit 0
> >> +fi
> >
> > So, am I right that in some future world where we have got rid of xend
> > and made this stuff work without udev everywhere (e.g. including driver
> > domains) there is a path to deprecating this snippet without requiring
> > everyone to go through some sort of transition?
> >
> > Or are we stuck with this envvar forever? It's not a big deal but if we
> > can invert it (e.g. by setting ENV{HOTPLUG_GATE}=${XENBUS}/evecute_udev
> > in xen-backend.rules and ...if -n "${HOTPLUG_GATE}" && xenstore-read
> > ${HOTPLUG_GATE}... etc) then that would be nicer?
>
>
> I will change it to something like:
>
> if [ -z "${HOTPLUG_GATE}" ] && `xenstore-read "$HOTPLUG_GATE" 2>/dev/null` && \
> [ -z "$XL_CALL" ]; then
Do you still need $XL_CALL?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
2012-04-17 8:56 ` Ian Campbell
@ 2012-04-17 9:13 ` Roger Pau Monne
0 siblings, 0 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-17 9:13 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
El 17/04/2012, a las 09:54, Roger Pau Monne escribió:
> On Tue, 2012-04-17 at 09:54 +0100, Roger Pau Monne wrote:
>> El 16/04/2012, a las 17:44, Ian Campbell escribió:
>>>> +execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
>>>> +if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
>>>> + exit 0
>>>> +fi
>>>
>>> So, am I right that in some future world where we have got rid of xend
>>> and made this stuff work without udev everywhere (e.g. including driver
>>> domains) there is a path to deprecating this snippet without requiring
>>> everyone to go through some sort of transition?
>>>
>>> Or are we stuck with this envvar forever? It's not a big deal but if we
>>> can invert it (e.g. by setting ENV{HOTPLUG_GATE}=${XENBUS}/evecute_udev
>>> in xen-backend.rules and ...if -n "${HOTPLUG_GATE}" && xenstore-read
>>> ${HOTPLUG_GATE}... etc) then that would be nicer?
>>
>>
>> I will change it to something like:
>>
>> if [ -z "${HOTPLUG_GATE}" ] && `xenstore-read "$HOTPLUG_GATE" 2>/dev/null` && \
>> [ -z "$XL_CALL" ]; then
>
> Do you still need $XL_CALL?
No, with HOTPLUG_GATE it's enough, thanks again.
>
> Ian.
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-16 16:50 ` Ian Campbell
2012-04-16 17:05 ` Roger Pau Monne
@ 2012-04-17 14:18 ` Roger Pau Monne
2012-04-17 14:21 ` Ian Campbell
1 sibling, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-17 14:18 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
El 16/04/2012, a las 17:50, Ian Campbell escribió:
> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
>> Clean xenstore, to prevent leaving empty directories in the tree, like:
>>
>> /local/domain/0/backend/console = "" (n0)
>> /local/domain/0/backend/console/3 = "" (n0)
>>
>> That was left after a guest was destroyed.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>> ---
>> tools/libxl/libxl_device.c | 5 ++---
>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index d773d71..4161d1bd 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -609,12 +609,11 @@ out_ok:
>>
>> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
>> {
>> - libxl_ctx *ctx = libxl__gc_owner(gc);
>> char *be_path = libxl__device_backend_path(gc, dev);
>> char *fe_path = libxl__device_frontend_path(gc, dev);
>>
>> - xs_rm(ctx->xsh, XBT_NULL, be_path);
>> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
>> + libxl__xs_path_cleanup(gc, fe_path);
>> + libxl__xs_path_cleanup(gc, be_path);
>
> is xs_rm not recursive? At least from the CLI it seems to be
Ok, I got messed up here, what my function did was purge a directory tree from top to bottom, so using your example, doing a:
libxl__xs_path_cleanup(gc, "/foo/bar/baz");
Would have erased /foo/bar/baz, /foo/bar and /foo, because they where all empty (had no more files or subfolders inside) after deleting "baz".
So I guess this is really needed.
>
> quartz:~# xenstore-write /foo/bar/baz 123
> quartz:~# xenstore-ls -f | grep foo
> /foo = ""
> /foo/bar = ""
> /foo/bar/baz = "123"
> quartz:~# xenstore-rm /foo
> quartz:~# xenstore-ls -f | grep foo
> quartz:~#
>
> looking at xenstore_client.c it seems to only call xs_rm() and not do
> any traversal itself?
>
>>
>> libxl__device_destroy_tapdisk(gc, be_path);
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-17 14:18 ` Roger Pau Monne
@ 2012-04-17 14:21 ` Ian Campbell
2012-04-17 14:32 ` Roger Pau Monne
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-04-17 14:21 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Tue, 2012-04-17 at 15:18 +0100, Roger Pau Monne wrote:
> El 16/04/2012, a las 17:50, Ian Campbell escribió:
> > On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
> >> Clean xenstore, to prevent leaving empty directories in the tree, like:
> >>
> >> /local/domain/0/backend/console = "" (n0)
> >> /local/domain/0/backend/console/3 = "" (n0)
> >>
> >> That was left after a guest was destroyed.
> >>
> >> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> >> ---
> >> tools/libxl/libxl_device.c | 5 ++---
> >> 1 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> >> index d773d71..4161d1bd 100644
> >> --- a/tools/libxl/libxl_device.c
> >> +++ b/tools/libxl/libxl_device.c
> >> @@ -609,12 +609,11 @@ out_ok:
> >>
> >> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
> >> {
> >> - libxl_ctx *ctx = libxl__gc_owner(gc);
> >> char *be_path = libxl__device_backend_path(gc, dev);
> >> char *fe_path = libxl__device_frontend_path(gc, dev);
> >>
> >> - xs_rm(ctx->xsh, XBT_NULL, be_path);
> >> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
> >> + libxl__xs_path_cleanup(gc, fe_path);
> >> + libxl__xs_path_cleanup(gc, be_path);
> >
> > is xs_rm not recursive? At least from the CLI it seems to be
>
> Ok, I got messed up here, what my function did was purge a directory
> tree from top to bottom, so using your example, doing a:
>
> libxl__xs_path_cleanup(gc, "/foo/bar/baz");
>
> Would have erased /foo/bar/baz, /foo/bar and /foo, because they where
> all empty (had no more files or subfolders inside) after deleting
> "baz".
IFF they are empty? So the presence of /foo/bob would have made
only /foo/bar/baz and /foo/bar but not /foo get deleted?
IOW it behaves like "xenstore-rm -t" (t for tidy)?
>
> So I guess this is really needed.
>
> >
> > quartz:~# xenstore-write /foo/bar/baz 123
> > quartz:~# xenstore-ls -f | grep foo
> > /foo = ""
> > /foo/bar = ""
> > /foo/bar/baz = "123"
> > quartz:~# xenstore-rm /foo
> > quartz:~# xenstore-ls -f | grep foo
> > quartz:~#
> >
> > looking at xenstore_client.c it seems to only call xs_rm() and not do
> > any traversal itself?
> >
> >>
> >> libxl__device_destroy_tapdisk(gc, be_path);
> >>
> >
> >
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-17 14:21 ` Ian Campbell
@ 2012-04-17 14:32 ` Roger Pau Monne
2012-04-17 14:37 ` Ian Campbell
0 siblings, 1 reply; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-17 14:32 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
El 17/04/2012, a las 15:18, Roger Pau Monne escribió:
> On Tue, 2012-04-17 at 15:18 +0100, Roger Pau Monne wrote:
>> El 16/04/2012, a las 17:50, Ian Campbell escribió:
>>> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
>>>> Clean xenstore, to prevent leaving empty directories in the tree, like:
>>>>
>>>> /local/domain/0/backend/console = "" (n0)
>>>> /local/domain/0/backend/console/3 = "" (n0)
>>>>
>>>> That was left after a guest was destroyed.
>>>>
>>>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>>>> ---
>>>> tools/libxl/libxl_device.c | 5 ++---
>>>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>>>> index d773d71..4161d1bd 100644
>>>> --- a/tools/libxl/libxl_device.c
>>>> +++ b/tools/libxl/libxl_device.c
>>>> @@ -609,12 +609,11 @@ out_ok:
>>>>
>>>> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
>>>> {
>>>> - libxl_ctx *ctx = libxl__gc_owner(gc);
>>>> char *be_path = libxl__device_backend_path(gc, dev);
>>>> char *fe_path = libxl__device_frontend_path(gc, dev);
>>>>
>>>> - xs_rm(ctx->xsh, XBT_NULL, be_path);
>>>> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
>>>> + libxl__xs_path_cleanup(gc, fe_path);
>>>> + libxl__xs_path_cleanup(gc, be_path);
>>>
>>> is xs_rm not recursive? At least from the CLI it seems to be
>>
>> Ok, I got messed up here, what my function did was purge a directory
>> tree from top to bottom, so using your example, doing a:
>>
>> libxl__xs_path_cleanup(gc, "/foo/bar/baz");
>>
>> Would have erased /foo/bar/baz, /foo/bar and /foo, because they where
>> all empty (had no more files or subfolders inside) after deleting
>> "baz".
>
> IFF they are empty? So the presence of /foo/bob would have made
> only /foo/bar/baz and /foo/bar but not /foo get deleted?
Yes, if there's a /foo/bob that would prevent the deletion of /foo.
>
> IOW it behaves like "xenstore-rm -t" (t for tidy)?
>From what I saw, yes (although I didn't even know of "xenstore-rm -t").
>
>>
>> So I guess this is really needed.
>>
>>>
>>> quartz:~# xenstore-write /foo/bar/baz 123
>>> quartz:~# xenstore-ls -f | grep foo
>>> /foo = ""
>>> /foo/bar = ""
>>> /foo/bar/baz = "123"
>>> quartz:~# xenstore-rm /foo
>>> quartz:~# xenstore-ls -f | grep foo
>>> quartz:~#
>>>
>>> looking at xenstore_client.c it seems to only call xs_rm() and not do
>>> any traversal itself?
>>>
>>>>
>>>> libxl__device_destroy_tapdisk(gc, be_path);
>>>>
>>>
>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-17 14:32 ` Roger Pau Monne
@ 2012-04-17 14:37 ` Ian Campbell
2012-04-17 14:49 ` Roger Pau Monne
0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2012-04-17 14:37 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
On Tue, 2012-04-17 at 15:32 +0100, Roger Pau Monne wrote:
> El 17/04/2012, a las 15:18, Roger Pau Monne escribió:
> > On Tue, 2012-04-17 at 15:18 +0100, Roger Pau Monne wrote:
> >> El 16/04/2012, a las 17:50, Ian Campbell escribió:
> >>> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
> >>>> Clean xenstore, to prevent leaving empty directories in the tree, like:
> >>>>
> >>>> /local/domain/0/backend/console = "" (n0)
> >>>> /local/domain/0/backend/console/3 = "" (n0)
> >>>>
> >>>> That was left after a guest was destroyed.
> >>>>
> >>>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> >>>> ---
> >>>> tools/libxl/libxl_device.c | 5 ++---
> >>>> 1 files changed, 2 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> >>>> index d773d71..4161d1bd 100644
> >>>> --- a/tools/libxl/libxl_device.c
> >>>> +++ b/tools/libxl/libxl_device.c
> >>>> @@ -609,12 +609,11 @@ out_ok:
> >>>>
> >>>> int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
> >>>> {
> >>>> - libxl_ctx *ctx = libxl__gc_owner(gc);
> >>>> char *be_path = libxl__device_backend_path(gc, dev);
> >>>> char *fe_path = libxl__device_frontend_path(gc, dev);
> >>>>
> >>>> - xs_rm(ctx->xsh, XBT_NULL, be_path);
> >>>> - xs_rm(ctx->xsh, XBT_NULL, fe_path);
> >>>> + libxl__xs_path_cleanup(gc, fe_path);
> >>>> + libxl__xs_path_cleanup(gc, be_path);
> >>>
> >>> is xs_rm not recursive? At least from the CLI it seems to be
> >>
> >> Ok, I got messed up here, what my function did was purge a directory
> >> tree from top to bottom, so using your example, doing a:
> >>
> >> libxl__xs_path_cleanup(gc, "/foo/bar/baz");
> >>
> >> Would have erased /foo/bar/baz, /foo/bar and /foo, because they where
> >> all empty (had no more files or subfolders inside) after deleting
> >> "baz".
> >
> > IFF they are empty? So the presence of /foo/bob would have made
> > only /foo/bar/baz and /foo/bar but not /foo get deleted?
>
>
> Yes, if there's a /foo/bob that would prevent the deletion of /foo.
> >
> > IOW it behaves like "xenstore-rm -t" (t for tidy)?
>
> From what I saw, yes (although I didn't even know of "xenstore-rm -t").
Me neither till I looked at xenstore_client.c to see how my experiments
related to xs_rm...
>
> >
> >>
> >> So I guess this is really needed.
> >>
> >>>
> >>> quartz:~# xenstore-write /foo/bar/baz 123
> >>> quartz:~# xenstore-ls -f | grep foo
> >>> /foo = ""
> >>> /foo/bar = ""
> >>> /foo/bar/baz = "123"
> >>> quartz:~# xenstore-rm /foo
> >>> quartz:~# xenstore-ls -f | grep foo
> >>> quartz:~#
> >>>
> >>> looking at xenstore_client.c it seems to only call xs_rm() and not do
> >>> any traversal itself?
> >>>
> >>>>
> >>>> libxl__device_destroy_tapdisk(gc, be_path);
> >>>>
> >>>
> >>>
> >>
> >>
> >
> >
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy
2012-04-17 14:37 ` Ian Campbell
@ 2012-04-17 14:49 ` Roger Pau Monne
0 siblings, 0 replies; 18+ messages in thread
From: Roger Pau Monne @ 2012-04-17 14:49 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
El 17/04/2012, a las 15:37, Ian Campbell escribió:
> On Tue, 2012-04-17 at 15:32 +0100, Roger Pau Monne wrote:
> Me neither till I looked at xenstore_client.c to see how my experiments
> related to xs_rm…
I've looked at the code, but I prefer mine, since it makes use of the handy libxl__xs_directory libxl function (and I think is cleaner to use a for loop than the goto that's used on xenstore_client.c). I've added it again to the series, and I'm going to resend it with the necessary fixes.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-04-17 14:49 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-16 15:06 [PATCH 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
2012-04-16 15:06 ` [PATCH 1/5] libxl: allow libxl__exec to take a parameter containing the env variables Roger Pau Monne
2012-04-16 15:06 ` [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd Roger Pau Monne
2012-04-16 16:44 ` Ian Campbell
2012-04-17 8:54 ` Roger Pau Monne
2012-04-17 8:56 ` Ian Campbell
2012-04-17 9:13 ` Roger Pau Monne
2012-04-16 15:06 ` [PATCH 3/5] libxl: call hotplug scripts from libxl for vif Roger Pau Monne
2012-04-16 15:06 ` [PATCH 4/5] libxl: add "downscript=no" to Qemu call Roger Pau Monne
2012-04-16 16:48 ` Ian Campbell
2012-04-16 15:06 ` [PATCH 5/5] libxl: clean xenstore console directories recursively on destroy Roger Pau Monne
2012-04-16 16:50 ` Ian Campbell
2012-04-16 17:05 ` Roger Pau Monne
2012-04-17 14:18 ` Roger Pau Monne
2012-04-17 14:21 ` Ian Campbell
2012-04-17 14:32 ` Roger Pau Monne
2012-04-17 14:37 ` Ian Campbell
2012-04-17 14:49 ` Roger Pau Monne
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.