# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337180254 -3600
# Node ID 3f65f6466bf6008a2cccc374ffd9ab799dd41da4
# Parent c8af0666c1448bbdcb3fddc155c6cc1978bc0206
xl: remove all local "ctx" variables.
xl has a global "ctx" variable, so there should be no need to pass a
ctx to any function.
The exception is to functions which are used as libxl callbacks. In
this case the ctx passed to the callback must necessarily always be
the same as the global one and so we rename the parameter version to
ctx_unused.
This fixes a hang/deadlock with console autoconnect. postfork() would
correctly replace the global ctx with a new one in the child, but
autoconnect_console() would continue using the old context via the
local variable (i.e. the parent''s context) leading to deadlock on the
xenstore handle (which cannot be reliably shared between guests).
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r c8af0666c144 -r 3f65f6466bf6 tools/libxl/xl.c
--- a/tools/libxl/xl.c Wed May 16 14:37:25 2012 +0100
+++ b/tools/libxl/xl.c Wed May 16 15:57:34 2012 +0100
@@ -108,7 +108,7 @@ void postfork(void)
}
}
-pid_t xl_fork(libxl_ctx *ctx) {
+pid_t xl_fork(void) {
pid_t pid;
pid = fork();
diff -r c8af0666c144 -r 3f65f6466bf6 tools/libxl/xl.h
--- a/tools/libxl/xl.h Wed May 16 14:37:25 2012 +0100
+++ b/tools/libxl/xl.h Wed May 16 15:57:34 2012 +0100
@@ -107,7 +107,7 @@ struct cmd_spec *cmdtable_lookup(const c
extern libxl_ctx *ctx;
extern xentoollog_logger_stdiostream *logger;
-pid_t xl_fork(libxl_ctx *ctx); /* like fork, but prints and dies if it fails */
+pid_t xl_fork(void); /* like fork, but prints and dies if it fails */
void postfork(void);
/* global options */
diff -r c8af0666c144 -r 3f65f6466bf6 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed May 16 14:37:25 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed May 16 15:57:34 2012 +0100
@@ -1298,7 +1298,7 @@ skip_vfb:
xlu_cfg_destroy(config);
}
-static void reload_domain_config(libxl_ctx *ctx, uint32_t domid,
+static void reload_domain_config(uint32_t domid,
uint8_t **config_data, int *config_len)
{
uint8_t *t_data;
@@ -1318,7 +1318,7 @@ static void reload_domain_config(libxl_c
/* Returns 1 if domain should be restarted,
* 2 if domain should be renamed then restarted, or 0 */
-static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
+static int handle_domain_death(uint32_t domid,
libxl_event *event,
uint8_t **config_data, int *config_len,
libxl_domain_config *d_config)
@@ -1377,12 +1377,12 @@ static int handle_domain_death(libxl_ctx
break;
case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
- reload_domain_config(ctx, domid, config_data, config_len);
+ reload_domain_config(domid, config_data, config_len);
restart = 2;
break;
case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
- reload_domain_config(ctx, domid, config_data, config_len);
+ reload_domain_config(domid, config_data, config_len);
restart = 1;
/* fall-through */
@@ -1418,7 +1418,7 @@ static void replace_string(char **str, c
}
-static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event *event,
+static int preserve_domain(uint32_t domid, libxl_event *event,
libxl_domain_config *d_config)
{
time_t now;
@@ -1496,7 +1496,7 @@ static int freemem(libxl_domain_build_in
return ERROR_NOMEM;
}
-static void autoconnect_console(libxl_ctx *ctx, libxl_event *ev, void *priv)
+static void autoconnect_console(libxl_ctx *ctx_ignored, libxl_event *ev, void
*priv)
{
pid_t *pid = priv;
@@ -1811,7 +1811,7 @@ start:
pid_t child1, got_child;
int nullfd;
- child1 = xl_fork(ctx);
+ child1 = xl_fork();
if (child1) {
printf("Daemon running with PID %d\n", child1);
@@ -1889,11 +1889,11 @@ start:
LOG("Domain %d has shut down, reason code %d 0x%x",
domid,
event->u.domain_shutdown.shutdown_reason,
event->u.domain_shutdown.shutdown_reason);
- switch (handle_domain_death(ctx, domid, event,
+ switch (handle_domain_death(domid, event,
(uint8_t **)&config_data,
&config_len,
&d_config)) {
case 2:
- if (!preserve_domain(ctx, domid, event, &d_config)) {
+ if (!preserve_domain(domid, event, &d_config)) {
/* If we fail then exit leaving the old domain in place. */
ret = -1;
goto out;
@@ -2929,7 +2929,7 @@ static void migrate_domain(const char *d
MUST( libxl_pipe(ctx, sendpipe) );
MUST( libxl_pipe(ctx, recvpipe) );
- child = xl_fork(ctx);
+ child = xl_fork();
if (!child) {
dup2(sendpipe[0], 0);
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337262494 -3600
# Node ID bb377172ef57bed114ad688d8019fef98a08267f
# Parent 5bc483bc723e8b9563ba3763fe926cc6ebe6a980
xl: remove all local "ctx" variables.
xl has a global "ctx" variable, so there should be no need to pass a
ctx to any function.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: remove autoconnect_console change to ctx_ignored -- IanJ already handled
that in "libxl, xl: fix bootloader immediate console attach"
diff -r 5bc483bc723e -r bb377172ef57 tools/libxl/xl.c
--- a/tools/libxl/xl.c Thu May 17 14:48:14 2012 +0100
+++ b/tools/libxl/xl.c Thu May 17 14:48:14 2012 +0100
@@ -108,7 +108,7 @@ void postfork(void)
}
}
-pid_t xl_fork(libxl_ctx *ctx) {
+pid_t xl_fork(void) {
pid_t pid;
pid = fork();
diff -r 5bc483bc723e -r bb377172ef57 tools/libxl/xl.h
--- a/tools/libxl/xl.h Thu May 17 14:48:14 2012 +0100
+++ b/tools/libxl/xl.h Thu May 17 14:48:14 2012 +0100
@@ -107,7 +107,7 @@ struct cmd_spec *cmdtable_lookup(const c
extern libxl_ctx *ctx;
extern xentoollog_logger_stdiostream *logger;
-pid_t xl_fork(libxl_ctx *ctx); /* like fork, but prints and dies if it fails */
+pid_t xl_fork(void); /* like fork, but prints and dies if it fails */
void postfork(void);
/* global options */
diff -r 5bc483bc723e -r bb377172ef57 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Thu May 17 14:48:14 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu May 17 14:48:14 2012 +0100
@@ -1298,7 +1298,7 @@ skip_vfb:
xlu_cfg_destroy(config);
}
-static void reload_domain_config(libxl_ctx *ctx, uint32_t domid,
+static void reload_domain_config(uint32_t domid,
uint8_t **config_data, int *config_len)
{
uint8_t *t_data;
@@ -1318,7 +1318,7 @@ static void reload_domain_config(libxl_c
/* Returns 1 if domain should be restarted,
* 2 if domain should be renamed then restarted, or 0 */
-static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
+static int handle_domain_death(uint32_t domid,
libxl_event *event,
uint8_t **config_data, int *config_len,
libxl_domain_config *d_config)
@@ -1377,12 +1377,12 @@ static int handle_domain_death(libxl_ctx
break;
case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
- reload_domain_config(ctx, domid, config_data, config_len);
+ reload_domain_config(domid, config_data, config_len);
restart = 2;
break;
case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
- reload_domain_config(ctx, domid, config_data, config_len);
+ reload_domain_config(domid, config_data, config_len);
restart = 1;
/* fall-through */
@@ -1418,7 +1418,7 @@ static void replace_string(char **str, c
}
-static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event *event,
+static int preserve_domain(uint32_t domid, libxl_event *event,
libxl_domain_config *d_config)
{
time_t now;
@@ -1813,7 +1813,7 @@ start:
pid_t child1, got_child;
int nullfd;
- child1 = xl_fork(ctx);
+ child1 = xl_fork();
if (child1) {
printf("Daemon running with PID %d\n", child1);
@@ -1891,11 +1891,11 @@ start:
LOG("Domain %d has shut down, reason code %d 0x%x",
domid,
event->u.domain_shutdown.shutdown_reason,
event->u.domain_shutdown.shutdown_reason);
- switch (handle_domain_death(ctx, domid, event,
+ switch (handle_domain_death(domid, event,
(uint8_t **)&config_data,
&config_len,
&d_config)) {
case 2:
- if (!preserve_domain(ctx, domid, event, &d_config)) {
+ if (!preserve_domain(domid, event, &d_config)) {
/* If we fail then exit leaving the old domain in place. */
ret = -1;
goto out;
@@ -2931,7 +2931,7 @@ static void migrate_domain(const char *d
MUST( libxl_pipe(ctx, sendpipe) );
MUST( libxl_pipe(ctx, recvpipe) );
- child = xl_fork(ctx);
+ child = xl_fork();
if (!child) {
dup2(sendpipe[0], 0);
Ian Campbell writes ("[PATCH] xl: remove all local "ctx"
variables"):> xl: remove all local "ctx" variables.
>
> xl has a global "ctx" variable, so there should be no need to
pass a
> ctx to any function.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Although I would prefer it if you''d leave xl_fork alone because my
xl child process fix patch is going to change its prototype anyway.
Ian.
On Fri, 2012-05-18 at 12:18 +0100, Ian Jackson wrote:> Ian Campbell writes ("[PATCH] xl: remove all local "ctx" variables"): > > xl: remove all local "ctx" variables. > > > > xl has a global "ctx" variable, so there should be no need to pass a > > ctx to any function. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Although I would prefer it if you''d leave xl_fork alone because my > xl child process fix patch is going to change its prototype anyway.I''m happy to refresh this patch after your xl child patch goes in, and drop the xl_fork stuff as appropriate. Ian.
On Fri, 2012-05-18 at 12:37 +0100, Ian Campbell wrote:> On Fri, 2012-05-18 at 12:18 +0100, Ian Jackson wrote: > > Ian Campbell writes ("[PATCH] xl: remove all local "ctx" variables"): > > > xl: remove all local "ctx" variables. > > > > > > xl has a global "ctx" variable, so there should be no need to pass a > > > ctx to any function. > > > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > > Although I would prefer it if you''d leave xl_fork alone because my > > xl child process fix patch is going to change its prototype anyway. > > I''m happy to refresh this patch after your xl child patch goes in, and > drop the xl_fork stuff as appropriate.I dropped the xl_fork hunks and committed. Ian.