On 05/08/2015 06:08 PM, Andrew Cooper wrote:
On 08/05/15 10:59, Hongyang Yang wrote:
In general, a good change, but some comments...
---
tools/libxc/xc_sr_save.c | 72
+++++++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 28 deletions(-)
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index cc3e6b1..2394bc4 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -607,13 +607,10 @@ static int send_domain_memory_nonlive(struct
xc_sr_context *ctx)
return rc;
}
-/*
- * Save a domain.
- */
-static int save(struct xc_sr_context *ctx, uint16_t guest_type)
+static int setup(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
- int rc, saved_rc = 0, saved_errno = 0;
+ int rc;
DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
(&ctx->save.dirty_bitmap_hbuf));
@@ -632,13 +629,51 @@ static int save(struct xc_sr_context *ctx,
uint16_t guest_type)
goto err;
}
- IPRINTF("Saving domain %d, type %s",
- ctx->domid, dhdr_type_to_str(guest_type));
-
rc = ctx->save.ops.setup(ctx);
if ( rc )
goto err;
+ rc = 0;
+
+ err:
+ return rc;
+
+}
+
+static void cleanup(struct xc_sr_context *ctx)
+{
+ xc_interface *xch = ctx->xch;
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ (&ctx->save.dirty_bitmap_hbuf));
+
+ xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
+ NULL, 0, NULL, 0, NULL);
+
+ if ( ctx->save.ops.cleanup(ctx) )
+ PERROR("Failed to clean up");
+
+ if ( dirty_bitmap )
+ xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
+
NRPAGES(bitmap_size(ctx->save.p2m_size)));
xc_hypercall_buffer_free_pages() if fine dealing with NULL, just like
free() is. You can drop the conditional.
Actually this is another trick that I need to deal with those
hypercall macros.
DECLARE_HYPERCALL_BUFFER_SHADOW will define a user pointer "dirty_bitmap"
and a shadow buffer, although xc_hypercall_buffer_free_pages takes
"dirty_bitmap" as an augument, but it is also a MACRO, without
"if ( dirty_bitmap )", the compiler will report "dirty_bitmap" unused
error...
Ah, in which case you would be better using
xc__hypercall_buffer_free_pages() and not creating the local shadow in
the first place.
I thought we'd better use those MACROs which described in the comments...
If it is OK to use xc__hypercall_buffer_free_pages(), I will fix it in
the next version.
~Andrew
.
--
Thanks,
Yang.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
