* [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
@ 2017-07-13 14:43 Daniel P. Berrange
2017-07-14 2:29 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrange @ 2017-07-13 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Fam Zheng, Philippe Mathieu-Daudé,
Daniel P. Berrange
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 docker-test-build@fedora NETWORK=1
while to enable a specific network backend, eg join the network
associated with the container 'wibble':
make docker-test-build@fedora NETWORK=container:wibble
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
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) \
-e TARGET_LIST=$(TARGET_LIST) \
-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
-e V=$V -e J=$J -e DEBUG=$(DEBUG) \
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
2017-07-13 14:43 [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 Daniel P. Berrange
@ 2017-07-14 2:29 ` Fam Zheng
2017-07-14 11:41 ` Daniel P. Berrange
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2017-07-14 2:29 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé
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 docker-test-build@fedora NETWORK=1
>
> while to enable a specific network backend, eg join the network
> associated with the container 'wibble':
>
> make docker-test-build@fedora NETWORK=container:wibble
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
2017-07-14 2:29 ` Fam Zheng
@ 2017-07-14 11:41 ` Daniel P. Berrange
2017-07-14 11:50 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrange @ 2017-07-14 11:41 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé
On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote:
> 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 docker-test-build@fedora NETWORK=1
> >
> > while to enable a specific network backend, eg join the network
> > associated with the container 'wibble':
> >
> > make docker-test-build@fedora NETWORK=container:wibble
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >
> > 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) \
This looks inverted to me - NETWORK=1, will generate --net=1 and
NETWORK=foo generates empty string.
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
2017-07-14 11:41 ` Daniel P. Berrange
@ 2017-07-14 11:50 ` Fam Zheng
2017-07-14 11:52 ` Daniel P. Berrange
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2017-07-14 11:50 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé
On Fri, 07/14 12:41, Daniel P. Berrange wrote:
> On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote:
> > 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 docker-test-build@fedora NETWORK=1
> > >
> > > while to enable a specific network backend, eg join the network
> > > associated with the container 'wibble':
> > >
> > > make docker-test-build@fedora NETWORK=container:wibble
> > >
> > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > > ---
> > >
> > > 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) \
>
> This looks inverted to me - NETWORK=1, will generate --net=1 and
> NETWORK=foo generates empty string.
You are right, I was confused (subst succeed is FALSE branch of if). I think
s/,,/,/ and it will work?
Fam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
2017-07-14 11:50 ` Fam Zheng
@ 2017-07-14 11:52 ` Daniel P. Berrange
2017-07-14 12:12 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrange @ 2017-07-14 11:52 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé
On Fri, Jul 14, 2017 at 07:50:03PM +0800, Fam Zheng wrote:
> On Fri, 07/14 12:41, Daniel P. Berrange wrote:
> > On Fri, Jul 14, 2017 at 10:29:30AM +0800, Fam Zheng wrote:
> > > 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 docker-test-build@fedora NETWORK=1
> > > >
> > > > while to enable a specific network backend, eg join the network
> > > > associated with the container 'wibble':
> > > >
> > > > make docker-test-build@fedora NETWORK=container:wibble
> > > >
> > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > > > ---
> > > >
> > > > 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) \
> >
> > This looks inverted to me - NETWORK=1, will generate --net=1 and
> > NETWORK=foo generates empty string.
>
> You are right, I was confused (subst succeed is FALSE branch of if). I think
> s/,,/,/ and it will work?
Yep
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1
2017-07-14 11:52 ` Daniel P. Berrange
@ 2017-07-14 12:12 ` Fam Zheng
0 siblings, 0 replies; 6+ messages in thread
From: Fam Zheng @ 2017-07-14 12:12 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Alex Bennée, qemu-devel, Philippe Mathieu-Daudé
On Fri, 07/14 12:52, Daniel P. Berrange wrote:
> > > > > @@ -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) \
> > >
> > > This looks inverted to me - NETWORK=1, will generate --net=1 and
> > > NETWORK=foo generates empty string.
> >
> > You are right, I was confused (subst succeed is FALSE branch of if). I think
> > s/,,/,/ and it will work?
>
> Yep
OK, so dropping the second "$(subst ...)" and the comma we have:
$(if $(NETWORK),$(if $(subst $(NETWORK),,1),--net=$(NETWORK)),--net=none) \
With that, queued for 2.10:
https://github.com/famz/qemu/tree/staging
Fam
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-14 12:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-13 14:43 [Qemu-devel] [PATCH v2] docker: Don't enable networking as a side-effect of DEBUG=1 Daniel P. Berrange
2017-07-14 2:29 ` Fam Zheng
2017-07-14 11:41 ` Daniel P. Berrange
2017-07-14 11:50 ` Fam Zheng
2017-07-14 11:52 ` Daniel P. Berrange
2017-07-14 12:12 ` Fam Zheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.