[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





Am 20.03.14 12:52, schrieb Ian Jackson:
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.
It compiled at my end as well, but I am sorry to report that the problem with the 40s delay persits. Attached please find the new xl-output created with xl -vvv create -F domain. This file again contains a few annotations with regards to where the delays happen. To my untrained eye it looks largely identical to the last xl-output with the obvious change of domain-id, addresses, line-numbers for debug output where changes in the sourve have happende and the use of the new function libxl__wait_for_backend_deprecated instead of libxl__wait_for_backend due to your patch, the latter of which I take as proof that your patches have been applied.

For other obvious changes I have commented in the file on those lines that I could identify: There are a few new lines which were not there last time and now there's also a new 10s delay which, however, is only visible through xl -vvvv -F domain as the prompt has already returned by the time this new delay happens and this delay therefore is not visible in dom0.

I hope that helps. Thanks for your continued support and best regards,

Atom2

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;
          }

Attachment: xl-output
Description: Text document

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.