Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

2021-06-10 Thread Wainer dos Santos Moschetta 

Hi,

On 6/8/21 8:55 PM, Cleber Rosa Junior wrote:



On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta 
mailto:waine...@redhat.com>> wrote:


Hi,

On 6/8/21 11:09 AM, Cleber Rosa wrote:
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
>
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
>
> Signed-off-by: Cleber Rosa mailto:cr...@redhat.com>>
> ---
>   python/qemu/utils/__init__.py |  2 ++
>   python/qemu/utils/accel.py    | 15 ++--
>   python/qemu/utils/feature.py  | 44
+++
>   3 files changed, 48 insertions(+), 13 deletions(-)
>   create mode 100644 python/qemu/utils/feature.py
>
> diff --git a/python/qemu/utils/__init__.py
b/python/qemu/utils/__init__.py
> index 7f1a5138c4..1d0789eaa2 100644
> --- a/python/qemu/utils/__init__.py
> +++ b/python/qemu/utils/__init__.py
> @@ -20,12 +20,14 @@
>
>   # pylint: disable=import-error
>   from .accel import kvm_available, list_accel, tcg_available
> +from .feature import list_feature
>
>
>   __all__ = (
>       'get_info_usernet_hostfwd_port',
>       'kvm_available',
>       'list_accel',
> +    'list_feature',
>       'tcg_available',
>   )
>
> diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
> index 297933df2a..b5bb80c6d3 100644
> --- a/python/qemu/utils/accel.py
> +++ b/python/qemu/utils/accel.py
> @@ -14,13 +14,11 @@
>   # the COPYING file in the top-level directory.
>   #
>
> -import logging
>   import os
> -import subprocess
>   from typing import List, Optional
>
> +from qemu.utils.feature import list_feature
>
> -LOG = logging.getLogger(__name__)
>
>   # Mapping host architecture to any additional architectures it can
>   # support which often includes its 32 bit cousin.
> @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>       @raise Exception: if failed to run `qemu -accel help`
>       @return a list of accelerator names.
>       """
> -    if not qemu_bin:
> -        return []
> -    try:
> -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
> - universal_newlines=True)
> -    except:
> -        LOG.debug("Failed to get the list of accelerators in
%s", qemu_bin)
> -        raise
> -    # Skip the first line which is the header.
> -    return [acc.strip() for acc in out.splitlines()[1:]]
> +    return list_feature(qemu_bin, 'accel')
>
>
>   def kvm_available(target_arch: Optional[str] = None,
> diff --git a/python/qemu/utils/feature.py
b/python/qemu/utils/feature.py
> new file mode 100644
> index 00..b4a5f929ab
> --- /dev/null
> +++ b/python/qemu/utils/feature.py
> @@ -0,0 +1,44 @@
> +"""
> +QEMU feature module:
> +
> +This module provides a utility for discovering the availability of
> +generic features.
> +"""
> +# Copyright (C) 2022 Red Hat Inc.
Cleber, please, tell me how is the future like! :)


I grabbed a sports almanac.  That's all I can say. :)

Now seriously, thanks for spotting the typo.

> +#
> +# Authors:
> +#  Cleber Rosa mailto:cr...@redhat.com>>
> +#
> +# This work is licensed under the terms of the GNU GPL, version
2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import logging
> +import subprocess
> +from typing import List
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +def list_feature(qemu_bin: str, feature: str) -> List[str]:
> +    """
> +    List available options the QEMU binary for a given feature
type.
> +
> +    By calling a "qemu $feature -help" and parsing its output.

I understand we need a mean to easily cancel the test if given
feature
is not present. However, I'm unsure this generic list_feature() is
what
we need.

The `-accel help` returns a simple list of strings (besides the
header,
which is dismissed). Whereas `-machine help` returns what could be
parsed as a tuple of (name, description).

Another

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

2021-06-08 Thread Wainer dos Santos Moschetta 

Hi,

On 6/8/21 11:09 AM, Cleber Rosa wrote:

Which can be used to check for any "feature" that is available as a
QEMU command line option, and that will return its list of available
options.

This is a generalization of the list_accel() utility function, which
is itself re-implemented in terms of the more generic feature.

Signed-off-by: Cleber Rosa 
---
  python/qemu/utils/__init__.py |  2 ++
  python/qemu/utils/accel.py| 15 ++--
  python/qemu/utils/feature.py  | 44 +++
  3 files changed, 48 insertions(+), 13 deletions(-)
  create mode 100644 python/qemu/utils/feature.py

diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
index 7f1a5138c4..1d0789eaa2 100644
--- a/python/qemu/utils/__init__.py
+++ b/python/qemu/utils/__init__.py
@@ -20,12 +20,14 @@
  
  # pylint: disable=import-error

  from .accel import kvm_available, list_accel, tcg_available
+from .feature import list_feature
  
  
  __all__ = (

  'get_info_usernet_hostfwd_port',
  'kvm_available',
  'list_accel',
+'list_feature',
  'tcg_available',
  )
  
diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py

index 297933df2a..b5bb80c6d3 100644
--- a/python/qemu/utils/accel.py
+++ b/python/qemu/utils/accel.py
@@ -14,13 +14,11 @@
  # the COPYING file in the top-level directory.
  #
  
-import logging

  import os
-import subprocess
  from typing import List, Optional
  
+from qemu.utils.feature import list_feature
  
-LOG = logging.getLogger(__name__)
  
  # Mapping host architecture to any additional architectures it can

  # support which often includes its 32 bit cousin.
@@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
  @raise Exception: if failed to run `qemu -accel help`
  @return a list of accelerator names.
  """
-if not qemu_bin:
-return []
-try:
-out = subprocess.check_output([qemu_bin, '-accel', 'help'],
-  universal_newlines=True)
-except:
-LOG.debug("Failed to get the list of accelerators in %s", qemu_bin)
-raise
-# Skip the first line which is the header.
-return [acc.strip() for acc in out.splitlines()[1:]]
+return list_feature(qemu_bin, 'accel')
  
  
  def kvm_available(target_arch: Optional[str] = None,

diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
new file mode 100644
index 00..b4a5f929ab
--- /dev/null
+++ b/python/qemu/utils/feature.py
@@ -0,0 +1,44 @@
+"""
+QEMU feature module:
+
+This module provides a utility for discovering the availability of
+generic features.
+"""
+# Copyright (C) 2022 Red Hat Inc.

Cleber, please, tell me how is the future like! :)

+#
+# Authors:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import logging
+import subprocess
+from typing import List
+
+
+LOG = logging.getLogger(__name__)
+
+
+def list_feature(qemu_bin: str, feature: str) -> List[str]:
+"""
+List available options the QEMU binary for a given feature type.
+
+By calling a "qemu $feature -help" and parsing its output.


I understand we need a mean to easily cancel the test if given feature 
is not present. However, I'm unsure this generic list_feature() is what 
we need.


The `-accel help` returns a simple list of strings (besides the header, 
which is dismissed). Whereas `-machine help` returns what could be 
parsed as a tuple of (name, description).


Another example is `-cpu help` which will print a similar list as 
`-machine`, plus a section with CPUID flags.


If confess I still don't have a better idea, although I feel it will 
require a OO design.


Thanks!

- Wainer


+
+@param qemu_bin (str): path to the QEMU binary.
+@param feature (str): feature name, matching the command line option.
+@raise Exception: if failed to run `qemu -feature help`
+@return a list of available options for the given feature.
+"""
+if not qemu_bin:
+return []
+try:
+out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
+  universal_newlines=True)
+except:
+LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
+raise
+# Skip the first line which is the header.
+return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]

Re: [PATCH 05/10] python/machine: Disable pylint warning for open() in _pre_launch

2021-05-14 Thread Wainer dos Santos Moschetta 

Hi,

On 5/12/21 6:46 PM, John Snow wrote:

Shift the open() call later so that the pylint pragma applies *only* to
that one open() call. Add a note that suggests why this is safe: the
resource is unconditionally cleaned up in _post_shutdown().



You can also put it in a pylint disable/enable block. E.g.:

    # pylint: disable=consider-using-with

    self._qemu_log_file = open(self._qemu_log_path, 'wb')

    # pylint: enable=consider-using-with

However I don't know if this is bad practice. :)

Reviewed-by: Wainer dos Santos Moschetta 



_post_shutdown is called after failed launches (see launch()), and
unconditionally after every call to shutdown(), and therefore also on
__exit__.

Signed-off-by: John Snow 
---
  python/qemu/machine.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c13ff9b32bf..8f86303b48f 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -308,7 +308,6 @@ def _pre_launch(self) -> None:
  self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
dir=self._test_dir)
  self._qemu_log_path = os.path.join(self._temp_dir, self._name + 
".log")
-self._qemu_log_file = open(self._qemu_log_path, 'wb')
  
  if self._console_set:

  self._remove_files.append(self._console_address)
@@ -323,6 +322,11 @@ def _pre_launch(self) -> None:
  nickname=self._name
  )
  
+# NOTE: Make sure any opened resources are *definitely* freed in

+# _post_shutdown()!
+# pylint: disable=consider-using-with
+self._qemu_log_file = open(self._qemu_log_path, 'wb')
+
  def _post_launch(self) -> None:
  if self._qmp_connection:
  self._qmp.accept()

Re: [PATCH 03/10] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-14 Thread Wainer dos Santos Moschetta 

Hi,

On 5/12/21 6:46 PM, John Snow wrote:

use run() instead of Popen() -- to assert to pylint that we are not
forgetting to close a long-running program.

Signed-off-by: John Snow 
---
  python/qemu/machine.py | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 41f51bd27d0..c13ff9b32bf 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,13 +223,16 @@ def send_fd_scm(self, fd: Optional[int] = None,
  assert fd is not None
  fd_param.append(str(fd))
  
-proc = subprocess.Popen(

-fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
-stderr=subprocess.STDOUT, close_fds=False
+proc = subprocess.run(
+fd_param,
+stdin=subprocess.DEVNULL,
+stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT,
+check=True,
+close_fds=False,
  )


Now it might throw a CalledProcessError given that `check=True`. 
Shouldn't it capture the exception and (possible) re-throw as an 
QEMUMachineError?


- Wainer


-output = proc.communicate()[0]
-if output:
-LOG.debug(output)
+if proc.stdout:
+LOG.debug(proc.stdout)
  
  return proc.returncode

Re: [PATCH 06/14] machine: remove 'query-cpus' QMP command

2021-02-25 Thread Wainer dos Santos Moschetta 
27;riscv': 'CpuInfoRISCV' } }
-
-##
-# @CpuInfoX86:
-#
-# Additional information about a virtual i386 or x86_64 CPU
-#
-# @pc: the 64-bit instruction pointer
-#
-# Since: 2.6
-##
-{ 'struct': 'CpuInfoX86', 'data': { 'pc': 'int' } }
-
-##
-# @CpuInfoSPARC:
-#
-# Additional information about a virtual SPARC CPU
-#
-# @pc: the PC component of the instruction pointer
-#
-# @npc: the NPC component of the instruction pointer
-#
-# Since: 2.6
-##
-{ 'struct': 'CpuInfoSPARC', 'data': { 'pc': 'int', 'npc': 'int' } }
-
-##
-# @CpuInfoPPC:
-#
-# Additional information about a virtual PPC CPU
-#
-# @nip: the instruction pointer
-#
-# Since: 2.6
-##
-{ 'struct': 'CpuInfoPPC', 'data': { 'nip': 'int' } }
-
-##
-# @CpuInfoMIPS:
-#
-# Additional information about a virtual MIPS CPU
-#
-# @PC: the instruction pointer
-#
-# Since: 2.6
-##
-{ 'struct': 'CpuInfoMIPS', 'data': { 'PC': 'int' } }
-
-##
-# @CpuInfoTricore:
-#
-# Additional information about a virtual Tricore CPU
-#
-# @PC: the instruction pointer
-#
-# Since: 2.6
-##
-{ 'struct': 'CpuInfoTricore', 'data': { 'PC': 'int' } }
-
-##
-# @CpuInfoRISCV:
-#
-# Additional information about a virtual RISCV CPU
-#
-# @pc: the instruction pointer
-#
-# Since 2.12
-##
-{ 'struct': 'CpuInfoRISCV', 'data': { 'pc': 'int' } }
-
  ##
  # @CpuS390State:
  #
@@ -180,53 +72,6 @@
  ##
  { 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
  
-##

-# @query-cpus:
-#
-# Returns a list of information about each virtual CPU.
-#
-# This command causes vCPU threads to exit to userspace, which causes
-# a small interruption to guest CPU execution. This will have a negative
-# impact on realtime guests and other latency sensitive guest workloads.
-#
-# Features:
-# @deprecated: This command is deprecated, because it interferes with
-#  the guest.  Use 'query-cpus-fast' instead to avoid the vCPU
-#  interruption.
-#
-# Returns: a list of @CpuInfo for each virtual CPU
-#
-# Since: 0.14
-#
-# Example:
-#
-# -> { "execute": "query-cpus" }
-# <- { "return": [
-#  {
-# "CPU":0,
-# "current":true,
-# "halted":false,
-# "qom_path":"/machine/unattached/device[0]",
-# "arch":"x86",
-# "pc":3227107138,
-# "thread_id":3134
-#  },
-#  {
-# "CPU":1,
-# "current":false,
-# "halted":true,
-#         "qom_path":"/machine/unattached/device[2]",
-# "arch":"x86",
-# "pc":7108165,
-# "thread_id":3135
-#  }
-#   ]
-#}
-#
-##
-{ 'command': 'query-cpus', 'returns': ['CpuInfo'],
-  'features': [ 'deprecated' ] }
-
  ##
  # @CpuInfoFast:
  #
@@ -266,9 +111,7 @@
  ##
  # @query-cpus-fast:
  #
-# Returns information about all virtual CPUs. This command does not
-# incur a performance penalty and should be used in production
-# instead of query-cpus.
+# Returns information about all virtual CPUs.
  #
  # Returns: list of @CpuInfoFast
  #
diff --git a/tests/acceptance/pc_cpu_hotplug_props.py 
b/tests/acceptance/pc_cpu_hotplug_props.py
index e49bf33fc5..f48f68fc6b 100644
--- a/tests/acceptance/pc_cpu_hotplug_props.py
+++ b/tests/acceptance/pc_cpu_hotplug_props.py
@@ -32,4 +32,4 @@ def test_no_die_id(self):
  self.vm.add_args('-cpu', 'qemu64')
  self.vm.add_args('-device', 
'qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0')
  self.vm.launch()
-self.assertEquals(len(self.vm.command('query-cpus')), 2)
+self.assertEquals(len(self.vm.command('query-cpus-fast')), 2)
diff --git a/tests/acceptance/x86_cpu_model_versions.py 
b/tests/acceptance/x86_cpu_model_versions.py
index 2b7461bb41..77ed8597a4 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -246,7 +246,7 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
  :avocado: tags=arch:x86_64
  """
  def get_cpu_prop(self, prop):
-cpu_path = self.vm.command('query-cpus')[0].get('qom_path')
+cpu_path = self.vm.command('query-cpus-fast')[0].get('qom-path')
  return self.vm.command('qom-get', path=cpu_path, property=prop)



The chan

Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory

2021-02-17 Thread Wainer dos Santos Moschetta 



On 2/15/21 7:27 PM, John Snow wrote:

On 2/15/21 1:50 PM, Wainer dos Santos Moschetta wrote:


In qtest.QEMUQtestMachine.__init__(), the argument named 'test_dir' 
still make sense, right?


- Wainer


It might upset pylint/mypy to rename parameters in the initializer for 
a parent class. If we rename it in the base class, we should rename it 
in the descendants, too.


(I say "might" because I have not yet worked out under the exact 
conditions that mypy will give you LSP warnings for initializer 
methods. It definitely doesn't always seem to, but I have run afoul of 
it enough times that I try to avoid it as a matter of habit now.)


Thanks for the explanation, John!

So Cleber just got another:

Reviewed-by: Wainer dos Santos Moschetta

Re: [PATCH 1/6] Python: close the log file kept by QEMUMachine before reading it

2021-02-17 Thread Wainer dos Santos Moschetta 



On 2/15/21 11:34 PM, Cleber Rosa wrote:

On Mon, Feb 15, 2021 at 03:30:16PM -0300, Wainer dos Santos Moschetta wrote:

Hi,

On 2/11/21 7:01 PM, Cleber Rosa wrote:

Closing a file that is open for writing, and then reading from it
sounds like a better idea than the opposite, given that the content
will be flushed.

Reference: https://docs.python.org/3/library/io.html#io.IOBase.close
Signed-off-by: Cleber Rosa 
---
   python/qemu/machine.py | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 7a40f4604b..6e44bda337 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -337,12 +337,12 @@ class QEMUMachine:
   self._qmp.close()
   self._qmp_connection = None
-self._load_io_log()
-
   if self._qemu_log_file is not None:
   self._qemu_log_file.close()
   self._qemu_log_file = None
+self._load_io_log()
+


IMO it's a too fragile fix. It needs the `self._qemu_log_file.close()` being
called before `self._load_io_log()` but a future change can make this
condition unmet again. Maybe you could document that in the code. Or change
the `_load_io_log()` to close the log file if it is open (also document it
in code).

- Wainer


I'm glad you see this is needed... and then something else.  I'll investigate
making this more robust as time allows it.

Question is: do you ack/nack this change?


hmm... /me thinking hmmm... okay, good deal. :)

Acked-by: Wainer dos Santos Moschetta 




Cheers,
- Cleber.

Re: [PATCH 6/6] tests/acceptance/virtio-gpu.py: preserve virtio-user-gpu log

2021-02-15 Thread Wainer dos Santos Moschetta 



On 2/11/21 7:01 PM, Cleber Rosa wrote:

At location already prepared for keeping the test's log files.

While at it, log info about its location (in the main test log
file), instead of printing it out.

Reference: 
https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.logdir
Signed-off-by: Cleber Rosa 
---
  tests/acceptance/virtio-gpu.py | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 8d689eb820..ab1a4c1a71 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -119,10 +119,11 @@ class VirtioGPUx86(Test):
  os.set_inheritable(vug_sock.fileno(), True)
  
  self._vug_log_path = os.path.join(

-self.vm.temp_dir, "vhost-user-gpu.log"
+self.logdir, "vhost-user-gpu.log"
  )
  self._vug_log_file = open(self._vug_log_path, "wb")
-print(self._vug_log_path)
+self.log.info('Complete vhost-user-gpu.log file can be '
+  'found at %s', self._vug_log_path)
  
  vugp = subprocess.Popen(

  [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],

Re: [PATCH 5/6] Acceptance Tests: distinguish between temp and logs dir

2021-02-15 Thread Wainer dos Santos Moschetta 

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 
---
  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 


  @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

Re: [PATCH 4/6] Acceptance Tests: log information when creating QEMUMachine

2021-02-15 Thread Wainer dos Santos Moschetta 



On 2/11/21 7:01 PM, Cleber Rosa wrote:

Including its base temporary directory, given that information useful
for debugging can be put there.

Signed-off-by: Cleber Rosa 
---
  tests/acceptance/avocado_qemu/__init__.py | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index b7ab836455..94b78fd7c8 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -170,10 +170,12 @@ class Test(avocado.Test):
  if self.qemu_bin is None:
  self.cancel("No QEMU binary defined or found in the build tree")
  
-def _new_vm(self, *args):

+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)
+self.log.debug('QEMUMachine "%s" created', name)
+self.log.debug('QEMUMachine "%s" temp_dir: %s', name, vm.temp_dir)
  if args:
  vm.add_args(*args)
  return vm
@@ -186,7 +188,7 @@ class Test(avocado.Test):
  if not name:
  name = str(uuid.uuid4())
  if self._vms.get(name) is None:
-self._vms[name] = self._new_vm(*args)
+self._vms[name] = self._new_vm(name, *args)
  if self.machine is not None:
  self._vms[name].set_machine(self.machine)
  return self._vms[name]

Re: [PATCH 3/6] Acceptance Tests: use the job work directory for created VMs

2021-02-15 Thread Wainer dos Santos Moschetta 



On 2/11/21 7:01 PM, Cleber Rosa wrote:

The QEMUMachine uses a base temporary directory for all temporary
needs.  By setting it to the Avocado's workdir, it's possible to
keep the temporary files during debugging sessions much more
easily by setting the "--keep-tmp" command line option.

Reference: 
https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.workdir
Reference: 
https://avocado-framework.readthedocs.io/en/85.0/config/index.html#run-keep-tmp
Signed-off-by: Cleber Rosa 
---
  tests/acceptance/avocado_qemu/__init__.py | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


The changes look good to me, so:

Reviewed-by: Wainer dos Santos Moschetta 

But I got confused, the patch's subject states "use the job work 
directory" while the documentation of `workdir` says it is a "writable 
directory that exists during the entire test execution (...)". In the 
end is it a job or test work directory?


- Wainer



diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index bf54e419da..b7ab836455 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -172,7 +172,8 @@ class Test(avocado.Test):
  
  def _new_vm(self, *args):

  self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
-vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
+vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir,
+ sock_dir=self._sd.name)
  if args:
  vm.add_args(*args)
  return vm

Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory

2021-02-15 Thread Wainer dos Santos Moschetta 

Hi,

On 2/11/21 7:01 PM, Cleber Rosa wrote:

Each instance of qemu.machine.QEMUMachine currently has a "test
directory", which may not have any relation to a "test", and it's
really a temporary directory.

Users instantiating the QEMUMachine class will be able to set the
location of the directory that will *contain* the QEMUMachine unique
temporary directory, so that parameter name has been changed from
test_dir to base_temp_dir.

A property has been added to allow users to access it without using
private attributes, and with that, the directory is created on first
use of the property.

Signed-off-by: Cleber Rosa 
---
  python/qemu/machine.py | 24 
  python/qemu/qtest.py   |  6 +++---
  tests/acceptance/virtio-gpu.py |  2 +-
  tests/qemu-iotests/iotests.py  |  2 +-
  4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..b379fcbe72 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -84,7 +84,7 @@ class QEMUMachine:
   args: Sequence[str] = (),
   wrapper: Sequence[str] = (),
   name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_dir: str = "/var/tmp",
   monitor_address: Optional[SocketAddrT] = None,
   socket_scm_helper: Optional[str] = None,
   sock_dir: Optional[str] = None,
@@ -97,10 +97,10 @@ class QEMUMachine:
  @param args: list of extra arguments
  @param wrapper: list of arguments used as prefix to qemu binary
  @param name: prefix for socket and log file names (default: qemu-PID)
-@param test_dir: where to create socket and log file
+@param base_temp_dir: default location where temporary files are 
created
  @param monitor_address: address for QMP monitor
  @param socket_scm_helper: helper program, required for send_fd_scm()
-@param sock_dir: where to create socket (overrides test_dir for sock)
+@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
  @note: Qemu process is not started until launch() is used.
@@ -112,8 +112,8 @@ class QEMUMachine:
  self._wrapper = wrapper
  
  self._name = name or "qemu-%d" % os.getpid()

-self._test_dir = test_dir
-self._sock_dir = sock_dir or self._test_dir
+self._base_temp_dir = base_temp_dir
+self._sock_dir = sock_dir or self._base_temp_dir
  self._socket_scm_helper = socket_scm_helper
  
  if monitor_address is not None:

@@ -303,9 +303,7 @@ class QEMUMachine:
  return args
  
  def _pre_launch(self) -> None:

-self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
-  dir=self._test_dir)
-self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
+self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
  self._qemu_log_file = open(self._qemu_log_path, 'wb')
  
  if self._console_set:

@@ -744,3 +742,13 @@ class QEMUMachine:
  file=self._console_log_path,
  drain=self._drain_console)
  return self._console_socket
+
+@property
+def temp_dir(self) -> str:
+"""
+Returns a temporary directory to be used for this machine
+"""
+if self._temp_dir is None:
+self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
+  dir=self._base_temp_dir)
+return self._temp_dir
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe..78b97d13cf 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -112,14 +112,14 @@ class QEMUQtestMachine(QEMUMachine):
   binary: str,
   args: Sequence[str] = (),
   name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_dir: str = "/var/tmp",



In qtest.QEMUQtestMachine.__init__(), the argument named 'test_dir' 
still make sense, right?


- Wainer


   socket_scm_helper: Optional[str] = None,
   sock_dir: Optional[str] = None):
  if name is None:
  name = "qemu-%d" % os.getpid()
  if sock_dir is None:
-sock_dir = test_dir
-super().__init__(binary, args, name=name, test_dir=test_dir,
+sock_dir = base_temp_dir
+super().__init__(binary, args, name=name, base_temp_dir=base_temp_dir,
   socket_scm_helper=socket_scm_helper,
   sock_dir=sock_dir)
  self._qtest: Optional[QEMUQtestProtocol] = None
diff --git a/tests/acceptance/virtio-gpu.py b/t

Re: [PATCH 1/6] Python: close the log file kept by QEMUMachine before reading it

2021-02-15 Thread Wainer dos Santos Moschetta 

Hi,

On 2/11/21 7:01 PM, Cleber Rosa wrote:

Closing a file that is open for writing, and then reading from it
sounds like a better idea than the opposite, given that the content
will be flushed.

Reference: https://docs.python.org/3/library/io.html#io.IOBase.close
Signed-off-by: Cleber Rosa 
---
  python/qemu/machine.py | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 7a40f4604b..6e44bda337 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -337,12 +337,12 @@ class QEMUMachine:
  self._qmp.close()
  self._qmp_connection = None
  
-self._load_io_log()

-
  if self._qemu_log_file is not None:
  self._qemu_log_file.close()
  self._qemu_log_file = None
  
+self._load_io_log()

+



IMO it's a too fragile fix. It needs the `self._qemu_log_file.close()` 
being called before `self._load_io_log()` but a future change can make 
this condition unmet again. Maybe you could document that in the code. 
Or change the `_load_io_log()` to close the log file if it is open (also 
document it in code).


- Wainer


  self._qemu_log_path = None
  
  if self._temp_dir is not None:

Re: [PATCH 2/2] travis: remove travis configuration and all references to Travis CI

2021-02-11 Thread Wainer dos Santos Moschetta 

Hi,

On 2/9/21 11:32 AM, Thomas Huth wrote:

On 09/02/2021 14.50, Daniel P. Berrangé wrote:

The Travis CI system QEMU has been using has removed the unlimited free
usage model, replacing it with a one-time only grant of CI minutes that
is not renewed. The QEMU CI jobs quickly exhaust maintainer's free CI
credits, leaving them unable to test with Travis. This is not a
sustainable situation, so we have no choice by to discontinue use of
Travis. GitLab CI is now the primary target, with Cirrus CI filling
in some platform gaps where needed.


I've currently got a series in flight that moves some of the remaining 
jobs to gitlab-CI:


https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01924.html

Could you please hold this patch 'til my series got merged first?

Also I think we could still wait some more weeks with the final 
removal of the travis-CI either 'til travis-ci.org got shut down 
completely (and thus we cannot use it for QEMU at all anymore), or 
until we finally got the s390x and aarch64 runners up and running in 
the gitlab-CI.


It's reasonable to me.

- Wainer




 Thomas

Re: [PATCH v2 04/12] libqtest: add qtest_remove_abrt_handler()

2021-01-04 Thread Wainer dos Santos Moschetta 



On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:

Add a function to remove previously-added abrt handler functions.

Now that a symmetric pair of add/remove functions exists we can also
balance the SIGABRT handler installation. The signal handler was
installed each time qtest_add_abrt_handler() was called. Now it is
installed when the abrt handler list becomes non-empty and removed again
when the list becomes empty.

The qtest_remove_abrt_handler() function will be used by
vhost-user-blk-test.

Signed-off-by: Stefan Hajnoczi 
---
  tests/qtest/libqos/libqtest.h | 18 ++
  tests/qtest/libqtest.c| 35 +--
  2 files changed, 47 insertions(+), 6 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 51287b9276..a68dcd79d4 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -649,8 +649,26 @@ void qtest_add_data_func_full(const char *str, void *data,
  g_free(path); \
  } while (0)
  
+/**

+ * qtest_add_abrt_handler:
+ * @fn: Handler function
+ * @data: Argument that is passed to the handler
+ *
+ * Add a handler function that is invoked on SIGABRT. This can be used to
+ * terminate processes and perform other cleanup. The handler can be removed
+ * with qtest_remove_abrt_handler().
+ */
  void qtest_add_abrt_handler(GHookFunc fn, const void *data);
  
+/**

+ * qtest_remove_abrt_handler:
+ * @data: Argument previously passed to qtest_add_abrt_handler()
+ *
+ * Remove an abrt handler that was previously added with
+ * qtest_add_abrt_handler().
+ */
+void qtest_remove_abrt_handler(void *data);
+
  /**
   * qtest_qmp_assert_success:
   * @qts: QTestState instance to operate on
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index cc2cec4a35..7cf247baf0 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -196,15 +196,30 @@ static void cleanup_sigabrt_handler(void)
  sigaction(SIGABRT, &sigact_old, NULL);
  }
  
+static bool hook_list_is_empty(GHookList *hook_list)

+{
+GHook *hook = g_hook_first_valid(hook_list, TRUE);
+
+if (!hook) {
+return false;
+}
+
+g_hook_unref(hook_list, hook);
+return true;
+}
+
  void qtest_add_abrt_handler(GHookFunc fn, const void *data)
  {
  GHook *hook;
  
-/* Only install SIGABRT handler once */

  if (!abrt_hooks.is_setup) {
  g_hook_list_init(&abrt_hooks, sizeof(GHook));
  }
-setup_sigabrt_handler();
+
+/* Only install SIGABRT handler once */
+if (hook_list_is_empty(&abrt_hooks)) {
+setup_sigabrt_handler();
+}
  
  hook = g_hook_alloc(&abrt_hooks);

  hook->func = fn;
@@ -213,6 +228,17 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
  g_hook_prepend(&abrt_hooks, hook);
  }
  
+void qtest_remove_abrt_handler(void *data)

+{
+GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data);
+g_hook_destroy_link(&abrt_hooks, hook);
+
+/* Uninstall SIGABRT handler on last instance */
+if (hook_list_is_empty(&abrt_hooks)) {
+cleanup_sigabrt_handler();
+}
+}
+
  static const char *qtest_qemu_binary(void)
  {
  const char *qemu_bin;
@@ -369,10 +395,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, 
int *sock_fd)
  
  void qtest_quit(QTestState *s)

  {
-g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
-
-/* Uninstall SIGABRT handler on last instance */
-cleanup_sigabrt_handler();
+qtest_remove_abrt_handler(s);
  
  qtest_kill_qemu(s);

  close(s->fd);

Re: [PATCH v2 03/12] libqtest: add qtest_kill_qemu()

2021-01-04 Thread Wainer dos Santos Moschetta 



On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:

Tests that manage multiple processes may wish to kill QEMU before
destroying the QTestState. Expose a function to do that.

The vhost-user-blk-test testcase will need this.

Signed-off-by: Stefan Hajnoczi 
---
  tests/qtest/libqos/libqtest.h | 11 +++
  tests/qtest/libqtest.c|  7 ---
  2 files changed, 15 insertions(+), 3 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index e5f1ec590c..51287b9276 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -74,6 +74,17 @@ QTestState *qtest_init_without_qmp_handshake(const char 
*extra_args);
   */
  QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
  
+/**

+ * qtest_kill_qemu:
+ * @s: #QTestState instance to operate on.
+ *
+ * Kill the QEMU process and wait for it to terminate. It is safe to call this
+ * function multiple times. Normally qtest_quit() is used instead because it
+ * also frees QTestState. Use qtest_kill_qemu() when you just want to kill QEMU
+ * and qtest_quit() will be called later.
+ */
+void qtest_kill_qemu(QTestState *s);
+
  /**
   * qtest_quit:
   * @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index bc389d422b..cc2cec4a35 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -133,7 +133,7 @@ void qtest_set_expected_status(QTestState *s, int status)
  s->expected_status = status;
  }
  
-static void kill_qemu(QTestState *s)

+void qtest_kill_qemu(QTestState *s)
  {
  pid_t pid = s->qemu_pid;
  int wstatus;
@@ -143,6 +143,7 @@ static void kill_qemu(QTestState *s)
  kill(pid, SIGTERM);
  TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
  assert(pid == s->qemu_pid);
+s->qemu_pid = -1;
  }
  
  /*

@@ -169,7 +170,7 @@ static void kill_qemu(QTestState *s)
  
  static void kill_qemu_hook_func(void *s)

  {
-kill_qemu(s);
+qtest_kill_qemu(s);
  }
  
  static void sigabrt_handler(int signo)

@@ -373,7 +374,7 @@ void qtest_quit(QTestState *s)
  /* Uninstall SIGABRT handler on last instance */
  cleanup_sigabrt_handler();
  
-kill_qemu(s);

+qtest_kill_qemu(s);
  close(s->fd);
  close(s->qmp_fd);
  g_string_free(s->rx, true);

Re: [PATCH v2 02/12] libqtest: add qtest_socket_server()

2021-01-04 Thread Wainer dos Santos Moschetta 



On 12/7/20 2:20 PM, Stefan Hajnoczi wrote:

Add an API that returns a new UNIX domain socket in the listen state.
The code for this was already there but only used internally in
init_socket().

This new API will be used by vhost-user-blk-test.

Signed-off-by: Stefan Hajnoczi 
---
  tests/qtest/libqos/libqtest.h |  8 +++
  tests/qtest/libqtest.c| 40 ---
  2 files changed, 31 insertions(+), 17 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 724f65aa94..e5f1ec590c 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -132,6 +132,14 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
  void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
  GCC_FMT_ATTR(2, 3);
  
+/**

+ * qtest_socket_server:
+ * @socket_path: the UNIX domain socket path
+ *
+ * Create and return a listen socket file descriptor, or abort on failure.
+ */
+int qtest_socket_server(const char *socket_path);
+
  /**
   * qtest_vqmp_fds:
   * @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index e49f3a1e45..bc389d422b 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -81,24 +81,8 @@ static void qtest_client_set_rx_handler(QTestState *s, 
QTestRecvFn recv);
  
  static int init_socket(const char *socket_path)

  {
-struct sockaddr_un addr;
-int sock;
-int ret;
-
-sock = socket(PF_UNIX, SOCK_STREAM, 0);
-g_assert_cmpint(sock, !=, -1);
-
-addr.sun_family = AF_UNIX;
-snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+int sock = qtest_socket_server(socket_path);
  qemu_set_cloexec(sock);
-
-do {
-ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
-} while (ret == -1 && errno == EINTR);
-g_assert_cmpint(ret, !=, -1);
-ret = listen(sock, 1);
-g_assert_cmpint(ret, !=, -1);
-
  return sock;
  }
  
@@ -636,6 +620,28 @@ QDict *qtest_qmp_receive_dict(QTestState *s)

  return qmp_fd_receive(s->qmp_fd);
  }
  
+int qtest_socket_server(const char *socket_path)

+{
+struct sockaddr_un addr;
+int sock;
+int ret;
+
+sock = socket(PF_UNIX, SOCK_STREAM, 0);
+g_assert_cmpint(sock, !=, -1);
+
+addr.sun_family = AF_UNIX;
+snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+
+do {
+ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
+} while (ret == -1 && errno == EINTR);
+g_assert_cmpint(ret, !=, -1);
+ret = listen(sock, 1);
+g_assert_cmpint(ret, !=, -1);
+
+return sock;
+}
+
  /**
   * Allow users to send a message without waiting for the reply,
   * in the case that they choose to discard all replies up until

Re: [PATCH 0/9] Alpine Linux build fix and CI pipeline

2020-12-22 Thread Wainer dos Santos Moschetta 

Hi,

On 12/21/20 5:25 AM, Jiaxun Yang wrote:


On Mon, Dec 21, 2020, at 9:06 AM, no-re...@patchew.org wrote:

Patchew URL:
https://patchew.org/QEMU/20201221005318.11866-1-jiaxun.y...@flygoat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201221005318.11866-1-jiaxun.y...@flygoat.com
Subject: [PATCH 0/9] Alpine Linux build fix and CI pipeline

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
 From https://github.com/patchew-project/qemu
  * [new tag]
patchew/20201221005318.11866-1-jiaxun.y...@flygoat.com ->
patchew/20201221005318.11866-1-jiaxun.y...@flygoat.com
Switched to a new branch 'test'
10095a9 gitlab-ci: Add alpine to pipeline
a177af3 tests: Rename PAGE_SIZE definitions
5fcb0ed accel/kvm: avoid using predefined PAGE_SIZE
e7febdf hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
ba307d5 elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
0ccf92b libvhost-user: Include poll.h instead of sys/poll.h
41a10db configure/meson: Only check sys/signal.h on non-Linux
0bcd2f2 configure: Add sys/timex.h to probe clk_adjtime
a16c7ff tests/docker: Add dockerfile for Alpine Linux

=== OUTPUT BEGIN ===
1/9 Checking commit a16c7ff7d859 (tests/docker: Add dockerfile for Alpine Linux)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20:
new file mode 100644

total: 0 errors, 1 warnings, 56 lines checked

Patch 1/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/9 Checking commit 0bcd2f2eae84 (configure: Add sys/timex.h to probe
clk_adjtime)
3/9 Checking commit 41a10dbdc8da (configure/meson: Only check
sys/signal.h on non-Linux)
4/9 Checking commit 0ccf92b8ec37 (libvhost-user: Include poll.h instead
of sys/poll.h)
5/9 Checking commit ba307d5a51aa (elf2dmp: Rename PAGE_SIZE to
ELF2DMP_PAGE_SIZE)
WARNING: line over 80 characters
#69: FILE: contrib/elf2dmp/main.c:284:
+h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size /
ELF2DMP_PAGE_SIZE;

WARNING: line over 80 characters
#79: FILE: contrib/elf2dmp/main.c:291:
+h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages <<
ELF2DMP_PAGE_BITS;

total: 0 errors, 2 warnings, 70 lines checked

Patch 5/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/9 Checking commit e7febdf0b056 (hw/block/nand: Rename PAGE_SIZE to
NAND_PAGE_SIZE)
ERROR: code indent should never use tabs
#26: FILE: hw/block/nand.c:117:
+# define PAGE_START(page)^I(PAGE(page) * (NAND_PAGE_SIZE + OOB_SIZE))$

ERROR: code indent should never use tabs
#46: FILE: hw/block/nand.c:134:
+# define NAND_PAGE_SIZE^I^I2048$

WARNING: line over 80 characters
#65: FILE: hw/block/nand.c:684:
+mem_and(iobuf + (soff | off), s->io, MIN(s->iolen,
NAND_PAGE_SIZE - off));

WARNING: line over 80 characters
#70: FILE: hw/block/nand.c:687:
+mem_and(s->storage + (page << OOB_SHIFT), s->io +
NAND_PAGE_SIZE - off,

total: 2 errors, 2 warnings, 120 lines checked

Patch 6/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/9 Checking commit 5fcb0ed1331a (accel/kvm: avoid using predefined PAGE_SIZE)
8/9 Checking commit a177af33938d (tests: Rename PAGE_SIZE definitions)
9/9 Checking commit 10095a92643d (gitlab-ci: Add alpine to pipeline)
=== OUTPUT END ===

Test command exited with code: 1

All pre-existing errors.


Apparently some style errors were introduced by the patches 05 and 06.

- Wainer





The full log is available at
http://patchew.org/logs/20201221005318.11866-1-jiaxun.y...@flygoat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH 1/9] tests/docker: Add dockerfile for Alpine Linux

2020-12-22 Thread Wainer dos Santos Moschetta 

Hi,

On 12/20/20 9:53 PM, Jiaxun Yang wrote:

Alpine Linux[1] is a security-oriented, lightweight Linux distribution
based on musl libc and busybox.

It it popular among Docker guests and embedded applications.

Adding it to test against different libc.

[1]: https://alpinelinux.org/

Signed-off-by: Jiaxun Yang 
---
  tests/docker/dockerfiles/alpine.docker | 56 ++
  1 file changed, 56 insertions(+)
  create mode 100644 tests/docker/dockerfiles/alpine.docker


Unless one passes `NOUSER=1` as an argument (e.g. `make NOUSER=1 
docker-image-alpine`) then the image build is going to fail due to the 
lack of useradd command on the base Alpine image. See the error below. 
As a result do build inside the container will fail (e.g. `make 
docker-test-build@alpine`). Apparently this issue can be solved by 
installing the "shadow" package in the image.


Without applying the patches 02-08 the build will fail on Alpine (tested 
with `make docker-test-build@alpine`). Actually patch 08 did not apply 
on master tree (error: patch failed: tests/migration/stress.c:174) so I 
could not successfully build it in the container.


I suggest that you move the patch 01 later on this series, just after 
all build issues are fixed so that it can be built inside the container.


---

$ make docker-image-alpine
  BUILD   alpine
Trying to pull registry.gitlab.com/qemu-project/qemu/qemu/alpine...
  manifest unknown: manifest unknown
Error: unable to pull registry.gitlab.com/qemu-project/qemu/qemu/alpine: 
Error initializing source 
docker://registry.gitlab.com/qemu-project/qemu/qemu/alpine:latest: Error 
reading manifest latest in 
registry.gitlab.com/qemu-project/qemu/qemu/alpine: manifest unknown: 
manifest unknown

/bin/sh: useradd: not found
Error: error building at STEP "RUN id wmoschet 2>/dev/null || useradd -u 
1000 -U wmoschet": error while running runtime: exit status 127

Traceback (most recent call last):
  File "/home/wmoschet/src/qemu/tests/docker/docker.py", line 709, in 


    sys.exit(main())
  File "/home/wmoschet/src/qemu/tests/docker/docker.py", line 705, in main
    return args.cmdobj.run(args, argv)
  File "/home/wmoschet/src/qemu/tests/docker/docker.py", line 498, in run
    dkr.build_image(tag, docker_dir, dockerfile,
  File "/home/wmoschet/src/qemu/tests/docker/docker.py", line 353, in 
build_image

    self._do_check(build_args,
  File "/home/wmoschet/src/qemu/tests/docker/docker.py", line 244, in 
_do_check

    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['podman', 'build', '-t', 
'qemu/alpine', '-f', '/tmp/docker_build063jqidd/tmpn9zbbxzu.docker', 
'--cache-from', 'registry.gitlab.com/qemu-project/qemu/qemu/alpine', 
'/tmp/docker_build063jqidd']' returned non-zero exit status 125.
make: *** [/home/wmoschet/src/qemu/tests/docker/Makefile.include:62: 
docker-image-alpine] Error 1

---



diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker
new file mode 100644
index 00..a1b80f08d2
--- /dev/null
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -0,0 +1,56 @@
+
+FROM alpine:edge
+
+RUN apk update
+RUN apk upgrade
+
+# Please keep this list sorted alphabetically
+ENV PACKAGES \
+   alsa-lib-dev \
+   bash \
+   bison \
+   build-base \
+   coreutils \
+   curl-dev \
+   flex \
+   git \
+   glib-dev \
+   glib-static \
+   gnutls-dev \
+   gtk+3.0-dev \
+   libaio-dev \
+   libcap-dev \
+   libcap-ng-dev \
+   libjpeg-turbo-dev \
+   libnfs-dev \
+   libpng-dev \
+   libseccomp-dev \
+   libssh-dev \
+   libusb-dev \
+   libxml2-dev \
+   linux-headers \
+   lzo-dev \
+   mesa-dev \
+   mesa-egl \
+   mesa-gbm \
+   meson \
+   ncurses-dev \
+   ninja \
+   paxmark \
+   perl \
+   pulseaudio-dev \
+   python3 \
+   py3-sphinx \
+   snappy-dev \


To solve the problem I mentioned above:

diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker

index a1b80f08d2..5be5198d00 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -41,6 +41,7 @@ ENV PACKAGES \
    pulseaudio-dev \
    python3 \
    py3-sphinx \
+   shadow \
    snappy-dev \
    spice-dev \
    texinfo \


+   spice-dev \
+   texinfo \
+   usbredir-dev \
+   util-linux-dev \
+   vde2-dev \
+   virglrenderer-dev \
+   vte3-dev \
+   xfsprogs-dev \
+   zlib-dev \
+   zlib-static
+
+RUN apk add $PACKAGES

Re: [PATCH 05/10] tests/acceptance: Remove shebang header

2020-01-30 Thread Wainer dos Santos Moschetta 



On 1/29/20 9:13 PM, Philippe Mathieu-Daudé wrote:

Patch created mechanically by running:

   $ chmod 644 $(git grep -lF '#!/usr/bin/env python' | xargs grep -L 'if 
__name__.*__main__')
   $ sed -i "/^#\!\/usr\/bin\/\(env\ \)\?python.\?$/d" $(git grep -lF 
'#!/usr/bin/env python' | xargs grep -L 'if __name__.*__main__')

Reported-by: Vladimir Sementsov-Ogievskiy 
Suggested-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/acceptance/virtio_seg_max_adjust.py  | 1 -
  tests/acceptance/x86_cpu_model_versions.py | 1 -
  2 files changed, 2 deletions(-)
  mode change 100755 => 100644 tests/acceptance/virtio_seg_max_adjust.py



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/acceptance/virtio_seg_max_adjust.py 
b/tests/acceptance/virtio_seg_max_adjust.py
old mode 100755
new mode 100644
index 5458573138..8d4f24da49
--- a/tests/acceptance/virtio_seg_max_adjust.py
+++ b/tests/acceptance/virtio_seg_max_adjust.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python
  #
  # Test virtio-scsi and virtio-blk queue settings for all machine types
  #
diff --git a/tests/acceptance/x86_cpu_model_versions.py 
b/tests/acceptance/x86_cpu_model_versions.py
index 90558d9a71..01ff614ec2 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python
  #
  # Basic validation of x86 versioned CPU models and CPU model aliases
  #

Re: [Qemu-block] [Qemu-devel] [RFC PATCH v2 1/3] python/qemu: split QEMUMachine out from underneath __init__.py

2019-06-28 Thread Wainer dos Santos Moschetta 



On 06/27/2019 06:28 PM, John Snow wrote:

It's not obvious that something named __init__.py actually houses
important code that isn't relevant to python packaging glue. Move the
QEMUMachine and related error classes out into their own module.

Adjust users to the new import location.

Signed-off-by: John Snow 
---
  python/qemu/__init__.py   | 502 +
  python/qemu/machine.py| 520 ++
  python/qemu/qtest.py  |   2 +-
  scripts/device-crash-test |   2 +-
  scripts/render_block_graph.py |   2 +-
  tests/acceptance/avocado_qemu/__init__.py |   2 +-
  tests/acceptance/virtio_version.py|   2 +-
  tests/migration/guestperf/engine.py   |  22 +-
  tests/qemu-iotests/235|   2 +-
  tests/vm/basevm.py|   3 +-
  10 files changed, 540 insertions(+), 519 deletions(-)
  create mode 100644 python/qemu/machine.py

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index dbaf8a5311..6c919a3d56 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -12,17 +12,11 @@
  # Based on qmp.py.
  #
  
-import errno

  import logging
  import os
-import subprocess
-import re
-import shutil
-import socket
-import tempfile
  
  from . import qmp

-
+from . import machine
  
  LOG = logging.getLogger(__name__)
  
@@ -39,497 +33,3 @@ def kvm_available(target_arch=None):

  if target_arch != ADDITIONAL_ARCHES.get(host_arch):
  return False
  return os.access("/dev/kvm", os.R_OK | os.W_OK)
-
-
-class QEMUMachineError(Exception):
-"""
-Exception called when an error in QEMUMachine happens.
-"""
-
-
-class QEMUMachineAddDeviceError(QEMUMachineError):
-"""
-Exception raised when a request to add a device can not be fulfilled
-
-The failures are caused by limitations, lack of information or conflicting
-requests on the QEMUMachine methods.  This exception does not represent
-failures reported by the QEMU binary itself.
-"""
-
-class MonitorResponseError(qmp.QMPError):
-"""
-Represents erroneous QMP monitor reply
-"""
-def __init__(self, reply):
-try:
-desc = reply["error"]["desc"]
-except KeyError:
-desc = reply
-super(MonitorResponseError, self).__init__(desc)
-self.reply = reply
-
-
-class QEMUMachine(object):
-"""
-A QEMU VM
-
-Use this object as a context manager to ensure the QEMU process 
terminates::
-
-with VM(binary) as vm:
-...
-# vm is guaranteed to be shut down here
-"""
-
-def __init__(self, binary, args=None, wrapper=None, name=None,
- test_dir="/var/tmp", monitor_address=None,
- socket_scm_helper=None):
-'''
-Initialize a QEMUMachine
-
-@param binary: path to the qemu binary
-@param args: list of extra arguments
-@param wrapper: list of arguments used as prefix to qemu binary
-@param name: prefix for socket and log file names (default: qemu-PID)
-@param test_dir: where to create socket and log file
-@param monitor_address: address for QMP monitor
-@param socket_scm_helper: helper program, required for send_fd_scm()
-@note: Qemu process is not started until launch() is used.
-'''
-if args is None:
-args = []
-if wrapper is None:
-wrapper = []
-if name is None:
-name = "qemu-%d" % os.getpid()
-self._name = name
-self._monitor_address = monitor_address
-self._vm_monitor = None
-self._qemu_log_path = None
-self._qemu_log_file = None
-self._popen = None
-self._binary = binary
-self._args = list(args) # Force copy args in case we modify them
-self._wrapper = wrapper
-self._events = []
-self._iolog = None
-self._socket_scm_helper = socket_scm_helper
-self._qmp = None
-self._qemu_full_args = None
-self._test_dir = test_dir
-self._temp_dir = None
-self._launched = False
-self._machine = None
-self._console_set = False
-self._console_device_type = None
-self._console_address = None
-self._console_socket = None
-
-# just in case logging wasn't configured by the main script:
-logging.basicConfig()
-
-def __enter__(self):
-return self
-
-def __exit__(self, exc_type, exc_val, exc_tb):
-self.shutdown()
-return False
-
-# This can be used to add an unused monitor instance.
-def add_monitor_null(self):
-self._args.append('-monitor')
-self._args.append('null')
-
-def add_fd(self, fd, fdset, opaque, opts=''):
-"""
-Pass a file descriptor to the VM
-"""
-options = ['fd=%d' % fd,
-   'set=%d' % f

Re: [Qemu-block] [Qemu-devel] [PATCH 4/6] cirrus / travis: Add gnu-sed and bash for macOS and FreeBSD

2019-04-24 Thread Wainer dos Santos Moschetta 

Hello Thomas,

On 04/24/2019 07:37 AM, Thomas Huth wrote:

We are going to enable the qemu-iotests during "make check" again,
and for running the iotests, we need bash and gnu-sed.

Signed-off-by: Thomas Huth 
---
  .cirrus.yml | 4 ++--
  .travis.yml | 1 +
  2 files changed, 3 insertions(+), 2 deletions(-)


It needs to get the Freebsd image [1] updated too, in order to `make 
BUILD_TARGET=check vm-build-freebsd` passes. Here it failed with:

---
env: bash: No such file or directory
gmake: *** [/var/tmp/qemu-test.6OlDFH/tests/Makefile.include:1101: 
check-tests/qemu-iotests-quick.sh] Error 1

gmake: *** Waiting for unfinished jobs
---

I'm not sure about the netbsd and openbsd images, they might need bash 
and gnu-sed as well.


[1] http://download.patchew.org/freebsd-11.1-amd64.img.xz

- Wainer



diff --git a/.cirrus.yml b/.cirrus.yml
index 47ef5bc604..8326a3a4b1 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -7,7 +7,7 @@ freebsd_12_task:
  cpu: 8
  memory: 8G
install_script: pkg install -y
-bison curl cyrus-sasl git glib gmake gnutls
+bash bison curl cyrus-sasl git glib gmake gnutls gsed
  nettle perl5 pixman pkgconf png usbredir
script:
  - mkdir build
@@ -20,7 +20,7 @@ macos_task:
osx_instance:
  image: mojave-base
install_script:
-- brew install pkg-config python glib pixman make sdl2
+- brew install pkg-config python gnu-sed glib pixman make sdl2
script:
  - ./configure --python=/usr/local/bin/python3 || { cat config.log; exit 
1; }
  - gmake -j$(sysctl -n hw.ncpu)
diff --git a/.travis.yml b/.travis.yml
index 2e06aee9d0..ba94644192 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -42,6 +42,7 @@ addons:
  packages:
- glib
- pixman
+  - gnu-sed
  
  
  # The channel name "irc.oftc.net#qemu" is encrypted against qemu/qemu

Re: [Qemu-block] [Qemu-devel] [PATCH 5/6] tests: Run the iotests during "make check" again

2019-04-24 Thread Wainer dos Santos Moschetta 

Hi Thomas,


On 04/24/2019 07:37 AM, Thomas Huth wrote:

People often forget to run the iotests before submitting patches or
pull requests - this is likely due to the fact that we do not run the
tests during our mandatory "make check" tests yet. Now that we've got
a new "ci" group of iotests that should be fine to run in every environ-
ment, it should be OK to enable the iotests during "make check" again.
Thus we now run the "ci" tests by default from the qemu-iotests-quick.sh
script, and only use the former "quick" group (that contains some tests
that are failing in some environments) when the user decided to run
"make check SPEED=thorough" or something similar.

Signed-off-by: Thomas Huth 
---
  tests/Makefile.include  |  2 +-
  tests/qemu-iotests-quick.sh | 17 -
  2 files changed, 17 insertions(+), 2 deletions(-)


One of Patchew's runners failed with this patch series:
https://patchew.org/QEMU/20190424103747.10173-1-th...@redhat.com/

I encountered same failures with `make check` locally on Fedora 29 
x86_64 (pulled latest qemu, and configured with defaults and 
x86_64-softmmu target):

---
Not run: 233
Failures: 069 103 114 133 140 143 197 215 226 244
Failed 10 of 91 tests
---

Let me know if you need more details.

- Wainer



diff --git a/tests/Makefile.include b/tests/Makefile.include
index 36fc73fef5..eb6a7a41e2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1168,7 +1168,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
  check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) 
check-tests/qapi-schema/doc-good.texi
  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
  check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-qapi-schema check-unit check-softfloat check-qtest 
check-decodetree
+check: check-qapi-schema check-unit check-softfloat check-qtest 
check-decodetree check-block
  check-clean:
rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), 
$(check-qtest-$(target)-y)) $(check-qtest-generic-y))
diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
index 0e554bb972..416b3fc48b 100755
--- a/tests/qemu-iotests-quick.sh
+++ b/tests/qemu-iotests-quick.sh
@@ -1,8 +1,23 @@
  #!/bin/sh
  
+# Honor the SPEED environment variable, just like we do it for the qtests.

+# The default is to run all tests that still work fine in a CI environments,
+# but if the user set SPEED=slow or SPEED=thorough, we also run all other
+# tests that are still marked as "quick"
+if [ "$SPEED" = "slow" -o "$SPEED" = "thorough" ]; then
+group=quick
+else
+group=ci
+fi
+
+if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
+echo "No qemu-system binary available. Skipped qemu-iotests."
+exit 0
+fi
+
  cd tests/qemu-iotests
  
  ret=0

-TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g quick || 
ret=1
+TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g "$group" 
|| ret=1
  
  exit $ret

Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/3] qemu-img: removing created when img_create fails

2019-03-25 Thread Wainer dos Santos Moschetta 




On 03/22/2019 06:49 PM, Daniel Henrique Barboza wrote:



On 3/22/19 4:02 PM, no-re...@patchew.org wrote:
Patchew URL: 
https://patchew.org/QEMU/20190322175241.5954-1-danielhb...@gmail.com/




Hi,

This series failed the docker-mingw@fedora build test. Please find 
the testing commands and
their output below. If you have Docker installed, you can probably 
reproduce it

locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

   CC  aarch64-softmmu/tcg/tcg-op.o
   CC  aarch64-softmmu/tcg/tcg-op-vec.o
/tmp/qemu-test/src/qemu-img.c: In function 'img_create':
/tmp/qemu-test/src/qemu-img.c:568:13: error: 'filename' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]

  error_reportf_err(local_err, "%s: ", filename);
  ^~
cc1: all warnings being treated as errors


Not sure why it complained about this. Travis didn't care 


How can I reproduce this docker use test? I have a Fedora29 box with 
Docker and

issuing "make docker-test-mingw(...)" as mentioned above doesn't work.



I could reproduce the above error on my Fedora 29 x86_64 machine. From 
the build dir, run as:


make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1

Compile QEMU sources with default configs also fails.

- Wainer





Thanks,


dhb



The full log is available at
http://patchew.org/logs/20190322175241.5954-1-danielhb...@gmail.com/testing.docker-mingw@fedora/?type=message. 


---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-block] [Qemu-devel] [PATCH RFC] qemu-io: Prefer stderr for error messages

2018-12-13 Thread Wainer dos Santos Moschetta 
rintf("read failed: %s\n", strerror(-ret));
+fprintf(stderr, "read failed: %s\n", strerror(-ret));
  goto out;
  }
  cnt = ret;
@@ -777,9 +782,9 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
  void *cmp_buf = g_malloc(pattern_count);
  memset(cmp_buf, pattern, pattern_count);
  if (memcmp(buf + pattern_offset, cmp_buf, pattern_count)) {
-printf("Pattern verification failed at offset %"
-   PRId64 ", %"PRId64" bytes\n",
-   offset + pattern_offset, pattern_count);
+fprintf(stderr, "Pattern verification failed at offset %"
+PRId64 ", %"PRId64" bytes\n",
+offset + pattern_offset, pattern_count);
  ret = -EINVAL;
  }
  g_free(cmp_buf);
@@ -895,7 +900,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
  gettimeofday(&t2, NULL);

  if (ret < 0) {
-printf("readv failed: %s\n", strerror(-ret));
+fprintf(stderr, "readv failed: %s\n", strerror(-ret));
  goto out;
  }
  cnt = ret;
@@ -906,8 +911,8 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
  void *cmp_buf = g_malloc(qiov.size);
  memset(cmp_buf, pattern, qiov.size);
  if (memcmp(buf, cmp_buf, qiov.size)) {
-printf("Pattern verification failed at offset %"
-   PRId64 ", %zu bytes\n", offset, qiov.size);
+fprintf(stderr, "Pattern verification failed at offset %"
+PRId64 ", %zu bytes\n", offset, qiov.size);
  ret = -EINVAL;
  }
  g_free(cmp_buf);
@@ -1027,22 +1032,23 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
  }

  if (bflag && zflag) {
-printf("-b and -z cannot be specified at the same time\n");
+fprintf(stderr, "-b and -z cannot be specified at the same time\n");
  return -EINVAL;
  }

  if ((flags & BDRV_REQ_FUA) && (bflag || cflag)) {
-printf("-f and -b or -c cannot be specified at the same time\n");
+fprintf(stderr,
+"-f and -b or -c cannot be specified at the same time\n");
  return -EINVAL;
  }

  if ((flags & BDRV_REQ_MAY_UNMAP) && !zflag) {
-printf("-u requires -z to be specified\n");
+fprintf(stderr, "-u requires -z to be specified\n");
  return -EINVAL;
  }

  if (zflag && Pflag) {
-printf("-z and -P cannot be specified at the same time\n");
+fprintf(stderr, "-z and -P cannot be specified at the same time\n");
  return -EINVAL;
  }

@@ -1058,21 +1064,23 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
  print_cvtnum_err(count, argv[optind]);
  return count;
  } else if (count > BDRV_REQUEST_MAX_BYTES) {
-printf("length cannot exceed %" PRIu64 ", given %s\n",
-   (uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
+fprintf(stderr, "length cannot exceed %" PRIu64 ", given %s\n",
+        (uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
  return -EINVAL;
  }

  if (bflag || cflag) {
  if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
-printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
-   offset);
+fprintf(stderr,
+"%" PRId64 " is not a sector-aligned value for 'offset'\n",
+offset);
  return -EINVAL;
  }

  if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
-printf("%"PRId64" is not a sector-aligned value for 'count'\n",
-   count);
+fprintf(stderr,
+"%"PRId64" is not a sector-aligned value for 'count'\n",


Likewise.

Reviewed-by: Wainer dos Santos Moschetta 

- Wainer


+count);
  return -EINVAL;
  }
  }
@@ -1094,7 +1102,7 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
  gettimeofday(&t2, NULL);

  if (ret < 0) {
-printf("write failed: %s\n", strerror(-ret));
+fprintf(stderr, "write failed: %s\n", strerror(-ret));
  goto out;
  }
  cnt = ret;
@@ -1208,7 +1216,7 @@ static int writev_f(BlockBackend *blk, int argc, char 
**argv)
  gettimeofday(&t2, NULL);

  if (ret < 0) {
-printf("writev failed: %s\n", strerror(-ret));
+fprintf(stderr, "writev failed: %s\n"

Re: [Qemu-block] [Qemu-devel] [PATCH] iotests: Skip 233 if certtool not installed

2018-11-21 Thread Wainer dos Santos Moschetta 




On 11/20/2018 08:52 PM, Eric Blake wrote:

The use of TLS while building qemu is optional. While the
'certtool' binary should be available on every platform that
supports building against TLS, that does not imply that the
developer has installed it.  Make the test gracefully skip
in that case.

Reported-by: Kevin Wolf 
Signed-off-by: Eric Blake 


Reviewed-by: Wainer dos Santos Moschetta 


---

On Fedora, libvirt requires libtls-utils to be present, but not qemu.

I'm fine if Kevin wants to pick this up in a pull request related
to iotests in general; if not, I'll do a pull request through my
NBD tree in time for -rc3.

  tests/qemu-iotests/common.tls | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index 39f17c1b999..eae81789bbc 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -31,6 +31,9 @@ tls_x509_cleanup()

  tls_x509_init()
  {
+(certtool --help) >/dev/null 2>&1 || \
+   _notrun "certtool utility not found, skipping test"
+
  mkdir -p "${tls_dir}"

  # use a fixed key so we don't waste system entropy on