Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1

[Fam Zheng]

On Thu, 07/13 15:43, Daniel P. Berrange wrote:
> When trying to debug problems with tests it is natural to set
> DEBUG=1 when starting the docker environment. Unfortunately
> this has a side-effect of enabling an eth0 network interface
> in the container, which changes the operating environment of
> the test suite. IOW tests with fail may suddenly start
> working again if DEBUG=1 is set, due to changed network setup.
> 
> Add a separate NETWORK variable to allow enablement of
> networking separately from DEBUG=1. This can be used in two
> ways. To enable the default docker network backend
> 
>   make address@hidden NETWORK=1
> 
> while to enable a specific network backend, eg join the network
> associated with the container 'wibble':
> 
>   make address@hidden NETWORK=container:wibble
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> 
> Changed in v2:
> 
>  - Remove reference to eth0
>  - Allow arbitrary docker network backend to be given
> 
>  tests/docker/Makefile.include | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 037cb9e9e7..ec8e8d935d 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -106,6 +106,8 @@ docker:
>       @echo '                         (default is 1)'
>       @echo '    DEBUG=1              Stop and drop to shell in the created 
> container'
>       @echo '                         before running the command.'
> +     @echo '    NETWORK=1            Enable virtual network interface with 
> default backend.'
> +     @echo '    NETWORK=$BACKEND     Enable virtual network interface with 
> $BACKEND.'
>       @echo '    NOUSER               Define to disable adding current user 
> to containers passwd.'
>       @echo '    NOCACHE=1            Ignore cache when build images.'
>       @echo '    EXECUTABLE=<path>    Include executable in image.'
> @@ -132,7 +134,8 @@ docker-run: docker-qemu-src
>               $(SRC_PATH)/tests/docker/docker.py run                  \
>                       $(if $(NOUSER),,-u $(shell id -u)) -t           \
>                       $(if $V,,--rm)                                  \
> -                     $(if $(DEBUG),-i,--net=none)                    \
> +                     $(if $(DEBUG),-i,)                              \
> +                     $(if $(NETWORK),$(if $(subst $(NETWORK),,1)$(subst 
> 1,,$(NETWORK)),--net=$(NETWORK),),--net=none) \

Isn't the first subst enough? We already know $(NETWORK) is non-empty. If it is
"1", $(subst 1,,1) is empty; otherwise, $(subst foo,,1) is non-empty:

                        $(if $(NETWORK),$(if $(subst 
$(NETWORK),,1),,--net=$(NETWORK)),--net=none) \

The second subst is only a necessary condition.

Another way to write it is:

                        $(if $(NETWORK),$(if $(findstring 
$(NETWORK),1),,--net=$(NETWORK)),--net=none) \

Fam

>                       -e TARGET_LIST=$(TARGET_LIST)                   \
>                       -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>                       -e V=$V -e J=$J -e DEBUG=$(DEBUG)               \
> -- 
> 2.13.0
>