[libvirt] [PATCH v2 07/10] add qemuDomainPCIAddrSetCreateFromDomain

Martin Kletzander mkletzan at redhat.com
Mon Jul 25 15:42:16 UTC 2016


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160725/1d611dd2/attachment-0001.sig>


More information about the libvir-list mailing list