On 11/19/2013 10:43 AM, Cédric Bosdonnat wrote:
> These changes are all about bringing events for network object like the
> ones existing for domains. This feature is needed for virt-manager to
> refresh its UI when networks are started/destroyed/defined/undefined.
> 
> The first commit is a refactoring of the domain events internal functions
> to extract the reusable parts of them. The second commit adds network
> events based on those now-common event functions. The network events
> are implemented in the test and bridge network drivers ATM.
> 
> Cédric Bosdonnat (2):
>   virDomainEvent refactoring to separate generic things from domain ones
>   Added network events API and unit tests
> 

Cool! This will certainly be useful for virt-manager (as you know since you
submitted a patch there as well :) ).

However to ease review it will help if you split these patches up into many
smaller units. I'd recommend 2 series:

the first will do the event refactor. try to put virObjectEvent find replace
into its own patch with nothing else: that's a very large piece of the change,
but if it's in a standalone patch it will be trivial to review for
correctness. I'd also add tests/objecteventtest.c in this step as well, but
add tests for the pre-existing domain events: that will give devs more
confidence that your refactoring doesn't introduce any regressions, and
hopefully it should be easy to adapt your network test cases to work with
domain events as well.

next series is the new APIs. take a look at docs/api_extension/*.patch for a
rough guideline of how to split up this series. It doesn't directly apply here
but it should give you some ideas. Add the network objecteventtest.c test
cases at the end of this series.

To be clear, you don't need to wait for series 1 to be reviewed committed
before submitting series 2 as well.

Thanks,
Cole

>  .gitignore                            |    2 +
>  daemon/libvirtd.h                     |    1 +
>  daemon/remote.c                       |  175 +++-
>  include/libvirt/libvirt.h.in          |   85 ++
>  python/generator.py                   |    2 +
>  python/libvirt-override-virConnect.py |   34 +
>  python/libvirt-override.c             |  150 ++++
>  src/Makefile.am                       |    6 +
>  src/conf/domain_event.c               | 1431 
> ++++++---------------------------
>  src/conf/domain_event.h               |  134 ++-
>  src/conf/object_event.c               | 1008 +++++++++++++++++++++++
>  src/conf/object_event.h               |  114 +++
>  src/conf/object_event_private.h       |  167 ++++
>  src/driver.h                          |   14 +
>  src/libvirt.c                         |  133 +++
>  src/libvirt_private.syms              |   18 +-
>  src/libvirt_public.syms               |    6 +
>  src/libxl/libxl_conf.h                |    2 +-
>  src/libxl/libxl_driver.c              |   28 +-
>  src/lxc/lxc_conf.h                    |    2 +-
>  src/lxc/lxc_driver.c                  |   38 +-
>  src/lxc/lxc_process.c                 |   12 +-
>  src/network/bridge_driver.c           |   91 ++-
>  src/network/bridge_driver_platform.h  |    3 +
>  src/parallels/parallels_utils.h       |    2 +-
>  src/qemu/qemu_conf.h                  |    2 +-
>  src/qemu/qemu_domain.c                |    6 +-
>  src/qemu/qemu_domain.h                |    2 +-
>  src/qemu/qemu_driver.c                |   50 +-
>  src/qemu/qemu_hotplug.c               |   10 +-
>  src/qemu/qemu_migration.c             |   12 +-
>  src/qemu/qemu_process.c               |   48 +-
>  src/remote/remote_driver.c            |  173 +++-
>  src/remote/remote_protocol.x          |   46 +-
>  src/test/test_driver.c                |  166 ++--
>  src/uml/uml_conf.h                    |    2 +-
>  src/uml/uml_driver.c                  |   26 +-
>  src/vbox/vbox_tmpl.c                  |   20 +-
>  src/xen/xen_driver.c                  |   10 +-
>  src/xen/xen_driver.h                  |    4 +-
>  src/xen/xen_inotify.c                 |    8 +-
>  src/xen/xs_internal.c                 |    4 +-
>  tests/Makefile.am                     |    7 +
>  tests/objecteventtest.c               |  228 ++++++
>  tests/qemuhotplugtest.c               |    2 +-
>  45 files changed, 2974 insertions(+), 1510 deletions(-)
>  create mode 100644 src/conf/object_event.c
>  create mode 100644 src/conf/object_event.h
>  create mode 100644 src/conf/object_event_private.h
>  create mode 100644 tests/objecteventtest.c
> 

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

Reply via email to