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

Re: [libvirt] [PATCH v2] bridge driver: extract platform specifics



On 07/24/2013 06:22 AM, Roman Bogorodskiy wrote:
> * Move platform specific things (e.g. firewalling and route
>   collision checks) into bridge_driver_platform
> * Create two platform specific implementations:
>     - bridge_driver_linux: Linux implementation using iptables,
>       it's actually the code moved from bridge_driver.c
>     - bridge_driver_nop: dumb implementation that does nothing
> ---
>  src/network/bridge_driver_platform.h |   79 ++++
>  src/network/bridge_driver.c          |  708 +--------------------------------
>  src/network/bridge_driver_linux.c    |  709 ++++++++++++++++++++++++++++++++++
>  src/network/bridge_driver_nop.c      |   78 ++++
>  src/network/bridge_driver_platform.c |   32 ++
>  po/POTFILES.in                       |    1 +
>  src/Makefile.am                      |    5 +-
>  7 files changed, 903 insertions(+), 709 deletions(-)
>  create mode 100644 src/network/bridge_driver_platform.h
>  create mode 100644 src/network/bridge_driver_linux.c
>  create mode 100644 src/network/bridge_driver_nop.c
>  create mode 100644 src/network/bridge_driver_platform.c

You need to ensure that the new files get shipped in the tarball, or
else 'make distcheck' will break.

> +++ b/src/Makefile.am
> @@ -725,8 +725,9 @@ PARALLELS_DRIVER_SOURCES =					\
>  		parallels/parallels_storage.c		\
>  		parallels/parallels_network.c
>  
> -NETWORK_DRIVER_SOURCES =					\
> -		network/bridge_driver.h network/bridge_driver.c
> +NETWORK_DRIVER_SOURCES =								\
> +		network/bridge_driver.h network/bridge_driver.c 			\
> +		network/bridge_driver_platform.h network/bridge_driver_platform.c

Long line.  Also, we've been using $(NULL) at the end of lists in new
modifications, so that we don't have to remember to add a \ if we add
something at the end of a multi-line list (yeah, we don't consistently
use $(NULL) everywhere yet, but anytime a patch touches Makefile.am, I
try to clean up what gets touched).

This only tells automake to ship the wrapper file, but we also need to
ship both the _linux and the _nop file.  The tarball must include
everything, even what wasn't used when built on your setup, so that
someone else still has a complete package for their setup.  But that's
easy enough to fix, so ACK and pushed with this squashed in:

diff --git i/src/Makefile.am w/src/Makefile.am
index af8a25e..ac66ecf 100644
--- i/src/Makefile.am
+++ w/src/Makefile.am
@@ -727,9 +727,12 @@ PARALLELS_DRIVER_SOURCES =					\
 		parallels/parallels_storage.c		\
 		parallels/parallels_network.c

-NETWORK_DRIVER_SOURCES =								\
-		network/bridge_driver.h network/bridge_driver.c 			\
-		network/bridge_driver_platform.h network/bridge_driver_platform.c
+NETWORK_DRIVER_SOURCES =					\
+		network/bridge_driver.h network/bridge_driver.c \
+		network/bridge_driver_platform.h 		\
+		network/bridge_driver_platform.c		\
+		$(NULL)
+EXTRA_DIST += network/bridge_driver_linux.c network/bridge_driver_nop.c

 INTERFACE_DRIVER_SOURCES =



-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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