Re: [PATCH] KVM test: Silence screendump thread by default

[Michael Goldish <mgoldish@redhat.com>]

On 04/16/2010 09:12 PM, Lucas Meneghel Rodrigues wrote:
> The VM screendump thread recently introduced generates
> a lot of output on debug logs. Such output is not needed
> most of the time (we are interested to see if a screenshot
> production attempt failed though) and distracts the user
> from other more important info.
> 
> So let's add an additional parameter on send_monitor_cmd
> that specifies if we actually want the monitor command logged,
> defaulting it to True so the rest of the callers won't have
> any change on their behavior. The screendump thread will
> call send_monitor_cmd with verbose=False unless in the
> configuration file one sets screendump_verbose=yes (defaults
> to no on the sample confifg file).
> 
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/kvm_preprocessing.py  |   10 +++++++++-
>  client/tests/kvm/kvm_vm.py             |    7 +++++--
>  client/tests/kvm/tests_base.cfg.sample |    1 +
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kvm/kvm_preprocessing.py
> index 50db65c..4b9290c 100644
> --- a/client/tests/kvm/kvm_preprocessing.py
> +++ b/client/tests/kvm/kvm_preprocessing.py
> @@ -401,6 +401,10 @@ def _take_screendumps(test, params, env):
>                                   kvm_utils.generate_random_string(6))
>      delay = float(params.get("screendump_delay", 5))
>      quality = int(params.get("screendump_quality", 30))
> +    if params.get("screendump_verbose") == 'yes':
> +        screendump_verbose = True
> +    else:
> +        screendump_verbose = False

Why not:

screendump_verbose = params.get("screendump_verbose") == "yes"

>      cache = {}
>  
> @@ -408,7 +412,11 @@ def _take_screendumps(test, params, env):
>          for vm in kvm_utils.env_get_all_vms(env):
>              if vm.is_dead():
>                  continue
> -            vm.send_monitor_cmd("screendump %s" % temp_filename)
> +            if screendump_verbose:
> +                vm.send_monitor_cmd("screendump %s" % temp_filename)
> +            else:
> +                vm.send_monitor_cmd("screendump %s" % temp_filename,
> +                                    verbose=False)

Also, why not:

vm.send_monitor_cmd("screendump %s" % temp_filename,
verbose=screendump_verbose)

>              if not os.path.exists(temp_filename):
>                  logging.warn("VM '%s' failed to produce a screendump", vm.name)
>                  continue
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 047505a..244355e 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -498,7 +498,7 @@ class VM:
>              lockfile.close()
>  
>  
> -    def send_monitor_cmd(self, command, block=True, timeout=20.0):
> +    def send_monitor_cmd(self, command, block=True, timeout=20.0, verbose=True):
>          """
>          Send command to the QEMU monitor.
>  
> @@ -541,8 +541,11 @@ class VM:
>                      time.sleep(0.01)
>              return (False, o)
>  
> +        # In certain conditions printing this debug output might be too much
> +        # Just print it if verbose is enabled (True by default)
> +        if verbose:
> +            logging.debug("Sending monitor command: %s" % command)
>          # Connect to monitor
> -        logging.debug("Sending monitor command: %s" % command)
>          try:
>              s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
>              s.setblocking(False)
> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
> index 4baa1dc..e74c7cb 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -23,6 +23,7 @@ keep_screendumps_on_error = yes
>  screendump_delay = 5
>  screendump_quality = 30
>  screendump_temp_dir = /dev/shm
> +screendump_verbose = no
>  
>  # Some default VM params
>  qemu_binary = qemu