qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, berrange@redhat.com,
	alex.williamson@redhat.com, pbonzini@redhat.com,
	david@gibson.dropbear.id.au, groug@kaod.org, armbru@redhat.com
Subject: [Qemu-devel] [PATCH for-2.10 0/3] qdev/vfio: defer DEVICE_DEL to avoid races with libvirt
Date: Wed, 26 Jul 2017 20:30:52 -0500	[thread overview]
Message-ID: <1501119055-4060-1-git-send-email-mdroth@linux.vnet.ibm.com> (raw)

This series was motivated by the discussion in this thread:

  https://www.redhat.com/archives/libvir-list/2017-June/msg01370.html

The issue this series addresses is that when libvirt unplugs a VFIO PCI device,
it may attempt to bind the host device back to the host driver when QEMU emits
the DEVICE_DELETED event for the corresponding vfio-pci device. However, the
VFIO group FD is not actually cleaned up until vfio-pci device is *finalized*
by QEMU, whereas the event is emitted earlier during device_unparent.
Depending on the host device and how long certain operations like resetting the
device might take, this can in result in libvirt trying to rebind the device
back to the host while it is still in use by VFIO, leading to host crashes or
other unexpected behavior.

In particular, Mellanox CX4 adapters on PowerNV hosts might not be fully
quiesced by vfio-pci's finalize() routine until up to 6s after the
DEVICE_DELETED was emitted, leading to detach-device on the libvirt side pretty
much always crashing the host.

Implementing this change requires 2 prereqs to ensure the same information is
available when the DEVICE_DELETED is finally emitted:

1) Storing the path in the composition patch, which is addressed by PATCH 1,
   which was plucked from another pending series from Greg Kurz:

   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg07922.html

   since we are now "disconnected" at the time the event is emitted, and

2) Deferring qemu_opts_del of the DeviceState->QemuOpts till finalize, since
   that is where DeviceState->id is stored. This was actually how it was
   done in the past, so PATCH 2 simply reverts the change which moved it to
   device_unparent.

>From there it's just a mechanical move of the event from device_unparent to
device_finalize.

 hw/core/qdev.c         | 30 +++++++++++++++++++-----------
 include/hw/qdev-core.h |  1 +
 2 files changed, 20 insertions(+), 11 deletions(-)

             reply	other threads:[~2017-07-27  1:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-27  1:30 Michael Roth [this message]
2017-07-27  1:30 ` [Qemu-devel] [PATCH for-2.10 1/3] qdev: store DeviceState's canonical path to use when unparenting Michael Roth
2017-07-27  1:30 ` [Qemu-devel] [PATCH for-2.10 2/3] Revert "qdev: Free QemuOpts when the QOM path goes away" Michael Roth
2017-07-31 15:51   ` Greg Kurz
2017-07-31 16:39     ` Michael Roth
2017-07-31 17:10       ` Greg Kurz
2017-07-27  1:30 ` [Qemu-devel] [PATCH for-2.10 3/3] qdev: defer DEVICE_DEL event until instance_finalize() Michael Roth
2017-07-31 17:11   ` Greg Kurz
2017-08-09 14:04   ` Auger Eric
2017-10-07  0:03     ` Michael Roth
2017-07-27  9:11 ` [Qemu-devel] [PATCH for-2.10 0/3] qdev/vfio: defer DEVICE_DEL to avoid races with libvirt Peter Maydell
2017-07-27 10:53   ` David Gibson
2017-07-27 11:50     ` Daniel P. Berrange
2017-08-08 19:40       ` Alex Williamson
2017-08-09  5:08         ` David Gibson
2017-09-05 19:35           ` Greg Kurz
2017-07-27 11:54     ` Michael Roth
2017-07-27 14:47     ` Alex Williamson
2017-07-28  3:14       ` David Gibson
2017-08-09 14:53 ` Auger Eric
2017-10-03 22:21 ` Michael Roth
2017-10-04  6:01   ` David Gibson
2017-10-06 10:23   ` David Gibson
2017-10-06 12:31     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1501119055-4060-1-git-send-email-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).