Re: Tools backports for 4.5 [and 1 more messages]

[Jim Fehlig <jfehlig@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On 06/23/2015 02:06 PM, Jim Fehlig wrote:
> On 06/23/2015 07:12 AM, Ian Jackson wrote:
>> Ian Jackson writes ("Re: [Xen-devel] Tools backports for 4.5 [and 1 more 
>> messages]"):
>>> Jan Beulich writes ("Re: [Xen-devel] Tools backports for 4.5"):
>>>>>>>> 3380f5b6270e ocaml/xenctrl: Check return values from hypercalls
>>>>>>>> c8945d516134 ocaml/xenctrl: Make failwith_xc() thread safe
>>>>>>>> 1a010ca99e9b ocaml/xenctrl: Fix stub_xc_readconsolering()
>>>>>>>> 96e0ee8386cf tools/hotplug: systemd: Don't ever kill xenstored
>>>>>>>> 588df84c0d70 tools/xenconsoled: Increase file descriptor limit
>> ...
>>> The above backports (771f857e..9d5b2b01) need to go to 4.4 and
>>> earlier as applicable.  I will look at that soon (probably tomorrow).
>> These:
>>
>>    libxl: Domain destroy: unlock userdata earlier

4.4 doesn't have lock/unlock functions for the userdata store, so not sure why 
this one is needed.

>>
>>    libxl: Domain destroy: fork

Attached is my stab at backporting this one, which helps the libvirt libxl 
driver quite a bit.

>>    tools/hotplug: systemd: Don't ever kill xenstored
>>    libxl: In libxl_set_vcpuonline check for maximum number of VCPUs against 
>> the cpumap

I didn't request these, and they don't fix any particular headache for me, so I 
didn't look further.

Regards,
Jim


[-- Attachment #2: libxl-Domain-destroy-fork.patch --]
[-- Type: text/x-diff, Size: 5035 bytes --]

>From 0e447fed880e302e053f486180b1f6c22f515403 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Thu, 5 Mar 2015 16:28:04 +0000
Subject: [PATCH] libxl: Domain destroy: fork

Call xc_domain_destroy in a subprocess.  That allows us to do so
asynchronously, rather than blocking the whole process calling libxl.

The changes in detail:

 * Provide an libxl__ev_child in libxl__domain_destroy_state, and
   initialise it in libxl__domain_destroy.  There is no possibility
   to `clean up' a libxl__ev_child, but there need to clean it up, as
   the control flow ensures that we only continue after the child has
   exited.

 * Call libxl__ev_child_fork at the right point and put the call to
   xc_domain_destroy and associated logging in the child.  (The child
   opens a new xenctrl handle because we mustn't use the parent's.)

 * Consequently, the success return path from domain_destroy_domid_cb
   no longer calls dis->callback.  Instead it simply returns.

 * We plumb the errorno value through the child's exit status, if it
   fits.  This means we normally do the logging only in the parent.

 * Incidentally, we fix the bug that we were treating the return value
   from xc_domain_destroy as an errno value when in fact it is a
   return value from do_domctl (in this case, 0 or -1 setting errno).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Conflicts:
	tools/libxl/libxl.c

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 tools/libxl/libxl.c          | 57 ++++++++++++++++++++++++++++++++++++++++----
 tools/libxl/libxl_internal.h |  1 +
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 59e3292..d91a178 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1367,6 +1367,10 @@ static void devices_destroy_cb(libxl__egc *egc,
                                libxl__devices_remove_state *drs,
                                int rc);
 
+static void domain_destroy_domid_cb(libxl__egc *egc,
+                                    libxl__ev_child *destroyer,
+                                    pid_t pid, int status);
+
 void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
 {
     STATE_AO_GC(dis->ao);
@@ -1376,6 +1380,8 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
     char *pid;
     int rc, dm_present;
 
+    libxl__ev_child_init(&dis->destroyer);
+
     rc = libxl_domain_info(ctx, NULL, domid);
     switch(rc) {
     case 0:
@@ -1472,17 +1478,58 @@ static void devices_destroy_cb(libxl__egc *egc,
 
     libxl__userdata_destroyall(gc, domid);
 
-    rc = xc_domain_destroy(ctx->xch, domid);
-    if (rc < 0) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_destroy failed for %d", domid);
+    rc = libxl__ev_child_fork(gc, &dis->destroyer, domain_destroy_domid_cb);
+    if (rc < 0) goto out;
+    if (!rc) { /* child */
+        ctx->xch = xc_interface_open(ctx->lg,0,0);
+        if (!ctx->xch) goto badchild;
+
+        rc = xc_domain_destroy(ctx->xch, domid);
+        if (rc < 0) goto badchild;
+        _exit(0);
+
+    badchild:
+        if (errno > 0  && errno < 126) {
+            _exit(errno);
+        } else {
+            LOGE(ERROR,
+ "xc_domain_destroy failed for %d (with difficult errno value %d)",
+                 domid, errno);
+            _exit(-1);
+        }
+    }
+    LOG(INFO, "forked pid %ld for destroy of domain %d", (long)rc, domid);
+
+    return;
+
+out:
+    dis->callback(egc, dis, rc);
+    return;
+}
+
+static void domain_destroy_domid_cb(libxl__egc *egc,
+                                    libxl__ev_child *destroyer,
+                                    pid_t pid, int status)
+{
+    libxl__destroy_domid_state *dis = CONTAINER_OF(destroyer, *dis, destroyer);
+    STATE_AO_GC(dis->ao);
+    int rc;
+
+    if (status) {
+        if (WIFEXITED(status) && WEXITSTATUS(status)<126) {
+            LOGEV(ERROR, WEXITSTATUS(status),
+                  "xc_domain_destroy failed for %"PRIu32"", dis->domid);
+        } else {
+            libxl_report_child_exitstatus(CTX, XTL_ERROR,
+                                          "async domain destroy", pid, status);
+        }
         rc = ERROR_FAIL;
         goto out;
     }
     rc = 0;
 
-out:
+ out:
     dis->callback(egc, dis, rc);
-    return;
 }
 
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9d17586..df63757 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2466,6 +2466,7 @@ struct libxl__destroy_domid_state {
     libxl__domid_destroy_cb *callback;
     /* private to implementation */
     libxl__devices_remove_state drs;
+    libxl__ev_child destroyer;
 };
 
 struct libxl__domain_destroy_state {
-- 
2.1.4


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel