On Tue, 2010-10-19 at 22:12 +0100, Brendan Cully wrote:
> 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:
It makes the callback return code match the convention used by
xc_domain_save itself. It isn't necessarily the best convention but I
thought the overall consistency was more important.
Ian.
>
> > 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