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

Re: [libvirt] [PATCH 3/7] qemu: eliminate almost-duplicate code in qemu_command.c



On 08/02/2013 10:55 AM, Laine Stump wrote:
> * The functions qemuDomainPCIAddressReserveAddr and
> qemuDomainPCIAddressReserveSlot were very similar (and should have
> been more similar) and were about to get more code added to them which
> would create even more duplicated code, so this patch gives
> qemuDomainPCIAddressReserveAddr a "reserveEntireSlot" arg, then
> replaces the body of qemuDomainPCIAddressReserveSlot with a call to
> qemuDomainPCIAddressReserveAddr.
> 
> You will notice that addrs->lastaddr was previously set in
> qemuDomainPCIAddressReserveAddr (but *not* set in
> qemuDomainPCIAddressReserveSlot). For consistency and cleanliness of
> code, that bit was removed and put into the one caller of
> qemuDomainPCIAddressReserveAddr (there is a similar place where the
> caller of qemuDomainPCIAddressReserveSlot sets lastaddr). This does
> guarantee identical functionality to pre-patch code, but in practice
> isn't really critical, because lastaddr is just keeping track of where
> to start when looking for a free slot - if it isn't updated, we will
> just start looking on a slot that's already occupied, then skip up to
> one that isn't.
> 
> * qemuCollectPCIAddress was essentially doing the same thing as
> qemuDomainPCIAddressReserveAddr, but with some extra special case
> checking at the beginning. The duplicate code has been replaced with
> a call to qemuDomainPCIAddressReserveAddr. This required adding a
> "fromConfig" boolean, which is only used to change the log error
> code from VIR_ERR_INTERNAL_ERROR (when the address was
> auto-generated by libvirt) to VIR_ERR_XML_ERROR (when the address is
> coming from the config); without this differentiation, it would be
> difficult to tell if an error was caused by something wrong in
> libvirt's auto-allocate code or just bad config.
> 
> * the bit of code in qemuDomainPCIAddressValidate that checks the
> connect type flags is going to be used in a couple more places where
> we don't need to also check the slot limits (because we're generating
> the slot number ourselves), so that has been pulled out into a
> separate qemuDomainPCIAddressFlagsCompatible function.
> ---
>  src/qemu/qemu_command.c | 232 ++++++++++++++++++++++++------------------------
>  src/qemu/qemu_command.h |   4 +-
>  2 files changed, 119 insertions(+), 117 deletions(-)
> 

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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