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(-)

-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Jun 23, 2017 at 4:58 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
>
> 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.

If this isn't too difficult to do, I think it'd be a good idea. The
massive src/libvirt-php.c is difficult to parse and diff between
releases. Breaking it up into separate logical files would make diffs
cleaner, and also probably make it easier to centralize the
abstractions between different PHP versions as support for them is
added.


-- 
真実はいつも一つ！/ Always, there's only one truth!

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list