Re: [PATCH v2 07/10] add qemuDomainPCIAddrSetCreateFromDomain

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

 



On Sat, Jul 23, 2016 at 03:47:12AM +0200, Tomasz Flendrich wrote:
The address sets (pci, ccw, virtio serial) are currently cached
in qemu private data, but all the information required to recreate
these sets is in the domain definition. Therefore I am removing
the redundant data and adding a way to recalculate these sets.

Add a function that calculates the pci address set from
the domain definition.
The part of pci address assignment that is not a dryRun is
separated from qemuDomainAssignPCIAddresses into a new function:
qemuDomainPCIAddrSetCreateFromDomain. The first time this function
is run, it can allocate some new addresses. After all the pci
addresses are assigned, on subsequent runs to this function, it just
recreates the pci address set.
---
src/qemu/qemu_domain_address.c | 48 +++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_domain_address.h |  5 +++++
2 files changed, 45 insertions(+), 8 deletions(-)


The patch, technically, is not flawed.  However the naming got me
confused a lot.  That's because even though the function is named
AddrSetCreate, it is actually the one that calls
qemuDomainAssignDevicePCISlots().  I think we should split the calling
function a bit more and have part of it named '...Prepare...' and then
the rest.  Also, even though it is still now totally broken, I'm somehow
afraid we'll be missing some of the preparation stuff at some point.
Although there is no possibility of such case currently.  I'll continue
with the review tomorrow, all previous patches are fine for now.

Have a nice day,
Martin

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]