[PATCH v7 03/12] tests/vm: pass args through to BaseVM's __init__

Robert Foley posted 12 patches 5 years, 5 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Cleber Rosa <crosa@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
[PATCH v7 03/12] tests/vm: pass args through to BaseVM's __init__
Posted by Robert Foley 5 years, 5 months ago
Signed-off-by: Robert Foley <robert.foley@linaro.org>
---
 tests/vm/basevm.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index a2d4054d72..fbefda0595 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -61,9 +61,9 @@ class BaseVM(object):
     # 4 is arbitrary, but greater than 2,
     # since we found we need to wait more than twice as long.
     tcg_ssh_timeout_multiplier = 4
-    def __init__(self, debug=False, vcpus=None, genisoimage=None):
+    def __init__(self, args):
         self._guest = None
-        self._genisoimage = genisoimage
+        self._genisoimage = args.genisoimage
         self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-",
                                                          suffix=".tmp",
                                                          dir="."))
@@ -76,7 +76,7 @@ class BaseVM(object):
         self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
         open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
 
-        self.debug = debug
+        self.debug = args.debug
         self._stderr = sys.stderr
         self._devnull = open(os.devnull, "w")
         if self.debug:
@@ -90,8 +90,8 @@ class BaseVM(object):
                        (",ipv6=no" if not self.ipv6 else ""),
             "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20"]
-        if vcpus and vcpus > 1:
-            self._args += ["-smp", "%d" % vcpus]
+        if args.jobs and args.jobs > 1:
+            self._args += ["-smp", "%d" % args.jobs]
         if kvm_available(self.arch):
             self._args += ["-enable-kvm"]
         else:
@@ -438,8 +438,7 @@ def main(vmcls):
             return 1
         logging.basicConfig(level=(logging.DEBUG if args.debug
                                    else logging.WARN))
-        vm = vmcls(debug=args.debug, vcpus=args.jobs,
-                   genisoimage=args.genisoimage)
+        vm = vmcls(args)
         if args.build_image:
             if os.path.exists(args.image) and not args.force:
                 sys.stderr.writelines(["Image file exists: %s\n" % args.image,
-- 
2.17.1


Re: [PATCH v7 03/12] tests/vm: pass args through to BaseVM's __init__
Posted by Alex Bennée 5 years, 5 months ago
Robert Foley <robert.foley@linaro.org> writes:

A brief rationale wouldn't go amiss in the commit message. e.g. "We will
shortly need to pass more parameters to the class so lets just pass args
rather than growing the parameter list."

Otherwise:

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


> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> ---
>  tests/vm/basevm.py | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index a2d4054d72..fbefda0595 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -61,9 +61,9 @@ class BaseVM(object):
>      # 4 is arbitrary, but greater than 2,
>      # since we found we need to wait more than twice as long.
>      tcg_ssh_timeout_multiplier = 4
> -    def __init__(self, debug=False, vcpus=None, genisoimage=None):
> +    def __init__(self, args):
>          self._guest = None
> -        self._genisoimage = genisoimage
> +        self._genisoimage = args.genisoimage
>          self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-",
>                                                           suffix=".tmp",
>                                                           dir="."))
> @@ -76,7 +76,7 @@ class BaseVM(object):
>          self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
>          open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
>  
> -        self.debug = debug
> +        self.debug = args.debug
>          self._stderr = sys.stderr
>          self._devnull = open(os.devnull, "w")
>          if self.debug:
> @@ -90,8 +90,8 @@ class BaseVM(object):
>                         (",ipv6=no" if not self.ipv6 else ""),
>              "-device", "virtio-net-pci,netdev=vnet",
>              "-vnc", "127.0.0.1:0,to=20"]
> -        if vcpus and vcpus > 1:
> -            self._args += ["-smp", "%d" % vcpus]
> +        if args.jobs and args.jobs > 1:
> +            self._args += ["-smp", "%d" % args.jobs]
>          if kvm_available(self.arch):
>              self._args += ["-enable-kvm"]
>          else:
> @@ -438,8 +438,7 @@ def main(vmcls):
>              return 1
>          logging.basicConfig(level=(logging.DEBUG if args.debug
>                                     else logging.WARN))
> -        vm = vmcls(debug=args.debug, vcpus=args.jobs,
> -                   genisoimage=args.genisoimage)
> +        vm = vmcls(args)
>          if args.build_image:
>              if os.path.exists(args.image) and not args.force:
>                  sys.stderr.writelines(["Image file exists: %s\n" % args.image,


-- 
Alex Bennée

Re: [PATCH v7 03/12] tests/vm: pass args through to BaseVM's __init__
Posted by Robert Foley 5 years, 5 months ago
On Wed, 20 May 2020 at 17:49, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Robert Foley <robert.foley@linaro.org> writes:
>
> A brief rationale wouldn't go amiss in the commit message. e.g. "We will
> shortly need to pass more parameters to the class so lets just pass args
> rather than growing the parameter list."

Good point. I will add this to the commit message.

Thanks & Regards,
-Rob

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


>
> > Signed-off-by: Robert Foley <robert.foley@linaro.org>
> > ---
> >  tests/vm/basevm.py | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> > index a2d4054d72..fbefda0595 100644
> > --- a/tests/vm/basevm.py
> > +++ b/tests/vm/basevm.py
> > @@ -61,9 +61,9 @@ class BaseVM(object):
> >      # 4 is arbitrary, but greater than 2,
> >      # since we found we need to wait more than twice as long.
> >      tcg_ssh_timeout_multiplier = 4
> > -    def __init__(self, debug=False, vcpus=None, genisoimage=None):
> > +    def __init__(self, args):
> >          self._guest = None
> > -        self._genisoimage = genisoimage
> > +        self._genisoimage = args.genisoimage
> >          self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-",
> >                                                           suffix=".tmp",
> >                                                           dir="."))
> > @@ -76,7 +76,7 @@ class BaseVM(object):
> >          self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
> >          open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
> >
> > -        self.debug = debug
> > +        self.debug = args.debug
> >          self._stderr = sys.stderr
> >          self._devnull = open(os.devnull, "w")
> >          if self.debug:
> > @@ -90,8 +90,8 @@ class BaseVM(object):
> >                         (",ipv6=no" if not self.ipv6 else ""),
> >              "-device", "virtio-net-pci,netdev=vnet",
> >              "-vnc", "127.0.0.1:0,to=20"]
> > -        if vcpus and vcpus > 1:
> > -            self._args += ["-smp", "%d" % vcpus]
> > +        if args.jobs and args.jobs > 1:
> > +            self._args += ["-smp", "%d" % args.jobs]
> >          if kvm_available(self.arch):
> >              self._args += ["-enable-kvm"]
> >          else:
> > @@ -438,8 +438,7 @@ def main(vmcls):
> >              return 1
> >          logging.basicConfig(level=(logging.DEBUG if args.debug
> >                                     else logging.WARN))
> > -        vm = vmcls(debug=args.debug, vcpus=args.jobs,
> > -                   genisoimage=args.genisoimage)
> > +        vm = vmcls(args)
> >          if args.build_image:
> >              if os.path.exists(args.image) and not args.force:
> >                  sys.stderr.writelines(["Image file exists: %s\n" % args.image,
>
>
> --
> Alex Bennée