qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Wainer dos Santos Moschetta <wainersm@redhat.com>
To: Cleber Rosa <crosa@redhat.com>, qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Thomas Huth" <thuth@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	"Max Reitz" <mreitz@redhat.com>, "John Snow" <jsnow@redhat.com>,
	"Willian Rampazzo" <wrampazz@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Beraldo Leal" <bleal@redhat.com>
Subject: Re: [PATCH 5/6] Acceptance Tests: distinguish between temp and logs dir
Date: Mon, 15 Feb 2021 16:30:49 -0300	[thread overview]
Message-ID: <06d1e7a1-70ca-225f-e8ec-30f295cee5df@redhat.com> (raw)
In-Reply-To: <20210211220146.2525771-6-crosa@redhat.com>

Hi,

On 2/11/21 7:01 PM, Cleber Rosa wrote:
> Logs can be very important to debug issues, and currently QEMUMachine
> instances will remove logs that are created under the temporary
> directories.
>
> With this change, the stdout and stderr generated by the QEMU process
> started by QEMUMachine will always be kept along the test results
> directory.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   python/qemu/machine.py                    | 16 ++++++++++++++--
>   tests/acceptance/avocado_qemu/__init__.py |  3 ++-
>   2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index b379fcbe72..1f4119e2b4 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -89,7 +89,8 @@ class QEMUMachine:
>                    socket_scm_helper: Optional[str] = None,
>                    sock_dir: Optional[str] = None,
>                    drain_console: bool = False,
> -                 console_log: Optional[str] = None):
> +                 console_log: Optional[str] = None,
> +                 log_dir: Optional[str] = None):
>           '''
>           Initialize a QEMUMachine
>   
> @@ -103,6 +104,7 @@ class QEMUMachine:
>           @param sock_dir: where to create socket (defaults to base_temp_dir)
>           @param drain_console: (optional) True to drain console socket to buffer
>           @param console_log: (optional) path to console log file
> +        @param log_dir: where to create and keep log files

"(optional) where to create and keep (...)".

You could also say it defaults to the temp dir, thus logs aren't kept 
guaranteed.

Otherwise,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>           @note: Qemu process is not started until launch() is used.
>           '''
>           # Direct user configuration
> @@ -114,6 +116,7 @@ class QEMUMachine:
>           self._name = name or "qemu-%d" % os.getpid()
>           self._base_temp_dir = base_temp_dir
>           self._sock_dir = sock_dir or self._base_temp_dir
> +        self._log_dir = log_dir
>           self._socket_scm_helper = socket_scm_helper
>   
>           if monitor_address is not None:
> @@ -303,7 +306,7 @@ class QEMUMachine:
>           return args
>   
>       def _pre_launch(self) -> None:
> -        self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
> +        self._qemu_log_path = os.path.join(self.log_dir, self._name + ".log")
>           self._qemu_log_file = open(self._qemu_log_path, 'wb')
>   
>           if self._console_set:
> @@ -752,3 +755,12 @@ class QEMUMachine:
>               self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
>                                                 dir=self._base_temp_dir)
>           return self._temp_dir
> +
> +    @property
> +    def log_dir(self) -> str:
> +        """
> +        Returns a directory to be used for writing logs
> +        """
> +        if self._log_dir is None:
> +            return self.temp_dir
> +        return self._log_dir
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 94b78fd7c8..ac9be1eb66 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -173,9 +173,10 @@ class Test(avocado.Test):
>       def _new_vm(self, name, *args):
>           self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
>           vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir,
> -                         sock_dir=self._sd.name)
> +                         sock_dir=self._sd.name, log_dir=self.logdir)
>           self.log.debug('QEMUMachine "%s" created', name)
>           self.log.debug('QEMUMachine "%s" temp_dir: %s', name, vm.temp_dir)
> +        self.log.debug('QEMUMachine "%s" log_dir: %s', name, vm.log_dir)
>           if args:
>               vm.add_args(*args)
>           return vm



  reply	other threads:[~2021-02-15 19:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 22:01 [PATCH 0/6] Python / Acceptance Tests: improve logging Cleber Rosa
2021-02-11 22:01 ` [PATCH 1/6] Python: close the log file kept by QEMUMachine before reading it Cleber Rosa
2021-02-15 18:30   ` Wainer dos Santos Moschetta
2021-02-16  2:34     ` Cleber Rosa
2021-02-17 19:53       ` Wainer dos Santos Moschetta
2021-02-15 22:04   ` John Snow
2021-02-15 22:19     ` Eric Blake
2021-02-16  2:35     ` Cleber Rosa
2021-02-11 22:01 ` [PATCH 2/6] Python: expose QEMUMachine's temporary directory Cleber Rosa
2021-02-11 23:35   ` Philippe Mathieu-Daudé
2021-02-12  0:11     ` Cleber Rosa
2021-02-15 22:31     ` John Snow
2021-02-15 18:50   ` Wainer dos Santos Moschetta
2021-02-15 22:27     ` John Snow
2021-02-17 19:58       ` Wainer dos Santos Moschetta
2021-02-15 22:25   ` John Snow
2021-02-11 22:01 ` [PATCH 3/6] Acceptance Tests: use the job work directory for created VMs Cleber Rosa
2021-02-15 19:04   ` Wainer dos Santos Moschetta
2021-02-15 23:13   ` John Snow
2021-02-11 22:01 ` [PATCH 4/6] Acceptance Tests: log information when creating QEMUMachine Cleber Rosa
2021-02-15 19:15   ` Wainer dos Santos Moschetta
2021-02-11 22:01 ` [PATCH 5/6] Acceptance Tests: distinguish between temp and logs dir Cleber Rosa
2021-02-15 19:30   ` Wainer dos Santos Moschetta [this message]
2021-02-11 22:01 ` [PATCH 6/6] tests/acceptance/virtio-gpu.py: preserve virtio-user-gpu log Cleber Rosa
2021-02-11 23:37   ` Philippe Mathieu-Daudé
2021-02-15 19:31   ` Wainer dos Santos Moschetta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06d1e7a1-70ca-225f-e8ec-30f295cee5df@redhat.com \
    --to=wainersm@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=bleal@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=wrampazz@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).