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@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

6 files changed, 65 insertions, 39 deletions

diff --git a/tools/libxc/ia64/xc_ia64_linux_save.c b/tools/libxc/ia64/xc_ia64_linux_save.c
index 1b028e6c17..4de4a7fe1a 100644
--- a/tools/libxc/ia64/xc_ia64_linux_save.c
+++ b/tools/libxc/ia64/xc_ia64_linux_save.c
@@ -397,8 +397,7 @@ out:
 int
 xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                uint32_t max_factor, uint32_t flags,
-               struct save_callbacks* callbacks,
-               int hvm, void (*switch_qemu_logdirty)(int, unsigned))
+               struct save_callbacks* callbacks, int hvm)
 {
     DECLARE_DOMCTL;
     xc_dominfo_t info;
@@ -450,6 +449,14 @@ xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
     void *p;
     efi_memory_desc_t *md;
     struct xen_ia64_p2m_table p2m_table;
+
+    if ( hvm && !callbacks->switch_qemu_logdirty )
+    {
+        ERROR("No switch_qemu_logdirty callback given.");
+        errno = EINVAL;
+        return 1;
+    }
+
     xc_ia64_p2m_init(&p2m_table);
 
     if (debug)
@@ -556,8 +563,10 @@ xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
         }
 
         /* Enable qemu-dm logging dirty pages to xen */
-        if (hvm)
-            switch_qemu_logdirty(dom, 1);
+        if (hvm && !callbacks->switch_qemu_logdirty(dom, 1, callbacks->data)) {
+            ERROR("Unable to enable qemu log-dirty mode");
+            goto out;
+        }
     } else {
 
         /* This is a non-live suspend. Issue the call back to get the
@@ -785,8 +794,9 @@ xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                               NULL, 0, NULL, 0, NULL ) < 0) {
             DPRINTF("Warning - couldn't disable shadow mode");
         }
-        if ( hvm )
-            switch_qemu_logdirty(dom, 0);
+        if ( hvm && !callbacks->switch_qemu_logdirty(dom, 0) ) {
+            DPRINTF("Warning - couldn't disable qemu log-dirty mode");
+        }
     }
 
     unlock_pages(to_send, bitmap_size);
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index b6e239200b..47f8a79c9d 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -873,8 +873,7 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd)
 
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                    uint32_t max_factor, uint32_t flags,
-                   struct save_callbacks* callbacks,
-                   int hvm, void (*switch_qemu_logdirty)(int, unsigned))
+                   struct save_callbacks* callbacks, int hvm)
 {
     xc_dominfo_t info;
     DECLARE_DOMCTL;
@@ -938,6 +937,13 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
 
     int completed = 0;
 
+    if ( hvm && !callbacks->switch_qemu_logdirty )
+    {
+        ERROR("No switch_qemu_logdirty callback provided.");
+        errno = EINVAL;
+        return 1;
+    }
+
     outbuf_init(xch, &ob, OUTBUF_SIZE);
 
     /* If no explicit control parameters given, use defaults */
@@ -1009,8 +1015,11 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
         }
 
         /* Enable qemu-dm logging dirty pages to xen */
-        if ( hvm )
-            switch_qemu_logdirty(dom, 1);
+        if ( hvm && !callbacks->switch_qemu_logdirty(dom, 1, callbacks->data) )
+        {
+            PERROR("Couldn't enable qemu log-dirty mode (errno %d)", errno);
+            goto out;
+        }
     }
     else
     {
@@ -1870,8 +1879,8 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
                                XEN_DOMCTL_SHADOW_OP_OFF,
                                NULL, 0, NULL, 0, NULL) < 0 )
             DPRINTF("Warning - couldn't disable shadow mode");
-        if ( hvm )
-            switch_qemu_logdirty(dom, 0);
+        if ( hvm && !callbacks->switch_qemu_logdirty(dom, 0, callbacks->data) )
+            DPRINTF("Warning - couldn't disable qemu log-dirty mode");
     }
 
     if ( live_shinfo )
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index 1a6acffdc3..b9d986e20e 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -40,7 +40,10 @@ struct save_callbacks {
      * 1: take another checkpoint */
     int (*checkpoint)(void* data);
 
-    /* to be provided as the first argument to each callback function */
+    /* Enable qemu-dm logging dirty pages to xen */
+    int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */
+
+    /* to be provided as the last argument to each callback function */
     void* data;
 };
 
@@ -54,8 +57,7 @@ struct save_callbacks {
  */
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                    uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */,
-                   struct save_callbacks* callbacks,
-                   int hvm, void (*switch_qemu_logdirty)(int, unsigned)); /* HVM only */
+                   struct save_callbacks* callbacks, int hvm);
 
 
 /**
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 13cdab5fd5..5352f65ef6 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -325,21 +325,23 @@ struct suspendinfo {
     unsigned int flags;
 };
 
-static void libxl__domain_suspend_common_switch_qemu_logdirty(int domid, unsigned int enable)
+static int libxl__domain_suspend_common_switch_qemu_logdirty(int domid, unsigned int enable, void *data)
 {
-    struct xs_handle *xsh;
-    char path[64];
-
-    snprintf(path, sizeof(path), "/local/domain/0/device-model/%u/logdirty/cmd", domid);
+    struct suspendinfo *si = data;
+    libxl_ctx *ctx = libxl__gc_owner(si->gc);
+    char *path;
+    bool rc;
 
-    xsh = xs_daemon_open();
+    path = libxl__sprintf(si->gc, "/local/domain/0/device-model/%u/logdirty/cmd", domid);
+    if (!path)
+        return 1;
 
     if (enable)
-        xs_write(xsh, XBT_NULL, path, "enable", strlen("enable"));
+        rc = xs_write(ctx->xsh, XBT_NULL, path, "enable", strlen("enable"));
     else
-        xs_write(xsh, XBT_NULL, path, "disable", strlen("disable"));
+        rc = xs_write(ctx->xsh, XBT_NULL, path, "disable", strlen("disable"));
 
-    xs_daemon_close(xsh);
+    return rc ? 0 : 1;
 }
 
 static int libxl__domain_suspend_common_callback(void *data)
@@ -437,11 +439,10 @@ int libxl__domain_suspend_common(libxl_ctx *ctx, uint32_t domid, int fd,
 
     memset(&callbacks, 0, sizeof(callbacks));
     callbacks.suspend = libxl__domain_suspend_common_callback;
+    callbacks.switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty;
     callbacks.data = &si;
 
-    xc_domain_save(ctx->xch, fd, domid, 0, 0, flags,
-                   &callbacks, hvm,
-                   &libxl__domain_suspend_common_switch_qemu_logdirty);
+    xc_domain_save(ctx->xch, fd, domid, 0, 0, flags, &callbacks, hvm);
 
     if (si.suspend_eventchn > 0)
         xc_suspend_evtchn_release(ctx->xch, si.xce, domid, si.suspend_eventchn);
@@ -450,6 +451,7 @@ int libxl__domain_suspend_common(libxl_ctx *ctx, uint32_t domid, int fd,
 
     rc = 0;
 out:
+    libxl__free_all(&gc);
     return rc;
 }
 
diff --git a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
index 9ec7c8736e..d7fad711de 100644
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
@@ -164,9 +164,9 @@ void checkpoint_close(checkpoint_state* s)
 
 /* 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, int fd,
     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(checkpoint_state *s, int enable)
     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(checkpoint_state *s, int enable)
     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(checkpoint_state *s, int enable)
        if (len)
            free(response);
        s->errstr = "qemu logdirty command failed";
-       return -1;
+       return 1;
     }
     free(response);
     fprintf(stderr, "qemu logdirty mode: %s\n", cmd);
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index cdf62afba6..4ae7e4cf9c 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -102,7 +102,7 @@ static int suspend(void* data)
  * active buffer. */
 
 
-static void switch_qemu_logdirty(int domid, unsigned int enable)
+static int switch_qemu_logdirty(int domid, unsigned int enable, void *data)
 {
     struct xs_handle *xs;
     char *path, *p, *ret_str, *cmd_str, **watch;
@@ -159,6 +159,8 @@ static void switch_qemu_logdirty(int domid, unsigned int enable)
     free(path);
     free(cmd_str);
     free(ret_str);
+
+    return 0;
 }
 
 int
@@ -205,9 +207,9 @@ main(int argc, char **argv)
     }
     memset(&callbacks, 0, sizeof(callbacks));
     callbacks.suspend = suspend;
+    callbacks.switch_qemu_logdirty = switch_qemu_logdirty;
     ret = xc_domain_save(si.xch, io_fd, si.domid, maxit, max_f, si.flags, 
-                         &callbacks, !!(si.flags & XCFLAGS_HVM),
-                         &switch_qemu_logdirty);
+                         &callbacks, !!(si.flags & XCFLAGS_HVM));
 
     if (si.suspend_evtchn > 0)
 	 xc_suspend_evtchn_release(si.xch, si.xce, si.domid, si.suspend_evtchn);