[libvirt] [PATCH 00/21] qemu_hotplug: refactor device detach functions to fix one serious and other minor bugs

Laine Stump posted 21 patches 4 weeks ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190321222901.12902-1-laine@laine.org
src/qemu/qemu_driver.c  |  102 +--
src/qemu/qemu_hotplug.c | 1407 ++++++++++++++++++---------------------
src/qemu/qemu_hotplug.h |   56 +-
tests/qemuhotplugtest.c |    8 +-
4 files changed, 655 insertions(+), 918 deletions(-)

[libvirt] [PATCH 00/21] qemu_hotplug: refactor device detach functions to fix one serious and other minor bugs

Posted by Laine Stump 4 weeks ago
This all started with the observation that we were sending the
DEVICE_REMOVED events for unplugged devices too soon (reported in
https://bugzilla.redhat.com/1658198 ). While noticing that *all* of
the device types send their DEVICE_REMOVED event too early, I also
noticed how eerily similar all the different detach functions are
(they have converged over time, as legacy code supporting old qemus
with different methods of deleting different types of devices has been
removed - pkrempa and mprivozn committed patches just in the last week
that contributed to this convergence and removed some duplicated
code).

I also saw a few other definite bugs, as well as a couple that I think
are bugs, but am not enough informed about zPCI to be certain (I've
Cc'ed the author and reviewers of the original patch adding that code
to the patches so that they can verify, or negate, my suspicions).

Many/most of these patches are either very simple / easily verifiable
code movement / removal / renaming of variables. Patch 20 is a bit
more involved; viewing the new versions of the functions along with
the patch would probably help it go much quicker.

The first two patches in the series eliminate what I'm 99% certain is
erroneous calling of the qemuDomainDetachExtensionDevice() when
detaching a PCI device on S390. The commit log details my reasons for
believing this code should be removed.


The rest of the series eliminates small differences iteratively,
arriving at a set of detach functions which are completely identical
at the end (and only called from one place), and then replaces all
those duplicates with a single block of common code in a higher level
function that calls all of the individual functions (it was their only
caller anyway). This eliminated the inconsistent application of
checking for multifunction PCI devices and for the existence of a
device alias. It also eliminated individual calls to
qemuDomainRemove*Device() calls, and replaced them with a single call
to qemuDomainRemoveDevice().

Eliminating all the calls (except one) to the individual
qemuDomainRemove*Device() functions in turn made it possible to move
the code that builds and queues the DEVICE_REMOVED event into a single
place in the one remaining higher level caller -
qemuDomainRemoveDevice() - thus eliminating the "premature event" for *all*
device types at once.

Laine Stump (21):
  qemu_hotplug: remove erroneous call to
    qemuDomainDetachExtensionDevice()
  qemu_hotplug: remove another erroneous
    qemuDomainDetachExtensionDevice() call
  qemu_hotplug: remove unnecessary check for valid PCI address
  qemu_hotplug: rename a virDomainDeviceInfoPtr to avoid confusion
  qemu_hotplug: eliminate multiple identical
    qemuDomainDetachHost*Device() functions
  qemu_hotplug: eliminate unnecessary call to
    qemuDomainDetachNetDevice()
  qemu_hotplug: refactor qemuDomainDetachDiskLive and
    qemuDomainDetachDiskDevice
  qemu_hotplug: don't call DetachThisHostDevice for hostdev network
    devices
  qemu_hotplug: merge qemuDomainDetachThisHostDevice into
    qemuDomainDetachHostDevice
  qemu_hotplug: move qemuDomainChangeGraphicsPasswords()
  qemu_hotplug: move (almost) all qemuDomainDetach*() functions together
  qemu_hotplug: move (Attach|Detach)Lease functions with others of same
    type
  qemu_hotplug: move qemuDomainDetachDeviceLive() to qemu_hotplug.c
  qemu_hotplug: remove extra function in middle of DetachController call
    chain
  qemu_hotplug: pull qemuDomainUpdateDeviceList out of
    qemuDomainDetachDeviceLive
  test: replace calls to individual detach functions with one call to
    main detach
  qemu_hotplug: rename dev to match in qemuDomainDetachDeviceLive
  qemu_hotplug: standardize the names/args/calling of
    qemuDomainDetach*()
  qemu_hotplug: new function qemuDomainRemoveAuditDevice()
  qemu_hotplug: consolidate all common detach code in
    qemuDomainDetachDeviceLive
  qemu_hotplug: delay sending DEVICE_REMOVED event until after *all*
    teardown

 src/qemu/qemu_driver.c  |  102 +--
 src/qemu/qemu_hotplug.c | 1407 ++++++++++++++++++---------------------
 src/qemu/qemu_hotplug.h |   56 +-
 tests/qemuhotplugtest.c |    8 +-
 4 files changed, 655 insertions(+), 918 deletions(-)

Yay! jtomko would be proud - more deletions than insertions!!!

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list