On Tuesday, 19 October 2010 at 09:21, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1287476238 -3600
> # Node ID 0b5d85ea10f8fff3f654c564c0e66900e83e8012
> # Parent ca91b58834a35c70f33f6ceb28c02c60f190b6ed
> tools: cleanup domain save switch_qemu_logdirty callback
>
> Move the function into struct save_callbacks with the others and add
> the void *closure to the callback arguments.
>
> Add and propagate an error return code from the callback.
>
> Use this in libxl to pass the save context to
> libxl__domain_suspend_common_switch_qemu_logdirty allowing us to reuse
> the parent's xenstore handle, gc context etc.
>
> Also add an apparently missing libxl__free_all to
> libxl__domain_suspend_common.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> Changes from v1:
> Compile test from missed call to xc_domain_save in
> tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
> Changes from v2:
> Check for missing callback and return error.
> Similarly update tools/libxc/ia64/xc_ia64_linux_save.c
> Add return value to callback and propagate failure.
> Update comment to indicate that closure is last argument to callback
> (all existing callbacks only had the one argument so the first/last
> distinction is moot)
This reads well, but I don't understand the purpose of all the return
code juggling you've done in libcheckpoint.c here:
> diff -r ca91b58834a3 -r 0b5d85ea10f8
> tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
> --- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c Tue Oct 19
> 09:07:47 2010 +0100
> +++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c Tue Oct 19
> 09:17:18 2010 +0100
> @@ -164,9 +164,9 @@ void checkpoint_close(checkpoint_state*
>
> /* we toggle logdirty ourselves around the xc_domain_save call --
> * it avoids having to pass around checkpoint_state */
> -static void noop_switch_logdirty(int domid, unsigned enable)
> +static int noop_switch_logdirty(int domid, unsigned enable, void *data)
> {
> - return;
> + return 0;
> }
>
> int checkpoint_start(checkpoint_state* s, int fd,
> @@ -185,12 +185,13 @@ int checkpoint_start(checkpoint_state* s
> hvm = s->domtype > dt_pv;
> if (hvm) {
> flags |= XCFLAGS_HVM;
> - if ((rc = switch_qemu_logdirty(s, 1)))
> - return rc;
> + if (!switch_qemu_logdirty(s, 1))
> + return -1;
> }
>
> - rc = xc_domain_save(s->xch, fd, s->domid, 0, 0, flags, callbacks, hvm,
> - noop_switch_logdirty);
> + callbacks->switch_qemu_logdirty = noop_switch_logdirty;
> +
> + rc = xc_domain_save(s->xch, fd, s->domid, 0, 0, flags, callbacks, hvm);
>
> if (hvm)
> switch_qemu_logdirty(s, 0);
> @@ -540,7 +541,7 @@ static int switch_qemu_logdirty(checkpoi
> strcpy(tail, "ret");
> if (!xs_watch(s->xsh, path, "qemu-logdirty-ret")) {
> s->errstr = "error watching qemu logdirty return";
> - return -1;
> + return 1;
> }
> /* null fire. XXX unify with shutdown watch! */
> vec = xs_read_watch(s->xsh, &len);
> @@ -550,7 +551,7 @@ static int switch_qemu_logdirty(checkpoi
> cmd = enable ? "enable" : "disable";
> if (!xs_write(s->xsh, XBT_NULL, path, cmd, strlen(cmd))) {
> s->errstr = "error signalling qemu logdirty";
> - return -1;
> + return 1;
> }
>
> vec = xs_read_watch(s->xsh, &len);
> @@ -564,7 +565,7 @@ static int switch_qemu_logdirty(checkpoi
> if (len)
> free(response);
> s->errstr = "qemu logdirty command failed";
> - return -1;
> + return 1;
> }
> free(response);
> fprintf(stderr, "qemu logdirty mode: %s\n", cmd);
I haven't compiled or run this, I'll wait for it to hit the tree.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home