Re: [Xen-devel] [PATCH] tools: add closure to xc_domain_save switch_qemu_logdirty callback

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Wed, 2010-10-13 at 12:14 +0100, Stefano Stabellini wrote:
> On Tue, 12 Oct 2010, Ian Campbell wrote:
> > @@ -1876,7 +1876,7 @@ int xc_domain_save(xc_interface *xch, in
> >                                 NULL, 0, NULL, 0, NULL) < 0 )
> >              DPRINTF("Warning - couldn't disable shadow mode");
> >          if ( hvm )
> > -            switch_qemu_logdirty(dom, 0);
> > +            callbacks->switch_qemu_logdirty(dom, 0, callbacks->data);
> >      }
> >  
> >      if ( live_shinfo )
> 
> I think that at the beginning of xc_domain_save we should check if
> callbacks->switch_qemu_logdirty is NULL and print an error an return in
> that case.

We didn't do so for the original function pointer parameter.

Not passing in this callback is a pretty fundamental error in the
caller, there's not really anything they can do with the error code.
This change will break compilation for any caller which has not been
updated so I don't think there is too much danger of toolstacks missing
the need for the change.

Propagating an EINVAL doesn't really help unless all callers reliably
test the return code and do something sensible with it.

On the other hand given that at least one caller has a valid reason not
to use the callback (unless this changes means it now could, as I
wondered in the original changelog but forgot to CC Brendan about) then
I think this would be more reasonable than EINVAL

Subject: libxc allow omission of hvm switch_qemu_logdirty on save

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r 067d47ab6dc9 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c      Wed Oct 13 11:08:22 2010 +0100
+++ b/tools/libxc/xc_domain_save.c      Wed Oct 13 13:45:25 2010 +0100
@@ -1014,7 +1014,7 @@ int xc_domain_save(xc_interface *xch, in
         }
 
         /* Enable qemu-dm logging dirty pages to xen */
-        if ( hvm )
+        if ( hvm && callbacks->switch_qemu_logdirty )
             callbacks->switch_qemu_logdirty(dom, 1, callbacks->data);
     }
     else
@@ -1875,7 +1875,7 @@ int xc_domain_save(xc_interface *xch, in
                                XEN_DOMCTL_SHADOW_OP_OFF,
                                NULL, 0, NULL, 0, NULL) < 0 )
             DPRINTF("Warning - couldn't disable shadow mode");
-        if ( hvm )
+        if ( hvm && callbacks->switch_qemu_logdirty )
             callbacks->switch_qemu_logdirty(dom, 0, callbacks->data);
     }
 
diff -r 067d47ab6dc9 tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c      Wed Oct 13 
11:08:22 2010 +0100
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c      Wed Oct 13 
13:45:25 2010 +0100
@@ -162,13 +162,6 @@ void checkpoint_close(checkpoint_state* 
   s->fd = -1;
 }
 
-/* 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, void *data)
-{
-    return;
-}
-
 int checkpoint_start(checkpoint_state* s, int fd,
                     struct save_callbacks* callbacks)
 {
@@ -189,7 +182,7 @@ int checkpoint_start(checkpoint_state* s
            return rc;
     }
 
-    callbacks->switch_qemu_logdirty = noop_switch_logdirty;
+    callbacks->switch_qemu_logdirty = NULL;
 
     rc = xc_domain_save(s->xch, fd, s->domid, 0, 0, flags, callbacks, hvm);
 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel