|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH RESEND] libxl: Deprecate synchronous waiting for the device model
Ian Jackson writes ("Re: [PATCH] libxl: Deprecate synchronous waiting for the
device model"):
> Ian Campbell writes ("Re: [PATCH] libxl: Deprecate synchronous waiting for
> the device model"):
> > On Mon, 2013-10-14 at 17:30 +0100, Ian Jackson wrote:
> > > /*
> > > - * libxl__wait_for_offspring - Wait for child state
> > > + * libxl__xenstore_child_wait_deprecated - Wait for daemonic child IPC
> > > [...]
> > > This function is currently used only by
> > > + * libxl__wait_for_device_model_deprecated.
> >
> > Could it become a static helper in tools/libxl/libxl_device.c (which is
> > where libxl__wait_for_device_model_deprecated seems to live) then? Tat
> > would get it out of the internal API immediately...
>
> At the moment its called in libxl_device.c but defined in
> libxl_exec.c. I'm not too keen on moving about something which we
> intend to get rid of.
Ping.
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Mon, 14 Oct 2013 17:26:01 +0100
Subject: [PATCH] libxl: Deprecate synchronous waiting for the device model
libxl__wait_for_device_model blocks, with the ctx lock held, waiting
for a response from the device model. If the dm doesn't respond
quickly (for example, because it has crashed), this may block the
whole process. Explain this in a comment, rename the function to
libxl__wait_for_device_model_deprecated, and explain what to use
instead.
libxl__wait_for_offspring is the core implementation for the above.
Its name leads people to think it might be generally useful for
waiting for children, which is far from the case. It only waits for
xenstore. Also it has the problems described above. Explain this,
rename it to libxl__xenstore_child_wait_deprecated, and explain what
to use instead.
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_device.c | 4 ++--
tools/libxl/libxl_dom.c | 4 ++--
tools/libxl/libxl_exec.c | 2 +-
tools/libxl/libxl_internal.h | 36 ++++++++++++++++++++++++++++--------
tools/libxl/libxl_pci.c | 8 ++++----
6 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bede011..142e936 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -846,7 +846,7 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
state = libxl__xs_read(gc, XBT_NULL, path);
if (state != NULL && !strcmp(state, "paused")) {
libxl__qemu_traditional_cmd(gc, domid, "continue");
- libxl__wait_for_device_model(gc, domid, "running",
+ libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL);
}
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 16a92a4..610f0c5 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1077,7 +1077,7 @@ static void devices_remove_callback(libxl__egc *egc,
return;
}
-int libxl__wait_for_device_model(libxl__gc *gc,
+int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
uint32_t domid, char *state,
libxl__spawn_starting *spawning,
int (*check_callback)(libxl__gc *gc,
@@ -1088,7 +1088,7 @@ int libxl__wait_for_device_model(libxl__gc *gc,
{
char *path;
path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
- return libxl__wait_for_offspring(gc, domid,
+ return libxl__xenstore_child_wait_deprecated(gc, domid,
LIBXL_DEVICE_MODEL_START_TIMEOUT,
"Device Model", path, state, spawning,
check_callback, check_callback_userdata);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 1812bdc..6cb39c1 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -965,7 +965,7 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
LOG(DEBUG, "Saving device model state to %s", filename);
libxl__qemu_traditional_cmd(gc, domid, "save");
- libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
+ libxl__wait_for_device_model_deprecated(gc, domid, "paused", NULL,
NULL, NULL);
break;
}
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
@@ -989,7 +989,7 @@ int libxl__domain_resume_device_model(libxl__gc *gc,
uint32_t domid)
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
libxl__qemu_traditional_cmd(gc, domid, "continue");
- libxl__wait_for_device_model(gc, domid, "running", NULL, NULL, NULL);
+ libxl__wait_for_device_model_deprecated(gc, domid, "running", NULL,
NULL, NULL);
break;
}
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 7eddaef..b6efa0f 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -157,7 +157,7 @@ out:
return rc ? SIGTERM : 0;
}
-int libxl__wait_for_offspring(libxl__gc *gc,
+int libxl__xenstore_child_wait_deprecated(libxl__gc *gc,
uint32_t domid,
uint32_t timeout, char *what,
char *path, char *state,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4f92522..d2301f8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1223,7 +1223,19 @@ _hidden int libxl__spawn_record_pid(libxl__gc*,
libxl__spawn_state*,
pid_t innerchild);
/*
- * libxl__wait_for_offspring - Wait for child state
+ * libxl__xenstore_child_wait_deprecated - Wait for daemonic child IPC
+ *
+ * This is a NOT function for waiting for ordinary child processes.
+ * If you want to run (fork/exec/wait) subprocesses from libxl:
+ * - Make your libxl entrypoint use the ao machinery
+ * - Use libxl__ev_fork, and use the callback programming style
+ *
+ * This function is intended for interprocess communication with a
+ * service process. If the service process does not respond quickly,
+ * the whole caller may be blocked. Therefore this function is
+ * deprecated. This function is currently used only by
+ * libxl__wait_for_device_model_deprecated.
+ *
* gc: allocation pool
* domid: guest to work with
* timeout: how many seconds to wait for the state to appear
@@ -1240,9 +1252,9 @@ _hidden int libxl__spawn_record_pid(libxl__gc*,
libxl__spawn_state*,
* in xenstore, and optionally for state in path.
* If path appears and state matches, check_callback is called.
* If check_callback returns > 0, waiting for path or state continues.
- * Otherwise libxl__wait_for_offspring returns.
+ * Otherwise libxl__xenstore_child_wait_deprecated returns.
*/
-_hidden int libxl__wait_for_offspring(libxl__gc *gc,
+_hidden int libxl__xenstore_child_wait_deprecated(libxl__gc *gc,
uint32_t domid,
uint32_t timeout, char *what,
char *path, char *state,
@@ -1295,12 +1307,20 @@ _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
int nr_consoles, libxl__device_console *consoles,
int nr_vfbs, libxl_device_vfb *vfbs,
int nr_disks, libxl_device_disk *disks);
- /* Caller must either: pass starting_r==0, or on successful
- * return pass *starting_r (which will be non-0) to
- * libxl__confirm_device_model_startup or libxl__detach_device_model. */
-_hidden int libxl__wait_for_device_model(libxl__gc *gc,
+
+/*
+ * This function will cause the whole libxl process to hang
+ * if the device model does not respond. It is deprecated.
+ *
+ * Instead of calling this function:
+ * - Make your libxl entrypoint use the ao machinery
+ * - Use libxl__ev_xswatch_register, and use the callback programming
+ * style
+ */
+_hidden int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
uint32_t domid, char *state,
- libxl__spawn_starting *spawning,
+ libxl__spawn_starting *spawning
+ /* NULL allowed */,
int (*check_callback)(libxl__gc *gc,
uint32_t domid,
const char *state,
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 26ba3e6..c8eceb4 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -823,7 +823,7 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t
domid,
}
libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
- rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
+ rc = libxl__wait_for_device_model_deprecated(gc, domid, NULL, NULL,
pci_ins_check, state);
path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter",
domid);
@@ -851,7 +851,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, i
switch (libxl__domain_type(gc, domid)) {
case LIBXL_DOMAIN_TYPE_HVM:
hvm = 1;
- if (libxl__wait_for_device_model(gc, domid, "running",
+ if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0) {
return ERROR_FAIL;
}
@@ -1137,7 +1137,7 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc,
uint32_t domid,
* device-model for function 0 */
if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
libxl__qemu_traditional_cmd(gc, domid, "pci-rem");
- if (libxl__wait_for_device_model(gc, domid, "pci-removed",
+ if (libxl__wait_for_device_model_deprecated(gc, domid, "pci-removed",
NULL, NULL, NULL) < 0) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in
time");
/* This depends on guest operating system acknowledging the
@@ -1179,7 +1179,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
switch (libxl__domain_type(gc, domid)) {
case LIBXL_DOMAIN_TYPE_HVM:
hvm = 1;
- if (libxl__wait_for_device_model(gc, domid, "running",
+ if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0)
goto out_fail;
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |