WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] tools: cleanup domain save switch_qemu_logdirty call

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH] tools: cleanup domain save switch_qemu_logdirty callback
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Tue, 19 Oct 2010 09:21:59 +0100
Cc: Brendan Cully <brendan@xxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Tue, 19 Oct 2010 01:22:44 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1287044260.2003.5491.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1287044260.2003.5491.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mercurial-patchbomb/1.5.2
# 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)

diff -r ca91b58834a3 -r 0b5d85ea10f8 tools/libxc/ia64/xc_ia64_linux_save.c
--- a/tools/libxc/ia64/xc_ia64_linux_save.c     Tue Oct 19 09:07:47 2010 +0100
+++ b/tools/libxc/ia64/xc_ia64_linux_save.c     Tue Oct 19 09:17:18 2010 +0100
@@ -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
     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
         }
 
         /* 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
                               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 -r ca91b58834a3 -r 0b5d85ea10f8 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c      Tue Oct 19 09:07:47 2010 +0100
+++ b/tools/libxc/xc_domain_save.c      Tue Oct 19 09:17:18 2010 +0100
@@ -873,8 +873,7 @@ static int save_tsc_info(xc_interface *x
 
 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, in
 
     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, in
         }
 
         /* 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, in
                                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 -r ca91b58834a3 -r 0b5d85ea10f8 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h    Tue Oct 19 09:07:47 2010 +0100
+++ b/tools/libxc/xenguest.h    Tue Oct 19 09:17:18 2010 +0100
@@ -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 -r ca91b58834a3 -r 0b5d85ea10f8 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Tue Oct 19 09:07:47 2010 +0100
+++ b/tools/libxl/libxl_dom.c   Tue Oct 19 09:17:18 2010 +0100
@@ -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];
+    struct suspendinfo *si = data;
+    libxl_ctx *ctx = libxl__gc_owner(si->gc);
+    char *path;
+    bool rc;
 
-    snprintf(path, sizeof(path), 
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
-
-    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_c
 
     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_c
 
     rc = 0;
 out:
+    libxl__free_all(&gc);
     return rc;
 }
 
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);
diff -r ca91b58834a3 -r 0b5d85ea10f8 tools/xcutils/xc_save.c
--- a/tools/xcutils/xc_save.c   Tue Oct 19 09:07:47 2010 +0100
+++ b/tools/xcutils/xc_save.c   Tue Oct 19 09:17:18 2010 +0100
@@ -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 dom
     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);

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