Re: [Qemu-devel] [PATCH 1/4] tests/vm: Support proxy / corporate firewall

[Fam Zheng]

On Mon, 07/02 12:11, Philippe Mathieu-Daudé wrote:
> Hi Fam,
> 
> On 07/02/2018 04:12 AM, Fam Zheng wrote:
> > On Thu, 06/28 12:35, Philippe Mathieu-Daudé wrote:
> >> If ftp_proxy/http_proxy/https_proxy standard environment variables
> >> are available, pass them to the vm images.
> >>
> >> As per 06cc3551714:
> >> This is required when building behind corporate proxy/firewall, but
> >> also help when using local cache server (ie: apt/yum).
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> >> ---
> >>  tests/vm/ubuntu.i386 | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
> >> index fc319e0e6e..be16ceed50 100755
> >> --- a/tests/vm/ubuntu.i386
> >> +++ b/tests/vm/ubuntu.i386
> >> @@ -68,6 +68,10 @@ class UbuntuX86VM(basevm.BaseVM):
> >>          self.boot(img_tmp, extra_args = ["-cdrom", 
> >> self._gen_cloud_init_iso()])
> >>          self.wait_ssh()
> >>          self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
> >> +        for k, v in os.environ.iteritems():
> >> +            kl = k.lower()
> >> +            if kl in ['ftp_proxy', 'http_proxy', 'https_proxy']:
> >> +                self.ssh_root_check("echo 'Acquire::{}::Proxy \"{}\";' >> 
> >> /etc/apt/apt.conf.d/01proxy".format(kl[:-6].upper(), v))
> > 
> > Reasonable, but do we want it for other apps and images? How about setting 
> > these
> > env vars to ssh commands?
> 
> I see 2 different network usages:
> 
> 1/ how the guest connect to the outer world, this goes via the firewall.
> Here the change only affects apt* based commands (via the apt.conf file).
> Do we have other commands requiring network connectivity? If we have,
> then yes, we should add the same env vars in the guest.
> One case I think of is "git submodule init" calling "git clone".

Yes, I think this case is what we are looking at here. But this patch is very
specific: it only affects one command in one VM, albeit we don't have many.
Doing this means we'll need to specially open code tests/vm/fedora,
tests/vm/debian, or any other images we introduce later, to be consistent with
ubuntu.i386. It is a poor way to do this, IMO.

What I mean is, can we do it more generically? Like setting the env var in guest
/etc/profile or change BaseVM._ssh_do() to inject env vars:

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3643117816..94501e7dc7 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -106,7 +106,9 @@ class BaseVM(object):
         if interactive:
             ssh_cmd += ['-t']
         assert not isinstance(cmd, str)
-        ssh_cmd += ["address@hidden" % user] + list(cmd)
+        env = ["%s=%s" % (k, v) for k, v in os.environ.items() if k in \
+                ["ftp_proxy", "http_proxy", "https_proxy"]]
+        ssh_cmd += ["address@hidden" % user] + env + list(cmd)
         logging.debug("ssh_cmd: %s", " ".join(ssh_cmd))
         r = subprocess.call(ssh_cmd)
         if check and r != 0: