[libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain

2018-06-28 Thread Anya Harter 
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

Re: [libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain

2018-07-02 Thread John Ferlan 



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

Re: [libvirt] [PATCH 1/2] qemu: move qemuDomainCCWAddrSetCreateFromDomain

2018-07-03 Thread Anya Harter 



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