Roger Pau Monne
2012-May-10 11:28 UTC
[PATCH v4 0/4] libxl: call hotplug scripts from libxl
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.
They should be applied after IanJ v8 "libxl: subprocess handling" and
IanC "libxl/xend: name tap devices vifX.Y-emu".
This version (v4) introduces many changes, and makes proper use of the
event library. It entangles device addition with domain creation,
following IanJ changes to domain creation, and also introduces AO
operations to domain destruction and specifically to device teardown.
Many changes are introduced, and also a lot of code motion, since
libxl_device_{disk,nic}_add are now mere wrappers arround their
respective libxl__device_{disk,nic}_add counterparts, which can be
called within an already running AO operation, and perform all the
work.
Add another parameter to libxl__exec call that contains the
environment variables to use when performing the execvp call.
Changes since v3:
* Use CTX instead of defining libxl_ctx *ctx.
* Error checking on setenv.
* Better comment on header for libxl__exec function.
* Added some const-correctness.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
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 | 20 ++++++++++++++++++--
tools/libxl/libxl_internal.h | 20 ++++++++++++++++++--
5 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8f63941..79d7708 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1251,7 +1251,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(gc, 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 fb1302b..0021a7b 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -359,6 +359,7 @@ static void bootloader_gotptys(libxl__egc *egc,
libxl__openpty_state *op)
libxl__bootloader_state *bl = CONTAINER_OF(op, *bl, openpty);
STATE_AO_GC(bl->ao);
int rc, r;
+ char *const env[] = { "TERM", "vt100", NULL };
if (bl->openpty.rc) {
rc = bl->openpty.rc;
@@ -452,8 +453,7 @@ static void bootloader_gotptys(libxl__egc *egc,
libxl__openpty_state *op)
/* child */
r = login_tty(libxl__carefd_fd(bl->ptys[0].slave));
if (r) { LOGE(ERROR, "login_tty failed"); exit(-1); }
- setenv("TERM", "vt100", 1);
- libxl__exec(-1, -1, -1, bl->args[0], (char**)bl->args);
+ libxl__exec(gc, -1, -1, -1, bl->args[0], (char **) bl->args,
env);
exit(-1);
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b2bae68..5de3aef 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1006,7 +1006,7 @@ retry_transaction:
goto out_close;
if (!rc) { /* inner child */
setsid();
- libxl__exec(null, logfile_w, logfile_w, dm, args);
+ libxl__exec(gc, 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 b46b893..fba4cb4 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -66,8 +66,8 @@ static void check_open_fds(const char *what)
if (badness) abort();
}
-void libxl__exec(int stdinfd, int stdoutfd, int stderrfd, const char *arg0,
- char **args)
+void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd, int stderrfd,
+ const char *arg0, char *const args[], char *const env[])
/* call this in the child */
{
if (stdinfd != -1)
@@ -90,8 +90,24 @@ 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) {
+ if (setenv(env[i], env[i+1], 1) < 0) {
+ switch (errno) {
+ case EINVAL:
+ LOGEV(ERROR, errno, "invalid env variables (%s =
%s)",
+ env[i], env[i+1]);
+ break;
+ case ENOMEM:
+ libxl__alloc_failed(CTX, __func__, 0, 0);
+ }
+ goto out;
+ }
+ }
+ }
execvp(arg0, args);
+out:
fprintf(stderr, "libxl: cannot execute %s: %s\n", arg0,
strerror(errno));
_exit(-1);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8a90622..9ab76f8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1138,8 +1138,24 @@ _hidden int libxl__wait_for_offspring(libxl__gc *gc,
/* low-level stuff, for synchronous subprocesses etc. */
-_hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
- const char *arg0, char **args); // logs errors, never returns
+/*
+ * env should be passed using the following format,
+ *
+ * env[0]: name of env variable
+ * env[1]: value of env variable
+ * env[n]: ...
+ *
+ * So it efectively becomes something like:
+ * export env[n]=env[n+1]
+ * (where n%2 = 0)
+ *
+ * The last entry of the array always has to be NULL.
+ *
+ * Logs errors, never returns.
+ */
+_hidden void libxl__exec(libxl__gc *gc, int stdinfd, int stdoutfd,
+ int stderrfd, const char *arg0, char *const args[],
+ char *const env[]);
/* from xl_create */
_hidden int libxl__domain_make(libxl__gc *gc,
--
1.7.7.5 (Apple Git-26)
Add a function which behaves like "xenstore-rm -t", and which will be
used to
clean xenstore after unplug since we will be no longer executing
xen-hotplug-cleanup script, that used to do that for us.
Changes since v3:
* Better error checking and function description.
Changes since v2:
* Moved the function to libxl_xshelp.c and added the prototype to
libxl_internal.h.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/libxl/libxl_internal.h | 7 +++++++
tools/libxl/libxl_xshelp.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9ab76f8..fc2024d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -494,6 +494,13 @@ _hidden bool libxl__xs_mkdir(libxl__gc *gc,
xs_transaction_t t,
_hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
+/*
+ * This is a recursive delete, from top to bottom. What this function does
+ * is remove empty folders that contained the deleted entry.
+ *
+ * It mimics xenstore-rm -t behaviour.
+ */
+_hidden int libxl__xs_path_cleanup(libxl__gc *gc, char *user_path);
/*
* Event generation functions provided by the libxl event core to the
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 3ea8d08..7339236 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -135,6 +135,36 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
return s;
}
+int libxl__xs_path_cleanup(libxl__gc *gc, char *user_path)
+{
+ unsigned int nb = 0;
+ char *path, *last;
+
+ if (!user_path) {
+ LOGE(ERROR, "null path provided");
+ return ERROR_FAIL;
+ }
+
+ path = libxl__strdup(gc, user_path);
+ xs_rm(CTX->xsh, XBT_NULL, path);
+
+ for (last = strrchr(path, ''/''); last != NULL; last =
strrchr(path, ''/'')) {
+ *last = ''\0'';
+
+ if (!strlen(path)) break;
+
+ if (!libxl__xs_directory(gc, XBT_NULL, path, &nb)) {
+ LOG(DEBUG, "failed to read directory contents of %s",
path);
+ return ERROR_FAIL;
+ }
+
+ if (nb == 0) {
+ xs_rm(CTX->xsh, XBT_NULL, path);
+ }
+ }
+ return 0;
+}
+
/*
* Local variables:
* mode: C
--
1.7.7.5 (Apple Git-26)
Roger Pau Monne
2012-May-10 11:28 UTC
[PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
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).
* Disk addition waits for backend to switch to state 2
(XenbusInitWait) and then launches the hotplug script.
* Created and internal function (libxl__device_disk_add), that can be
called within an already running AO, and that will plug the
requested disk and execute the callback afterward.
libxl_device_disk_add is a user of this newly created function.
* 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, that has
been converted to an AO operation function
* The internal API for hotplug scripts has been added, which
consists of one function; libxl__device_hotplug that takes the
device and a struct with the necessary info about 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 a disable_udev=1 to
xenstore libxl local directory and with the modification of the
udev rules every call from udev gets UDEV_CALL passed, that points
to disable_udev. If UDEV_CALL is passed and points to a value, the
hotplug script is not executed.
Changes since v3 (changes based on IanJ review):
* Store disable_udev in libxl/disable_udev, which now is not device
dependant, but backend domain dependant.
* Added a global callback to device addition/destruction, that is now
called when we have finished all the oprtations and handles the
call to ao_complete if no other operations are pending.
* Unified both device creation/destruction flows, so now all of them
use the same data structure and callbacks.
* Used the event interface in a proper way, and merged disk addition
with the new async flow of the domain creation.
* Make use of the new functions LOG, LOGE, GCNEW_ARRAY...
* Passed NULL as ao_how in Python bindings.
Changes since v1:
* Replaced the hotplug snippet that prevents execution from udev
when necessary, and now we set the env var from udev rules instead
of libxl.
* Replaced the libxl_device_disk_add "if" with a "switch".
* Removed libxl__xs_path_cleanup (replaced with xs_rm).
* Fixed several spelling mistakes.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
docs/man/xl.conf.pod.5 | 8 +
tools/examples/xl.conf | 5 +
tools/hotplug/Linux/xen-backend.rules | 6 +-
tools/hotplug/Linux/xen-hotplug-common.sh | 6 +
tools/libxl/Makefile | 3 +-
tools/libxl/libxl.c | 351 +++++++--------------
tools/libxl/libxl.h | 7 +-
tools/libxl/libxl_create.c | 102 ++++++-
tools/libxl/libxl_device.c | 497 +++++++++++++++++++++++++----
tools/libxl/libxl_dm.c | 136 +++++----
tools/libxl/libxl_dom.c | 170 ++++++++++
tools/libxl/libxl_hotplug.c | 84 +++++
tools/libxl/libxl_internal.h | 170 ++++++++++-
tools/libxl/libxl_linux.c | 126 ++++++++
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl.c | 4 +
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 15 +-
tools/python/xen/lowlevel/xl/xl.c | 2 +-
19 files changed, 1291 insertions(+), 403 deletions(-)
create mode 100644 tools/libxl/libxl_hotplug.c
diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 8bd45ea..72825a0 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -55,6 +55,14 @@ default.
Default: C<1>
+=item B<run_hotplug_scripts=BOOLEAN>
+
+If disabled hotplug scripts will be called from udev, as it used to
+be in the previous releases. With the default option, hotplug scripts
+will be launched by xl directly.
+
+Default: C<1>
+
=item B<lockfile="PATH">
Sets the path to the lock file used by xl to serialise certain
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 56d3b3b..75b00e0 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -12,3 +12,8 @@
# default output format used by "xl list -l"
#output_format="json"
+
+# default option to run hotplug scripts from xl
+# if disabled the old behaviour will be used, and hotplug scripts will be
+# launched by udev.
+#run_hotplug_scripts=1
diff --git a/tools/hotplug/Linux/xen-backend.rules
b/tools/hotplug/Linux/xen-backend.rules
index 405387f..d55ff11 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -1,11 +1,11 @@
-SUBSYSTEM=="xen-backend", KERNEL=="tap*",
RUN+="/etc/xen/scripts/blktap $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
RUN+="/etc/xen/scripts/block $env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="tap*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap
$env{ACTION}"
+SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block
$env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*",
RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*",
RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online
type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline
type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*",
RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
-SUBSYSTEM=="xen-backend", ACTION=="remove",
RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
+SUBSYSTEM=="xen-backend", ACTION=="remove",
ENV{UDEV_CALL}="1",
RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
SUBSYSTEM=="xen", KERNEL=="blktap[0-9]*",
NAME="xen/%k", MODE="0600"
SUBSYSTEM=="blktap2", KERNEL=="blktap[0-9]*",
NAME="xen/blktap-2/%k", MODE="0600"
diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh
b/tools/hotplug/Linux/xen-hotplug-common.sh
index 8f6557d..6443a63 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 from udev if the domain
+# has been launched from libxl
+if [ -n "${UDEV_CALL}" ] && \
+ `xenstore-read "libxl/disable_udev" >/dev/null 2>&1`;
then
+ exit 0
+fi
dir=$(dirname "$0")
. "$dir/hotplugpath.sh"
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 5d9227e..9abadff 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -66,7 +66,8 @@ 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_aoutils.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 79d7708..b438fbf 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1058,82 +1058,36 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx,
libxl_evgen_disk_eject *evg) {
GC_INIT(ctx);
libxl__evdisable_disk_eject(gc, evg);
GC_FREE;
-}
+}
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
-{
- GC_INIT(ctx);
- char *dom_path;
- char *vm_path;
- char *pid;
- int rc, dm_present;
+/* Callback for domain destruction */
- rc = libxl_domain_info(ctx, NULL, domid);
- switch(rc) {
- case 0:
- break;
- case ERROR_INVAL:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "non-existant domain %d",
domid);
- default:
- return rc;
- }
+static void domain_destroy_cb(libxl__egc *, libxl__domain_destroy_state *,
int);
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
- dm_present = 1;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
- pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"/local/domain/%d/image/device-model-pid", domid));
- dm_present = (pid != NULL);
- break;
- default:
- abort();
- }
-
- dom_path = libxl__xs_get_dompath(gc, domid);
- if (!dom_path) {
- rc = ERROR_FAIL;
- goto out;
- }
-
- if (libxl__device_pci_destroy_all(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "pci shutdown failed for domid
%d", domid);
- rc = xc_domain_pause(ctx->xch, domid);
- if (rc < 0) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_pause
failed for %d", domid);
- }
- if (dm_present) {
- if (libxl__destroy_device_model(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__destroy_device_model
failed for %d", domid);
-
- libxl__qmp_cleanup(gc, domid);
- }
- if (libxl__devices_destroy(gc, domid) < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "libxl__devices_destroy failed for %d", domid);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__domain_destroy_state *dds;
- vm_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"%s/vm", dom_path));
- if (vm_path)
- if (!xs_rm(ctx->xsh, XBT_NULL, vm_path))
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs_rm failed for
%s", vm_path);
+ GCNEW(dds);
+ dds->ao = ao;
+ dds->domid = domid;
+ dds->callback = domain_destroy_cb;
+ libxl__domain_destroy(egc, dds);
- if (!xs_rm(ctx->xsh, XBT_NULL, dom_path))
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs_rm failed for
%s", dom_path);
+ return AO_INPROGRESS;
+}
- xs_rm(ctx->xsh, XBT_NULL, libxl__xs_libxl_path(gc, domid));
+static void domain_destroy_cb(libxl__egc *egc, libxl__domain_destroy_state
*dds,
+ int rc)
+{
+ STATE_AO_GC(dds->ao);
- libxl__userdata_destroyall(gc, domid);
+ if (rc)
+ LOGE(ERROR, "destruction of domain %u failed",
dds->domid);
- rc = xc_domain_destroy(ctx->xch, domid);
- if (rc < 0) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_destroy
failed for %d", domid);
- rc = ERROR_FAIL;
- goto out;
- }
- rc = 0;
-out:
- GC_FREE;
- return rc;
+ libxl__ao_complete(egc, ao, rc);
}
int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
libxl_console_type type)
@@ -1261,171 +1215,56 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t
domid, int autopass)
/******************************************************************************/
-int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
+/* generic callback for devices that only need to set ao_complete */
+static void libxl__device_cb(libxl__egc *egc, libxl__ao_device *aorm)
{
- int rc;
+ STATE_AO_GC(aorm->ao);
- rc = libxl__device_disk_set_backend(gc, disk);
- if (rc) return rc;
+ if (aorm->rc) {
+ LOGE(ERROR, "unable to %s %s with id %u",
+ aorm->action == DEVICE_CONNECT ? "add" :
"remove",
+ libxl__device_kind_to_string(aorm->dev->kind),
+ aorm->dev->devid);
+ goto out;
+ }
- return rc;
+out:
+ libxl__ao_complete(egc, ao, aorm->rc);
+ return;
}
-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;
- }
+int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
+{
+ int rc;
- device->domid = domid;
- device->devid = devid;
- device->kind = LIBXL__DEVICE_KIND_VBD;
+ rc = libxl__device_disk_set_backend(gc, disk);
+ if (rc) return rc;
- return 0;
+ return rc;
}
-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);
- flexarray_t *front;
- flexarray_t *back;
- char *dev;
- libxl__device device;
- int major, minor, rc;
-
- rc = libxl__device_disk_setdefault(gc, disk);
- if (rc) goto out;
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__ao_device *device;
+ int rc;
- front = flexarray_make(16, 1);
- if (!front) {
- rc = ERROR_NOMEM;
+ GCNEW(device);
+ libxl__init_ao_device(device, ao, NULL);
+ device->callback = libxl__device_cb;
+ rc = libxl__device_disk_add(egc, domid, disk, device);
+ if (rc) {
+ LOGE(ERROR, "unable to add disk %s", disk->pdev_path);
goto out;
}
- back = flexarray_make(16, 1);
- if (!back) {
- rc = ERROR_NOMEM;
- goto out_free;
- }
-
- if (disk->script) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
- " not yet supported, sorry");
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- rc = libxl__device_from_disk(gc, domid, disk, &device);
- if (rc != 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
- " virtual disk identifier %s", disk->vdev);
- goto out_free;
- }
-
- switch (disk->backend) {
- case LIBXL_DISK_BACKEND_PHY:
- dev = disk->pdev_path;
- do_backend_phy:
- libxl__device_physdisk_major_minor(dev, &major, &minor);
- flexarray_append(back, "physical-device");
- flexarray_append(back, libxl__sprintf(gc, "%x:%x", major,
minor));
-
- flexarray_append(back, "params");
- flexarray_append(back, dev);
-
- assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
- break;
- case LIBXL_DISK_BACKEND_TAP:
- dev = libxl__blktap_devpath(gc, disk->pdev_path,
disk->format);
- if (!dev) {
- rc = ERROR_FAIL;
- goto out_free;
- }
- flexarray_append(back, "tapdisk-params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format),
- disk->pdev_path));
-
- /* now create a phy device to export the device to the guest */
- goto do_backend_phy;
- case LIBXL_DISK_BACKEND_QDISK:
- flexarray_append(back, "params");
- flexarray_append(back, libxl__sprintf(gc, "%s:%s",
- libxl__device_disk_string_of_format(disk->format),
disk->pdev_path));
- assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
- break;
- default:
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend
type: %d\n", disk->backend);
- rc = ERROR_INVAL;
- goto out_free;
- }
-
- flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
- flexarray_append(back, "online");
- flexarray_append(back, "1");
- flexarray_append(back, "removable");
- flexarray_append(back, libxl__sprintf(gc, "%d",
(disk->removable) ? 1 : 0));
- flexarray_append(back, "bootable");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- flexarray_append(back, "dev");
- flexarray_append(back, disk->vdev);
- flexarray_append(back, "type");
- flexarray_append(back,
libxl__device_disk_string_of_backend(disk->backend));
- flexarray_append(back, "mode");
- flexarray_append(back, disk->readwrite ? "w" : "r");
- flexarray_append(back, "device-type");
- flexarray_append(back, disk->is_cdrom ? "cdrom" :
"disk");
-
- flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d",
disk->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
- flexarray_append(front, "virtual-device");
- flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
- flexarray_append(front, "device-type");
- flexarray_append(front, disk->is_cdrom ? "cdrom" :
"disk");
-
- libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back,
back->count),
- libxl__xs_kvs_of_flexarray(gc, front,
front->count));
- 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,
@@ -1433,19 +1272,25 @@ int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t
domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_disk(gc, domid, disk, &device);
+ GCNEW(device);
+ rc = libxl__device_from_disk(gc, domid, disk, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -1663,12 +1508,14 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk)
ret = 0;
+ /* fixme-ao */
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) {
+ /* fixme-ao */
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++)
@@ -1911,19 +1758,25 @@ int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t
domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_nic(gc, domid, nic, &device);
+ GCNEW(device);
+ rc = libxl__device_from_nic(gc, domid, nic, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -2273,19 +2126,25 @@ int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t
domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_vkb(gc, domid, vkb, &device);
+ GCNEW(device);
+ rc = libxl__device_from_vkb(gc, domid, vkb, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
@@ -2406,19 +2265,25 @@ int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t
domid,
const libxl_asyncop_how *ao_how)
{
AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
+ libxl__device *device;
+ libxl__ao_device *aorm = 0;
int rc;
- rc = libxl__device_from_vfb(gc, domid, vfb, &device);
+ GCNEW(device);
+ rc = libxl__device_from_vfb(gc, domid, vfb, device);
if (rc != 0) goto out;
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
+ GCNEW(aorm);
+ libxl__init_ao_device(aorm, ao, NULL);
+ aorm->action = DEVICE_DISCONNECT;
+ aorm->dev = device;
+ aorm->callback = libxl__device_cb;
- return AO_INPROGRESS;
+ libxl__initiate_device_remove(egc, aorm);
out:
- return AO_ABORT(rc);
+ if (rc) return AO_ABORT(rc);
+ return AO_INPROGRESS;
}
int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 0ff4a83..0ab1531 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -530,7 +530,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 */
@@ -660,7 +661,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 788e553..db58fd0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -400,7 +400,7 @@ int libxl__domain_make(libxl__gc *gc,
libxl_domain_create_info *info,
* on exit (even error exit), domid may be valid and refer to a domain */
{
libxl_ctx *ctx = libxl__gc_owner(gc);
- int flags, ret, rc;
+ int flags, ret, rc, nb_vm;
char *uuid_string;
char *dom_path, *vm_path, *libxl_path;
struct xs_permissions roperm[2];
@@ -521,6 +521,28 @@ retry_transaction:
libxl__sprintf(gc, "%s/hvmloader/generation-id-address",
dom_path),
rwperm, ARRAY_SIZE(rwperm));
+ if (libxl_list_vm(ctx, &nb_vm) < 0) {
+ LOGE(ERROR, "cannot get number of running guests");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (libxl_defbool_val(info->run_hotplug_scripts)) {
+ if (!libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
+ LOGE(ERROR, "cannot change hotplug execution option once set,
"
+ "please shutdown all guests before changing
it");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ libxl__xs_write(gc, t, DISABLE_UDEV_PATH, "1");
+ } else {
+ if (libxl__xs_read(gc, t, DISABLE_UDEV_PATH) && (nb_vm - 1)) {
+ LOGE(ERROR, "cannot change hotplug execution option once set,
"
+ "please shutdown all guests before changing
it");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ xs_rm(ctx->xsh, t, DISABLE_UDEV_PATH);
+ }
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path),
uuid_string, strlen(uuid_string));
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path),
info->name, strlen(info->name));
@@ -577,15 +599,25 @@ static void domcreate_bootloader_done(libxl__egc *egc,
libxl__bootloader_state *bl,
int rc);
+static void domcreate_disk_connected(libxl__egc *egc,
+ libxl__ao_device *aorm);
+
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
/* Our own function to clean up and call the user''s callback.
- * The final call in the sequence. */
+ * The final call in the sequence if domain creation is successful. */
static void domcreate_complete(libxl__egc *egc,
libxl__domain_create_state *dcs,
int rc);
+/* If creation is not successful, this callback will be executed
+ * when domain destruction is finished */
+
+static void domcreate_destruction_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc);
+
static void initiate_domain_create(libxl__egc *egc,
libxl__domain_create_state *dcs)
{
@@ -700,8 +732,13 @@ static void domcreate_bootloader_done(libxl__egc *egc,
store_libxl_entry(gc, domid, &d_config->b_info);
+ GCNEW_ARRAY(dcs->devices, d_config->num_disks);
+ dcs->num_devices = d_config->num_disks;
for (i = 0; i < d_config->num_disks; i++) {
- ret = libxl_device_disk_add(ctx, domid, &d_config->disks[i]);
+ libxl__init_ao_device(&dcs->devices[i], ao,
&dcs->devices);
+ dcs->devices[i].callback = domcreate_disk_connected;
+ ret = libxl__device_disk_add(egc, domid, &d_config->disks[i],
+ &dcs->devices[i]);
if (ret) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
"cannot add disk %d to domain: %d", i, ret);
@@ -718,6 +755,42 @@ static void domcreate_bootloader_done(libxl__egc *egc,
goto error_out;
}
}
+ return;
+
+ error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs,
devices);
+ int i, last, ret = 0;
+
+ /* convenience aliases */
+ const uint32_t domid = dcs->guest_domid;
+ libxl_domain_config *const d_config = dcs->guest_config;
+ libxl__domain_build_state *const state = &dcs->build_state;
+ libxl_ctx *const ctx = CTX;
+
+ ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
+ dcs->num_devices, &last);
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting disk devices");
+ goto error_out;
+ }
+
+ /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
+ * Fill in any field required by either, including both relevant
+ * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
+ dcs->dmss.dm.spawn.ao = ao;
+ dcs->dmss.dm.guest_config = dcs->guest_config;
+ dcs->dmss.dm.build_state = &dcs->build_state;
+ dcs->dmss.dm.callback = domcreate_devmodel_started;
+ dcs->dmss.callback = domcreate_devmodel_started;
+
switch (d_config->c_info.type) {
case LIBXL_DOMAIN_TYPE_HVM:
{
@@ -853,16 +926,31 @@ static void domcreate_complete(libxl__egc *egc,
if (rc) {
if (dcs->guest_domid) {
- int rc2 = libxl_domain_destroy(CTX, dcs->guest_domid);
- if (rc2)
- LOG(ERROR, "unable to destroy domain %d following"
- " failed creation", dcs->guest_domid);
+ dcs->dds.ao = ao;
+ dcs->dds.domid = dcs->guest_domid;
+ dcs->dds.callback = domcreate_destruction_cb;
+ libxl__domain_destroy(egc, &dcs->dds);
+ return;
}
dcs->guest_domid = -1;
}
dcs->callback(egc, dcs, rc, dcs->guest_domid);
}
+static void domcreate_destruction_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc)
+{
+ STATE_AO_GC(dds->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(dds, *dcs, dds);
+
+ if (rc)
+ LOG(ERROR, "unable to destroy domain %u following failed
creation",
+ dds->domid);
+
+ dcs->callback(egc, dcs, rc, dcs->guest_domid);
+}
+
/*----- application-facing domain creation interface -----*/
typedef struct {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c7e057d..275ab43 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -58,6 +58,48 @@ int libxl__parse_backend_path(libxl__gc *gc,
return libxl__device_kind_from_string(strkind, &dev->backend_kind);
}
+static int libxl__num_devices(libxl__gc *gc, uint32_t domid)
+{
+ char *path;
+ unsigned int num_kinds, num_devs;
+ char **kinds = NULL, **devs = NULL;
+ int i, j, rc = 0;
+ libxl__device dev;
+ libxl__device_kind kind;
+ int numdevs = 0;
+
+ path = GCSPRINTF("/local/domain/%d/device", domid);
+ kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
+ if (!kinds) {
+ if (errno != ENOENT) {
+ LOGE(ERROR, "unable to get xenstore device listing %s",
path);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ num_kinds = 0;
+ }
+ for (i = 0; i < num_kinds; i++) {
+ if (libxl__device_kind_from_string(kinds[i], &kind))
+ continue;
+
+ path = GCSPRINTF("/local/domain/%d/device/%s", domid,
kinds[i]);
+ devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
+ if (!devs)
+ continue;
+ for (j = 0; j < num_devs; j++) {
+ path = GCSPRINTF("/local/domain/%d/device/%s/%s/backend",
+ domid, kinds[i], devs[j]);
+ path = libxl__xs_read(gc, XBT_NULL, path);
+ if (path && libxl__parse_backend_path(gc, path, &dev)
== 0) {
+ numdevs++;
+ }
+ }
+ }
+out:
+ if (rc) return rc;
+ return numdevs;
+}
+
int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
char **bents, char **fents)
{
@@ -110,6 +152,178 @@ retry_transaction:
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:
+ LOGE(ERROR, "unrecognized disk backend type: %d",
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__egc *egc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ flexarray_t *front = NULL;
+ flexarray_t *back = NULL;
+ char *dev;
+ libxl__device *device;
+ int major, minor, rc;
+
+ rc = libxl__device_disk_setdefault(gc, disk);
+ if (rc) goto out;
+
+ front = flexarray_make(16, 1);
+ if (!front) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+ back = flexarray_make(20, 1);
+ if (!back) {
+ rc = ERROR_NOMEM;
+ goto out_free;
+ }
+
+ if (disk->script) {
+ LOGE(ERROR, "External block scripts not yet supported,
sorry");
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ GCNEW(device);
+ rc = libxl__device_from_disk(gc, domid, disk, device);
+ if (rc != 0) {
+ LOGE(ERROR, "Invalid or unsupported virtual disk identifier
%s",
+ disk->vdev);
+ goto out;
+ }
+
+ switch (disk->backend) {
+ case LIBXL_DISK_BACKEND_PHY:
+ dev = disk->pdev_path;
+ do_backend_phy:
+ libxl__device_physdisk_major_minor(dev, &major, &minor);
+ flexarray_append(back, "physical-device");
+ flexarray_append(back, GCSPRINTF("%x:%x", major, minor));
+
+ flexarray_append(back, "params");
+ flexarray_append(back, dev);
+
+ flexarray_append(back, "script");
+ flexarray_append(back, GCSPRINTF("%s/%s",
+ libxl__xen_script_dir_path(),
+ "block"));
+
+ assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
+ break;
+ case LIBXL_DISK_BACKEND_TAP:
+ dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
+ if (!dev) {
+ rc = ERROR_FAIL;
+ goto out_free;
+ }
+ flexarray_append(back, "tapdisk-params");
+ flexarray_append(back, GCSPRINTF("%s:%s",
+ libxl__device_disk_string_of_format(disk->format),
+ disk->pdev_path));
+
+ flexarray_append(back, "script");
+ flexarray_append(back, GCSPRINTF("%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:
+ flexarray_append(back, "params");
+ flexarray_append(back, GCSPRINTF("%s:%s",
+ libxl__device_disk_string_of_format(disk->format),
+ disk->pdev_path));
+ assert(device->backend_kind == LIBXL__DEVICE_KIND_QDISK);
+ break;
+ default:
+ LOGE(ERROR, "unrecognized disk backend type: %d",
disk->backend);
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, GCSPRINTF("%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "removable");
+ flexarray_append(back, GCSPRINTF("%d", (disk->removable) ? 1 :
0));
+ flexarray_append(back, "bootable");
+ flexarray_append(back, GCSPRINTF("%d", 1));
+ flexarray_append(back, "state");
+ flexarray_append(back, GCSPRINTF("%d", 1));
+ flexarray_append(back, "dev");
+ flexarray_append(back, disk->vdev);
+ flexarray_append(back, "type");
+ flexarray_append(back,
libxl__device_disk_string_of_backend(disk->backend));
+ flexarray_append(back, "mode");
+ flexarray_append(back, disk->readwrite ? "w" : "r");
+ flexarray_append(back, "device-type");
+ flexarray_append(back, disk->is_cdrom ? "cdrom" :
"disk");
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, GCSPRINTF("%d", disk->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, GCSPRINTF("%d", 1));
+ flexarray_append(front, "virtual-device");
+ flexarray_append(front, GCSPRINTF("%d", device->devid));
+ flexarray_append(front, "device-type");
+ flexarray_append(front, disk->is_cdrom ? "cdrom" :
"disk");
+
+ libxl__device_generic_add(gc, device,
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front, front->count));
+
+ aorm->dev = device;
+ aorm->action = DEVICE_CONNECT;
+ libxl__initiate_device_add(egc, aorm);
+
+ rc = 0;
+
+out_free:
+ flexarray_free(back);
+ flexarray_free(front);
+out:
+ return rc;
+}
+
typedef struct {
libxl__gc *gc;
libxl_device_disk *disk;
@@ -356,49 +570,116 @@ int libxl__device_disk_dev_number(const char *virtpath,
int *pdisk,
return -1;
}
-
-typedef struct {
- libxl__ao *ao;
- libxl__ev_devstate ds;
-} libxl__ao_device_remove;
-
-static void device_remove_cleanup(libxl__gc *gc,
- libxl__ao_device_remove *aorm) {
+static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *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__ao_complete(egc, aorm->ao, rc);
- device_remove_cleanup(gc, aorm);
+void libxl__init_ao_device(libxl__ao_device *aorm, libxl__ao *ao,
+ libxl__ao_device **base)
+{
+ aorm->ao = ao;
+ aorm->state = DEVICE_ACTIVE;
+ aorm->rc = 0;
+ aorm->base = base;
}
-int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
- libxl__device *dev)
+int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
+ libxl__ao_device *list, int num, int *last)
{
- AO_GC;
- libxl_ctx *ctx = libxl__gc_owner(gc);
- xs_transaction_t t;
- char *be_path = libxl__device_backend_path(gc, dev);
+ int i, ret = 0;
+
+ device->state = DEVICE_FINISHED;
+ *last = 1;
+ for (i = 0; i < num; i++) {
+ if (list[i].state != DEVICE_FINISHED && *last) {
+ *last = 0;
+ if (!device->rc) break;
+ }
+ if (device->rc && device->action == DEVICE_CONNECT) {
+ /* Cancel pending events */
+ if (list[i].pid) {
+ libxl__ev_time_deregister(gc, &list[i].ev);
+ kill(list[i].pid, SIGKILL);
+ } else {
+ libxl__ev_devstate_cancel(gc, &list[i].ds);
+ }
+ ret = list[i].rc;
+ }
+ }
+ return ret;
+}
+
+/* Common callbacks for device add/remove */
+
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+ int rc);
+
+void libxl__initiate_device_add(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ char *be_path = libxl__device_backend_path(gc, aorm->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_remove *aorm = 0;
+ if (aorm->dev->backend_kind == LIBXL__DEVICE_KIND_QDISK) {
+ aorm->callback(egc, aorm);
+ return;
+ }
+
+ if (atoi(state) == XenbusStateInitWait)
+ goto out_ok;
+
+ libxl__ev_devstate_init(&aorm->ds);
+ rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_backend_callback,
+ state_path, XenbusStateInitWait,
+ LIBXL_INIT_TIMEOUT * 1000);
+ if (rc) {
+ LOGE(ERROR, "unable to initialize device %s", be_path);
+ goto out_fail;
+ }
+
+ return;
+
+out_fail:
+ assert(rc);
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
+
+out_ok:
+ libxl__device_hotplug(egc, aorm);
+ return;
+}
+
+void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ xs_transaction_t t = 0;
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+ char *state;
+ int rc = 0;
+
+ if (aorm->force)
+ libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc,
aorm->dev));
+ /* Remove frontend xs paths to force backend disconnection */
+
+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) != XenbusStateConnected &&
+ atoi(state) != XenbusStateClosing) {
libxl__device_destroy_tapdisk(gc, be_path);
- xs_rm(ctx->xsh, XBT_NULL, 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, GCSPRINTF("%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)) {
if (errno == EAGAIN)
@@ -408,60 +689,67 @@ 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);
- aorm = libxl__zalloc(gc, sizeof(*aorm));
- aorm->ao = ao;
libxl__ev_devstate_init(&aorm->ds);
-
- rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_remove_callback,
+ rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_backend_callback,
state_path, XenbusStateClosed,
LIBXL_DESTROY_TIMEOUT * 1000);
- if (rc) goto out_fail;
+ if (rc) {
+ LOGE(ERROR, "unable to remove device %s", be_path);
+ goto out_fail;
+ }
- return 0;
+ return;
out_fail:
assert(rc);
- device_remove_cleanup(gc, aorm);
- return rc;
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
out_ok:
- libxl__ao_complete(egc, ao, 0);
- return 0;
+ if (t) xs_transaction_end(ctx->xsh, t, 0);
+ libxl__device_hotplug(egc, aorm);
+ return;
}
-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__device_destroy_tapdisk(gc, be_path);
+/* Callback for device destruction */
- return 0;
-}
+static void device_remove_callback(libxl__egc *egc, libxl__ao_device *aorm);
-int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
+void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
+ STATE_AO_GC(drs->ao);
char *path;
unsigned int num_kinds, num_devs;
char **kinds = NULL, **devs = NULL;
- int i, j;
- libxl__device dev;
+ int i, j, rc = 0, numdev = 0;
+ libxl__device *dev;
libxl__device_kind kind;
- path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
+ drs->num_devices = libxl__num_devices(gc, drs->domid);
+ if (drs->num_devices < 0) {
+ LOGE(ERROR, "unable to get number of devices for domain %u",
+ drs->domid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ GCNEW_ARRAY(drs->aorm, drs->num_devices);
+ for (i = 0; i < drs->num_devices; i++) {
+ libxl__init_ao_device(&drs->aorm[i], drs->ao,
&drs->aorm);
+ }
+
+ path = GCSPRINTF("/local/domain/%d/device", drs->domid);
kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
if (!kinds) {
if (errno != ENOENT) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get
xenstore"
- " device listing %s", path);
+ LOGE(ERROR, "unable to get xenstore device listing %s",
path);
+ rc = ERROR_FAIL;
goto out;
}
num_kinds = 0;
@@ -470,37 +758,106 @@ int libxl__devices_destroy(libxl__gc *gc, uint32_t domid)
if (libxl__device_kind_from_string(kinds[i], &kind))
continue;
- path = libxl__sprintf(gc, "/local/domain/%d/device/%s",
domid, kinds[i]);
+ path = GCSPRINTF("/local/domain/%d/device/%s", drs->domid,
kinds[i]);
devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
if (!devs)
continue;
for (j = 0; j < num_devs; j++) {
- path = libxl__sprintf(gc,
"/local/domain/%d/device/%s/%s/backend",
- domid, kinds[i], devs[j]);
+ path = GCSPRINTF("/local/domain/%d/device/%s/%s/backend",
+ drs->domid, kinds[i], devs[j]);
path = libxl__xs_read(gc, XBT_NULL, path);
- if (path && libxl__parse_backend_path(gc, path, &dev)
== 0) {
- dev.domid = domid;
- dev.kind = kind;
- dev.devid = atoi(devs[j]);
-
- libxl__device_destroy(gc, &dev);
+ GCNEW(dev);
+ if (path && libxl__parse_backend_path(gc, path, dev) == 0)
{
+ dev->domid = drs->domid;
+ dev->kind = kind;
+ dev->devid = atoi(devs[j]);
+ drs->aorm[numdev].action = DEVICE_DISCONNECT;
+ drs->aorm[numdev].dev = dev;
+ drs->aorm[numdev].callback = device_remove_callback;
+ libxl__initiate_device_remove(egc, &drs->aorm[numdev]);
+ numdev++;
}
}
}
/* console 0 frontend directory is not under
/local/domain/<domid>/device */
- path = libxl__sprintf(gc, "/local/domain/%d/console/backend",
domid);
+ path = GCSPRINTF("/local/domain/%d/console/backend",
drs->domid);
path = libxl__xs_read(gc, XBT_NULL, path);
+ GCNEW(dev);
if (path && strcmp(path, "") &&
- libxl__parse_backend_path(gc, path, &dev) == 0) {
- dev.domid = domid;
- dev.kind = LIBXL__DEVICE_KIND_CONSOLE;
- dev.devid = 0;
+ libxl__parse_backend_path(gc, path, dev) == 0) {
+ dev->domid = drs->domid;
+ dev->kind = LIBXL__DEVICE_KIND_CONSOLE;
+ dev->devid = 0;
- libxl__device_destroy(gc, &dev);
+ libxl__device_destroy(gc, dev);
}
out:
+ if (!numdev) drs->callback(egc, drs, rc);
+ return;
+}
+
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+ int rc)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(ds, *aorm, ds);
+ STATE_AO_GC(aorm->ao);
+
+ device_backend_cleanup(gc, aorm);
+
+ if (rc == ERROR_TIMEDOUT && aorm->action == DEVICE_DISCONNECT
&&
+ !aorm->force) {
+ aorm->force = 1;
+ libxl__initiate_device_remove(egc, aorm);
+ return;
+ }
+
+ if (rc) {
+ LOGE(ERROR, "unable to %s device with path %s",
+ aorm->action == DEVICE_DISCONNECT ?
"disconnect" :
+ "connect",
+ libxl__device_backend_path(gc, aorm->dev));
+ goto error;
+ }
+
+ libxl__device_hotplug(egc, aorm);
+ return;
+
+error:
+ aorm->rc = rc;
+ aorm->callback(egc, aorm);
+ return;
+}
+
+static void device_remove_callback(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__devices_remove_state *drs = CONTAINER_OF(aorm->base, *drs, aorm);
+ int rc = 0, last;
+
+ if (aorm->action == DEVICE_DISCONNECT) {
+ libxl__xs_path_cleanup(gc, libxl__device_frontend_path(gc,
aorm->dev));
+ libxl__xs_path_cleanup(gc, libxl__device_backend_path(gc,
aorm->dev));
+ }
+
+ rc = libxl__ao_device_check_last(gc, aorm, drs->aorm,
+ drs->num_devices, &last);
+
+ if (last)
+ drs->callback(egc, drs, rc);
+ return;
+}
+
+int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
+{
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *fe_path = libxl__device_frontend_path(gc, dev);
+
+ libxl__xs_path_cleanup(gc, be_path);
+ libxl__xs_path_cleanup(gc, fe_path);
+
+ libxl__device_destroy_tapdisk(gc, be_path);
return 0;
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5de3aef..b2622ee 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -660,6 +660,8 @@ retry_transaction:
return 0;
}
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
libxl__dm_spawn_state *stubdom_dmss,
int rc);
@@ -668,8 +670,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state *sdss)
{
STATE_AO_GC(sdss->dm.spawn.ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
- int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret;
- libxl__device_console *console;
+ int i, ret;
libxl_device_vfb vfb;
libxl_device_vkb vkb;
char **args;
@@ -787,22 +788,66 @@ retry_transaction:
if (errno == EAGAIN)
goto retry_transaction;
+ GCNEW_ARRAY(sdss->devices, dm_config->num_disks);
+ sdss->num_devices = dm_config->num_disks;
for (i = 0; i < dm_config->num_disks; i++) {
- ret = libxl_device_disk_add(ctx, dm_domid,
&dm_config->disks[i]);
- if (ret)
+ libxl__init_ao_device(&sdss->devices[i], ao,
&sdss->devices);
+ sdss->devices[i].callback = spawn_stub_disk_connected;
+ ret = libxl__device_disk_add(egc, dm_domid,
&dm_config->disks[i],
+ &sdss->devices[i]);
+ if (ret) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "cannot add disk %d to domain: %d", i, ret);
+ ret = ERROR_FAIL;
goto out_free;
+ }
+ }
+
+ free(args);
+ return;
+
+out_free:
+ free(args);
+out:
+ assert(ret);
+ spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+}
+
+static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aorm->base, *sdss,
devices);
+ int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret = 0, last;
+ libxl__device_console *console;
+
+ /* convenience aliases */
+ libxl_domain_config *const dm_config = &sdss->dm_config;
+ libxl_domain_config *const guest_config = sdss->dm.guest_config;
+ const int guest_domid = sdss->dm.guest_domid;
+ libxl__domain_build_state *const d_state = sdss->dm.build_state;
+ libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
+ uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+ ret = libxl__ao_device_check_last(gc, aorm, sdss->devices,
+ sdss->num_devices, &last);
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting disk devices");
+ goto out;
}
+
for (i = 0; i < dm_config->num_vifs; i++) {
ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
if (ret)
- goto out_free;
+ goto out;
}
ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
if (ret)
- goto out_free;
+ goto out;
ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0]);
if (ret)
- goto out_free;
+ goto out;
if (guest_config->b_info.u.hvm.serial)
num_console++;
@@ -810,7 +855,7 @@ retry_transaction:
console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
if (!console) {
ret = ERROR_NOMEM;
- goto out_free;
+ goto out;
}
for (i = 0; i < num_console; i++) {
@@ -846,7 +891,7 @@ retry_transaction:
ret = libxl__device_console_add(gc, dm_domid, &console[i],
i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL);
if (ret)
- goto out_free;
+ goto out;
}
sdss->pvqemu.guest_domid = dm_domid;
@@ -856,13 +901,9 @@ retry_transaction:
libxl__spawn_local_dm(egc, &sdss->pvqemu);
- free(args);
return;
-out_free:
- free(args);
out:
- assert(ret);
spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
}
@@ -883,7 +924,7 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
out:
if (rc) {
if (dm_domid)
- libxl_domain_destroy(CTX, dm_domid);
+ libxl_domain_destroy(CTX, dm_domid, 0);
}
sdss->callback(egc, &sdss->dm, rc);
}
@@ -1075,62 +1116,35 @@ static void device_model_spawn_outcome(libxl__egc *egc,
int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
char *pid;
int ret;
- pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc,
"/local/domain/%d/image/device-model-pid", domid));
- if (!pid) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn''t find
device model''s pid");
- ret = ERROR_INVAL;
- goto out;
- }
- if (!atoi(pid)) {
- int stubdomid = libxl_get_stubdom_id(ctx, domid);
- const char *savefile;
-
- if (!stubdomid) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn''t
find device model''s pid");
+ pid = libxl__xs_read(gc, XBT_NULL,
+
GCSPRINTF("/local/domain/%d/image/device-model-pid",
+ domid));
+ if (!pid || !atoi(pid)) {
+ LOGE(ERROR, "Couldn''t find device model''s
pid");
ret = ERROR_INVAL;
goto out;
- }
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom,
domid=%d", stubdomid);
- ret = libxl_domain_destroy(ctx, stubdomid);
- if (ret)
- goto out;
-
- savefile = libxl__device_model_savefile(gc, domid);
- ret = unlink(savefile);
- /*
- * On suspend libxl__domain_save_device_model will have already
- * unlinked the save file.
- */
- if (ret && errno == ENOENT) ret = 0;
- if (ret) {
- LIBXL__LOG_ERRNO(ctx, XTL_ERROR,
- "failed to remove device-model savefile
%s\n",
- savefile);
- goto out;
- }
+ }
+ ret = kill(atoi(pid), SIGHUP);
+ if (ret < 0 && errno == ESRCH) {
+ LOG(DEBUG, "Device Model already exited");
+ ret = 0;
+ } else if (ret == 0) {
+ LOG(DEBUG, "Device Model signaled");
+ ret = 0;
} else {
- ret = kill(atoi(pid), SIGHUP);
- if (ret < 0 && errno == ESRCH) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model already
exited");
- ret = 0;
- } else if (ret == 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device Model
signaled");
- ret = 0;
- } else {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to kill Device
Model [%d]",
- atoi(pid));
- ret = ERROR_FAIL;
- goto out;
- }
+ LOGEV(ERROR, errno, "failed to kill Device Model [%d]",
atoi(pid));
+ ret = ERROR_FAIL;
+ goto out;
}
out:
- xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc,
"/local/domain/0/device-model/%d", domid));
- xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc,
"/local/domain/%d/hvmloader", domid));
+ xs_rm(CTX->xsh, XBT_NULL,
GCSPRINTF("/local/domain/0/device-model/%d",
+ domid));
+ xs_rm(CTX->xsh, XBT_NULL,
GCSPRINTF("/local/domain/%d/hvmloader",
+ domid));
return ret;
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index c246211..9b016e8 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -973,6 +973,176 @@ out:
return rc;
}
+/* Callbacks for libxl__domain_destroy */
+static void stubdom_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc);
+static void domain_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc);
+
+void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
+{
+ STATE_AO_GC(dds->ao);
+ uint32_t stubdomid = libxl_get_stubdom_id(CTX, dds->domid);
+
+ if (stubdomid) {
+ dds->stubdom.ao = ao;
+ dds->stubdom.domid = stubdomid;
+ dds->stubdom.callback = stubdom_callback;
+ libxl__destroy_domid(egc, &dds->stubdom);
+ } else {
+ dds->stubdom_finished = 1;
+ }
+
+ dds->domain.ao = ao;
+ dds->domain.domid = dds->domid;
+ dds->domain.callback = domain_callback;
+ libxl__destroy_domid(egc, &dds->domain);
+}
+
+static void stubdom_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc)
+{
+ STATE_AO_GC(dis->ao);
+ libxl__domain_destroy_state *dds = CONTAINER_OF(dis, *dds, stubdom);
+ const char *savefile;
+
+ if (rc)
+ LOGE(ERROR, "unable to destroy stubdom with domid %u",
dis->domid);
+
+ dds->stubdom_finished = 1;
+ savefile = libxl__device_model_savefile(gc, dis->domid);
+ rc = unlink(savefile);
+ /*
+ * On suspend libxl__domain_save_device_model will have already
+ * unlinked the save file.
+ */
+ if (rc && errno == ENOENT) rc = 0;
+ if (rc) {
+ LOGEV(ERROR, errno, "failed to remove device-model savefile
%s",
+ savefile);
+ }
+
+ if (dds->domain_finished)
+ dds->callback(egc, dds, rc);
+}
+
+static void domain_callback(libxl__egc *egc, libxl__destroy_domid_state *dis,
+ int rc)
+{
+ STATE_AO_GC(dis->ao);
+ libxl__domain_destroy_state *dds = CONTAINER_OF(dis, *dds, domain);
+
+ if (rc)
+ LOGE(ERROR, "unable to destroy guest with domid %u",
dis->domid);
+
+ dds->domain_finished = 1;
+ if (dds->stubdom_finished)
+ dds->callback(egc, dds, rc);
+}
+
+/* Callbacks for libxl__domain_clean */
+
+/* Device destruction callback */
+static void devices_destroy_cb(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc);
+
+void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
+{
+ STATE_AO_GC(dis->ao);
+ int rc;
+
+ rc = libxl_domain_info(CTX, NULL, dis->domid);
+ switch (rc) {
+ case 0:
+ break;
+ case ERROR_INVAL:
+ LOGE(ERROR, "non-existant domain %d", dis->domid);
+ default:
+ goto out;
+ }
+
+ switch (libxl__domain_type(gc, dis->domid)) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ dis->dm_present = 1;
+ break;
+ case LIBXL_DOMAIN_TYPE_PV:
+ dis->pid = libxl__xs_read(gc, XBT_NULL,
+
GCSPRINTF("/local/domain/%d/image/device-model-pid",
+ dis->domid));
+ dis->dm_present = (dis->pid != NULL);
+ break;
+ default:
+ abort();
+ }
+
+ dis->dom_path = libxl__xs_get_dompath(gc, dis->domid);
+ if (!dis->dom_path) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ if (libxl__device_pci_destroy_all(gc, dis->domid) < 0)
+ LOGE(ERROR, "pci shutdown failed for domid %d",
dis->domid);
+ rc = xc_domain_pause(CTX->xch, dis->domid);
+ if (rc < 0) {
+ LOGEV(ERROR, rc, "xc_domain_pause failed for %d",
dis->domid);
+ }
+ if (dis->dm_present) {
+ rc = libxl__destroy_device_model(gc, dis->domid);
+ if (rc < 0)
+ LOGE(ERROR, "libxl__destroy_device_model failed for %d",
+ dis->domid);
+ libxl__qmp_cleanup(gc, dis->domid);
+ }
+ dis->drs.ao = ao;
+ dis->drs.domid = dis->domid;
+ dis->drs.callback = devices_destroy_cb;
+ libxl__devices_destroy(egc, &dis->drs);
+ return;
+
+out:
+ assert(rc);
+ dis->callback(egc, dis, rc);
+ return;
+}
+
+static void devices_destroy_cb(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc)
+{
+ STATE_AO_GC(drs->ao);
+ libxl__destroy_domid_state *dis = CONTAINER_OF(drs, *dis, drs);
+
+ if (rc < 0)
+ LOGE(ERROR, "libxl__devices_destroy failed for %d",
dis->domid);
+
+ dis->vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/vm",
+ dis->dom_path));
+ if (dis->vm_path)
+ if (!xs_rm(CTX->xsh, XBT_NULL, dis->vm_path))
+ LOGE(ERROR, "xs_rm failed for %s", dis->vm_path);
+
+ if (!xs_rm(CTX->xsh, XBT_NULL, dis->dom_path))
+ LOGE(ERROR, "xs_rm failed for %s", dis->dom_path);
+
+ xs_rm(CTX->xsh, XBT_NULL, libxl__xs_libxl_path(gc, dis->domid));
+
+ libxl__userdata_destroyall(gc, dis->domid);
+
+ rc = xc_domain_destroy(CTX->xch, dis->domid);
+ if (rc < 0) {
+ LOGEV(ERROR, rc, "xc_domain_destroy failed for %d",
dis->domid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ rc = 0;
+
+out:
+ dis->callback(egc, dis, rc);
+ return;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_hotplug.c b/tools/libxl/libxl_hotplug.c
new file mode 100644
index 0000000..686a38d
--- /dev/null
+++ b/tools/libxl/libxl_hotplug.c
@@ -0,0 +1,84 @@
+/*
+ * 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 device_hotplug_timeout(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(ev, *aorm, ev);
+ STATE_AO_GC(aorm->ao);
+
+ if (!aorm) return;
+ if (libxl__ev_child_inuse(&aorm->child)) {
+ if (kill(aorm->pid, SIGKILL)) {
+ LOGEV(ERROR, errno, "unable to kill hotplug script %s
[%ld]",
+ aorm->what, (unsigned long)aorm->pid);
+ goto out;
+ }
+ }
+
+out:
+ libxl__ev_time_deregister(gc, &aorm->ev);
+ return;
+}
+
+int libxl__hotplug_launch(libxl__gc *gc, libxl__ao_device *aorm,
+ const char *arg0, char *const args[],
+ char *const env[], libxl__ev_child_callback *death)
+{
+ int rc = 0;
+ pid_t pid = -1;
+
+ libxl__ev_time_init(&aorm->ev);
+ libxl__ev_child_init(&aorm->child);
+
+ rc = libxl__ev_time_register_rel(gc, &aorm->ev,
device_hotplug_timeout,
+ LIBXL_HOTPLUG_TIMEOUT * 1000);
+ if (rc) {
+ LOGE(ERROR, "unable to register timeout for hotplug script
%s", arg0);
+ goto out;
+ }
+
+ pid = libxl__ev_child_fork(gc, &aorm->child, death);
+ if (pid == -1) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (!pid) {
+ /* child */
+ libxl__exec(gc, -1, -1, -1, arg0, args, env);
+ LOGE(ERROR, "unable execute hotplug scripts for device
%"PRIu32 " on "
+ "dom %"PRIu32, aorm->dev->devid,
aorm->dev->backend_domid);
+ abort();
+ }
+out:
+ if (libxl__ev_child_inuse(&aorm->child)) {
+ aorm->pid = pid;
+ /* we will get a callback when the child dies */
+ return 0;
+ }
+ libxl__ev_time_deregister(gc, &aorm->ev);
+ return rc;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fc2024d..faeb965 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -71,6 +71,8 @@
#include "_libxl_types_internal_json.h"
#define LIBXL_DESTROY_TIMEOUT 10
+#define LIBXL_INIT_TIMEOUT 10
+#define LIBXL_HOTPLUG_TIMEOUT 10
#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
#define LIBXL_XENCONSOLE_LIMIT 1048576
#define LIBXL_XENCONSOLE_PROTOCOL "vt100"
@@ -85,6 +87,7 @@
#define STUBDOM_CONSOLE_SERIAL 3
#define STUBDOM_SPECIAL_CONSOLES 3
#define TAP_DEVICE_SUFFIX "-emu"
+#define DISABLE_UDEV_PATH "libxl/disable_udev"
#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
@@ -779,6 +782,9 @@ _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_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,
@@ -795,7 +801,6 @@ _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__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
/*
@@ -826,13 +831,6 @@ _hidden const char *libxl__device_nic_devname(libxl__gc
*gc,
uint32_t devid,
libxl_nic_type type);
-/* Arranges that dev will be removed from its guest. When
- * this is done, the ao will be completed. An error
- * return from libxl__initiate_device_remove means that the ao
- * will _not_ be completed and the caller must do so. */
-_hidden int libxl__initiate_device_remove(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.
@@ -918,6 +916,65 @@ _hidden int libxl__create_pci_backend(libxl__gc *gc,
uint32_t domid,
libxl_device_pci *pcidev, int num);
_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
+/*----- device addition/removal -----*/
+
+/* During the init/destruction process, the device can be in several states:
+ *
+ * DEVICE_UNKNOWN: device in unknown state (default value when struct is
+ * initialized).
+ *
+ * DEVICE_WAIT_BACKEND: waiting for the backend to switch to XenbusStateInit or
+ * XenbusStateClosed.
+ *
+ * DEVICE_WAIT_HOTPLUG: waiting for hotplug script to finish execution.
+ *
+ * DEVICE_FINISHED: device is connected/disconnected.
+ */
+typedef enum {
+ DEVICE_ACTIVE,
+ DEVICE_FINISHED
+} libxl__device_state;
+
+/* Action to perform (either connect or disconnect) */
+typedef enum {
+ DEVICE_CONNECT,
+ DEVICE_DISCONNECT
+} libxl__device_action;
+
+typedef struct libxl__ao_device libxl__ao_device;
+typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
+
+_hidden void libxl__init_ao_device(libxl__ao_device *aorm, libxl__ao *ao,
+ libxl__ao_device **base);
+_hidden int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device
*device,
+ libxl__ao_device *list,
+ int num, int *last);
+
+struct libxl__ao_device {
+ libxl__ao *ao;
+ /* State in which the device is */
+ libxl__device_state state;
+ /* action being performed */
+ libxl__device_action action;
+ /* device teardown (back for xenstore backend to connect/disconnect) */
+ libxl__device *dev;
+ int force;
+ /* device hotplug execution */
+ pid_t pid;
+ char *what;
+ libxl__device_callback *callback;
+ /* private for implementation */
+ int rc;
+ libxl__ev_time ev;
+ libxl__ev_child child;
+ libxl__ev_devstate ds;
+ void *base;
+};
+
+_hidden int libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_disk *disk,
+ libxl__ao_device *aorm);
+
/*
*----- spawn -----
*
@@ -1106,6 +1163,8 @@ typedef struct {
libxl_domain_config dm_config;
libxl__domain_build_state dm_state;
libxl__dm_spawn_state pvqemu;
+ libxl__ao_device *devices;
+ int num_devices;
} libxl__stub_dm_spawn_state;
_hidden void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state*);
@@ -1797,6 +1856,98 @@ _hidden void
libxl__bootloader_init(libxl__bootloader_state *bl);
* If callback is passed rc==0, will have updated st->info appropriately */
_hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
+/*
+ * libxl__hotplug_launch is an internal function that should not be used
+ * directly, all hotplug script calls have to implement and use
+ * libxl__device_hotplug.
+ */
+_hidden void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm);
+_hidden int libxl__hotplug_launch(libxl__gc *gc, libxl__ao_device *aorm,
+ const char *arg0, char *const args[],
+ char *const env[],
+ libxl__ev_child_callback *death);
+
+/* Arranges that dev will be added to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aorm->callback will be called.
+ */
+_hidden void libxl__initiate_device_add(libxl__egc*, libxl__ao_device *aorm);
+
+/* Arranges that dev will be removed to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aorm->callback will be called.
+ */
+_hidden void libxl__initiate_device_remove(libxl__egc *egc,
+ libxl__ao_device *aorm);
+
+/*----- Domain destruction -----*/
+
+typedef struct libxl__domain_destroy_state libxl__domain_destroy_state;
+typedef struct libxl__destroy_domid_state libxl__destroy_domid_state;
+typedef struct libxl__devices_remove_state libxl__devices_remove_state;
+
+typedef void libxl__domain_destroy_cb(libxl__egc *egc,
+ libxl__domain_destroy_state *dds,
+ int rc);
+
+typedef void libxl__domain_clean_cb(libxl__egc *egc,
+ libxl__destroy_domid_state *dis,
+ int rc);
+
+typedef void libxl__devices_remove_callback(libxl__egc *egc,
+ libxl__devices_remove_state *drs,
+ int rc);
+
+struct libxl__devices_remove_state {
+ /* set by user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__devices_remove_callback *callback;
+ /* private */
+ libxl__ao_device *aorm;
+ int num_devices;
+};
+
+struct libxl__destroy_domid_state {
+ /* filled in by user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__domain_clean_cb *callback;
+ /* private to implementation */
+ libxl__devices_remove_state drs;
+ char *dom_path;
+ char *vm_path;
+ char *pid;
+ int dm_present;
+};
+
+struct libxl__domain_destroy_state {
+ /* filled by the user */
+ libxl__ao *ao;
+ uint32_t domid;
+ libxl__domain_destroy_cb *callback;
+ /* Private */
+ uint32_t stubdomid;
+ libxl__destroy_domid_state stubdom;
+ int stubdom_finished;
+ libxl__destroy_domid_state domain;
+ int domain_finished;
+};
+
+/* Entry point for domain destruction */
+_hidden void libxl__domain_destroy(libxl__egc *egc,
+ libxl__domain_destroy_state *dds);
+
+/* Used to destroy a domain with the passed id (it doesn''t check for
stubs) */
+_hidden void libxl__destroy_domid(libxl__egc *egc,
+ libxl__destroy_domid_state *dis);
+
+/* Entry point for devices destruction */
+_hidden void libxl__devices_destroy(libxl__egc *egc,
+ libxl__devices_remove_state *drs);
+
/*----- Domain creation -----*/
typedef struct libxl__domain_create_state libxl__domain_create_state;
@@ -1816,6 +1967,9 @@ struct libxl__domain_create_state {
int guest_domid;
libxl__domain_build_state build_state;
libxl__bootloader_state bl;
+ libxl__ao_device *devices;
+ int num_devices;
+ libxl__domain_destroy_state dds;
libxl__stub_dm_spawn_state dmss;
/* If we''re not doing stubdom, we use only dmss.dm,
* for the non-stubdom device model. */
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 925248b..315ce15 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -25,3 +25,129 @@ int libxl__try_phy_backend(mode_t st_mode)
return 1;
}
+
+/* Hotplug scripts helpers */
+
+static void cleanup(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ if (!aorm) return;
+ libxl__ev_time_deregister(gc, &aorm->ev);
+}
+
+static void callback(libxl__egc *egc, libxl__ev_child *child,
+ pid_t pid, int status)
+{
+ libxl__ao_device *aorm = CONTAINER_OF(child, *aorm, child);
+ STATE_AO_GC(aorm->ao);
+
+ cleanup(gc, aorm);
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, aorm->rc ? LIBXL__LOG_ERROR
+ : LIBXL__LOG_WARNING,
+ aorm->what, pid, status);
+ aorm->rc = ERROR_FAIL;
+ }
+ aorm->callback(egc, aorm);
+}
+
+static char **get_hotplug_env(libxl__gc *gc, libxl__device *dev)
+{
+ char *be_path = libxl__device_backend_path(gc, dev);
+ char *script;
+ const char *type = libxl__device_kind_to_string(dev->backend_kind);
+ char **env;
+ int nr = 0;
+
+ script = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF("%s/%s", be_path,
"script"));
+ if (!script) {
+ LOGEV(ERROR, errno, "unable to read script from %s",
be_path);
+ return NULL;
+ }
+
+ GCNEW_ARRAY(env, 9);
+ env[nr++] = "script";
+ env[nr++] = script;
+ env[nr++] = "XENBUS_TYPE";
+ env[nr++] = libxl__strdup(gc, type);
+ env[nr++] = "XENBUS_PATH";
+ env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid,
dev->devid);
+ env[nr++] = "XENBUS_BASE_PATH";
+ env[nr++] = "backend";
+ env[nr++] = NULL;
+
+ return env;
+}
+
+/* Hotplug scripts caller functions */
+
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *script;
+ char **args, **env;
+ int nr = 0, rc = 0;
+
+ script = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF("%s/%s", be_path,
"script"));
+ if (!script) {
+ LOGEV(ERROR, errno, "unable to read script from %s",
be_path);
+ return ERROR_FAIL;
+ }
+
+ env = get_hotplug_env(gc, aorm->dev);
+ if (!env)
+ return ERROR_FAIL;
+
+ GCNEW_ARRAY(args, 3);
+
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "add" :
"remove";
+ args[nr++] = NULL;
+
+ aorm->what = GCSPRINTF("%s %s", args[0], args[1]);
+ LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ goto out_free;
+ }
+
+ rc = 0;
+
+out_free:
+ return rc;
+}
+
+void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ char *disable_udev = libxl__xs_read(gc, XBT_NULL, DISABLE_UDEV_PATH);
+
+ /* Check if we have to run hotplug scripts */
+ if (!disable_udev) goto out;
+
+ switch (aorm->dev->backend_kind) {
+ case LIBXL__DEVICE_KIND_VBD:
+ aorm->rc = libxl__hotplug_disk(gc, aorm);
+ if (aorm->rc) goto error;
+ break;
+ default:
+ /* If no need to execute any hotplug scripts,
+ * call the callback manually
+ */
+ aorm->rc = 0;
+ aorm->callback(egc, aorm);
+ break;
+ }
+
+ return;
+
+error:
+ assert(aorm->rc);
+ LOGE(ERROR, "unable to queue execution of hotplug script for device
"
+ "with path %s", libxl__device_backend_path(gc,
aorm->dev));
+out:
+ aorm->callback(egc, aorm);
+ return;
+}
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 551e367..b1351de 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -220,6 +220,7 @@ libxl_domain_create_info =
Struct("domain_create_info",[
("xsdata", libxl_key_value_list),
("platformdata", libxl_key_value_list),
("poolid", uint32),
+ ("run_hotplug_scripts",libxl_defbool),
], dir=DIR_IN)
MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index d4db1f8..d992c32 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;
+libxl_defbool run_hotplug_scripts;
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;
+ libxl_defbool_setdefault(&run_hotplug_scripts, true);
+ xlu_cfg_get_defbool(config, "run_hotplug_scripts",
&run_hotplug_scripts, 0);
+
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 2b6714a..43d727a 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -109,6 +109,7 @@ void postfork(void);
/* global options */
extern int autoballoon;
+extern libxl_defbool run_hotplug_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 8908581..3f1bbf6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -562,6 +562,7 @@ static void parse_config_data(const char
*configfile_filename_report,
}
}
+ c_info->run_hotplug_scripts = run_hotplug_scripts;
c_info->type = LIBXL_DOMAIN_TYPE_PV;
if (!xlu_cfg_get_string (config, "builder", &buf, 0)
&&
!strncmp(buf, "hvm", strlen(buf)))
@@ -1333,7 +1334,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:
@@ -1899,7 +1900,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)
@@ -2386,7 +2387,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); }
}
@@ -2660,7 +2661,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);
}
@@ -2892,7 +2893,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);
@@ -3010,7 +3011,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);
@@ -5077,7 +5078,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 c4f7c52..ce7a2b2 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -447,7 +447,7 @@ static PyObject *pyxl_domain_destroy(XlObject *self,
PyObject *args)
int domid;
if ( !PyArg_ParseTuple(args, "i", &domid) )
return NULL;
- if ( libxl_domain_destroy(self->ctx, domid) ) {
+ if ( libxl_domain_destroy(self->ctx, domid, NULL) ) {
PyErr_SetString(xl_error_obj, "cannot destroy domain");
return NULL;
}
--
1.7.7.5 (Apple Git-26)
Roger Pau Monne
2012-May-10 11:28 UTC
[PATCH v4 4/4] libxl: call hotplug scripts from libxl for vif
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.
Network devices are added after the domain model has launched, since
we need the tap inteface to be present when the hotplug script
is executed.
PV nic devices are set to LIBXL_NIC_TYPE_VIF, since the default value
is LIBXL_NIC_TYPE_IOEMU regardless of the guest type.
Changes since v3:
* Entangle network device addition with the new AO domain creation,
assute that network interfaces are always added after device model
creation. Make sure the same happens for stubdomain network
interfaces.
* Make use of the new functions LOG, LOGE, GCNEW_ARRAY...
* Remove disable_xl_vif_scripts, since now it''s a global option that
affects both vifs and disks hotplug scripts.
* Added proper handling of IOEMU type of nics, we need to
execute two different hotplug scripts. Now both scripts are
chained, and one is executed after the other has finished (we call
the next script from the callback of the previous one). Previously
we used to launch both scripts simultaneously.
Changes since v2:
* Added fancy functions to fetch tap and vif names, now the prefix of
the tap device has been saved in a constant, called
TAP_DEVICE_PREFIX.
* Wait for timeout before nuking frontend xenstore entries.
* Changed disable_vif_scripts to disable_xl_vif_scripts, it''s easier
to understand.
* Default nic type set by libxl__device_nic_setdefault is VIF now
(was IOEMU).
* Save nic type and udev script exection inside
/libxl/devices/<domid>/nic/<devid>/ instead of saving it in the
backend path.
Changes since v1:
* Propagated changes from previous patch (disable_udev and
libxl_device_nic_add switch).
* Modified udev rules to add the new env variable.
* Added support for named interfaces.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
tools/hotplug/Linux/xen-backend.rules | 6 +-
tools/libxl/libxl.c | 122 ++++---------------------------
tools/libxl/libxl.h | 3 +-
tools/libxl/libxl_create.c | 78 ++++++++++++++++++--
tools/libxl/libxl_device.c | 122 +++++++++++++++++++++++++++++++
tools/libxl/libxl_dm.c | 64 +++++++++++++++-
tools/libxl/libxl_internal.h | 9 ++
tools/libxl/libxl_linux.c | 130 ++++++++++++++++++++++++++++++++-
tools/libxl/xl_cmdimpl.c | 2 +-
9 files changed, 412 insertions(+), 124 deletions(-)
diff --git a/tools/hotplug/Linux/xen-backend.rules
b/tools/hotplug/Linux/xen-backend.rules
index d55ff11..c591a3f 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -2,8 +2,8 @@ SUBSYSTEM=="xen-backend", KERNEL=="tap*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scr
SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block
$env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*",
RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*",
RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online
type_if=vif"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline
type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ENV{UDEV_CALL}="1", ACTION=="online",
RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*",
ENV{UDEV_CALL}="1", ACTION=="offline",
RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*",
RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
SUBSYSTEM=="xen-backend", ACTION=="remove",
ENV{UDEV_CALL}="1",
RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
@@ -13,4 +13,4 @@ KERNEL=="blktap-control",
NAME="xen/blktap-2/control", MODE="0600"
KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m",
MODE="0600"
-SUBSYSTEM=="net", KERNEL=="vif*-emu",
ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION}
type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu",
ACTION=="add", ENV{UDEV_CALL}="1",
RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b438fbf..5a681b1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1634,123 +1634,29 @@ int libxl__device_nic_setdefault(libxl__gc *gc,
libxl_device_nic *nic)
libxl__xen_script_dir_path()) < 0 )
return ERROR_FAIL;
if (!nic->nictype)
- nic->nictype = LIBXL_NIC_TYPE_IOEMU;
+ nic->nictype = LIBXL_NIC_TYPE_VIF;
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);
- flexarray_t *front;
- flexarray_t *back;
- libxl__device device;
- char *dompath, **l;
- unsigned int nb, rc;
-
- rc = libxl__device_nic_setdefault(gc, nic);
- if (rc) goto out;
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__ao_device *device;
+ int rc;
- front = flexarray_make(16, 1);
- if (!front) {
- rc = ERROR_NOMEM;
+ GCNEW(device);
+ libxl__init_ao_device(device, ao, NULL);
+ device->callback = libxl__device_cb;
+ rc = libxl__device_nic_add(egc, domid, nic, device);
+ if (rc) {
+ LOGE(ERROR, "unable to add nic %s", nic->ifname);
goto out;
}
- back = flexarray_make(16, 1);
- if (!back) {
- rc = ERROR_NOMEM;
- goto out_free;
- }
-
- if (nic->devid == -1) {
- if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
- rc = ERROR_FAIL;
- goto out_free;
- }
- if (!(l = libxl__xs_directory(gc, XBT_NULL,
- libxl__sprintf(gc,
"%s/device/vif", dompath), &nb))) {
- nic->devid = 0;
- } else {
- nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
- }
- }
-
- rc = libxl__device_from_nic(gc, domid, nic, &device);
- if ( rc != 0 ) goto out_free;
-
- flexarray_append(back, "frontend-id");
- flexarray_append(back, libxl__sprintf(gc, "%d", domid));
- flexarray_append(back, "online");
- flexarray_append(back, "1");
- flexarray_append(back, "state");
- flexarray_append(back, libxl__sprintf(gc, "%d", 1));
- if (nic->script) {
- flexarray_append(back, "script");
- flexarray_append(back, nic->script[0]==''/'' ?
nic->script
- : libxl__sprintf(gc, "%s/%s",
- libxl__xen_script_dir_path(),
- nic->script));
- }
-
- if (nic->ifname) {
- flexarray_append(back, "vifname");
- flexarray_append(back, nic->ifname);
- }
-
- flexarray_append(back, "mac");
- flexarray_append(back,libxl__sprintf(gc,
- LIBXL_MAC_FMT,
LIBXL_MAC_BYTES(nic->mac)));
- if (nic->ip) {
- flexarray_append(back, "ip");
- flexarray_append(back, libxl__strdup(gc, nic->ip));
- }
-
- if (nic->rate_interval_usecs > 0) {
- flexarray_append(back, "rate");
- flexarray_append(back, libxl__sprintf(gc,
"%"PRIu64",%"PRIu32"",
- nic->rate_bytes_per_interval,
- nic->rate_interval_usecs));
- }
- flexarray_append(back, "bridge");
- flexarray_append(back, libxl__strdup(gc, nic->bridge));
- flexarray_append(back, "handle");
- flexarray_append(back, libxl__sprintf(gc, "%d", nic->devid));
-
- flexarray_append(front, "backend-id");
- flexarray_append(front, libxl__sprintf(gc, "%d",
nic->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, libxl__sprintf(gc, "%d", 1));
- flexarray_append(front, "handle");
- flexarray_append(front, libxl__sprintf(gc, "%d", nic->devid));
- flexarray_append(front, "mac");
- flexarray_append(front, libxl__sprintf(gc,
- LIBXL_MAC_FMT,
LIBXL_MAC_BYTES(nic->mac)));
- libxl__device_generic_add(gc, &device,
- libxl__xs_kvs_of_flexarray(gc, back,
back->count),
- libxl__xs_kvs_of_flexarray(gc, front,
front->count));
-
- /* FIXME: wait for plug */
- 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 0ab1531..6689baf 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -688,7 +688,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 db58fd0..69f4e4d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -605,6 +605,11 @@ static void domcreate_disk_connected(libxl__egc *egc,
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
+static void domcreate_nics_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
+static void domcreate_attach_pci(libxl__egc *egc,
+ libxl__domain_create_state *dcs);
+
/* Our own function to clean up and call the user''s callback.
* The final call in the sequence if domain creation is successful. */
static void domcreate_complete(libxl__egc *egc,
@@ -747,13 +752,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
}
}
for (i = 0; i < d_config->num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, domid, &d_config->vifs[i]);
- if (ret) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "cannot add nic %d to domain: %d", i, ret);
- ret = ERROR_FAIL;
- goto error_out;
- }
+ /* We have to init the nic here, because we still haven''t
+ * called libxl_device_nic_add at this point, but qemu needs
+ * the nic information to be complete.
+ */
+ libxl__device_nic_setdefault(gc, &d_config->vifs[i]);
}
return;
@@ -885,6 +888,67 @@ static void domcreate_devmodel_started(libxl__egc *egc,
}
}
+ /* Plug nic interfaces */
+ if (!ret && d_config->num_vifs > 0) {
+ /* Attach nics */
+ GCNEW_ARRAY(dcs->devices, d_config->num_vifs);
+ dcs->num_devices = d_config->num_vifs;
+ for (i = 0; i < d_config->num_vifs; i++) {
+ libxl__init_ao_device(&dcs->devices[i], ao,
&dcs->devices);
+ dcs->devices[i].callback = domcreate_nics_connected;
+ ret = libxl__device_nic_add(egc, domid, &d_config->vifs[i],
+ &dcs->devices[i]);
+ if (ret) {
+ LOGE(ERROR, "cannot add nic %d to domain: %d", i,
domid);
+ ret = ERROR_FAIL;
+ goto error_out;
+ }
+ }
+ return;
+ }
+
+ domcreate_attach_pci(egc, dcs);
+ return;
+
+error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_nics_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs,
devices);
+ int last, ret = 0;
+
+ ret = libxl__ao_device_check_last(gc, aorm, dcs->devices,
+ dcs->num_devices, &last);
+
+ if (!last) return;
+ if (last && ret) {
+ LOGE(ERROR, "error connecting nics devices");
+ goto error_out;
+ }
+
+ domcreate_attach_pci(egc, dcs);
+ return;
+
+error_out:
+ assert(ret);
+ domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_attach_pci(libxl__egc *egc,
+ libxl__domain_create_state *dcs)
+{
+ STATE_AO_GC(dcs->ao);
+ int i, ret = 0;
+ libxl_ctx *ctx = CTX;
+ int domid = dcs->guest_domid;
+
+ /* convenience aliases */
+ libxl_domain_config *const d_config = dcs->guest_config;
+
for (i = 0; i < d_config->num_pcidevs; i++)
libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 275ab43..6b431a2 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -191,6 +191,20 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
return 0;
}
+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_disk_add(libxl__egc *egc, uint32_t domid,
libxl_device_disk *disk,
libxl__ao_device *aorm)
@@ -324,6 +338,114 @@ out:
return rc;
}
+int libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_nic *nic, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ flexarray_t *front;
+ flexarray_t *back;
+ libxl__device *device;
+ char *dompath, **l;
+ unsigned int nb, rc;
+
+ rc = libxl__device_nic_setdefault(gc, nic);
+ if (rc) goto out;
+
+ front = flexarray_make(16, 1);
+ if (!front) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+ back = flexarray_make(18, 1);
+ if (!back) {
+ rc = ERROR_NOMEM;
+ goto out_free;
+ }
+
+ if (nic->devid == -1) {
+ if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
+ rc = ERROR_FAIL;
+ goto out_free;
+ }
+ if (!(l = libxl__xs_directory(gc, XBT_NULL,
GCSPRINTF("%s/device/vif",
+ dompath),
&nb))) {
+ nic->devid = 0;
+ } else {
+ nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
+ }
+ }
+
+ GCNEW(device);
+ rc = libxl__device_from_nic(gc, domid, nic, device);
+ if (rc != 0) goto out_free;
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, GCSPRINTF("%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "state");
+ flexarray_append(back, GCSPRINTF("%d", 1));
+ if (nic->script) {
+ flexarray_append(back, "script");
+ flexarray_append(back, nic->script[0] == ''/'' ?
+ nic->script :
+ GCSPRINTF("%s/%s",
libxl__xen_script_dir_path(),
+ nic->script));
+ }
+
+ if (nic->ifname) {
+ flexarray_append(back, "vifname");
+ flexarray_append(back, nic->ifname);
+ }
+
+ flexarray_append(back, "mac");
+ flexarray_append(back, GCSPRINTF(LIBXL_MAC_FMT,
LIBXL_MAC_BYTES(nic->mac)));
+ if (nic->ip) {
+ flexarray_append(back, "ip");
+ flexarray_append(back, libxl__strdup(gc, nic->ip));
+ }
+
+ if (nic->rate_interval_usecs > 0) {
+ flexarray_append(back, "rate");
+ flexarray_append(back,
GCSPRINTF("%"PRIu64",%"PRIu32"",
+ nic->rate_bytes_per_interval,
+ nic->rate_interval_usecs));
+ }
+
+ flexarray_append(back, "bridge");
+ flexarray_append(back, libxl__strdup(gc, nic->bridge));
+ flexarray_append(back, "handle");
+ flexarray_append(back, GCSPRINTF("%d", nic->devid));
+ flexarray_append(back, "type");
+ flexarray_append(back, GCSPRINTF("%s",
+
libxl_nic_type_to_string(nic->nictype)));
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, GCSPRINTF("%d", nic->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, GCSPRINTF("%d", 1));
+ flexarray_append(front, "handle");
+ flexarray_append(front, GCSPRINTF("%d", nic->devid));
+ flexarray_append(front, "mac");
+ flexarray_append(front, GCSPRINTF(LIBXL_MAC_FMT,
+ LIBXL_MAC_BYTES(nic->mac)));
+ libxl__device_generic_add(gc, device,
+ libxl__xs_kvs_of_flexarray(gc, back, back->count),
+ libxl__xs_kvs_of_flexarray(gc, front,
front->count));
+
+ aorm->dev = device;
+ aorm->action = DEVICE_CONNECT;
+ libxl__initiate_device_add(egc, aorm);
+
+ rc = 0;
+out_free:
+ flexarray_free(back);
+ flexarray_free(front);
+out:
+ return rc;
+}
+
+
typedef struct {
libxl__gc *gc;
libxl_device_disk *disk;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b2622ee..ff5f8ad 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -666,6 +666,12 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
libxl__dm_spawn_state *stubdom_dmss,
int rc);
+static void stubdom_nics_connected(libxl__egc *egc, libxl__ao_device *aorm);
+
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+ libxl__dm_spawn_state *stubdom_dmss,
+ int rc);
+
void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
{
STATE_AO_GC(sdss->dm.spawn.ao);
@@ -838,9 +844,11 @@ static void spawn_stub_disk_connected(libxl__egc *egc,
libxl__ao_device *aorm)
}
for (i = 0; i < dm_config->num_vifs; i++) {
- ret = libxl_device_nic_add(ctx, dm_domid, &dm_config->vifs[i]);
- if (ret)
- goto out;
+ /* We have to init the nic here, because we still haven''t
+ * called libxl_device_nic_add at this point, but qemu needs
+ * the nic information to be complete.
+ */
+ libxl__device_nic_setdefault(gc, &dm_config->vifs[i]);
}
ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]);
if (ret)
@@ -915,9 +923,59 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
CONTAINER_OF(stubdom_dmss, *sdss, pvqemu);
STATE_AO_GC(sdss->dm.spawn.ao);
uint32_t dm_domid = sdss->pvqemu.guest_domid;
+ libxl_domain_config *d_config = stubdom_dmss->guest_config;
+ int ret2, i;
if (rc) goto out;
+ if (!rc && d_config->num_vifs > 0) {
+ GCNEW_ARRAY(stubdom_dmss->devices, d_config->num_vifs);
+ stubdom_dmss->num_devices = d_config->num_vifs;
+ for (i = 0; i < d_config->num_vifs; i++) {
+ libxl__init_ao_device(&stubdom_dmss->devices[i], ao,
+ &stubdom_dmss->devices);
+ stubdom_dmss->devices[i].callback = stubdom_nics_connected;
+ ret2 = libxl__device_nic_add(egc, dm_domid,
&d_config->vifs[i],
+ &stubdom_dmss->devices[i]);
+ if (ret2) {
+ LOGE(ERROR, "cannot add nic %d to domain: %d", i,
dm_domid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ }
+ return;
+ }
+
+out:
+ stubdom_pvqemu_cb(egc, stubdom_dmss, rc);
+}
+
+static void stubdom_nics_connected(libxl__egc *egc, libxl__ao_device *aorm)
+{
+ STATE_AO_GC(aorm->ao);
+ libxl__dm_spawn_state *dmss = CONTAINER_OF(aorm->base, *dmss, devices);
+ int last, ret = 0;
+
+ ret = libxl__ao_device_check_last(gc, aorm, dmss->devices,
+ dmss->num_devices, &last);
+
+ if (!last) return;
+ if (last && ret)
+ LOGE(ERROR, "error connecting nics devices");
+
+ stubdom_pvqemu_cb(egc, dmss, ret);
+ return;
+}
+
+static void stubdom_pvqemu_cb(libxl__egc *egc,
+ libxl__dm_spawn_state *stubdom_dmss,
+ int rc)
+{
+ libxl__stub_dm_spawn_state *sdss + CONTAINER_OF(stubdom_dmss, *sdss,
pvqemu);
+ STATE_AO_GC(sdss->dm.spawn.ao);
+ uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
rc = libxl_domain_unpause(CTX, dm_domid);
if (rc) goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index faeb965..91eba63 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -782,6 +782,9 @@ _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);
@@ -965,6 +968,7 @@ struct libxl__ao_device {
libxl__device_callback *callback;
/* private for implementation */
int rc;
+ int vif_executed;
libxl__ev_time ev;
libxl__ev_child child;
libxl__ev_devstate ds;
@@ -974,6 +978,9 @@ struct libxl__ao_device {
_hidden int libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
libxl_device_disk *disk,
libxl__ao_device *aorm);
+_hidden int libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__ao_device *aorm);
/*
*----- spawn -----
@@ -1147,6 +1154,8 @@ struct libxl__dm_spawn_state {
libxl_domain_config *guest_config;
libxl__domain_build_state *build_state; /* relates to guest_domid */
libxl__dm_spawn_cb *callback;
+ libxl__ao_device *devices;
+ int num_devices;
};
_hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 315ce15..6e752bd 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -28,6 +28,26 @@ 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)
+{
+ char *snictype, *be_path;
+ libxl_nic_type nictype;
+
+ be_path = libxl__device_backend_path(gc, dev);
+ snictype = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF("%s/%s", be_path,
"type"));
+ if (!snictype) {
+ LOGE(ERROR, "unable to read nictype from %s", be_path);
+ return -1;
+ }
+ if (libxl_nic_type_from_string(snictype, &nictype) < 0) {
+ LOGE(ERROR, "unable to parse nictype from %s", be_path);
+ return -1;
+ }
+
+ return nictype;
+}
+
static void cleanup(libxl__gc *gc, libxl__ao_device *aorm)
{
if (!aorm) return;
@@ -47,7 +67,18 @@ static void callback(libxl__egc *egc, libxl__ev_child *child,
: LIBXL__LOG_WARNING,
aorm->what, pid, status);
aorm->rc = ERROR_FAIL;
+ goto out;
+ }
+
+ if (aorm->dev->backend_kind == LIBXL__DEVICE_KIND_VIF &&
+ get_nic_type(gc, aorm->dev) == LIBXL_NIC_TYPE_IOEMU &&
+ !aorm->vif_executed) {
+ aorm->vif_executed = 1;
+ libxl__device_hotplug(egc, aorm);
+ return;
}
+
+out:
aorm->callback(egc, aorm);
}
@@ -66,7 +97,7 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device
*dev)
return NULL;
}
- GCNEW_ARRAY(env, 9);
+ GCNEW_ARRAY(env, 13);
env[nr++] = "script";
env[nr++] = script;
env[nr++] = "XENBUS_TYPE";
@@ -75,6 +106,24 @@ static char **get_hotplug_env(libxl__gc *gc, libxl__device
*dev)
env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid,
dev->devid);
env[nr++] = "XENBUS_BASE_PATH";
env[nr++] = "backend";
+ if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+ switch (get_nic_type(gc, dev)) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ env[nr++] = "INTERFACE";
+ env[nr++] = libxl__strdup(gc, libxl__device_nic_devname(gc,
+ dev->domid,
dev->devid,
+ LIBXL_NIC_TYPE_IOEMU));
+ case LIBXL_NIC_TYPE_VIF:
+ env[nr++] = "vif";
+ env[nr++] = libxl__strdup(gc, libxl__device_nic_devname(gc,
+ dev->domid,
dev->devid,
+ LIBXL_NIC_TYPE_VIF));
+ break;
+ default:
+ return NULL;
+ }
+ }
+
env[nr++] = NULL;
return env;
@@ -119,6 +168,81 @@ out_free:
return rc;
}
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__ao_device *aorm)
+{
+ char *be_path = libxl__device_backend_path(gc, aorm->dev);
+ char *what, *script;
+ char **args, **env;
+ int nr = 0, rc = 0;
+ libxl_nic_type nictype;
+
+ script = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s", be_path,
+
"script"));
+ if (!script) {
+ LOGE(ERROR, "unable to read script from %s", be_path);
+ return -1;
+ }
+
+ nictype = get_nic_type(gc, aorm->dev);
+ if (nictype < 0) {
+ LOGE(ERROR, "error when fetching nic type");
+ return -1;
+ }
+
+ env = get_hotplug_env(gc, aorm->dev);
+ if (!env)
+ return -1;
+
+ GCNEW_ARRAY(args, 4);
+
+ switch (nictype) {
+ case LIBXL_NIC_TYPE_IOEMU:
+ if (!aorm->vif_executed) goto execute_vif;
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "add" :
"remove";
+ args[nr++] = libxl__strdup(gc, "type_if=tap");
+ args[nr++] = NULL;
+ what = GCSPRINTF("%s %s", args[0], aorm->action ==
DEVICE_CONNECT ?
+ "connect" :
"disconnect");
+ LOG(DEBUG, "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ LOGE(ERROR, "unable execute hotplug scripts for vif device
%"PRIu32,
+ aorm->dev->devid);
+ goto out;
+ }
+ break;
+ case LIBXL_NIC_TYPE_VIF:
+execute_vif:
+ args[nr++] = script;
+ args[nr++] = aorm->action == DEVICE_CONNECT ? "online" :
"offline";
+ args[nr++] = libxl__strdup(gc, "type_if=vif");
+ args[nr++] = NULL;
+ what = GCSPRINTF("%s %s", args[0], aorm->action ==
DEVICE_CONNECT ?
+ "connect" :
"disconnect");
+ LOG(DEBUG, "Calling hotplug script: %s %s %s",
+ args[0], args[1], args[2]);
+ rc = libxl__hotplug_launch(gc, aorm, args[0], args, env, callback);
+ if (rc) {
+ LOGE(ERROR, "unable execute hotplug scripts for vif device
%"PRIu32,
+ aorm->dev->devid);
+ goto out;
+ }
+ break;
+ default:
+ /* Unknown network type */
+ LOG(DEBUG, "unknown network card type with id %"PRIu32,
+ aorm->dev->devid);
+ return 0;
+ }
+
+ rc = 0;
+
+out:
+ return rc;
+}
+
void libxl__device_hotplug(libxl__egc *egc, libxl__ao_device *aorm)
{
STATE_AO_GC(aorm->ao);
@@ -132,6 +256,10 @@ void libxl__device_hotplug(libxl__egc *egc,
libxl__ao_device *aorm)
aorm->rc = libxl__hotplug_disk(gc, aorm);
if (aorm->rc) goto error;
break;
+ case LIBXL__DEVICE_KIND_VIF:
+ aorm->rc = libxl__hotplug_nic(gc, aorm);
+ if (aorm->rc) goto error;
+ break;
default:
/* If no need to execute any hotplug scripts,
* call the callback manually
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3f1bbf6..162bb82 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4967,7 +4967,7 @@ int main_networkattach(int argc, char **argv)
return 0;
}
- 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)
Roger Pau Monne writes ("[Xen-devel] [PATCH v4 1/4] libxl: pass env vars to
libxl__exec"):> + if (setenv(env[i], env[i+1], 1) < 0) {
> + switch (errno) {
> + case EINVAL:
> + LOGEV(ERROR, errno, "invalid env variables (%s =
%s)",
> + env[i], env[i+1]);
> + break;
> + case ENOMEM:
> + libxl__alloc_failed(CTX, __func__, 0, 0);
> + }
> + goto out;
This silently exits if errno isn''t one of the expected values.
TBH I''m not sure you need to bother switching on errno here. Whatever
the errno is we''re going to log a message and exit nonzero, so you
might as well simply LOGEV even in the ENOMEM case.
Ian.
Roger Pau Monne writes ("[Xen-devel] [PATCH v4 2/4] libxl: add
libxl__xs_path_cleanup"):> Add a function which behaves like "xenstore-rm -t", and which
will be
> used to
> clean xenstore after unplug since we will be no longer executing
> xen-hotplug-cleanup script, that used to do that for us.
I still think this needs to run in a transaction.
Otherwise, if another xenstore user adds an entry to one of the
directories we think is empty, we will unwittingly remove that entry.
And in general it may be necessary to use the caller''s transaction.
So I would make libxl__xs_path_cleanup always require a non-null
transaction to be passed in by the caller.
Ian.
Ian Jackson
2012-May-10 16:30 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Roger Pau Monne writes ("[Xen-devel] [PATCH v4 3/4] libxl: call hotplug
scripts from libxl for vbd"):> 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).
Thanks.
> -SUBSYSTEM=="xen-backend", KERNEL=="tap*",
RUN+="/etc/xen/scripts/blktap $env{ACTION}"
> -SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
RUN+="/etc/xen/scripts/block $env{ACTION}"
> +SUBSYSTEM=="xen-backend", KERNEL=="tap*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap
$env{ACTION}"
> +SUBSYSTEM=="xen-backend", KERNEL=="vbd*",
ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block
$env{ACTION}"
Wouldn''t it be better to have an env var set by libxl to mean
"_not_
called from udev?". That would mean that if the udev rules
weren''t
updated for some reason it would all still work. On many setups these
are config files which may require the user to merge changes, etc., so
this is a real concern.
> +# Hack to prevent the execution of hotplug scripts from udev if the domain
> +# has been launched from libxl
> +if [ -n "${UDEV_CALL}" ] && \
> + `xenstore-read "libxl/disable_udev" >/dev/null
2>&1`; then
This reads something from xenstore and executes it as a shell command!
(Also it will go wrong if the value read is empty eg becaue the key
doesn''t exist.)
Also didn''t I say that in xenstore we normally record booleans as
decimal integers, "0" for false and "1" for true ?
> -}
> +}
>
> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
> -{
> - GC_INIT(ctx);
> - char *dom_path;
> - char *vm_path;
> - char *pid;
> - int rc, dm_present;
> +/* Callback for domain destruction */
The rest of this is very very difficult to review because of the
amount of code motion, variable renaming, and so forth.
Do you think it would be possible to reorganise things so that the
diff is as minimal as possible ? Techniques you will probably find
useful include:
* Declare some convenience variables such as
uint32_t const domid = dis->domid;
This is best done near the top of each function along with the new
function prototype etc., so it doesn''t introduce a mid-function
patch hunk. This will avoid irrelevant noise in the diff caused by
the movement of state into the operation state struct.
* Declare callback functions at the top of the file so that they can
appear after the point where they are referenced, and write all the
code in the order in which it actually occurs. This is useful
anyway, but it probably means that the code won''t need to move
because it''s probably already in that order. If it isn''t
you may
need to move it about separately.
* In general, avoid moving any code if at all possible in the main
patch. If you need to move code, do it in a pre- or post-patch.
* Only take the opportunity to make unrelated changes (eg, changing
from libxl__sprintf to GCSPRINTF) if you have to make another
change to the very same line of code.
* If any code motion is needed, split it out into a pre-patch which
contains only code motion.
* Be prepared to leave wrinkles in the code style or layout and fix
them up in a later patch.
* If it is necessary to switch a variable from a "struct foo" to a
"struct foo*", split out a separate pre-patch which changes it to a
"struct foo[1]". This separate patch will then obviously have no
functional change and wiull simply contain a lot of removing of
"&"
etc. The actual patch with the meat can leave all those references
unchanged.
* Likewise if you want to change something from a "struct foo*" to a
"struct foo" you can do the reverse: in your main patch change it
to "struct foo[1]" instead. Then, you can add a post-patch to fix
that up by changing "struct foo[1]" to "struct foo" which
again
will have no functional change.
You will see me applying these techniques in my recent child process
series; I can point you at specific commits etc. if you want more
hints.
Of course this may not be feasible. If for some parts of the code
this approach is not feasible, and the result is always going to look
like a rewrite, mention in the commit message that such-and-such
function(s) or parts of code have essentially been rewritten.
Thanks,
Ian.
Roger Pau Monne
2012-May-14 12:38 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Ian Jackson escribió:> Roger Pau Monne writes ("[Xen-devel] [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd"): >> 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). > > Thanks. > >> -SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACTION}" >> -SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}" >> +SUBSYSTEM=="xen-backend", KERNEL=="tap*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/blktap $env{ACTION}" >> +SUBSYSTEM=="xen-backend", KERNEL=="vbd*", ENV{UDEV_CALL}="1", RUN+="/etc/xen/scripts/block $env{ACTION}" > > Wouldn''t it be better to have an env var set by libxl to mean "_not_ > called from udev?". That would mean that if the udev rules weren''t > updated for some reason it would all still work. On many setups these > are config files which may require the user to merge changes, etc., so > this is a real concern.Since the 4.2 release already implies updating udev rules (due to the change from tap* to *emu), I think it''s better to set the var on udev, that way when udev is dropped we won''t need to perform any change to libxl.> >> +# Hack to prevent the execution of hotplug scripts from udev if the domain >> +# has been launched from libxl >> +if [ -n "${UDEV_CALL}" ]&& \ >> + `xenstore-read "libxl/disable_udev">/dev/null 2>&1`; then > > This reads something from xenstore and executes it as a shell command! > (Also it will go wrong if the value read is empty eg becaue the key > doesn''t exist.)Are you sure about this? This command never returns anything, because it is redirected to /dev/null, so we only evaluate if it is able to read libxl/disable_udev. If libxl/disable_udev exists this test is passed.> > Also didn''t I say that in xenstore we normally record booleans as > decimal integers, "0" for false and "1" for true ? > >> -} >> +} >> >> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid) >> -{ >> - GC_INIT(ctx); >> - char *dom_path; >> - char *vm_path; >> - char *pid; >> - int rc, dm_present; >> +/* Callback for domain destruction */ > > The rest of this is very very difficult to review because of the > amount of code motion, variable renaming, and so forth.Yes, will try to do so. I''m currently moving the changes to mach tip and also splitting them.> > Do you think it would be possible to reorganise things so that the > diff is as minimal as possible ? Techniques you will probably find > useful include: > > * Declare some convenience variables such as > uint32_t const domid = dis->domid; > This is best done near the top of each function along with the new > function prototype etc., so it doesn''t introduce a mid-function > patch hunk. This will avoid irrelevant noise in the diff caused by > the movement of state into the operation state struct. > > * Declare callback functions at the top of the file so that they can > appear after the point where they are referenced, and write all the > code in the order in which it actually occurs. This is useful > anyway, but it probably means that the code won''t need to move > because it''s probably already in that order. If it isn''t you may > need to move it about separately. > > * In general, avoid moving any code if at all possible in the main > patch. If you need to move code, do it in a pre- or post-patch. > > * Only take the opportunity to make unrelated changes (eg, changing > from libxl__sprintf to GCSPRINTF) if you have to make another > change to the very same line of code. > > * If any code motion is needed, split it out into a pre-patch which > contains only code motion. > > * Be prepared to leave wrinkles in the code style or layout and fix > them up in a later patch. > > * If it is necessary to switch a variable from a "struct foo" to a > "struct foo*", split out a separate pre-patch which changes it to a > "struct foo[1]". This separate patch will then obviously have no > functional change and wiull simply contain a lot of removing of "&" > etc. The actual patch with the meat can leave all those references > unchanged. > > * Likewise if you want to change something from a "struct foo*" to a > "struct foo" you can do the reverse: in your main patch change it > to "struct foo[1]" instead. Then, you can add a post-patch to fix > that up by changing "struct foo[1]" to "struct foo" which again > will have no functional change. > > You will see me applying these techniques in my recent child process > series; I can point you at specific commits etc. if you want more > hints. > > Of course this may not be feasible. If for some parts of the code > this approach is not feasible, and the result is always going to look > like a rewrite, mention in the commit message that such-and-such > function(s) or parts of code have essentially been rewritten. > > Thanks, > Ian.
Ian Campbell
2012-May-14 12:51 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
On Mon, 2012-05-14 at 13:38 +0100, Roger Pau Monne wrote:> >> +# Hack to prevent the execution of hotplug scripts from udev if the domain > >> +# has been launched from libxl > >> +if [ -n "${UDEV_CALL}" ]&& \ > >> + `xenstore-read "libxl/disable_udev">/dev/null 2>&1`; then > > > > This reads something from xenstore and executes it as a shell command! > > (Also it will go wrong if the value read is empty eg becaue the key > > doesn''t exist.) > > Are you sure about this? This command never returns anything, because it > is redirected to /dev/null, so we only evaluate if it is able to read > libxl/disable_udev. If libxl/disable_udev exists this test is passed.You don''t need the backticks for that though. With the backticks it will execute whatever happens to be in the key -- I guess it''s something quite benign right now or you''d have seen errors. Ian.
Roger Pau Monne
2012-May-14 13:30 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Ian Campbell escribió:> On Mon, 2012-05-14 at 13:38 +0100, Roger Pau Monne wrote: >>>> +# Hack to prevent the execution of hotplug scripts from udev if the domain >>>> +# has been launched from libxl >>>> +if [ -n "${UDEV_CALL}" ]&& \ >>>> + `xenstore-read "libxl/disable_udev">/dev/null 2>&1`; then >>> This reads something from xenstore and executes it as a shell command! >>> (Also it will go wrong if the value read is empty eg becaue the key >>> doesn't exist.) >> Are you sure about this? This command never returns anything, because it >> is redirected to /dev/null, so we only evaluate if it is able to read >> libxl/disable_udev. If libxl/disable_udev exists this test is passed. > > You don't need the backticks for that though. With the backticks it will > execute whatever happens to be in the key -- I guess it's something > quite benign right now or you'd have seen errors.Yes, it's not the first time that I've made that mistake. Since it never returns anything I guess that's why I never saw any errors. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Jackson
2012-May-14 14:12 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH v4 3/4] libxl: call hotplug
scripts from libxl for vbd"):> Ian Jackson escribió:
> > Wouldn''t it be better to have an env var set by libxl to mean
"_not_
> > called from udev?". That would mean that if the udev rules
weren''t
> > updated for some reason it would all still work. On many setups these
> > are config files which may require the user to merge changes, etc., so
> > this is a real concern.
>
> Since the 4.2 release already implies updating udev rules (due to the
> change from tap* to *emu), I think it''s better to set the var on
udev,
> that way when udev is dropped we won''t need to perform any change
to libxl.
Ah. (The change to libxl is not an essential one so we won''t
"need"
to make it, but OK.)
> >> +if [ -n "${UDEV_CALL}" ]&& \
> >> + `xenstore-read "libxl/disable_udev">/dev/null
2>&1`; then
> >
> > This reads something from xenstore and executes it as a shell command!
> > (Also it will go wrong if the value read is empty eg becaue the key
> > doesn''t exist.)
>
> Are you sure about this? This command never returns anything, because it
> is redirected to /dev/null, so we only evaluate if it is able to read
> libxl/disable_udev. If libxl/disable_udev exists this test is passed.
Uh, so it does. Why are you using `` at all then ? This is a very
odd construction.
Thanks,
Ian.
Ian Jackson
2012-May-14 14:14 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Ian Campbell writes ("Re: [Xen-devel] [PATCH v4 3/4] libxl: call hotplug
scripts from libxl for vbd"):> On Mon, 2012-05-14 at 13:38 +0100, Roger Pau Monne wrote:
> > Are you sure about this? This command never returns anything, because
it
> > is redirected to /dev/null, so we only evaluate if it is able to read
> > libxl/disable_udev. If libxl/disable_udev exists this test is passed.
>
> You don''t need the backticks for that though. With the backticks
it will
> execute whatever happens to be in the key -- I guess it''s
something
> quite benign right now or you''d have seen errors.
In fact Roger was right on the narrow point: because the >/dev/null is
inside the backticks, the backticks never see any output and it''s
equivalent to `false` or `true`. I''m pleased that it''s not
just me
that read it the way you did at first :-).
But you are of course right in that it''s a silly construction. If the
output is not wanted, `` should not be used.
Ian.
Roger Pau Monne
2012-May-14 14:20 UTC
Re: [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd
Ian Jackson escribió:> Ian Campbell writes ("Re: [Xen-devel] [PATCH v4 3/4] libxl: call hotplug scripts from libxl for vbd"): >> On Mon, 2012-05-14 at 13:38 +0100, Roger Pau Monne wrote: >>> Are you sure about this? This command never returns anything, because it >>> is redirected to /dev/null, so we only evaluate if it is able to read >>> libxl/disable_udev. If libxl/disable_udev exists this test is passed. >> You don't need the backticks for that though. With the backticks it will >> execute whatever happens to be in the key -- I guess it's something >> quite benign right now or you'd have seen errors. > > In fact Roger was right on the narrow point: because the>/dev/null is > inside the backticks, the backticks never see any output and it's > equivalent to `false` or `true`. I'm pleased that it's not just me > that read it the way you did at first :-).The fact that it works is just a question of "luck", but the construction is definitely wrong. I always forget that you don't need to use the ` when testing in the conditional for the execution of a command. Sorry about that.> But you are of course right in that it's a silly construction. If the > output is not wanted, `` should not be used. > > Ian._______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel