[libvirt] [PATCH 0/4] qemu: Fix leaks in handling PCI devices

[Laine Stump]

On 12/04/2012 05:38 AM, Jiri Denemark wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=877095
>
> Patch 1/4 fixes a possible double free on error path and the other patches
> deal with memory and file descriptor leaks in PCI device attachment code.
>
> Jiri Denemark (4):
>   qemu: Don't free PCI device if adding it to activePciHostdevs fails
>   util: Slightly refactor PCI list functions
>   qemu: Fix memory (and FD) leak on PCI device detach
>   qemu: Do not keep PCI device config file open
>
>  src/libvirt_private.syms |  3 +++
>  src/qemu/qemu_hostdev.c  | 22 ++++++++--------
>  src/util/pci.c           | 66 ++++++++++++++++++++++++++++--------------------
>  src/util/pci.h           |  5 ++++
>  4 files changed, 58 insertions(+), 38 deletions(-)
>


I haven't looked at the individual patches, but just a comment of
something to look out for if you're going to be doing any changes to the
API as Dan suggested:

The network driver can have pools of physical netdevs that it allocates
to guests either for use with macvtap or pci passthrough. Currently
these pools of netdevs are self-contained and ignorant of the world
around them, but it would be very nice if libvirt could have a unified
idea of what host devices are in use so that, for example, a device
could be listed in multiple netdev pools, but would then not be blindly
assigned to a guest if it was already in use via a different network's
pool (or via standard hostdev assignment). Likewise, if someone tried to
do a hostdev assignment of a network device that was already in use by a
guest for a macvtap network interface, that would be caught and prevented.

I'm not suggesting that any of that work is done here, but just to keep
that in mind when making any changes.


On top of all this is the idea that we've tossed around about giving the
networking functions a semi-public API and perhaps even their own daemon
so that they can be easily called from an unprivileged libvirtd - if we
really do this by putting the network functions in a separate daemon,
that daemon would need access back to the centralized pci functions
(which maybe argues for keeping the networking functions in libvirtd
itself, but giving them public APIs and teaching unprivileged libvirtd
to call privileged libvirtd when it needs those functions. But now I'm
seriously hijacking this thread, so I'll shut up :-)