Re: [libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain
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 >> --- >> 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); >> >> -- libvir-list mailing list l
Re: [libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain
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 > --- > 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). 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 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). > 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); > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain
from src/qemu/qemu_domain_address.[ch] to src/conf/domain_addr.[ch] Signed-off-by: Anya Harter --- 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(-) 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); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list