* [PATCH 01/13] libxl: unify libxl__device_destroy and device_hotplug_done
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 02/13] libxl: react correctly to bootloader pty master POLLHUP Ian Jackson
` (12 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
device_hotplug_done contains an open-coded but improved version of
libxl__device_destroy. So move the contents of device_hotplug_done
into libxl__device_destroy, deleting the old code, and replace it at
its old location with a function call.
Add the missing call to libxl__xs_transaction_abort (which was present
in neither version and technically speaking is always a no-op with
this code as it stands at the moment because no-one does "goto out"
other than after libxl__xs_transaction_start or _commit).
Also fix the error handling: the rc from the destroy should be
propagated into the aodev.
Reported-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
-
Changes in v5 of series:
* Also add missing xs abort.
---
tools/libxl/libxl_device.c | 36 +++++++++++++-----------------------
1 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index da0c3ea..95b169e 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -513,22 +513,24 @@ 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);
xs_transaction_t t = 0;
- int rc = 0;
+ int rc;
+
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
- do {
- t = xs_transaction_start(CTX->xsh);
libxl__xs_path_cleanup(gc, t, fe_path);
libxl__xs_path_cleanup(gc, t, be_path);
- rc = !xs_transaction_end(CTX->xsh, t, 0);
- } while (rc && errno == EAGAIN);
- if (rc) {
- LOGE(ERROR, "unable to finish transaction");
- goto out;
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc < 0) goto out;
}
libxl__device_destroy_tapdisk(gc, be_path);
out:
+ libxl__xs_transaction_abort(gc, &t);
return rc;
}
@@ -993,29 +995,17 @@ error:
static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev)
{
STATE_AO_GC(aodev->ao);
- char *be_path = libxl__device_backend_path(gc, aodev->dev);
- char *fe_path = libxl__device_frontend_path(gc, aodev->dev);
- xs_transaction_t t = 0;
int rc;
device_hotplug_clean(gc, aodev);
/* Clean xenstore if it's a disconnection */
if (aodev->action == DEVICE_DISCONNECT) {
- for (;;) {
- rc = libxl__xs_transaction_start(gc, &t);
- if (rc) goto out;
-
- libxl__xs_path_cleanup(gc, t, fe_path);
- libxl__xs_path_cleanup(gc, t, be_path);
-
- rc = libxl__xs_transaction_commit(gc, &t);
- if (!rc) break;
- if (rc < 0) goto out;
- }
+ rc = libxl__device_destroy(gc, aodev->dev);
+ if (!aodev->rc)
+ aodev->rc = rc;
}
-out:
aodev->callback(egc, aodev);
return;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 02/13] libxl: react correctly to bootloader pty master POLLHUP
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
2012-08-02 17:27 ` [PATCH 01/13] libxl: unify libxl__device_destroy and device_hotplug_done Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-03 8:32 ` Ian Campbell
2012-08-02 17:27 ` [PATCH 03/13] libxl: fix device counting race in libxl__devices_destroy Ian Jackson
` (11 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Receive POLLHUP on the bootloader master pty is not an error.
Hopefully it means that the bootloader has exited and therefore the
pty slave side has no process group any more. (At least NetBSD
indicates POLLHUP on the master in this case.)
So send the bootloader SIGTERM; if it has already exited then this has
no effect (except that on some versions of NetBSD it erroneously
returns ESRCH and we print a harmless warning) and we will then
collect the bootloader's exit status and be satisfied.
However, we remember that we have done this so that if we got POLLHUP
for some other reason than that the bootloader exited we report
something resembling a useful message.
In order to implement this we need to provide a way for users of
datacopier to handle POLLHUP rather than treating it as fatal.
We rename bootloader_abort to bootloader_stop since it now no longer
only applies to error situations.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
-
Changes in v5:
* Correctly call dc->callback_pollhup, not dc->callback,
in datacopier_pollhup_handled.
Changes in v4:
* Track whether we sent SIGTERM due to POLLHUP so we can report
messages properly.
Changes in v3:
* datacopier provides new interface for handling POLLHUP
* Do not ignore errors on the xenconsole pty
* Rename bootloader_abort.
---
tools/libxl/libxl_aoutils.c | 23 +++++++++++++++++++++++
tools/libxl/libxl_bootloader.c | 39 +++++++++++++++++++++++++++++----------
tools/libxl/libxl_internal.h | 7 +++++--
3 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 99972a2..983a60a 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -97,11 +97,31 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc,
LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
}
+static int datacopier_pollhup_handled(libxl__egc *egc,
+ libxl__datacopier_state *dc,
+ short revents, int onwrite)
+{
+ STATE_AO_GC(dc->ao);
+
+ if (dc->callback_pollhup && (revents & POLLHUP)) {
+ LOG(DEBUG, "received POLLHUP on %s during copy of %s",
+ onwrite ? dc->writewhat : dc->readwhat,
+ dc->copywhat);
+ libxl__datacopier_kill(dc);
+ dc->callback_pollhup(egc, dc, onwrite, -1);
+ return 1;
+ }
+ return 0;
+}
+
static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents) {
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
STATE_AO_GC(dc->ao);
+ if (datacopier_pollhup_handled(egc, dc, revents, 0))
+ return;
+
if (revents & ~POLLIN) {
LOG(ERROR, "unexpected poll event 0x%x (should be POLLIN)"
" on %s during copy of %s", revents, dc->readwhat, dc->copywhat);
@@ -163,6 +183,9 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, towrite);
STATE_AO_GC(dc->ao);
+ if (datacopier_pollhup_handled(egc, dc, revents, 1))
+ return;
+
if (revents & ~POLLOUT) {
LOG(ERROR, "unexpected poll event 0x%x (should be POLLOUT)"
" on %s during copy of %s", revents, dc->writewhat, dc->copywhat);
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index ef5a91b..bfc1b56 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -215,6 +215,7 @@ void libxl__bootloader_init(libxl__bootloader_state *bl)
libxl__domaindeathcheck_init(&bl->deathcheck);
bl->keystrokes.ao = bl->ao; libxl__datacopier_init(&bl->keystrokes);
bl->display.ao = bl->ao; libxl__datacopier_init(&bl->display);
+ bl->got_pollhup = 0;
}
static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
@@ -275,7 +276,7 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
}
/* might be called at any time, provided it's init'd */
-static void bootloader_abort(libxl__egc *egc,
+static void bootloader_stop(libxl__egc *egc,
libxl__bootloader_state *bl, int rc)
{
STATE_AO_GC(bl->ao);
@@ -285,8 +286,8 @@ static void bootloader_abort(libxl__egc *egc,
libxl__datacopier_kill(&bl->display);
if (libxl__ev_child_inuse(&bl->child)) {
r = kill(bl->child.pid, SIGTERM);
- if (r) LOGE(WARN, "after failure, failed to kill bootloader [%lu]",
- (unsigned long)bl->child.pid);
+ if (r) LOGE(WARN, "%sfailed to kill bootloader [%lu]",
+ rc ? "after failure, " : "", (unsigned long)bl->child.pid);
}
bl->rc = rc;
}
@@ -508,7 +509,10 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
bl->keystrokes.maxsz = BOOTLOADER_BUF_OUT;
bl->keystrokes.copywhat =
GCSPRINTF("bootloader input for domain %"PRIu32, bl->domid);
- bl->keystrokes.callback = bootloader_keystrokes_copyfail;
+ bl->keystrokes.callback = bootloader_keystrokes_copyfail;
+ bl->keystrokes.callback_pollhup = bootloader_keystrokes_copyfail;
+ /* pollhup gets called with errnoval==-1 which is not otherwise
+ * possible since errnos are nonnegative, so it's unambiguous */
rc = libxl__datacopier_start(&bl->keystrokes);
if (rc) goto out;
@@ -516,7 +520,8 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
bl->display.maxsz = BOOTLOADER_BUF_IN;
bl->display.copywhat =
GCSPRINTF("bootloader output for domain %"PRIu32, bl->domid);
- bl->display.callback = bootloader_display_copyfail;
+ bl->display.callback = bootloader_display_copyfail;
+ bl->display.callback_pollhup = bootloader_display_copyfail;
rc = libxl__datacopier_start(&bl->display);
if (rc) goto out;
@@ -562,30 +567,42 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
/* perhaps one of these will be called, but perhaps not */
static void bootloader_copyfail(libxl__egc *egc, const char *which,
- libxl__bootloader_state *bl, int onwrite, int errnoval)
+ libxl__bootloader_state *bl, int ondisplay, int onwrite, int errnoval)
{
STATE_AO_GC(bl->ao);
+ int rc = ERROR_FAIL;
+
+ if (errnoval==-1) {
+ /* POLLHUP */
+ if (!!ondisplay != !!onwrite) {
+ rc = 0;
+ bl->got_pollhup = 1;
+ } else {
+ LOG(ERROR, "unexpected POLLHUP on %s", which);
+ }
+ }
if (!onwrite && !errnoval)
LOG(ERROR, "unexpected eof copying %s", which);
- bootloader_abort(egc, bl, ERROR_FAIL);
+
+ bootloader_stop(egc, bl, rc);
}
static void bootloader_keystrokes_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
- bootloader_copyfail(egc, "bootloader input", bl, onwrite, errnoval);
+ bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, errnoval);
}
static void bootloader_display_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
- bootloader_copyfail(egc, "bootloader output", bl, onwrite, errnoval);
+ bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval);
}
static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck *dc)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, deathcheck);
- bootloader_abort(egc, bl, ERROR_FAIL);
+ bootloader_stop(egc, bl, ERROR_FAIL);
}
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
@@ -599,6 +616,8 @@ static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
libxl__datacopier_kill(&bl->display);
if (status) {
+ if (bl->got_pollhup && WIFSIGNALED(status) && WTERMSIG(status)==SIGTERM)
+ LOG(ERROR, "got POLLHUP, sent SIGTERM");
LOG(ERROR, "bootloader failed - consult logfile %s", bl->logfile);
libxl_report_child_exitstatus(CTX, XTL_ERROR, "bootloader",
pid, status);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 58004b3..2d6c71a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2076,7 +2076,9 @@ typedef struct libxl__datacopier_buf libxl__datacopier_buf;
* errnoval==0 means we got eof and all data was written
* errnoval!=0 means we had a read error, logged
* onwrite==-1 means some other internal failure, errnoval not valid, logged
- * in all cases copier is killed before calling this callback */
+ * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1);
+ * or if callback_pollhup==0 this is an internal failure, as above.
+ * In all cases copier is killed before calling this callback */
typedef void libxl__datacopier_callback(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
@@ -2095,6 +2097,7 @@ struct libxl__datacopier_state {
const char *copywhat, *readwhat, *writewhat; /* for error msgs */
FILE *log; /* gets a copy of everything */
libxl__datacopier_callback *callback;
+ libxl__datacopier_callback *callback_pollhup;
/* remaining fields are private to datacopier */
libxl__ev_fd toread, towrite;
ssize_t used;
@@ -2279,7 +2282,7 @@ struct libxl__bootloader_state {
int nargs, argsspace;
const char **args;
libxl__datacopier_state keystrokes, display;
- int rc;
+ int rc, got_pollhup;
};
_hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 02/13] libxl: react correctly to bootloader pty master POLLHUP
2012-08-02 17:27 ` [PATCH 02/13] libxl: react correctly to bootloader pty master POLLHUP Ian Jackson
@ 2012-08-03 8:32 ` Ian Campbell
0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2012-08-03 8:32 UTC (permalink / raw)
To: Ian Jackson; +Cc: Roger Pau Monne, xen-devel@lists.xen.org
On Thu, 2012-08-02 at 18:27 +0100, Ian Jackson wrote:
> Receive POLLHUP on the bootloader master pty is not an error.
> Hopefully it means that the bootloader has exited and therefore the
> pty slave side has no process group any more. (At least NetBSD
> indicates POLLHUP on the master in this case.)
>
> So send the bootloader SIGTERM; if it has already exited then this has
> no effect (except that on some versions of NetBSD it erroneously
> returns ESRCH and we print a harmless warning) and we will then
> collect the bootloader's exit status and be satisfied.
>
> However, we remember that we have done this so that if we got POLLHUP
> for some other reason than that the bootloader exited we report
> something resembling a useful message.
>
> In order to implement this we need to provide a way for users of
> datacopier to handle POLLHUP rather than treating it as fatal.
>
> We rename bootloader_abort to bootloader_stop since it now no longer
> only applies to error situations.
>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> -
> Changes in v5:
> * Correctly call dc->callback_pollhup, not dc->callback,
> in datacopier_pollhup_handled.
Therefore:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Changes in v4:
> * Track whether we sent SIGTERM due to POLLHUP so we can report
> messages properly.
>
> Changes in v3:
> * datacopier provides new interface for handling POLLHUP
> * Do not ignore errors on the xenconsole pty
> * Rename bootloader_abort.
> ---
> tools/libxl/libxl_aoutils.c | 23 +++++++++++++++++++++++
> tools/libxl/libxl_bootloader.c | 39 +++++++++++++++++++++++++++++----------
> tools/libxl/libxl_internal.h | 7 +++++--
> 3 files changed, 57 insertions(+), 12 deletions(-)
>
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 99972a2..983a60a 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -97,11 +97,31 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc,
> LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
> }
>
> +static int datacopier_pollhup_handled(libxl__egc *egc,
> + libxl__datacopier_state *dc,
> + short revents, int onwrite)
> +{
> + STATE_AO_GC(dc->ao);
> +
> + if (dc->callback_pollhup && (revents & POLLHUP)) {
> + LOG(DEBUG, "received POLLHUP on %s during copy of %s",
> + onwrite ? dc->writewhat : dc->readwhat,
> + dc->copywhat);
> + libxl__datacopier_kill(dc);
> + dc->callback_pollhup(egc, dc, onwrite, -1);
> + return 1;
> + }
> + return 0;
> +}
> +
> static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
> int fd, short events, short revents) {
> libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
> STATE_AO_GC(dc->ao);
>
> + if (datacopier_pollhup_handled(egc, dc, revents, 0))
> + return;
> +
> if (revents & ~POLLIN) {
> LOG(ERROR, "unexpected poll event 0x%x (should be POLLIN)"
> " on %s during copy of %s", revents, dc->readwhat, dc->copywhat);
> @@ -163,6 +183,9 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
> libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, towrite);
> STATE_AO_GC(dc->ao);
>
> + if (datacopier_pollhup_handled(egc, dc, revents, 1))
> + return;
> +
> if (revents & ~POLLOUT) {
> LOG(ERROR, "unexpected poll event 0x%x (should be POLLOUT)"
> " on %s during copy of %s", revents, dc->writewhat, dc->copywhat);
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index ef5a91b..bfc1b56 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -215,6 +215,7 @@ void libxl__bootloader_init(libxl__bootloader_state *bl)
> libxl__domaindeathcheck_init(&bl->deathcheck);
> bl->keystrokes.ao = bl->ao; libxl__datacopier_init(&bl->keystrokes);
> bl->display.ao = bl->ao; libxl__datacopier_init(&bl->display);
> + bl->got_pollhup = 0;
> }
>
> static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
> @@ -275,7 +276,7 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
> }
>
> /* might be called at any time, provided it's init'd */
> -static void bootloader_abort(libxl__egc *egc,
> +static void bootloader_stop(libxl__egc *egc,
> libxl__bootloader_state *bl, int rc)
> {
> STATE_AO_GC(bl->ao);
> @@ -285,8 +286,8 @@ static void bootloader_abort(libxl__egc *egc,
> libxl__datacopier_kill(&bl->display);
> if (libxl__ev_child_inuse(&bl->child)) {
> r = kill(bl->child.pid, SIGTERM);
> - if (r) LOGE(WARN, "after failure, failed to kill bootloader [%lu]",
> - (unsigned long)bl->child.pid);
> + if (r) LOGE(WARN, "%sfailed to kill bootloader [%lu]",
> + rc ? "after failure, " : "", (unsigned long)bl->child.pid);
> }
> bl->rc = rc;
> }
> @@ -508,7 +509,10 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
> bl->keystrokes.maxsz = BOOTLOADER_BUF_OUT;
> bl->keystrokes.copywhat =
> GCSPRINTF("bootloader input for domain %"PRIu32, bl->domid);
> - bl->keystrokes.callback = bootloader_keystrokes_copyfail;
> + bl->keystrokes.callback = bootloader_keystrokes_copyfail;
> + bl->keystrokes.callback_pollhup = bootloader_keystrokes_copyfail;
> + /* pollhup gets called with errnoval==-1 which is not otherwise
> + * possible since errnos are nonnegative, so it's unambiguous */
> rc = libxl__datacopier_start(&bl->keystrokes);
> if (rc) goto out;
>
> @@ -516,7 +520,8 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
> bl->display.maxsz = BOOTLOADER_BUF_IN;
> bl->display.copywhat =
> GCSPRINTF("bootloader output for domain %"PRIu32, bl->domid);
> - bl->display.callback = bootloader_display_copyfail;
> + bl->display.callback = bootloader_display_copyfail;
> + bl->display.callback_pollhup = bootloader_display_copyfail;
> rc = libxl__datacopier_start(&bl->display);
> if (rc) goto out;
>
> @@ -562,30 +567,42 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
>
> /* perhaps one of these will be called, but perhaps not */
> static void bootloader_copyfail(libxl__egc *egc, const char *which,
> - libxl__bootloader_state *bl, int onwrite, int errnoval)
> + libxl__bootloader_state *bl, int ondisplay, int onwrite, int errnoval)
> {
> STATE_AO_GC(bl->ao);
> + int rc = ERROR_FAIL;
> +
> + if (errnoval==-1) {
> + /* POLLHUP */
> + if (!!ondisplay != !!onwrite) {
> + rc = 0;
> + bl->got_pollhup = 1;
> + } else {
> + LOG(ERROR, "unexpected POLLHUP on %s", which);
> + }
> + }
> if (!onwrite && !errnoval)
> LOG(ERROR, "unexpected eof copying %s", which);
> - bootloader_abort(egc, bl, ERROR_FAIL);
> +
> + bootloader_stop(egc, bl, rc);
> }
> static void bootloader_keystrokes_copyfail(libxl__egc *egc,
> libxl__datacopier_state *dc, int onwrite, int errnoval)
> {
> libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
> - bootloader_copyfail(egc, "bootloader input", bl, onwrite, errnoval);
> + bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, errnoval);
> }
> static void bootloader_display_copyfail(libxl__egc *egc,
> libxl__datacopier_state *dc, int onwrite, int errnoval)
> {
> libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
> - bootloader_copyfail(egc, "bootloader output", bl, onwrite, errnoval);
> + bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval);
> }
>
> static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck *dc)
> {
> libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, deathcheck);
> - bootloader_abort(egc, bl, ERROR_FAIL);
> + bootloader_stop(egc, bl, ERROR_FAIL);
> }
>
> static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
> @@ -599,6 +616,8 @@ static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
> libxl__datacopier_kill(&bl->display);
>
> if (status) {
> + if (bl->got_pollhup && WIFSIGNALED(status) && WTERMSIG(status)==SIGTERM)
> + LOG(ERROR, "got POLLHUP, sent SIGTERM");
> LOG(ERROR, "bootloader failed - consult logfile %s", bl->logfile);
> libxl_report_child_exitstatus(CTX, XTL_ERROR, "bootloader",
> pid, status);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 58004b3..2d6c71a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2076,7 +2076,9 @@ typedef struct libxl__datacopier_buf libxl__datacopier_buf;
> * errnoval==0 means we got eof and all data was written
> * errnoval!=0 means we had a read error, logged
> * onwrite==-1 means some other internal failure, errnoval not valid, logged
> - * in all cases copier is killed before calling this callback */
> + * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1);
> + * or if callback_pollhup==0 this is an internal failure, as above.
> + * In all cases copier is killed before calling this callback */
> typedef void libxl__datacopier_callback(libxl__egc *egc,
> libxl__datacopier_state *dc, int onwrite, int errnoval);
>
> @@ -2095,6 +2097,7 @@ struct libxl__datacopier_state {
> const char *copywhat, *readwhat, *writewhat; /* for error msgs */
> FILE *log; /* gets a copy of everything */
> libxl__datacopier_callback *callback;
> + libxl__datacopier_callback *callback_pollhup;
> /* remaining fields are private to datacopier */
> libxl__ev_fd toread, towrite;
> ssize_t used;
> @@ -2279,7 +2282,7 @@ struct libxl__bootloader_state {
> int nargs, argsspace;
> const char **args;
> libxl__datacopier_state keystrokes, display;
> - int rc;
> + int rc, got_pollhup;
> };
>
> _hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 03/13] libxl: fix device counting race in libxl__devices_destroy
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
2012-08-02 17:27 ` [PATCH 01/13] libxl: unify libxl__device_destroy and device_hotplug_done Ian Jackson
2012-08-02 17:27 ` [PATCH 02/13] libxl: react correctly to bootloader pty master POLLHUP Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 04/13] libxl: fix formatting of DEFINE_DEVICES_ADD Ian Jackson
` (10 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monne
Don't have a fixed number of devices in the aodevs array, and instead
size it depending on the devices present in xenstore. Somewhat
formalise the multiple device addition/removal machinery to make this
clearer and easier to do.
As a side-effect we fix a few "lost thread of control" bug which would
occur if there were no devices of a particular kind. (Various if
statements which checked for there being no devices have become
redundant, but are retained to avoid making the patch bigger.)
Specifically:
* Users of libxl__ao_devices are no longer expected to know in
advance how many device operations they are going to do. Instead
they can initiate them one at a time, between bracketing calls to
"begin" and "prepared".
* The array of aodevs used for this is dynamically sized; to support
this it's an array of pointers rather than of structs.
* Users of libxl__ao_devices are presented with a more opaque interface.
They are are no longer expected to, themselves,
- look into the array of aodevs (this is now private)
- know that the individual addition/removal completions are
handled by libxl__ao_devices_callback (this callback function
is now a private function for the multidev machinery)
- ever deal with populating the contents of an aodevs
* The doc comments relating to some of the members of
libxl__ao_device are clarified. (And the member `aodevs' is moved
to put it with the other members with the same status.)
* The multidev machinery allocates an aodev to represent the
operation of preparing all of the other operations. See
the comment in libxl__multidev_begin.
A wrinkle is that the functions are called "multidev" but the structs
are called "libxl__ao_devices" and "aodevs". I have given these
functions this name to distinguish them from "libxl__ao_device" and
"aodev" and so forth by more than just the use of the plural "s"
suffix.
In the next patch we will rename the structs.
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@eu.citrix.com>
-
Changes in v4:
* Actually honour errors in rc argument to libxl__multidev_prepared.
* Fix the doc comment for libxl__add_*.
* In comments, consistently use "multidev" not "multidevs".
Changes in v3:
* New multidev interfaces - extensive changes.
---
tools/libxl/libxl_create.c | 8 +-
tools/libxl/libxl_device.c | 129 +++++++++++++++++++-----------------------
tools/libxl/libxl_dm.c | 8 +-
tools/libxl/libxl_internal.h | 75 +++++++++++++++----------
4 files changed, 111 insertions(+), 109 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index aafacd8..3265d69 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -909,10 +909,10 @@ static void domcreate_rebuild_done(libxl__egc *egc,
store_libxl_entry(gc, domid, &d_config->b_info);
- dcs->aodevs.size = d_config->num_disks;
+ libxl__multidev_begin(ao, &dcs->aodevs);
dcs->aodevs.callback = domcreate_launch_dm;
- libxl__prepare_ao_devices(ao, &dcs->aodevs);
libxl__add_disks(egc, ao, domid, 0, d_config, &dcs->aodevs);
+ libxl__multidev_prepared(egc, &dcs->aodevs, 0);
return;
@@ -1039,10 +1039,10 @@ static void domcreate_devmodel_started(libxl__egc *egc,
/* Plug nic interfaces */
if (d_config->num_nics > 0) {
/* Attach nics */
- dcs->aodevs.size = d_config->num_nics;
+ libxl__multidev_begin(ao, &dcs->aodevs);
dcs->aodevs.callback = domcreate_attach_pci;
- libxl__prepare_ao_devices(ao, &dcs->aodevs);
libxl__add_nics(egc, ao, domid, 0, d_config, &dcs->aodevs);
+ libxl__multidev_prepared(egc, &dcs->aodevs, 0);
return;
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 95b169e..79dd502 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -58,50 +58,6 @@ 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;
- if (kind == LIBXL__DEVICE_KIND_CONSOLE)
- 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__nic_type(libxl__gc *gc, libxl__device *dev, libxl_nic_type *nictype)
{
char *snictype, *be_path;
@@ -445,40 +401,81 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
libxl__ev_child_init(&aodev->child);
}
-void libxl__prepare_ao_devices(libxl__ao *ao, libxl__ao_devices *aodevs)
+/* multidev */
+
+void libxl__multidev_begin(libxl__ao *ao, libxl__ao_devices *aodevs)
{
AO_GC;
- GCNEW_ARRAY(aodevs->array, aodevs->size);
- for (int i = 0; i < aodevs->size; i++) {
- aodevs->array[i].aodevs = aodevs;
- libxl__prepare_ao_device(ao, &aodevs->array[i]);
+ aodevs->ao = ao;
+ aodevs->array = 0;
+ aodevs->used = aodevs->allocd = 0;
+
+ /* We allocate an aodev to represent the operation of preparing
+ * all of the other operations. This operation is completed when
+ * we have started all the others (ie, when the user calls
+ * _prepared). That arranges automatically that
+ * (i) we do not think we have finished even if one of the
+ * operations completes while we are still preparing
+ * (ii) if we are starting zero operations, we do still
+ * make the callback as soon as we know this fact
+ * (iii) we have a nice consistent way to deal with any
+ * error that might occur while deciding what to initiate
+ */
+ aodevs->preparation = libxl__multidev_prepare(aodevs);
+}
+
+static void multidev_one_callback(libxl__egc *egc, libxl__ao_device *aodev);
+
+libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices *aodevs) {
+ STATE_AO_GC(aodevs->ao);
+ libxl__ao_device *aodev;
+
+ GCNEW(aodev);
+ aodev->aodevs = aodevs;
+ aodev->callback = multidev_one_callback;
+ libxl__prepare_ao_device(ao, aodev);
+
+ if (aodevs->used >= aodevs->allocd) {
+ aodevs->allocd = aodevs->used * 2 + 5;
+ GCREALLOC_ARRAY(aodevs->array, aodevs->allocd);
}
+ aodevs->array[aodevs->used++] = aodev;
+
+ return aodev;
}
-void libxl__ao_devices_callback(libxl__egc *egc, libxl__ao_device *aodev)
+static void multidev_one_callback(libxl__egc *egc, libxl__ao_device *aodev)
{
STATE_AO_GC(aodev->ao);
libxl__ao_devices *aodevs = aodev->aodevs;
int i, error = 0;
aodev->active = 0;
- for (i = 0; i < aodevs->size; i++) {
- if (aodevs->array[i].active)
+
+ for (i = 0; i < aodevs->used; i++) {
+ if (aodevs->array[i]->active)
return;
- if (aodevs->array[i].rc)
- error = aodevs->array[i].rc;
+ if (aodevs->array[i]->rc)
+ error = aodevs->array[i]->rc;
}
aodevs->callback(egc, aodevs, error);
return;
}
+void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
+ int rc)
+{
+ aodevs->preparation->rc = rc;
+ multidev_one_callback(egc, aodevs->preparation);
+}
+
/******************************************************************************/
/* Macro for defining the functions that will add a bunch of disks when
- * inside an async op.
+ * inside an async op with multidev.
* This macro is added to prevent repetition of code.
*
* The following functions are defined:
@@ -495,9 +492,9 @@ void libxl__ao_devices_callback(libxl__egc *egc, libxl__ao_device *aodev)
int i; \
int end = start + d_config->num_##type##s; \
for (i = start; i < end; i++) { \
- aodevs->array[i].callback = libxl__ao_devices_callback; \
+ libxl__ao_device *aodev = libxl__multidev_prepare(aodevs); \
libxl__device_##type##_add(egc, domid, &d_config->type##s[i-start],\
- &aodevs->array[i]); \
+ aodev); \
} \
}
@@ -547,20 +544,13 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
char *path;
unsigned int num_kinds, num_dev_xsentries;
char **kinds = NULL, **devs = NULL;
- int i, j, numdev = 0, rc = 0;
+ int i, j, rc = 0;
libxl__device *dev;
libxl__ao_devices *aodevs = &drs->aodevs;
libxl__ao_device *aodev;
libxl__device_kind kind;
- aodevs->size = libxl__num_devices(gc, drs->domid);
- if (aodevs->size < 0) {
- LOG(ERROR, "unable to get number of devices for domain %u", drs->domid);
- rc = aodevs->size;
- goto out;
- }
-
- libxl__prepare_ao_devices(drs->ao, aodevs);
+ libxl__multidev_begin(ao, aodevs);
aodevs->callback = devices_remove_callback;
path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
@@ -598,13 +588,11 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
libxl__device_destroy(gc, dev);
continue;
}
- aodev = &aodevs->array[numdev];
+ aodev = libxl__multidev_prepare(aodevs);
aodev->action = DEVICE_DISCONNECT;
aodev->dev = dev;
- aodev->callback = libxl__ao_devices_callback;
aodev->force = drs->force;
libxl__initiate_device_remove(egc, aodev);
- numdev++;
}
}
}
@@ -626,8 +614,7 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
}
out:
- if (!numdev) drs->callback(egc, drs, rc);
- return;
+ libxl__multidev_prepared(egc, aodevs, rc);
}
/* Callbacks for device related operations */
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f2e9572..177642b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -856,10 +856,10 @@ retry_transaction:
if (errno == EAGAIN)
goto retry_transaction;
- sdss->aodevs.size = dm_config->num_disks;
+ libxl__multidev_begin(ao, &sdss->aodevs);
sdss->aodevs.callback = spawn_stub_launch_dm;
- libxl__prepare_ao_devices(ao, &sdss->aodevs);
libxl__add_disks(egc, ao, dm_domid, 0, dm_config, &sdss->aodevs);
+ libxl__multidev_prepared(egc, &sdss->aodevs, 0);
free(args);
return;
@@ -982,10 +982,10 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
if (rc) goto out;
if (d_config->num_nics > 0) {
- sdss->aodevs.size = d_config->num_nics;
+ libxl__multidev_begin(ao, &sdss->aodevs);
sdss->aodevs.callback = stubdom_pvqemu_cb;
- libxl__prepare_ao_devices(ao, &sdss->aodevs);
libxl__add_nics(egc, ao, dm_domid, 0, d_config, &sdss->aodevs);
+ libxl__multidev_prepared(egc, &sdss->aodevs, 0);
return;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2d6c71a..07e92fb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1816,20 +1816,6 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
*/
_hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev);
-/* Prepare a bunch of devices for addition/removal. Every ao_device in
- * ao_devices is set to 'active', and the ao_device 'base' field is set to
- * the one pointed by aodevs.
- */
-_hidden void libxl__prepare_ao_devices(libxl__ao *ao,
- libxl__ao_devices *aodevs);
-
-/* Generic callback to use when adding/removing several devices, this will
- * check if the given aodev is the last one, and call the callback in the
- * parent libxl__ao_devices struct, passing the appropriate error if found.
- */
-_hidden void libxl__ao_devices_callback(libxl__egc *egc,
- libxl__ao_device *aodev);
-
struct libxl__ao_device {
/* filled in by user */
libxl__ao *ao;
@@ -1837,32 +1823,60 @@ struct libxl__ao_device {
libxl__device *dev;
int force;
libxl__device_callback *callback;
- /* private for implementation */
- int active;
+ /* return value, zeroed by user on entry, is valid on callback */
int rc;
+ /* private for multidev */
+ int active;
+ libxl__ao_devices *aodevs; /* reference to the containing multidev */
+ /* private for add/remove implementation */
libxl__ev_devstate backend_ds;
/* Bodge for Qemu devices, also used for timeout of hotplug execution */
libxl__ev_time timeout;
- /* Used internally to have a reference to the upper libxl__ao_devices
- * struct when present */
- libxl__ao_devices *aodevs;
/* device hotplug execution */
const char *what;
int num_exec;
libxl__ev_child child;
};
-/* Helper struct to simply the plug/unplug of multiple devices at the same
- * time.
- *
- * This structure holds several devices, and the callback is only called
- * when all the devices inside of the array have finished.
- */
+/*
+ * Multiple devices "multidev" handling.
+ *
+ * Firstly, you should
+ * libxl__multidev_begin
+ * multidev->callback = ...
+ * Then zero or more times
+ * libxl__multidev_prepare
+ * libal__initiate_device_{remove/addition}.
+ * Finally, once
+ * libxl__multidev_prepared
+ * which will result (perhaps reentrantly) in one call to callback().
+ */
+
+/* Starts preparing to add/remove a bunch of devices. */
+_hidden void libxl__multidev_begin(libxl__ao *ao, libxl__ao_devices*);
+
+/* Prepares to add/remove one of many devices. Returns a libxl__ao_device
+ * which has had libxl__prepare_ao_device called, and which has also
+ * had ->callback set. The user should not mess with aodev->callback. */
+_hidden libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices*);
+
+/* Notifies the multidev machinery that we have now finished preparing
+ * and initiating devices. multidev->callback may then be called as
+ * soon as there are no prepared but not completed operations
+ * outstanding, perhaps reentrantly. If rc!=0 (error should have been
+ * logged) multidev->callback will get a non-zero rc.
+ * callback may be set by the user at any point before prepared. */
+_hidden void libxl__multidev_prepared(libxl__egc*, libxl__ao_devices*, int rc);
+
typedef void libxl__devices_callback(libxl__egc*, libxl__ao_devices*, int rc);
struct libxl__ao_devices {
- libxl__ao_device *array;
- int size;
+ /* set by user: */
libxl__devices_callback *callback;
+ /* for private use by libxl__...ao_devices... machinery: */
+ libxl__ao *ao;
+ libxl__ao_device **array;
+ int used, allocd;
+ libxl__ao_device *preparation;
};
/*
@@ -2372,10 +2386,11 @@ _hidden void libxl__devices_destroy(libxl__egc *egc,
libxl__devices_remove_state *drs);
/* Helper function to add a bunch of disks. This should be used when
- * the caller is inside an async op. "devices" will NOT be prepared by this
- * function, so the caller must make sure to call _prepare before calling this
- * function. The start parameter contains the position inside the aodevs array
- * that should be used to store the state of this devices.
+ * the caller is inside an async op. "devices" will NOT be prepared by
+ * this function, so the caller must make sure to call
+ * libxl__multidev_begin before calling this function. The start
+ * parameter contains the position inside the aodevs array that should
+ * be used to store the state of this devices.
*
* The "callback" will be called for each device, and the user is responsible
* for calling libxl__ao_device_check_last on the callback.
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/13] libxl: fix formatting of DEFINE_DEVICES_ADD
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (2 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 03/13] libxl: fix device counting race in libxl__devices_destroy Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 05/13] libxl: abolish useless `start' parameter to libxl__add_* Ian Jackson
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
These lines were exactly 80 columns wide, which produces hideous wrap
damage in an 80 column emacs. Reformat using emacs's C-c \,
which puts the \ in column 72 (by default) where possible.
Whitespace change only.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_device.c | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 79dd502..4a53181 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -483,19 +483,19 @@ void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
* libxl__add_nics
*/
-#define DEFINE_DEVICES_ADD(type) \
- void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
- int start, libxl_domain_config *d_config, \
- libxl__ao_devices *aodevs) \
- { \
- AO_GC; \
- int i; \
- int end = start + d_config->num_##type##s; \
- for (i = start; i < end; i++) { \
- libxl__ao_device *aodev = libxl__multidev_prepare(aodevs); \
- libxl__device_##type##_add(egc, domid, &d_config->type##s[i-start],\
- aodev); \
- } \
+#define DEFINE_DEVICES_ADD(type) \
+ void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
+ int start, libxl_domain_config *d_config, \
+ libxl__ao_devices *aodevs) \
+ { \
+ AO_GC; \
+ int i; \
+ int end = start + d_config->num_##type##s; \
+ for (i = start; i < end; i++) { \
+ libxl__ao_device *aodev = libxl__multidev_prepare(aodevs); \
+ libxl__device_##type##_add(egc, domid, &d_config->type##s[i-start], \
+ aodev); \
+ } \
}
DEFINE_DEVICES_ADD(disk)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/13] libxl: abolish useless `start' parameter to libxl__add_*
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (3 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 04/13] libxl: fix formatting of DEFINE_DEVICES_ADD Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 06/13] libxl: rename aodevs to multidev Ian Jackson
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
0 is always passed for this parameter and the code doesn't, actually,
use it, now.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_create.c | 4 ++--
tools/libxl/libxl_device.c | 7 +++----
tools/libxl/libxl_dm.c | 4 ++--
tools/libxl/libxl_internal.h | 8 +++-----
4 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3265d69..5275373 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -911,7 +911,7 @@ static void domcreate_rebuild_done(libxl__egc *egc,
libxl__multidev_begin(ao, &dcs->aodevs);
dcs->aodevs.callback = domcreate_launch_dm;
- libxl__add_disks(egc, ao, domid, 0, d_config, &dcs->aodevs);
+ libxl__add_disks(egc, ao, domid, d_config, &dcs->aodevs);
libxl__multidev_prepared(egc, &dcs->aodevs, 0);
return;
@@ -1041,7 +1041,7 @@ static void domcreate_devmodel_started(libxl__egc *egc,
/* Attach nics */
libxl__multidev_begin(ao, &dcs->aodevs);
dcs->aodevs.callback = domcreate_attach_pci;
- libxl__add_nics(egc, ao, domid, 0, d_config, &dcs->aodevs);
+ libxl__add_nics(egc, ao, domid, d_config, &dcs->aodevs);
libxl__multidev_prepared(egc, &dcs->aodevs, 0);
return;
}
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4a53181..27fbd21 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -485,15 +485,14 @@ void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
#define DEFINE_DEVICES_ADD(type) \
void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
- int start, libxl_domain_config *d_config, \
+ libxl_domain_config *d_config, \
libxl__ao_devices *aodevs) \
{ \
AO_GC; \
int i; \
- int end = start + d_config->num_##type##s; \
- for (i = start; i < end; i++) { \
+ for (i = 0; i < d_config->num_##type##s; i++) { \
libxl__ao_device *aodev = libxl__multidev_prepare(aodevs); \
- libxl__device_##type##_add(egc, domid, &d_config->type##s[i-start], \
+ libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \
aodev); \
} \
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 177642b..66aa45e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -858,7 +858,7 @@ retry_transaction:
libxl__multidev_begin(ao, &sdss->aodevs);
sdss->aodevs.callback = spawn_stub_launch_dm;
- libxl__add_disks(egc, ao, dm_domid, 0, dm_config, &sdss->aodevs);
+ libxl__add_disks(egc, ao, dm_domid, dm_config, &sdss->aodevs);
libxl__multidev_prepared(egc, &sdss->aodevs, 0);
free(args);
@@ -984,7 +984,7 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
if (d_config->num_nics > 0) {
libxl__multidev_begin(ao, &sdss->aodevs);
sdss->aodevs.callback = stubdom_pvqemu_cb;
- libxl__add_nics(egc, ao, dm_domid, 0, d_config, &sdss->aodevs);
+ libxl__add_nics(egc, ao, dm_domid, d_config, &sdss->aodevs);
libxl__multidev_prepared(egc, &sdss->aodevs, 0);
return;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 07e92fb..bb3eb5f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2388,19 +2388,17 @@ _hidden void libxl__devices_destroy(libxl__egc *egc,
/* Helper function to add a bunch of disks. This should be used when
* the caller is inside an async op. "devices" will NOT be prepared by
* this function, so the caller must make sure to call
- * libxl__multidev_begin before calling this function. The start
- * parameter contains the position inside the aodevs array that should
- * be used to store the state of this devices.
+ * libxl__multidev_begin before calling this function.
*
* The "callback" will be called for each device, and the user is responsible
* for calling libxl__ao_device_check_last on the callback.
*/
_hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
- int start, libxl_domain_config *d_config,
+ libxl_domain_config *d_config,
libxl__ao_devices *aodevs);
_hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
- int start, libxl_domain_config *d_config,
+ libxl_domain_config *d_config,
libxl__ao_devices *aodevs);
/*----- device model creation -----*/
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/13] libxl: rename aodevs to multidev
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (4 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 05/13] libxl: abolish useless `start' parameter to libxl__add_* Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 07/13] libxl: do not blunder on if bootloader fails (again) Ian Jackson
` (7 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
To be consistent with the new function naming, rename
libxl__ao_devices to libxl__multidev and all variables aodevs to
multidev.
No functional change.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_create.c | 30 +++++++++---------
tools/libxl/libxl_device.c | 68 +++++++++++++++++++++---------------------
tools/libxl/libxl_dm.c | 30 +++++++++---------
tools/libxl/libxl_internal.h | 26 ++++++++--------
4 files changed, 77 insertions(+), 77 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 5275373..5f0d26f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -599,10 +599,10 @@ static void domcreate_bootloader_done(libxl__egc *egc,
libxl__bootloader_state *bl,
int rc);
-static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
+static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
-static void domcreate_attach_pci(libxl__egc *egc, libxl__ao_devices *aodevs,
+static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
static void domcreate_console_available(libxl__egc *egc,
@@ -909,10 +909,10 @@ static void domcreate_rebuild_done(libxl__egc *egc,
store_libxl_entry(gc, domid, &d_config->b_info);
- libxl__multidev_begin(ao, &dcs->aodevs);
- dcs->aodevs.callback = domcreate_launch_dm;
- libxl__add_disks(egc, ao, domid, d_config, &dcs->aodevs);
- libxl__multidev_prepared(egc, &dcs->aodevs, 0);
+ libxl__multidev_begin(ao, &dcs->multidev);
+ dcs->multidev.callback = domcreate_launch_dm;
+ libxl__add_disks(egc, ao, domid, d_config, &dcs->multidev);
+ libxl__multidev_prepared(egc, &dcs->multidev, 0);
return;
@@ -921,10 +921,10 @@ static void domcreate_rebuild_done(libxl__egc *egc,
domcreate_complete(egc, dcs, ret);
}
-static void domcreate_launch_dm(libxl__egc *egc, libxl__ao_devices *aodevs,
+static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
int ret)
{
- libxl__domain_create_state *dcs = CONTAINER_OF(aodevs, *dcs, aodevs);
+ libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
STATE_AO_GC(dcs->ao);
int i;
@@ -1039,14 +1039,14 @@ static void domcreate_devmodel_started(libxl__egc *egc,
/* Plug nic interfaces */
if (d_config->num_nics > 0) {
/* Attach nics */
- libxl__multidev_begin(ao, &dcs->aodevs);
- dcs->aodevs.callback = domcreate_attach_pci;
- libxl__add_nics(egc, ao, domid, d_config, &dcs->aodevs);
- libxl__multidev_prepared(egc, &dcs->aodevs, 0);
+ libxl__multidev_begin(ao, &dcs->multidev);
+ dcs->multidev.callback = domcreate_attach_pci;
+ libxl__add_nics(egc, ao, domid, d_config, &dcs->multidev);
+ libxl__multidev_prepared(egc, &dcs->multidev, 0);
return;
}
- domcreate_attach_pci(egc, &dcs->aodevs, 0);
+ domcreate_attach_pci(egc, &dcs->multidev, 0);
return;
error_out:
@@ -1054,10 +1054,10 @@ error_out:
domcreate_complete(egc, dcs, ret);
}
-static void domcreate_attach_pci(libxl__egc *egc, libxl__ao_devices *aodevs,
+static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
int ret)
{
- libxl__domain_create_state *dcs = CONTAINER_OF(aodevs, *dcs, aodevs);
+ libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
STATE_AO_GC(dcs->ao);
int i;
libxl_ctx *ctx = CTX;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 27fbd21..9fc63f1 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -403,13 +403,13 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
/* multidev */
-void libxl__multidev_begin(libxl__ao *ao, libxl__ao_devices *aodevs)
+void libxl__multidev_begin(libxl__ao *ao, libxl__multidev *multidev)
{
AO_GC;
- aodevs->ao = ao;
- aodevs->array = 0;
- aodevs->used = aodevs->allocd = 0;
+ multidev->ao = ao;
+ multidev->array = 0;
+ multidev->used = multidev->allocd = 0;
/* We allocate an aodev to represent the operation of preparing
* all of the other operations. This operation is completed when
@@ -422,25 +422,25 @@ void libxl__multidev_begin(libxl__ao *ao, libxl__ao_devices *aodevs)
* (iii) we have a nice consistent way to deal with any
* error that might occur while deciding what to initiate
*/
- aodevs->preparation = libxl__multidev_prepare(aodevs);
+ multidev->preparation = libxl__multidev_prepare(multidev);
}
static void multidev_one_callback(libxl__egc *egc, libxl__ao_device *aodev);
-libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices *aodevs) {
- STATE_AO_GC(aodevs->ao);
+libxl__ao_device *libxl__multidev_prepare(libxl__multidev *multidev) {
+ STATE_AO_GC(multidev->ao);
libxl__ao_device *aodev;
GCNEW(aodev);
- aodev->aodevs = aodevs;
+ aodev->multidev = multidev;
aodev->callback = multidev_one_callback;
libxl__prepare_ao_device(ao, aodev);
- if (aodevs->used >= aodevs->allocd) {
- aodevs->allocd = aodevs->used * 2 + 5;
- GCREALLOC_ARRAY(aodevs->array, aodevs->allocd);
+ if (multidev->used >= multidev->allocd) {
+ multidev->allocd = multidev->used * 2 + 5;
+ GCREALLOC_ARRAY(multidev->array, multidev->allocd);
}
- aodevs->array[aodevs->used++] = aodev;
+ multidev->array[multidev->used++] = aodev;
return aodev;
}
@@ -448,28 +448,28 @@ libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices *aodevs) {
static void multidev_one_callback(libxl__egc *egc, libxl__ao_device *aodev)
{
STATE_AO_GC(aodev->ao);
- libxl__ao_devices *aodevs = aodev->aodevs;
+ libxl__multidev *multidev = aodev->multidev;
int i, error = 0;
aodev->active = 0;
- for (i = 0; i < aodevs->used; i++) {
- if (aodevs->array[i]->active)
+ for (i = 0; i < multidev->used; i++) {
+ if (multidev->array[i]->active)
return;
- if (aodevs->array[i]->rc)
- error = aodevs->array[i]->rc;
+ if (multidev->array[i]->rc)
+ error = multidev->array[i]->rc;
}
- aodevs->callback(egc, aodevs, error);
+ multidev->callback(egc, multidev, error);
return;
}
-void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
- int rc)
+void libxl__multidev_prepared(libxl__egc *egc,
+ libxl__multidev *multidev, int rc)
{
- aodevs->preparation->rc = rc;
- multidev_one_callback(egc, aodevs->preparation);
+ multidev->preparation->rc = rc;
+ multidev_one_callback(egc, multidev->preparation);
}
/******************************************************************************/
@@ -486,12 +486,12 @@ void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
#define DEFINE_DEVICES_ADD(type) \
void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
libxl_domain_config *d_config, \
- libxl__ao_devices *aodevs) \
+ libxl__multidev *multidev) \
{ \
AO_GC; \
int i; \
for (i = 0; i < d_config->num_##type##s; i++) { \
- libxl__ao_device *aodev = libxl__multidev_prepare(aodevs); \
+ libxl__ao_device *aodev = libxl__multidev_prepare(multidev); \
libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \
aodev); \
} \
@@ -532,8 +532,8 @@ out:
/* Callback for device destruction */
-static void devices_remove_callback(libxl__egc *egc, libxl__ao_devices *aodevs,
- int rc);
+static void devices_remove_callback(libxl__egc *egc,
+ libxl__multidev *multidev, int rc);
void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
{
@@ -545,12 +545,12 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
char **kinds = NULL, **devs = NULL;
int i, j, rc = 0;
libxl__device *dev;
- libxl__ao_devices *aodevs = &drs->aodevs;
+ libxl__multidev *multidev = &drs->multidev;
libxl__ao_device *aodev;
libxl__device_kind kind;
- libxl__multidev_begin(ao, aodevs);
- aodevs->callback = devices_remove_callback;
+ libxl__multidev_begin(ao, multidev);
+ multidev->callback = devices_remove_callback;
path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
@@ -587,7 +587,7 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
libxl__device_destroy(gc, dev);
continue;
}
- aodev = libxl__multidev_prepare(aodevs);
+ aodev = libxl__multidev_prepare(multidev);
aodev->action = DEVICE_DISCONNECT;
aodev->dev = dev;
aodev->force = drs->force;
@@ -613,7 +613,7 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
}
out:
- libxl__multidev_prepared(egc, aodevs, rc);
+ libxl__multidev_prepared(egc, multidev, rc);
}
/* Callbacks for device related operations */
@@ -1003,10 +1003,10 @@ static void device_hotplug_clean(libxl__gc *gc, libxl__ao_device *aodev)
assert(!libxl__ev_child_inuse(&aodev->child));
}
-static void devices_remove_callback(libxl__egc *egc, libxl__ao_devices *aodevs,
- int rc)
+static void devices_remove_callback(libxl__egc *egc,
+ libxl__multidev *multidev, int rc)
{
- libxl__devices_remove_state *drs = CONTAINER_OF(aodevs, *drs, aodevs);
+ libxl__devices_remove_state *drs = CONTAINER_OF(multidev, *drs, multidev);
STATE_AO_GC(drs->ao);
drs->callback(egc, drs, rc);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 66aa45e..0c0084f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -714,10 +714,10 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
int rc);
static void spawn_stub_launch_dm(libxl__egc *egc,
- libxl__ao_devices *aodevs, int ret);
+ libxl__multidev *aodevs, int ret);
static void stubdom_pvqemu_cb(libxl__egc *egc,
- libxl__ao_devices *aodevs,
+ libxl__multidev *aodevs,
int rc);
static void spaw_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
@@ -856,10 +856,10 @@ retry_transaction:
if (errno == EAGAIN)
goto retry_transaction;
- libxl__multidev_begin(ao, &sdss->aodevs);
- sdss->aodevs.callback = spawn_stub_launch_dm;
- libxl__add_disks(egc, ao, dm_domid, dm_config, &sdss->aodevs);
- libxl__multidev_prepared(egc, &sdss->aodevs, 0);
+ libxl__multidev_begin(ao, &sdss->multidev);
+ sdss->multidev.callback = spawn_stub_launch_dm;
+ libxl__add_disks(egc, ao, dm_domid, dm_config, &sdss->multidev);
+ libxl__multidev_prepared(egc, &sdss->multidev, 0);
free(args);
return;
@@ -872,9 +872,9 @@ out:
}
static void spawn_stub_launch_dm(libxl__egc *egc,
- libxl__ao_devices *aodevs, int ret)
+ libxl__multidev *multidev, int ret)
{
- libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aodevs, *sdss, aodevs);
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(multidev, *sdss, multidev);
STATE_AO_GC(sdss->dm.spawn.ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
int i, num_console = STUBDOM_SPECIAL_CONSOLES;
@@ -982,22 +982,22 @@ static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
if (rc) goto out;
if (d_config->num_nics > 0) {
- libxl__multidev_begin(ao, &sdss->aodevs);
- sdss->aodevs.callback = stubdom_pvqemu_cb;
- libxl__add_nics(egc, ao, dm_domid, d_config, &sdss->aodevs);
- libxl__multidev_prepared(egc, &sdss->aodevs, 0);
+ libxl__multidev_begin(ao, &sdss->multidev);
+ sdss->multidev.callback = stubdom_pvqemu_cb;
+ libxl__add_nics(egc, ao, dm_domid, d_config, &sdss->multidev);
+ libxl__multidev_prepared(egc, &sdss->multidev, 0);
return;
}
out:
- stubdom_pvqemu_cb(egc, &sdss->aodevs, rc);
+ stubdom_pvqemu_cb(egc, &sdss->multidev, rc);
}
static void stubdom_pvqemu_cb(libxl__egc *egc,
- libxl__ao_devices *aodevs,
+ libxl__multidev *multidev,
int rc)
{
- libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(aodevs, *sdss, aodevs);
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(multidev, *sdss, multidev);
STATE_AO_GC(sdss->dm.spawn.ao);
uint32_t dm_domid = sdss->pvqemu.guest_domid;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index bb3eb5f..6528694 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1797,7 +1797,7 @@ typedef enum {
} libxl__device_action;
typedef struct libxl__ao_device libxl__ao_device;
-typedef struct libxl__ao_devices libxl__ao_devices;
+typedef struct libxl__multidev libxl__multidev;
typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
/* This functions sets the necessary libxl__ao_device struct values to use
@@ -1827,7 +1827,7 @@ struct libxl__ao_device {
int rc;
/* private for multidev */
int active;
- libxl__ao_devices *aodevs; /* reference to the containing multidev */
+ libxl__multidev *multidev; /* reference to the containing multidev */
/* private for add/remove implementation */
libxl__ev_devstate backend_ds;
/* Bodge for Qemu devices, also used for timeout of hotplug execution */
@@ -1853,12 +1853,12 @@ struct libxl__ao_device {
*/
/* Starts preparing to add/remove a bunch of devices. */
-_hidden void libxl__multidev_begin(libxl__ao *ao, libxl__ao_devices*);
+_hidden void libxl__multidev_begin(libxl__ao *ao, libxl__multidev*);
/* Prepares to add/remove one of many devices. Returns a libxl__ao_device
* which has had libxl__prepare_ao_device called, and which has also
* had ->callback set. The user should not mess with aodev->callback. */
-_hidden libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices*);
+_hidden libxl__ao_device *libxl__multidev_prepare(libxl__multidev*);
/* Notifies the multidev machinery that we have now finished preparing
* and initiating devices. multidev->callback may then be called as
@@ -1866,10 +1866,10 @@ _hidden libxl__ao_device *libxl__multidev_prepare(libxl__ao_devices*);
* outstanding, perhaps reentrantly. If rc!=0 (error should have been
* logged) multidev->callback will get a non-zero rc.
* callback may be set by the user at any point before prepared. */
-_hidden void libxl__multidev_prepared(libxl__egc*, libxl__ao_devices*, int rc);
+_hidden void libxl__multidev_prepared(libxl__egc*, libxl__multidev*, int rc);
-typedef void libxl__devices_callback(libxl__egc*, libxl__ao_devices*, int rc);
-struct libxl__ao_devices {
+typedef void libxl__devices_callback(libxl__egc*, libxl__multidev*, int rc);
+struct libxl__multidev {
/* set by user: */
libxl__devices_callback *callback;
/* for private use by libxl__...ao_devices... machinery: */
@@ -2342,7 +2342,7 @@ struct libxl__devices_remove_state {
libxl__devices_remove_callback *callback;
int force; /* libxl_device_TYPE_destroy rather than _remove */
/* private */
- libxl__ao_devices aodevs;
+ libxl__multidev multidev;
int num_devices;
};
@@ -2386,7 +2386,7 @@ _hidden void libxl__devices_destroy(libxl__egc *egc,
libxl__devices_remove_state *drs);
/* Helper function to add a bunch of disks. This should be used when
- * the caller is inside an async op. "devices" will NOT be prepared by
+ * the caller is inside an async op. "multidev" will NOT be prepared by
* this function, so the caller must make sure to call
* libxl__multidev_begin before calling this function.
*
@@ -2395,11 +2395,11 @@ _hidden void libxl__devices_destroy(libxl__egc *egc,
*/
_hidden void libxl__add_disks(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
libxl_domain_config *d_config,
- libxl__ao_devices *aodevs);
+ libxl__multidev *multidev);
_hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
libxl_domain_config *d_config,
- libxl__ao_devices *aodevs);
+ libxl__multidev *multidev);
/*----- device model creation -----*/
@@ -2435,7 +2435,7 @@ typedef struct {
libxl__domain_build_state dm_state;
libxl__dm_spawn_state pvqemu;
libxl__destroy_domid_state dis;
- libxl__ao_devices aodevs;
+ libxl__multidev multidev;
} libxl__stub_dm_spawn_state;
_hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
@@ -2467,7 +2467,7 @@ struct libxl__domain_create_state {
libxl__save_helper_state shs;
/* necessary if the domain creation failed and we have to destroy it */
libxl__domain_destroy_state dds;
- libxl__ao_devices aodevs;
+ libxl__multidev multidev;
};
/*----- Domain suspend (save) functions -----*/
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/13] libxl: do not blunder on if bootloader fails (again)
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (5 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 06/13] libxl: rename aodevs to multidev Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 08/13] libxl: remus: mark TODOs more clearly Ian Jackson
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Do not lose the rc value passed to bootloader_callback. Do not lose
the rc value from the bl when the local disk detach succeeds.
While we're here rationalise the use of bl->rc to make things clearer.
Set it to zero at the start and always update it conditionally; copy
it into bootloader_callback's argument each time.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_bootloader.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index bfc1b56..e103ee9 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -206,6 +206,7 @@ static int parse_bootloader_result(libxl__egc *egc,
void libxl__bootloader_init(libxl__bootloader_state *bl)
{
assert(bl->ao);
+ bl->rc = 0;
bl->dls.diskpath = NULL;
bl->openpty.ao = bl->ao;
bl->dls.ao = bl->ao;
@@ -255,6 +256,9 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
static void bootloader_callback(libxl__egc *egc, libxl__bootloader_state *bl,
int rc)
{
+ if (!bl->rc)
+ bl->rc = rc;
+
bootloader_cleanup(egc, bl);
bl->dls.callback = bootloader_local_detached_cb;
@@ -270,9 +274,11 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
if (rc) {
LOG(ERROR, "unable to detach locally attached disk");
+ if (!bl->rc)
+ bl->rc = rc;
}
- bl->callback(egc, bl, rc);
+ bl->callback(egc, bl, bl->rc);
}
/* might be called at any time, provided it's init'd */
@@ -289,7 +295,8 @@ static void bootloader_stop(libxl__egc *egc,
if (r) LOGE(WARN, "%sfailed to kill bootloader [%lu]",
rc ? "after failure, " : "", (unsigned long)bl->child.pid);
}
- bl->rc = rc;
+ if (!bl->rc)
+ bl->rc = rc;
}
/*----- main flow of control -----*/
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/13] libxl: remus: mark TODOs more clearly
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (6 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 07/13] libxl: do not blunder on if bootloader fails (again) Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 09/13] libxl: remove an unused numainfo parameter Ian Jackson
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Change the TODOs in the remus code to "REMUS TODO" which will make
them easier to grep for later. AIUI all of these are essential for
use of remus in production.
Also add a new TODO and a new assert, to check rc on entry to
remus_checkpoint_dm_saved.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_dom.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index d749983..06d5e4f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1110,7 +1110,7 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
static int libxl__remus_domain_suspend_callback(void *data)
{
- /* TODO: Issue disk and network checkpoint reqs. */
+ /* REMUS TODO: Issue disk and network checkpoint reqs. */
return libxl__domain_suspend_common_callback(data);
}
@@ -1124,7 +1124,7 @@ static int libxl__remus_domain_resume_callback(void *data)
if (libxl_domain_resume(CTX, dss->domid, /* Fast Suspend */1))
return 0;
- /* TODO: Deal with disk. Start a new network output buffer */
+ /* REMUS TODO: Deal with disk. Start a new network output buffer */
return 1;
}
@@ -1151,8 +1151,9 @@ static void libxl__remus_domain_checkpoint_callback(void *data)
static void remus_checkpoint_dm_saved(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc)
{
- /* TODO: Wait for disk and memory ack, release network buffer */
- /* TODO: make this asynchronous */
+ /* REMUS TODO: Wait for disk and memory ack, release network buffer */
+ /* REMUS TODO: make this asynchronous */
+ assert(!rc); /* REMUS TODO handle this error properly */
usleep(dss->interval * 1000);
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1);
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/13] libxl: remove an unused numainfo parameter
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (7 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 08/13] libxl: remus: mark TODOs more clearly Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 10/13] libxl: idl: always initialise KeyedEnum keyvar in member init Ian Jackson
` (4 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
---
tools/libxl/libxl_numa.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c
index 5301ec4..2c8e59f 100644
--- a/tools/libxl/libxl_numa.c
+++ b/tools/libxl/libxl_numa.c
@@ -231,7 +231,7 @@ static int nodemap_to_nr_vcpus(libxl__gc *gc, libxl_cputopology *tinfo,
* candidates with just one node).
*/
static int count_cpus_per_node(libxl_cputopology *tinfo, int nr_cpus,
- libxl_numainfo *ninfo, int nr_nodes)
+ int nr_nodes)
{
int cpus_per_node = 0;
int j, i;
@@ -340,7 +340,7 @@ int libxl__get_numa_candidate(libxl__gc *gc,
if (!min_nodes) {
int cpus_per_node;
- cpus_per_node = count_cpus_per_node(tinfo, nr_cpus, ninfo, nr_nodes);
+ cpus_per_node = count_cpus_per_node(tinfo, nr_cpus, nr_nodes);
if (cpus_per_node == 0)
min_nodes = 1;
else
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/13] libxl: idl: always initialise KeyedEnum keyvar in member init
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (8 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 09/13] libxl: remove an unused numainfo parameter Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-02 17:27 ` [PATCH 11/13] libxl: correct some comments regarding event API and fds Ian Jackson
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Campbell
libxl: idl: always initialise the KeyedEnum keyvar in the member init function
Previously we only initialised it if an explicit keyvar_init_val was
given but not if the default was implicitly 0.
In the generated code this only changes the unused libxl_event_init_type
function:
void libxl_event_init_type(libxl_event *p, libxl_event_type type)
{
+ assert(!p->type);
+ p->type = type;
switch (p->type) {
case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
break;
However I think it is wrong that this function is unused, this and
libxl_event_init should be used by libxl__event_new. As it happens
both are just memset to zero but for correctness we should use the
init functions (in case the IDL changes).
In the generator we also need to properly handle init_var == 0 which
the current if statements incorrectly treat as False. This doesn't
actually have any impact on the generated code.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/gentypes.py | 11 +++++++----
tools/libxl/libxl_event.c | 5 ++++-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 1d13201..30f29ba 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -162,17 +162,20 @@ def libxl_C_type_member_init(ty, field):
ku.keyvar.type.make_arg(ku.keyvar.name))
s += "{\n"
- if ku.keyvar.init_val:
+ if ku.keyvar.init_val is not None:
init_val = ku.keyvar.init_val
- elif ku.keyvar.type.init_val:
+ elif ku.keyvar.type.init_val is not None:
init_val = ku.keyvar.type.init_val
else:
init_val = None
+ (nparent,fexpr) = ty.member(ty.pass_arg("p"), ku.keyvar, isref=True)
if init_val is not None:
- (nparent,fexpr) = ty.member(ty.pass_arg("p"), ku.keyvar, isref=True)
s += " assert(%s == %s);\n" % (fexpr, init_val)
- s += " %s = %s;\n" % (fexpr, ku.keyvar.name)
+ else:
+ s += " assert(!%s);\n" % (fexpr)
+ s += " %s = %s;\n" % (fexpr, ku.keyvar.name)
+
(nparent,fexpr) = ty.member(ty.pass_arg("p"), field, isref=True)
s += _libxl_C_type_init(ku, fexpr, parent=nparent, subinit=True)
s += "}\n"
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 1af64c8..939906c 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1163,7 +1163,10 @@ libxl_event *libxl__event_new(libxl__egc *egc,
libxl_event *ev;
ev = libxl__zalloc(NOGC,sizeof(*ev));
- ev->type = type;
+
+ libxl_event_init(ev);
+ libxl_event_init_type(ev, type);
+
ev->domid = domid;
return ev;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/13] libxl: correct some comments regarding event API and fds
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (9 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 10/13] libxl: idl: always initialise KeyedEnum keyvar in member init Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-03 8:35 ` Ian Campbell
2012-08-02 17:27 ` [PATCH 12/13] libxl: add a comment re the memory management API instability Ian Jackson
` (2 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
* libxl may indeed register more than one callback for the same fd,
with some restrictions. The allowable range of responses to this by
the application means that this should pose no problems for users.
But the documentation comment should be fixed.
* Document the relaxed synchronicity semantics of the fd_modify
registration callback.
* A couple of comments referred to old names for functions.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_event.h | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
index 3344bc8..cead71b 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -320,13 +320,24 @@ typedef struct libxl_osevent_hooks {
* *for_registration_update is honoured by libxl and will be passed
* to future modify or deregister calls.
*
- * libxl will only attempt to register one callback for any one fd.
+ * libxl may want to register more than one callback for any one fd;
+ * in that case: (i) each such registration will have at least one bit
+ * set in revents which is unique to that registration; (ii) if an
+ * event occurs which is relevant for multiple registrations the
+ * application's event system is may call libxl_osevent_occurred_fd
+ * for one, some, or all of those registrations.
+ *
+ * If fd_modify is used, it is permitted for the application's event
+ * system to still make calls to libxl_osevent_occurred_fd for the
+ * "old" set of requested events; these will be safely ignored by
+ * libxl.
+ *
* libxl will remember the value stored in *for_app_registration_out
* (or *for_app_registration_update) by a successful call to
* register (or modify), and pass it to subsequent calls to modify
* or deregister.
*
- * register_fd_hooks may be called only once for each libxl_ctx.
+ * osevent_register_hooks may be called only once for each libxl_ctx.
* libxl may make calls to register/modify/deregister from within
* any libxl function (indeed, it will usually call register from
* register_event_hooks). Conversely, the application MUST NOT make
@@ -357,7 +368,7 @@ void libxl_osevent_register_hooks(libxl_ctx *ctx,
/* It is NOT legal to call _occurred_ reentrantly within any libxl
* function. Specifically it is NOT legal to call it from within
* a register callback. Conversely, libxl MAY call register/deregister
- * from within libxl_event_registered_call_*.
+ * from within libxl_event_occurred_call_*.
*/
void libxl_osevent_occurred_fd(libxl_ctx *ctx, void *for_libxl,
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 11/13] libxl: correct some comments regarding event API and fds
2012-08-02 17:27 ` [PATCH 11/13] libxl: correct some comments regarding event API and fds Ian Jackson
@ 2012-08-03 8:35 ` Ian Campbell
2012-08-03 10:53 ` Ian Jackson
0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-08-03 8:35 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xen.org
On Thu, 2012-08-02 at 18:27 +0100, Ian Jackson wrote:
> * libxl may indeed register more than one callback for the same fd,
> with some restrictions. The allowable range of responses to this by
> the application means that this should pose no problems for users.
> But the documentation comment should be fixed.
>
> * Document the relaxed synchronicity semantics of the fd_modify
> registration callback.
>
> * A couple of comments referred to old names for functions.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> tools/libxl/libxl_event.h | 17 ++++++++++++++---
> 1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
> index 3344bc8..cead71b 100644
> --- a/tools/libxl/libxl_event.h
> +++ b/tools/libxl/libxl_event.h
> @@ -320,13 +320,24 @@ typedef struct libxl_osevent_hooks {
> * *for_registration_update is honoured by libxl and will be passed
> * to future modify or deregister calls.
> *
> - * libxl will only attempt to register one callback for any one fd.
> + * libxl may want to register more than one callback for any one fd;
> + * in that case: (i) each such registration will have at least one bit
> + * set in revents which is unique to that registration; (ii) if an
> + * event occurs which is relevant for multiple registrations the
> + * application's event system is may call libxl_osevent_occurred_fd
is may ?
Probably meant just "may".
Otherwise:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(no need to resend, if you confirm the intended words are as I suggest
I'll tweak on commit)
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 11/13] libxl: correct some comments regarding event API and fds
2012-08-03 8:35 ` Ian Campbell
@ 2012-08-03 10:53 ` Ian Jackson
0 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2012-08-03 10:53 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
Ian Campbell writes ("Re: [Xen-devel] [PATCH 11/13] libxl: correct some comments regarding event API and fds"):
> On Thu, 2012-08-02 at 18:27 +0100, Ian Jackson wrote:
> > - * libxl will only attempt to register one callback for any one fd.
> > + * libxl may want to register more than one callback for any one fd;
> > + * in that case: (i) each such registration will have at least one bit
> > + * set in revents which is unique to that registration; (ii) if an
> > + * event occurs which is relevant for multiple registrations the
> > + * application's event system is may call libxl_osevent_occurred_fd
>
> is may ?
>
> Probably meant just "may".
Yes.
> Otherwise:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> (no need to resend, if you confirm the intended words are as I suggest
> I'll tweak on commit)
It's probably easier if I commit the series myself, when I've finished
collecting the acks etc., as I already have it in a git branch.
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 12/13] libxl: add a comment re the memory management API instability
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (10 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 11/13] libxl: correct some comments regarding event API and fds Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-03 8:35 ` Ian Campbell
2012-08-02 17:27 ` [PATCH 13/13] libxl: -Wunused-parameter Ian Jackson
2012-08-03 8:59 ` [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Campbell
13 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl.h | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5ec2d74..f11abc2 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -567,6 +567,17 @@ int libxl_domain_core_dump(libxl_ctx *ctx, uint32_t domid,
int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb);
int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t target_memkb, int relative, int enforce);
int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target);
+
+
+/*
+ * WARNING
+ * This memory management API is unstable even in Xen 4.2.
+ * It has a numer of deficiencies and we intend to replace it.
+ *
+ * The semantics of these functions should not be relied on to be very
+ * coherent or stable. We will however endeavour to keep working
+ * existing programs which use them in roughly the same way as libxl.
+ */
/* how much free memory in the system a domain needs to be built */
int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
uint32_t *need_memkb);
@@ -577,6 +588,7 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t memory_k
/* wait for the memory target of a domain to be reached */
int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
+
int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
/* libxl_primary_console_exec finds the domid and console number
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/13] libxl: -Wunused-parameter
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (11 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 12/13] libxl: add a comment re the memory management API instability Ian Jackson
@ 2012-08-02 17:27 ` Ian Jackson
2012-08-03 8:08 ` Ian Campbell
2012-08-03 8:59 ` [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Campbell
13 siblings, 1 reply; 21+ messages in thread
From: Ian Jackson @ 2012-08-02 17:27 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Campbell
*** DO NOT APPLY ***
We have recently had a couple of bugs which basically involved
ignoring the rc parameter to a callback function. I thought I would
try -Wunused-parameter. Here are the results.
I found three further problems:
* libxl_wait_for_free_memory takes a domid parameter but its
semantics don't seem to call for that. However this function has
an API stability warning, and we will leave it be in case the domid
turns out to be useful later for some kind of compatibility bodge.
* qmp_synchronous_send has an `ask_timeout' parameter which is
ignored.
* The autogenerated function libxl_event_init_type ignored the type
parameter. This is now fixed by Ian Campbell in an earlier
patch in this series.
Things I needed to do to get the rest of the code to compile:
* Remove one harmless unused parameter from an internal function.
(Earlier in this series.)
* Add an assert to make the error handling in the broken remus code
slightly less broken. (Earlier in this series.)
* Provide machinery in the Makefile for passing different CFLAGS to
libxl as opposed to xl and libxlu. The flex- and bison-generated
files in libxlu can't be compiled with -Wunused-parameter.
* Define a new helper macro
#define USE(var) ((void)(var))
and use it 43 times. The pattern is something like
USE(egc);
in a function which takes egc but doesn't need it. If the
parameter is later used, this is harmless. In functions
which are placeholders the USE statement should be placed in the
middle of the function where the parameter would be used if the
function is changed later, so that the USE gets deleted by the
patch introducing the implementation.
* Define a new helper macro for use only in other macros
#define MAYBE_UNUSED __attribute__((unused))
and use it in 10 different places.
* Define new macros for helping declare common types of callback
functions. For example:
#define EV_XSWATCH_CALLBACK_PARAMS(egc, watch, wpath, epath) \
libxl__egc *egc MAYBE_UNUSED, \
libxl__ev_xswatch *watch MAYBE_UNUSED, \
const char *wpath MAYBE_UNUSED, \
const char *epath MAYBE_UNUSED
which is used like this:
-static void some_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
- const char *wpath, const char *epath)
+static void some_callback(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, watch, wpath, epath))
{
... now we use (or not) egc, watch, wpath, etc. or not as we like
This somewhat resembles a Traditional K&R C typeless function
definition. The types of the parameters are actually defined
for the compiler of course, along with the information that
the parameters might be unused.
There are 4 macros of this kind with 22 call sites.
IMO on the cost (65 places in ordinary code where we have to write
something somewhat ugly) is worth the benefit (finding, if we had
deployed this right away, around 6 bugs). But it's arguable.
*** DO NOT APPLY ***
Anyway, this patch must not be applied right now because it causes the
build to fail.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
-
Changes in v5 of series:
* Use REMUS TODO in comment.
* Ignore domid parameter to libxl_wait_for_free_memory.
* Do not unnecessarily but harmlessly USE(priv) in pci_ins_check.
* Mention that libxl_event_init_type problem is now fixed.
---
tools/libxl/Makefile | 4 +++-
tools/libxl/libxl.c | 31 +++++++++++++++++++++++++++----
tools/libxl/libxl_aoutils.c | 8 ++++----
tools/libxl/libxl_blktap2.c | 1 +
tools/libxl/libxl_bootloader.c | 6 ++++++
tools/libxl/libxl_create.c | 2 ++
tools/libxl/libxl_device.c | 9 +++++----
tools/libxl/libxl_dm.c | 4 ++++
tools/libxl/libxl_dom.c | 8 ++++----
tools/libxl/libxl_event.c | 22 +++++++++++++---------
tools/libxl/libxl_exec.c | 8 ++++----
tools/libxl/libxl_fork.c | 1 +
tools/libxl/libxl_internal.h | 24 ++++++++++++++++++++++--
tools/libxl/libxl_pci.c | 5 +++++
tools/libxl/libxl_qmp.c | 17 +++++++++--------
tools/libxl/libxl_save_callout.c | 4 ++--
tools/libxl/libxl_utils.c | 2 ++
17 files changed, 114 insertions(+), 42 deletions(-)
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 63a8157..f5ff115 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -13,6 +13,8 @@ XLUMINOR = 0
CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
-Wno-declaration-after-statement -Wformat-nonliteral
+CFLAGS_FOR_LIBXL = -Wunused-parameter
+
CFLAGS += -I. -fPIC
ifeq ($(CONFIG_Linux),y)
@@ -71,7 +73,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
libxl_qmp.o libxl_event.o libxl_fork.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
+$(LIBXL_OBJS): CFLAGS += $(CFLAGS_FOR_LIBXL) $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \
_libxl_save_msgs_callout.h _libxl_save_msgs_helper.h \
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 726a70e..4f387df 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -28,6 +28,8 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
struct stat stat_buf;
int rc;
+ USE(flags);
+
if (version != LIBXL_VERSION) { rc = ERROR_VERSION; goto out; }
ctx = malloc(sizeof(*ctx));
@@ -700,6 +702,8 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
libxl__domain_suspend_state *dss;
int rc;
+ USE(recv_fd); /* REMUS TODO: get rid of this and actually use it! */
+
libxl_domain_type type = libxl__domain_type(gc, domid);
if (type == LIBXL_DOMAIN_TYPE_INVALID) {
rc = ERROR_FAIL;
@@ -979,8 +983,9 @@ static void domain_death_occurred(libxl__egc *egc,
LIBXL_TAILQ_INSERT_HEAD(&CTX->death_reported, evg, entry);
}
-static void domain_death_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
- const char *wpath, const char *epath) {
+static void domain_death_xswatch_callback(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, watch, wpath, epath))
+{
EGC_GC;
libxl_evgen_domain_death *evg;
uint32_t domid;
@@ -1137,8 +1142,9 @@ void libxl_evdisable_domain_death(libxl_ctx *ctx,
GC_FREE;
}
-static void disk_eject_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
- const char *wpath, const char *epath) {
+static void disk_eject_xswatch_callback(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, w, wpath, epath))
+{
EGC_GC;
libxl_evgen_disk_eject *evg = (void*)w;
char *backend;
@@ -2525,6 +2531,8 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
libxl_device_nic *nic,
libxl__device *device)
{
+ USE(gc);
+
device->backend_devid = nic->devid;
device->backend_domid = nic->backend_domid;
device->backend_kind = LIBXL__DEVICE_KIND_VIF;
@@ -2903,6 +2911,9 @@ out:
int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
{
+ USE(gc);
+
+ USE(vkb);
return 0;
}
@@ -2910,6 +2921,7 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
libxl_device_vkb *vkb,
libxl__device *device)
{
+ USE(gc);
device->backend_devid = vkb->devid;
device->backend_domid = vkb->backend_domid;
device->backend_kind = LIBXL__DEVICE_KIND_VKBD;
@@ -2991,6 +3003,7 @@ out:
int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
{
+ USE(gc);
libxl_defbool_setdefault(&vfb->vnc.enable, true);
if (libxl_defbool_val(vfb->vnc.enable)) {
if (!vfb->vnc.listen) {
@@ -3011,6 +3024,7 @@ static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
libxl_device_vfb *vfb,
libxl__device *device)
{
+ USE(gc);
device->backend_devid = vfb->devid;
device->backend_domid = vfb->backend_domid;
device->backend_kind = LIBXL__DEVICE_KIND_VFB;
@@ -3567,6 +3581,8 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t
uint32_t freemem_slack;
GC_INIT(ctx);
+ USE(domid); /* this may turn out to be useful for compatibility, later */
+
rc = libxl__get_free_memory_slack(gc, &freemem_slack);
if (rc < 0)
goto out;
@@ -3937,8 +3953,13 @@ libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx)
static int sched_arinc653_domain_set(libxl__gc *gc, uint32_t domid,
const libxl_domain_sched_params *scinfo)
{
+ USE(gc);
+
/* Currently, the ARINC 653 scheduler does not take any domain-specific
configuration, so we simply return success. */
+ USE(domid);
+ USE(scinfo);
+
return 0;
}
@@ -4386,6 +4407,8 @@ int libxl_xen_console_read_line(libxl_ctx *ctx,
void libxl_xen_console_read_finish(libxl_ctx *ctx,
libxl_xen_console_reader *cr)
{
+ USE(ctx);
+
free(cr->buffer);
free(cr);
}
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 983a60a..6d85f56 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -114,8 +114,8 @@ static int datacopier_pollhup_handled(libxl__egc *egc,
return 0;
}
-static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
- int fd, short events, short revents) {
+static void datacopier_readable(EV_FD_CALLBACK_PARAMS
+ (egc, ev, fd, events, revents)) {
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
STATE_AO_GC(dc->ao);
@@ -178,8 +178,8 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
datacopier_check_state(egc, dc);
}
-static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
- int fd, short events, short revents) {
+static void datacopier_writable(EV_FD_CALLBACK_PARAMS(
+ egc, ev, fd, events, revents)) {
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, towrite);
STATE_AO_GC(dc->ao);
diff --git a/tools/libxl/libxl_blktap2.c b/tools/libxl/libxl_blktap2.c
index 2c40182..660a669 100644
--- a/tools/libxl/libxl_blktap2.c
+++ b/tools/libxl/libxl_blktap2.c
@@ -19,6 +19,7 @@
int libxl__blktap_enabled(libxl__gc *gc)
{
+ USE(gc);
const char *msg;
return !tap_ctl_check(&msg);
}
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index e103ee9..902dfe6 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -88,6 +88,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
static int setup_xenconsoled_pty(libxl__egc *egc, libxl__bootloader_state *bl,
char *slave_path, size_t slave_path_len)
{
+ USE(egc);
STATE_AO_GC(bl->ao);
struct termios termattr;
int r, rc;
@@ -141,6 +142,7 @@ static const char *bootloader_result_command(libxl__gc *gc, const char *buf,
static int parse_bootloader_result(libxl__egc *egc,
libxl__bootloader_state *bl)
{
+ USE(egc);
STATE_AO_GC(bl->ao);
char buf[PATH_MAX*2];
FILE *f = 0;
@@ -221,6 +223,7 @@ void libxl__bootloader_init(libxl__bootloader_state *bl)
static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
{
+ USE(egc);
STATE_AO_GC(bl->ao);
int i;
@@ -256,6 +259,8 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
static void bootloader_callback(libxl__egc *egc, libxl__bootloader_state *bl,
int rc)
{
+ USE(egc);
+
if (!bl->rc)
bl->rc = rc;
@@ -285,6 +290,7 @@ static void bootloader_local_detached_cb(libxl__egc *egc,
static void bootloader_stop(libxl__egc *egc,
libxl__bootloader_state *bl, int rc)
{
+ USE(egc);
STATE_AO_GC(bl->ao);
int r;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 5f0d26f..5d56d67 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -62,6 +62,8 @@ void libxl_domain_config_dispose(libxl_domain_config *d_config)
int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info)
{
+ USE(gc);
+
if (!c_info->type)
return ERROR_INVAL;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 9fc63f1..80c5511 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -44,6 +44,8 @@ int libxl__parse_backend_path(libxl__gc *gc,
const char *path,
libxl__device *dev)
{
+ USE(gc);
+
/* /local/domain/<domid>/backend/<kind>/<domid>/<devid> */
char strkind[16]; /* Longest is actually "console" */
int rc = sscanf(path, "/local/domain/%d/backend/%15[^/]/%u/%d",
@@ -796,8 +798,7 @@ out:
return;
}
-static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+static void device_qemu_timeout(EV_TIME_CALLBACK_PARAMS(egc, ev, requested_abs))
{
libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
STATE_AO_GC(aodev->ao);
@@ -919,8 +920,8 @@ out:
return;
}
-static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+static void device_hotplug_timeout_cb(EV_TIME_CALLBACK_PARAMS
+ (egc, ev, requested_abs))
{
libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
STATE_AO_GC(aodev->ao);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c0084f..6995306 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -640,6 +640,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
libxl_device_vfb *vfb,
libxl_device_vkb *vkb)
{
+ USE(gc);
const libxl_domain_build_info *b_info = &guest_config->b_info;
if (b_info->type != LIBXL_DOMAIN_TYPE_HVM)
@@ -1177,6 +1178,7 @@ static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
{
libxl__dm_spawn_state *dmss = CONTAINER_OF(spawn, *dmss, spawn);
STATE_AO_GC(spawn->ao);
+ USE(egc);
if (!xsdata)
return;
@@ -1262,6 +1264,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
int nr_vfbs, libxl_device_vfb *vfbs,
int nr_disks, libxl_device_disk *disks)
{
+ USE(gc);
int i, ret = 0;
/*
@@ -1283,6 +1286,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
}
if (nr_vfbs > 0) {
+ USE(vfbs);
ret = 1;
goto out;
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 06d5e4f..4a36652 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -756,8 +756,8 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
switch_logdirty_done(egc,dss,-1);
}
-static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+static void switch_logdirty_timeout(EV_TIME_CALLBACK_PARAMS
+ (egc, ev, requested_abs))
{
libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss, logdirty.timeout);
STATE_AO_GC(dss->ao);
@@ -765,8 +765,8 @@ static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
switch_logdirty_done(egc,dss,-1);
}
-static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
- const char *watch_path, const char *event_path)
+static void switch_logdirty_xswatch(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, watch, wpath, epath))
{
libxl__domain_suspend_state *dss =
CONTAINER_OF(watch, *dss, logdirty.watch);
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 939906c..3ec9361 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -197,6 +197,8 @@ static void time_done_debug(libxl__gc *gc, const char *func,
"ev_time=%p done rc=%d .func=%p infinite=%d abs=%lu.%06lu",
ev, rc, ev->func, ev->infinite,
(unsigned long)ev->abs.tv_sec, (unsigned long)ev->abs.tv_usec);
+#else
+ USE(gc); USE(func); USE(ev); USE(rc);
#endif
}
@@ -381,8 +383,7 @@ static void libxl__set_watch_slot_contents(libxl__ev_watch_slot *slot,
slot->empty.sle_next = (void*)w;
}
-static void watchfd_callback(libxl__egc *egc, libxl__ev_fd *ev,
- int fd, short events, short revents)
+static void watchfd_callback(EV_FD_CALLBACK_PARAMS(egc,ev, fd,events,revents))
{
EGC_GC;
@@ -571,8 +572,8 @@ void libxl__ev_xswatch_deregister(libxl__gc *gc, libxl__ev_xswatch *w)
* waiting for device state
*/
-static void devstate_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
- const char *watch_path, const char *event_path)
+static void devstate_watch_callback(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, watch, watch_path, event_path))
{
EGC_GC;
libxl__ev_devstate *ds = CONTAINER_OF(watch, *ds, watch);
@@ -605,8 +606,7 @@ static void devstate_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
ds->callback(egc, ds, rc);
}
-static void devstate_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+static void devstate_timeout(EV_TIME_CALLBACK_PARAMS(egc, ev, requested_abs))
{
EGC_GC;
libxl__ev_devstate *ds = CONTAINER_OF(ev, *ds, timeout);
@@ -662,8 +662,8 @@ int libxl__ev_devstate_wait(libxl__gc *gc, libxl__ev_devstate *ds,
* futile.
*/
-static void domaindeathcheck_callback(libxl__egc *egc, libxl__ev_xswatch *w,
- const char *watch_path, const char *event_path)
+static void domaindeathcheck_callback(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, w, watch_path, event_path))
{
libxl__domaindeathcheck *dc = CONTAINER_OF(w, *dc, watch);
EGC_GC;
@@ -1019,6 +1019,7 @@ void libxl_osevent_occurred_fd(libxl_ctx *ctx, void *for_libxl,
assert(fd == ev->fd);
revents &= ev->events;
+ USE(events); /* we use our own idea of what we asked for */
if (revents)
ev->func(egc, ev, fd, ev->events, revents);
@@ -1151,6 +1152,8 @@ void libxl__event_occurred(libxl__egc *egc, libxl_event *event)
void libxl_event_free(libxl_ctx *ctx, libxl_event *event)
{
+ USE(ctx);
+
/* Exceptionally, this function may be called from libxl, with ctx==0 */
libxl_event_dispose(event);
free(event);
@@ -1648,7 +1651,8 @@ int libxl__ao_inprogress(libxl__ao *ao,
* for how. But we want to copy *how. So we have this dummy function
* whose address is stored in callback if the app passed how==NULL. */
static void dummy_asyncprogress_callback_ignore
- (libxl_ctx *ctx, libxl_event *ev, void *for_callback) { }
+ (libxl_ctx *ctx, libxl_event *ev, void *for_callback)
+ { USE(ctx); USE(ev); USE(for_callback); }
void libxl__ao_progress_gethow(libxl_asyncprogress_how *in_state,
const libxl_asyncprogress_how *from_app) {
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 0477386..ed6b44e 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -280,6 +280,7 @@ void libxl__spawn_init(libxl__spawn_state *ss)
int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
{
+ USE(egc);
STATE_AO_GC(ss->ao);
int r;
pid_t child;
@@ -387,8 +388,7 @@ static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss)
spawn_detach(gc, ss);
}
-static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+static void spawn_timeout(EV_TIME_CALLBACK_PARAMS(egc, ev, requested_abs))
{
/* Before event, was Attached. */
EGC_GC;
@@ -397,8 +397,8 @@ static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
spawn_fail(egc, ss); /* must be last */
}
-static void spawn_watch_event(libxl__egc *egc, libxl__ev_xswatch *xsw,
- const char *watch_path, const char *event_path)
+static void spawn_watch_event(EV_XSWATCH_CALLBACK_PARAMS
+ (egc, xsw, wpath, epath))
{
/* On entry, is Attached. */
EGC_GC;
diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index 044ddad..0379604 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -157,6 +157,7 @@ int libxl__carefd_fd(const libxl__carefd *cf)
static void sigchld_handler(int signo)
{
+ USE(signo);
int e = libxl__self_pipe_wakeup(sigchld_owner->sigchld_selfpipe[1]);
assert(!e); /* errors are probably EBADF, very bad */
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 6528694..2381388 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -101,6 +101,8 @@
#define DISABLE_UDEV_PATH "libxl/disable_udev"
#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
+#define USE(var) ((void)(var))
+#define MAYBE_UNUSED __attribute__((unused))
#define LIBXL__LOGGING_ENABLED
@@ -153,6 +155,14 @@ typedef void libxl__ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev,
* It is not permitted to listen for the same or overlapping events
* on the same fd using multiple different libxl__ev_fd's.
*/
+
+/* Declare your callback functions with this helper and you avoid unused
+ * parameter warnings (and don't have to list all the types either): */
+#define EV_FD_CALLBACK_PARAMS(egc, ev, fd, events, revents) \
+ libxl__egc *egc MAYBE_UNUSED, libxl__ev_fd *ev MAYBE_UNUSED, \
+ int fd MAYBE_UNUSED, \
+ short events MAYBE_UNUSED, short revents MAYBE_UNUSED
+
struct libxl__ev_fd {
/* caller should include this in their own struct */
/* read-only for caller, who may read only when registered: */
@@ -168,6 +178,11 @@ struct libxl__ev_fd {
typedef struct libxl__ev_time libxl__ev_time;
typedef void libxl__ev_time_callback(libxl__egc *egc, libxl__ev_time *ev,
const struct timeval *requested_abs);
+
+#define EV_TIME_CALLBACK_PARAMS(egc, ev, requested_abs) \
+ libxl__egc *egc MAYBE_UNUSED, libxl__ev_time *ev MAYBE_UNUSED, \
+ const struct timeval *requested_abs MAYBE_UNUSED
+
struct libxl__ev_time {
/* caller should include this in their own struct */
/* read-only for caller, who may read only when registered: */
@@ -180,8 +195,13 @@ struct libxl__ev_time {
};
typedef struct libxl__ev_xswatch libxl__ev_xswatch;
-typedef void libxl__ev_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch*,
- const char *watch_path, const char *event_path);
+typedef void libxl__ev_xswatch_callback(libxl__egc *egc,
+ libxl__ev_xswatch *watch, const char *watch_path, const char *event_path);
+
+#define EV_XSWATCH_CALLBACK_PARAMS(egc, watch, wpath, epath) \
+ libxl__egc *egc MAYBE_UNUSED, libxl__ev_xswatch *watch MAYBE_UNUSED, \
+ const char *wpath MAYBE_UNUSED, const char *epath MAYBE_UNUSED
+
struct libxl__ev_xswatch {
/* caller should include this in their own struct */
/* read-only for caller, who may read only when registered: */
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 9c92ae6..5e866f2 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -798,8 +798,11 @@ static int pci_multifunction_check(libxl__gc *gc, libxl_device_pci *pcidev, unsi
static int pci_ins_check(libxl__gc *gc, uint32_t domid, const char *state, void *priv)
{
+ USE(gc);
char *orig_state = priv;
+ USE(domid);
+
if ( !strcmp(state, "pci-insert-failed") )
return -1;
if ( !strcmp(state, "pci-inserted") )
@@ -1007,6 +1010,8 @@ static int libxl__device_pci_reset(libxl__gc *gc, unsigned int domain, unsigned
int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci)
{
+ USE(gc);
+ USE(pci);
return 0;
}
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index e33b130..c354c71 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -46,6 +46,10 @@
typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
const libxl__json_object *tree,
void *opaque);
+#define QMP_CALLBACK_PARAMS(qmp, tree, opaque) \
+ libxl__qmp_handler *qmp MAYBE_UNUSED, \
+ const libxl__json_object *tree MAYBE_UNUSED, \
+ void *opaque MAYBE_UNUSED
typedef struct qmp_request_context {
int rc;
@@ -109,9 +113,7 @@ static int store_serial_port_info(libxl__qmp_handler *qmp,
return ret;
}
-static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
- const libxl__json_object *o,
- void *unused)
+static int register_serials_chardev_callback(QMP_CALLBACK_PARAMS(qmp,o,unused))
{
const libxl__json_object *obj = NULL;
const libxl__json_object *label = NULL;
@@ -165,9 +167,7 @@ static int qmp_write_domain_console_item(libxl__gc *gc, int domid,
return libxl__xs_write(gc, XBT_NULL, path, "%s", value);
}
-static int qmp_register_vnc_callback(libxl__qmp_handler *qmp,
- const libxl__json_object *o,
- void *unused)
+static int qmp_register_vnc_callback(QMP_CALLBACK_PARAMS(qmp,o,unused))
{
GC_INIT(qmp->ctx);
const libxl__json_object *obj;
@@ -203,8 +203,7 @@ out:
return rc;
}
-static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
- const libxl__json_object *o, void *unused)
+static int qmp_capabilities_callback(QMP_CALLBACK_PARAMS(qmp,o,unused))
{
qmp->connected = true;
@@ -228,6 +227,8 @@ static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
static libxl__qmp_message_type qmp_response_type(libxl__qmp_handler *qmp,
const libxl__json_object *o)
{
+ USE(qmp);
+
libxl__qmp_message_type type;
libxl__json_map_node *node = NULL;
int i = 0;
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 078b7ee..78bb67e 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -252,8 +252,8 @@ static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
(unsigned long)shs->child.pid);
}
-static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
- int fd, short events, short revents)
+static void helper_stdout_readable(EV_FD_CALLBACK_PARAMS
+ (egc, ev, fd, events, revents))
{
libxl__save_helper_state *shs = CONTAINER_OF(ev, *shs, readable);
STATE_AO_GC(shs->ao);
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index f7b44a0..a7c34a9 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -230,6 +230,7 @@ out:
int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
{
+ USE(ctx);
char *p;
int rc = 0;
@@ -513,6 +514,7 @@ void libxl_bitmap_dispose(libxl_bitmap *map)
void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
const libxl_bitmap *sptr)
{
+ USE(ctx);
int sz;
assert(dptr->size == sptr->size);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 13/13] libxl: -Wunused-parameter
2012-08-02 17:27 ` [PATCH 13/13] libxl: -Wunused-parameter Ian Jackson
@ 2012-08-03 8:08 ` Ian Campbell
0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2012-08-03 8:08 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xen.org
On Thu, 2012-08-02 at 18:27 +0100, Ian Jackson wrote:
> diff --git a/tools/libxl/libxl_blktap2.c b/tools/libxl/libxl_blktap2.c
> index 2c40182..660a669 100644
> --- a/tools/libxl/libxl_blktap2.c
> +++ b/tools/libxl/libxl_blktap2.c
> @@ -19,6 +19,7 @@
>
> int libxl__blktap_enabled(libxl__gc *gc)
> {
> + USE(gc);
> const char *msg;
> return !tap_ctl_check(&msg);
> }
You'll need to consider libxl_noblktap2.c too. And perhaps some other
conditionally compiled stuff, e.g. libxl_netbsd.c
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups
2012-08-02 17:27 [PATCH v5 00/13] libxl: Assorted bugfixes and cleanups Ian Jackson
` (12 preceding siblings ...)
2012-08-02 17:27 ` [PATCH 13/13] libxl: -Wunused-parameter Ian Jackson
@ 2012-08-03 8:59 ` Ian Campbell
2012-08-03 11:05 ` Ian Jackson
13 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-08-03 8:59 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xen.org
On Thu, 2012-08-02 at 18:27 +0100, Ian Jackson wrote:
> These should go into 4.2 soon:
I've applied 01..12 but skipped 11 pending you confirmation of the
intended wording.
>
> A * 01/13 libxl: unify libxl__device_destroy and device_hotplug_done
> * 02/13 libxl: react correctly to bootloader pty master POLLHUP
> A 03/13 libxl: fix device counting race in libxl__devices_destroy
> A 04/13 libxl: fix formatting of DEFINE_DEVICES_ADD
> A 05/13 libxl: abolish useless `start' parameter to libxl__add_*
> A 06/13 libxl: rename aodevs to multidev
> A 07/13 libxl: do not blunder on if bootloader fails (again)
>
> These are harmless enough (but make no functional difference right
> now) and should go in as well:
>
> A 08/13 libxl: remus: mark TODOs more clearly
> A 09/13 libxl: remove an unused numainfo parameter
> A 10/13 libxl: idl: always initialise KeyedEnum keyvar in member init
>
> These are API doc fixes for 4.2:
>
> + 11/13 libxl: correct some comments regarding event API and fds
> + 12/13 libxl: add a comment re the memory management API instability
>
> And this one is still waiting on the qmp ask_timeout question:
>
> X * 13/13 libxl: -Wunused-parameter
>
> Key:
>
> A acked
> X DO NOT APPLY
> * modified since v4
> + new patch, not posted before
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread