Xen devel - Aug 2012 - [PATCH] libxl: make domain resume API asynchronous

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1345046301 -3600
# Node ID 7cec0543f67cefe3755bbad0c2262fa2e820d746
# Parent  30bf79cc14d932fbe6ff572d0438e5a432f69b0a
libxl: make domain resume API asynchronous

Although the current implementation has no asynchromous parts I can
envisage it needing to do bits of create/destroy like functionality
which may need async support in the future.

To do this make the meat into an internal libxl__domain_resume
function in order to satisfy the no-internal-callers rule for the
async function.

Since I needed to touch the logging to s/ctx/CTX/ anyway switch to the
LOG* helper macros.

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

diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Aug 15 14:45:21 2012 +0100
+++ b/tools/libxl/libxl.c	Wed Aug 15 16:58:21 2012 +0100
@@ -396,15 +396,12 @@ int libxl_domain_rename(libxl_ctx *ctx, 
     return rc;
 }
 
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
+int libxl__domain_resume(libxl__gc *gc, uint32_t domid, int suspend_cancel)
 {
-    GC_INIT(ctx);
     int rc = 0;
 
-    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                        "xc_domain_resume failed for domain %u",
-                        domid);
+    if (xc_domain_resume(CTX->xch, domid, suspend_cancel)) {
+        LOGE(ERROR, "xc_domain_resume failed for domain %u", domid);
         rc = ERROR_FAIL;
         goto out;
     }
@@ -418,24 +415,29 @@ int libxl_domain_resume(libxl_ctx *ctx, 
     if (type == LIBXL_DOMAIN_TYPE_HVM) {
         rc = libxl__domain_resume_device_model(gc, domid);
         if (rc) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "failed to resume device model for domain
%u:%d",
-                       domid, rc);
+            LOG(ERROR, "failed to resume device model for domain
%u:%d",
+                domid, rc);
             goto out;
         }
     }
 
-    if (!xs_resume_domain(ctx->xsh, domid)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                        "xs_resume_domain failed for domain %u",
-                        domid);
+    if (!xs_resume_domain(CTX->xsh, domid)) {
+        LOGE(ERROR, "xs_resume_domain failed for domain %u", domid);
         rc = ERROR_FAIL;
     }
 out:
-    GC_FREE;
     return rc;
 }
 
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel,
+                        const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    int rc = libxl__domain_resume(gc, domid, suspend_cancel);
+    libxl__ao_complete(egc, ao, rc);
+    return AO_INPROGRESS;
+}
+
 /*
  * Preserves a domain but rewrites xenstore etc to make it unique so
  * that the domain can be restarted.
diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Wed Aug 15 14:45:21 2012 +0100
+++ b/tools/libxl/libxl.h	Wed Aug 15 16:58:21 2012 +0100
@@ -529,7 +529,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
  *   If this parameter is true, use co-operative resume. The guest
  *   must support this.
  */
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel,
+                        const libxl_asyncop_how *ao_how)
+                        LIBXL_EXTERNAL_CALLERS_ONLY;
 
 int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
                              uint32_t domid, int send_fd, int recv_fd,
diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Wed Aug 15 14:45:21 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Wed Aug 15 16:58:21 2012 +0100
@@ -1121,7 +1121,7 @@ static int libxl__remus_domain_resume_ca
     STATE_AO_GC(dss->ao);
 
     /* Resumes the domain and the device model */
-    if (libxl_domain_resume(CTX, dss->domid, /* Fast Suspend */1))
+    if (libxl__domain_resume(gc, dss->domid, /* Fast Suspend */1))
         return 0;
 
     /* REMUS TODO: Deal with disk. Start a new network output buffer */
diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Wed Aug 15 14:45:21 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Wed Aug 15 16:58:21 2012 +0100
@@ -899,6 +899,9 @@ _hidden int libxl__domain_resume_device_
 
 _hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
 
+_hidden int libxl__domain_resume(libxl__gc *gc, uint32_t domid,
+                                 int suspend_cancel);
+
 /* returns 0 or 1, or a libxl error code */
 _hidden int libxl__domain_pvcontrol_available(libxl__gc *gc, uint32_t domid);
 
diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Aug 15 14:45:21 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed Aug 15 16:58:21 2012 +0100
@@ -2859,7 +2859,7 @@ static int save_domain(const char *p, co
     close(fd);
 
     if (checkpoint)
-        libxl_domain_resume(ctx, domid, 1);
+        libxl_domain_resume(ctx, domid, 1, 0);
     else
         libxl_domain_destroy(ctx, domid, 0);
 
@@ -3110,7 +3110,7 @@ static void migrate_domain(const char *d
         if (common_domname) {
             libxl_domain_rename(ctx, domid, away_domname, common_domname);
         }
-        rc = libxl_domain_resume(ctx, domid, 0);
+        rc = libxl_domain_resume(ctx, domid, 0, 0);
         if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
 
         fprintf(stderr, "Migration failed due to problems at
target.\n");
@@ -3132,7 +3132,7 @@ static void migrate_domain(const char *d
     close(send_fd);
     migration_child_report(recv_fd);
     fprintf(stderr, "Migration failed, resuming at sender.\n");
-    libxl_domain_resume(ctx, domid, 0);
+    libxl_domain_resume(ctx, domid, 0, 0);
     exit(-ERROR_FAIL);
 
  failed_badly:
@@ -6654,7 +6654,7 @@ int main_remus(int argc, char **argv)
         fprintf(stderr, "Failed to suspend domain at primary.\n");
     else {
         fprintf(stderr, "Remus: Backup failed? resuming domain at
primary.\n");
-        libxl_domain_resume(ctx, domid, 1);
+        libxl_domain_resume(ctx, domid, 1, 0);
     }
 
     close(send_fd);

Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1345046301 -3600
> # Node ID 7cec0543f67cefe3755bbad0c2262fa2e820d746
> # Parent  30bf79cc14d932fbe6ff572d0438e5a432f69b0a
> libxl: make domain resume API asynchronous
> 
> Although the current implementation has no asynchromous parts I can
> envisage it needing to do bits of create/destroy like functionality
> which may need async support in the future.
> 
> To do this make the meat into an internal libxl__domain_resume
> function in order to satisfy the no-internal-callers rule for the
> async function.
> 
> Since I needed to touch the logging to s/ctx/CTX/ anyway switch to the
> LOG* helper macros.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Just a minor comment below.
> 
> diff -r 30bf79cc14d9 -r 7cec0543f67c tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Wed Aug 15 14:45:21 2012 +0100
> +++ b/tools/libxl/libxl.c	Wed Aug 15 16:58:21 2012 +0100
> @@ -396,15 +396,12 @@ int libxl_domain_rename(libxl_ctx *ctx, 
>      return rc;
>  }
>  
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int
suspend_cancel)
> +int libxl__domain_resume(libxl__gc *gc, uint32_t domid, int
suspend_cancel)
>  {
> -    GC_INIT(ctx);
You can also use libxl_ctx *ctx = CTX; so there''s no need to change all
occurrences of ctx with CTX (not that I have a problem with that).

Ian Campbell writes ("[PATCH] libxl: make domain resume API
asynchronous"):
> libxl: make domain resume API asynchronous
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>