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

[libvirt] [PATCH 00/10] conf: Clean up virDomain*Def creation



This series is required to solve a known limitation in the
current incarnation of device isolation support[1], namely
the inability to isolate host devices coming from IOMMU
group 0.

The issue lies in the fact that virDomainDeviceInfo, and
all virDomain*Def that embed it, are usually allocated
through VIR_ALLOC(), which result in all their fields being
initialized to zero.

That's worked out just fine so far, because zero was a
sensible default value for all existing fields; however,
when implementing isolation groups, we add a new
virDomainDeviceInfo::isolationGroup field which we need
to be initialized to -1 instead so that it doesn't overlap
with IOMMU group 0 mentioned above.

Solving the issue involves creating twenty or so
virDomain*DefNew() functions and using them instead of
VIR_ALLOC() every time a virDomain*Def needs to be created,
which is arguably a pretty good idea regardless.

The series could probably be organized so that eg. the
patch order makes more sense, but honestly I'm pretty
tired right now and I just don't have it in me. Moreover,
the goal of the series and its implementation are both
straighforward enough that I feel reviewers will have no
problem following along.

My main concern with this series is that, while converting
from VIR_ALLOC() to virDomain*DefNew(), I might simply have
missed some instances. That would not cause any issue right
away, but once we introduce isolation groups it would lead
to suboptimal PCI addresses being allocated at best, and to
perfectly legitimate hotplug requests being denied at worst.

The fact that the test suite[2] passes gives me a lot of
confidence that I haven't overlooked anything, but if
anyone has clever ideas on how to be actually sure rather
than merely confident please do let me know.


[1] https://www.redhat.com/archives/libvir-list/2017-June/msg01018.html
[2] That is, the version of it where the default isolation
    group being initialized correctly actually matters

Andrea Bolognani (10):
  conf: Make virDomainDeviceGetInfo() private
  conf: Rename virDomainHostdevDefAlloc() to virDomainHostdevDefNew()
  conf: Clean up virDomainHostdevDefNew()
  conf: Clean up virDomain{Memory,Shmem}DefParseXML()
  conf: Move some virDomainDeviceInfo functions
  conf: Introduce virDomainDeviceInfoNew()
  conf: Clean up virDomainDeviceInfo functions
  conf, qemu: Use virDomainDeviceInfoNew()
  conf: Provide missing virDomain*DefNew() functions
  conf: Call virDomainDeviceInfoClear() in virDomain*DefNew()

 src/bhyve/bhyve_parse_command.c |   4 +-
 src/conf/device_conf.c          | 146 ++++++++++++
 src/conf/device_conf.h          |   9 +
 src/conf/domain_addr.c          |   6 +-
 src/conf/domain_conf.c          | 486 ++++++++++++++++++++++++----------------
 src/conf/domain_conf.h          |  25 ++-
 src/libvirt_private.syms        |  19 +-
 src/lxc/lxc_native.c            |   2 +-
 src/openvz/openvz_conf.c        |   2 +-
 src/qemu/qemu_command.c         |  12 +-
 src/qemu/qemu_domain.c          |  11 +-
 src/qemu/qemu_domain_address.c  |  31 +--
 src/qemu/qemu_hotplug.c         |   5 +-
 src/qemu/qemu_parse_command.c   |  31 +--
 src/vbox/vbox_common.c          |  14 +-
 src/vmx/vmx.c                   |   2 +-
 src/vz/vz_sdk.c                 |   6 +-
 src/xen/xen_driver.c            |   2 +-
 src/xenapi/xenapi_driver.c      |   2 +-
 src/xenconfig/xen_common.c      |   4 +-
 src/xenconfig/xen_sxpr.c        |  10 +-
 src/xenconfig/xen_xl.c          |   4 +-
 src/xenconfig/xen_xm.c          |   2 +-
 tests/virhostdevtest.c          |   2 +-
 24 files changed, 561 insertions(+), 276 deletions(-)

-- 
2.7.5


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