Re: [Xen-devel] [Xen-users] substantial shutdown delay for PV guests with PCI -passthrough

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Atom2 writes ("Re: [Xen-devel] [Xen-users] substantial shutdown delay for PV 
guests with PCI -passthrough"):
> the patch unfortunately doesn't apply to my sources - some comments to 
> the reasons why further below.

Here's a backport.  I have compiled but not executed it.

> Sorry for my delay in answering - this is a resend as the first e-Mail 
> with uncompressed attachments did not go through.

> Just FYI: the version I am using is 4.3.1-r5; I have attached the 
> relevant source files referred to by your patches.

Thanks, but our revision control system enables us to retrieve old
versions very easily :-).  So there is not any need to provide us with
these files.

Having said that, I have no record of 4.3.1-rc5.  But I'm pretty sure
the patch below, which is against staging-4.3, will apply to your
tree.  It applies cleanly to 4.3.0-rc5 and 4.3.1-rc2, which are my two
guesses as to which version you mean.

Thanks,
Ian.

From f9df128cd4d4ad6c7ed6ffd9bd8ba0633af78389 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Wed, 19 Mar 2014 15:47:02 +0000
Subject: [PATCH] libxl: Tolerate backend state "6" on pciback remove

When shutting down a domain with pci passthrough, it can happen that
the backend has actually shut down (xenbus state 6) before we try to
remove it.  When this happens, libxl would time out waiting for the
backend to reach state 4.

Instead, deal with this by having libxl__wait_for_backend take a list
of suitable states.

The arrangements are still fundamentally incorrect:
 - libxl__wait_for_backend is a slow synchronous function, which is
   forbidden;
 - There is no way to deal properly with the various xenbus states
   that might arise (including erroneous ones).
We will hopefully fix this later, although it's not trivial.  For
the moment, rename the function to libxl__wait_for_backend_deprecated.

Reported-by: Atom2 <ariel.atom2@xxxxxxxxxx>
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Atom2 <ariel.atom2@xxxxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Roger Pau Monne <roger.pau@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>

Backported to 4.3.  Conflicts:
        tools/libxl/libxl_device.c
        tools/libxl/libxl_internal.h
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.c          |    2 +-
 tools/libxl/libxl_device.c   |   21 ++++++++++++++-------
 tools/libxl/libxl_internal.h |    3 ++-
 tools/libxl/libxl_pci.c      |    8 +++++---
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3d9543b..c0cc0b7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2689,7 +2689,7 @@ static void local_device_attach_cb(libxl__egc *egc, 
libxl__ao_device *aodev)
     if (rc < 0)
         goto out;
     be_path = libxl__device_backend_path(gc, &device);
-    rc = libxl__wait_for_backend(gc, be_path, "4");
+    rc = libxl__wait_for_backend_deprecated(gc, be_path, "4", (char*)0);
     if (rc < 0)
         goto out;
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index ea845b7..779b38b 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1094,7 +1094,8 @@ int libxl__wait_for_device_model(libxl__gc *gc,
                                      check_callback, check_callback_userdata);
 }
 
-int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state)
+int libxl__wait_for_backend_deprecated(libxl__gc *gc, const char *be_path,
+                                       ...)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int watchdog = 100;
@@ -1115,13 +1116,19 @@ int libxl__wait_for_backend(libxl__gc *gc, char 
*be_path, char *state)
             }
             goto out;
         } else {
-            if (!strcmp(p, state)) {
-                rc = 0;
-                goto out;
-            } else {
-                usleep(100000);
-                watchdog--;
+            const char *want;
+            va_list al;
+            va_start(al,be_path);
+            while ((want = va_arg(al, char*))) {
+                if (!strcmp(p, want)) {
+                    va_end(al);
+                    rc = 0;
+                    goto out;
+                }
             }
+            va_end(al);
+            usleep(100000);
+            watchdog--;
         }
     }
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Backend %s not ready", be_path);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f051d91..4485c56 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -944,7 +944,8 @@ _hidden char *libxl__device_frontend_path(libxl__gc *gc, 
libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
+_hidden int libxl__wait_for_backend_deprecated(libxl__gc *gc,
+                   const char *be_path, ...) __attribute__((sentinel));
 _hidden int libxl__nic_type(libxl__gc *gc, libxl__device *dev,
                             libxl_nic_type *nictype);
 
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 0295e0b..e22852c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -126,7 +126,7 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, 
uint32_t domid, libxl_d
         return ERROR_FAIL;
 
     if (!starting && domtype == LIBXL_DOMAIN_TYPE_PV) {
-        if (libxl__wait_for_backend(gc, be_path, "4") < 0)
+        if (libxl__wait_for_backend_deprecated(gc, be_path, "4", (char*)0) < 0)
             return ERROR_FAIL;
     }
 
@@ -169,7 +169,8 @@ static int libxl__device_pci_remove_xenstore(libxl__gc *gc, 
uint32_t domid, libx
         return ERROR_FAIL;
 
     if (domtype == LIBXL_DOMAIN_TYPE_PV) {
-        if (libxl__wait_for_backend(gc, be_path, "4") < 0) {
+        if (libxl__wait_for_backend_deprecated(gc, be_path, "4", "6", (char*)0)
+            < 0) {
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "pci backend at %s is not 
ready", be_path);
             return ERROR_FAIL;
         }
@@ -198,7 +199,8 @@ retry_transaction:
             goto retry_transaction;
 
     if (domtype == LIBXL_DOMAIN_TYPE_PV) {
-        if (libxl__wait_for_backend(gc, be_path, "4") < 0) {
+        if (libxl__wait_for_backend_deprecated(gc, be_path, "4", "6", (char*)0)
+            < 0) {
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "pci backend at %s is not 
ready", be_path);
             return ERROR_FAIL;
         }
-- 
1.7.10.4



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