On 07/02/2018 04:48 PM, John Ferlan wrote:

On 06/28/2018 04:23 PM, Anya Harter wrote:

...

from src/qemu/qemu_domain_address.[ch] to src/conf/domain_addr.[ch]

Signed-off-by: Anya Harter <aharter@redhat.com> --- src/conf/domain_addr.c | 24 ++++++++++++++++++++++++ src/conf/domain_addr.h | 4 ++++ src/qemu/qemu_domain_address.c | 22 ---------------------- src/qemu/qemu_domain_address.h | 4 ---- 4 files changed, 28 insertions(+), 26 deletions(-)

While I can appreciate the 2 patch approach for ease of review, patch 1 and patch 2 should be combined. Once you move a function into src/conf/* - give it the vir* prefix.

Since you now have a function inside src/conf called from a driver, you must add the function to src/libvirt_private.syms (otherwise your build fails - at least mine did).

Woops! I missed that step.

Additionally, once patches are combined virDomainCCWAddressSetCreate, virDomainCCWAddressValidate, and virDomainCCWAddressAllocate (moved in this patch) all become local/static to domain_addr and thus can be removed from domain_addr.h and libvirt_private.syms

I'm going to do this as part of a separate patch series which makes several functions static which are unused elsewhere.

John

BTW: The ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) from the .h file that would then be "lost" could be added after the "static int" in the .c file, although that's not a requirement. We're finding those are really only useful if someone pass NULL as an argument and less useful if an argument itself is NULL (which isn't the case here).

I will keep this in mind for my other patch series. Thanks, Anya

...

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 39f22b82eb..6c39608e01 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1209,6 +1209,30 @@ virDomainCCWAddressSetCreate(void) }

+virDomainCCWAddressSetPtr +qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) +{ + virDomainCCWAddressSetPtr addrs = NULL; + + if (!(addrs = virDomainCCWAddressSetCreate())) + goto error; + + if (virDomainDeviceInfoIterate(def, virDomainCCWAddressValidate, + addrs) < 0) + goto error; + + if (virDomainDeviceInfoIterate(def, virDomainCCWAddressAllocate, + addrs) < 0) + goto error; + + return addrs; + + error: + virDomainCCWAddressSetFree(addrs); + return NULL; +} + + #define VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS 31

diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 3236b7d6de..039efcaaf1 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -205,6 +205,10 @@ int virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); virDomainCCWAddressSetPtr virDomainCCWAddressSetCreate(void);

+virDomainCCWAddressSetPtr +qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) + ATTRIBUTE_NONNULL(1); + struct _virDomainVirtioSerialController { unsigned int idx; virBitmapPtr ports; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e9f460d77a..b1aef64d89 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -376,28 +376,6 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, } }

-virDomainCCWAddressSetPtr -qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) -{ - virDomainCCWAddressSetPtr addrs = NULL; - - if (!(addrs = virDomainCCWAddressSetCreate())) - goto error; - - if (virDomainDeviceInfoIterate(def, virDomainCCWAddressValidate, - addrs) < 0) - goto error; - - if (virDomainDeviceInfoIterate(def, virDomainCCWAddressAllocate, - addrs) < 0) - goto error; - - return addrs; - - error: - virDomainCCWAddressSetFree(addrs); - return NULL; -}

/* * Three steps populating CCW devnos diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index 83f8e81cad..89d7a5ac3e 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -59,10 +59,6 @@ void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info, const char *devstr);

-virDomainCCWAddressSetPtr -qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) - ATTRIBUTE_NONNULL(1); - int qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def, virDomainMemoryDefPtr mem);