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

Re: [libvirt] [libvirt-php PATCH 0/7] add bindings for NWFilter APIs



On 06/22/2017 09:14 PM, Dawid Zamirski wrote:
> Hello,
> 
> This series adds support for libvirt's virNWFilter* APIs. Since it
> introduces new resource type, I took the opportunity to cleanup the
> driver code a little:
> 
> * in patches 1-3: added macros to take care of the differences in how
>   PHP5 and PHP7 handle resource types.
> * in patch 4: libvirt_doman_get_connect was segfaulting when called
>   multiple times because it was not bumping reference count on the
>   resource it was returning to the calling code.
> * in patch 5: added a macro to take care of the differences in how PHP5
>   and PHP7 initialize arrays.
> * patches 6 and 7: implement the missing binding to NWFilter APIs.
> 
> 
> Dawid Zamirski (7):
>   move macros to header file.
>   add wrappers for PHP resource handling.
>   update code to use resource handling macros
>   fix libvirt_doman_get_connect implementation.
>   add and use VIRT_ARRAY_INIT macro
>   add nwfilter resource type
>   implement NWFilter API bindings.
> 
>  src/libvirt-php.c | 923 +++++++++++++++++++++++++++++++-----------------------
>  src/libvirt-php.h | 150 ++++++++-
>  2 files changed, 672 insertions(+), 401 deletions(-)
> 

ACK.

Just a side note, The first line of commit message (usually referred to
as subject line) should start with capital letter. I'll fix that before
pushing.

But this got me thinking, should we follow libvirt's example and finally
split src/libvirt-php.c into smaller files that would handle just one
object? For example:

libvirt-domain.c
libvirt-nwfilter.c
libvirt-storage.c
libvirt-network.c

and so on.

The other thing that comes to my mind - would you mind updating the
example under examples/ so that we can demonstrate how NWFilters work in
php?

Thanks!

Michal


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