1. Patchew
  2. QEMU
  3. View series

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

Daniel P. Berrange posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170712162550.8061-1-berrange@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
tests/docker/Makefile.include | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Daniel P. Berrange 6 years, 9 months ago 
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=1 option to allow enablement of
networking separately from DEBUG=1, since common debugging
tasks probably don't require networking anyway.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/docker/Makefile.include | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 037cb9e..a8c4b82 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -106,6 +106,7 @@ 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 eth0 virtual network interface.'
 	@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 +133,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),,--net=none)			\
 			-e TARGET_LIST=$(TARGET_LIST) 			\
 			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
 			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
-- 
2.9.4
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
Hi Alex, Fam,

I wanted to try this patch but got:

$ make docker-test-quick@centos6 NETWORK=1
   BUILD   centos6
The command '/bin/sh -c yum install -y epel-release' returned a non-zero 
code: 139
Traceback (most recent call last):
   File "./tests/docker/docker.py", line 382, in <module>
     sys.exit(main())
   File "./tests/docker/docker.py", line 379, in main
     return args.cmdobj.run(args, argv)
   File "./tests/docker/docker.py", line 301, in run
     extra_files_cksum=cksum)
   File "./tests/docker/docker.py", line 185, in build_image
     quiet=quiet)
   File "./tests/docker/docker.py", line 123, in _do_check
     return subprocess.check_call(self._command + cmd, **kwargs)
   File "/usr/lib/python2.7/subprocess.py", line 186, in check_call
     raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'build', '-t', 
'qemu:centos6', '-f', '/tmp/docker_buildIrIR2w/tmpxMPjZu.docker', 
'--build-arg=http_proxy=http://172.17.0.1:3142/', 
'/tmp/docker_buildIrIR2w']' returned non-zero exit status 139
tests/docker/Makefile.include:47: recipe for target 
'docker-image-centos6' failed
make: *** [docker-image-centos6] Error 1

looking further:

$ docker run --rm centos:6 cat /etc/redhat-release; echo $?
CentOS release 6.9 (Final)
0

$ docker run --rm centos:6 sh -c "cat /etc/redhat-release"; echo $?
139

uh?

$ docker run --rm centos:7 sh -c "cat /etc/redhat-release"; echo $?
CentOS Linux release 7.3.1611 (Core)
0

now trying old debian release:

$ docker run --rm -it debian:wheezy sh -c "cat /etc/debian_version"; echo $?
7.11
0

$ docker run --rm -it debian:wheezy bash -c "cat /etc/debian_version"; 
echo $?
139

hmmm

$ docker run --rm -it debian:jessie bash -c "cat /etc/debian_version"; 
echo $?
8.7
0

$ docker info
Server Version: 17.05.0-ce
Storage Driver: overlay2
  Backing Filesystem: extfs
Cgroup Driver: cgroupfs
Default Runtime: runc
Init Binary: docker-init
containerd version: 9048e5e50717ea4497b757314bad98ea3763c145
runc version: 9c2d8d184e5da67c95d601382adf14862e4f2228
init version: 949e6fa
Kernel Version: 4.11.0-1-amd64
Operating System: Debian GNU/Linux buster/sid
Architecture: x86_64

$ sudo journalctl -kb -l -o json-pretty
{
         "PRIORITY" : "6",
         "_TRANSPORT" : "kernel",
         "SYSLOG_FACILITY" : "0",
         "SYSLOG_IDENTIFIER" : "kernel",
         "MESSAGE" : "sh[23389] vsyscall attempted with vsyscall=none 
ip:ffffffffff600400 cs:33 sp:7ffcfd21a6c8 ax:ffffffffff600400 
si:7ffcfd21af6f di:0"
}
{
         "_TRANSPORT" : "kernel",
         "SYSLOG_FACILITY" : "0",
         "SYSLOG_IDENTIFIER" : "kernel",
         "MESSAGE" : "sh[23389]: segfault at ffffffffff600400 ip 
ffffffffff600400 sp 00007ffcfd21a6c8 error 15"
}

is it time to upgrade the docker image to centos:7 ?
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
On 07/12/2017 06:46 PM, Philippe Mathieu-Daudé wrote:
> now trying old debian release:
> 
> $ docker run --rm -it debian:wheezy sh -c "cat /etc/debian_version"; 
> echo $?
> 7.11
> 0
> 
> $ docker run --rm -it debian:wheezy bash -c "cat /etc/debian_version"; 
> echo $?
> 139

Indeed using debian:wheezy based dockerfile:

$ make docker-test-quick@debian7
[...]
Step 4/14 : RUN apt-get update
  ---> Running in 305758a09ca4
E: Method http has died unexpectedly!
E: Sub-process http received a segmentation fault.
The command '/bin/sh -c apt-get update' returned a non-zero code: 100

$ dmesg
sh[25336] vsyscall attempted with vsyscall=none ip:ffffffffff600400 
cs:33 sp:7fffa210e208 ax:ffffffffff600400 si:7fffa210ef60 di:0
sh[25336]: segfault at ffffffffff600400 ip ffffffffff600400 sp 
00007fffa210e208 error 15

note, this does test Fam's "docker.py: Improve subprocess exit code 
handling" :P
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
Hi Daniel,

On 07/12/2017 01:25 PM, 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=1 option to allow enablement of
> networking separately from DEBUG=1, since common debugging
> tasks probably don't require networking anyway.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>   tests/docker/Makefile.include | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 037cb9e..a8c4b82 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -106,6 +106,7 @@ 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 eth0 virtual network interface.'

"eth0" is not always true...

This patch could be more generic, maybe documented as:

   NETWORK=host     Use full host network stack (default no network).'

>   	@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 +133,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),,--net=none)			\

and here use directly:  --net=${NETWORK:-none}

so an experimented docker user could even run tests as:

   make docker-test-quick@centos6 NETWORK=container:qemu

(or NETWORK=bridge)

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

Regards,

Phil.
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Daniel P. Berrange 6 years, 9 months ago 
On Wed, Jul 12, 2017 at 06:55:45PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 07/12/2017 01:25 PM, 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=1 option to allow enablement of
> > networking separately from DEBUG=1, since common debugging
> > tasks probably don't require networking anyway.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >   tests/docker/Makefile.include | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> > index 037cb9e..a8c4b82 100644
> > --- a/tests/docker/Makefile.include
> > +++ b/tests/docker/Makefile.include
> > @@ -106,6 +106,7 @@ 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 eth0 virtual network interface.'
> 
> "eth0" is not always true...
> 
> This patch could be more generic, maybe documented as:
> 
>   NETWORK=host     Use full host network stack (default no network).'
> 
> >   	@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 +133,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),,--net=none)			\
> 
> and here use directly:  --net=${NETWORK:-none}
> 
> so an experimented docker user could even run tests as:
> 
>   make docker-test-quick@centos6 NETWORK=container:qemu
> 
> (or NETWORK=bridge)

This is a nice idea, though slightly more complicated. It would be good
to support NETWORK=1 as a short-cut for enabling the default docker
network backend, as well as being able to give an explicit backend for
those who really care about the flexibility.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Fam Zheng 6 years, 9 months ago 
On Wed, 07/12 17:25, 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.

Makes sense.

> 
> Add a separate NETWORK=1 option to allow enablement of
> networking separately from DEBUG=1, since common debugging
> tasks probably don't require networking anyway.

Not uncommon because fiddling with the package manager is often needed when
working on the test coverage. But that doesn't mean it's bad to control network
explicitly.

> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  tests/docker/Makefile.include | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 037cb9e..a8c4b82 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -106,6 +106,7 @@ 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 eth0 virtual network interface.'

As pointed out by Philippe, I'd just document it as "enable network".

>  	@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 +133,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),,--net=none)			\
>  			-e TARGET_LIST=$(TARGET_LIST) 			\
>  			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>  			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
> -- 
> 2.9.4
> 

If others don't come up with objections, I'll apply this.

Thanks,
Fam
Re: [Qemu-devel] [PATCH] Don't enable networking as a side-effect of DEBUG=1
Posted by Alex Bennée 6 years, 9 months ago 
Fam Zheng <famz@redhat.com> writes:

> On Wed, 07/12 17:25, 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.
>
> Makes sense.
>
>>
>> Add a separate NETWORK=1 option to allow enablement of
>> networking separately from DEBUG=1, since common debugging
>> tasks probably don't require networking anyway.
>
> Not uncommon because fiddling with the package manager is often needed when
> working on the test coverage. But that doesn't mean it's bad to control network
> explicitly.
>
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  tests/docker/Makefile.include | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 037cb9e..a8c4b82 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -106,6 +106,7 @@ 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 eth0 virtual network interface.'
>
> As pointed out by Philippe, I'd just document it as "enable network".
>
>>  	@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 +133,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),,--net=none)			\
>>  			-e TARGET_LIST=$(TARGET_LIST) 			\
>>  			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>>  			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
>> --
>> 2.9.4
>>
>
> If others don't come up with objections, I'll apply this.

Acked-by: Alex Bennée <alex.bennee@linaro.org>


--
Alex Bennée

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.