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 > 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 > Signed-off-by: Ian Jackson > CC: Atom2 > CC: Konrad Rzeszutek Wilk > CC: Roger Pau Monne > CC: Ian Campbell > > Backported to 4.3. Conflicts: > tools/libxl/libxl_device.c > tools/libxl/libxl_internal.h > Signed-off-by: Ian Jackson > --- > 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; > } >