Re: [PATCH v4 08/10] iotests.py: add verify_o_direct helper

2020-08-20 Thread Vladimir Sementsov-Ogievskiy 

21.08.2020 00:00, Nir Soffer wrote:

On Thu, Aug 20, 2020 at 9:49 PM Vladimir Sementsov-Ogievskiy
 wrote:


Add python notrun-helper similar to _check_o_direct for bash tests.
To be used in the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Stefan Hajnoczi 
---
  tests/qemu-iotests/iotests.py | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 717b5b652c..369e9918b4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1083,6 +1083,12 @@ def _verify_aio_mode(supported_aio_modes: Sequence[str] = 
()) -> None:
  if supported_aio_modes and (aiomode not in supported_aio_modes):
  notrun('not suitable for this aio mode: %s' % aiomode)

+def verify_o_direct() -> None:
+with FilePath('test_o_direct') as f:
+qemu_img_create('-f', 'raw', f, '1M')
+if 'O_DIRECT' in qemu_io('-f', 'raw', '-t', 'none', '-c', 'quit', f):
+notrun(f'file system at {test_dir} does not support O_DIRECT')


Why not:

 with FilePath('test_o_direct') as f:
 try:
 fd = os.open(f, os.O_DIRECT | os.O_CREAT | os.O_RDWR)
 except OSError as e:
 if e.errno != errno.EINVAL:
 raise
 notrun(...)
 else:
os.close(fd)

More verbose, but the intent is more clear, and we do not depend on the output
of qemu-io which is not a public API. For example if someone improves qemu-io
to fail with:

 Direct I/O is not supported

It would break the tests using this helper.



Agree, that's better. Will use it, thanks!




+
  def supports_quorum():
  return 'quorum' in qemu_img_pipe('--help')

--
2.21.3







--
Best regards,
Vladimir

Re: [PATCH v4 08/10] iotests.py: add verify_o_direct helper

2020-08-20 Thread Nir Soffer 
On Thu, Aug 20, 2020 at 9:49 PM Vladimir Sementsov-Ogievskiy
 wrote:
>
> Add python notrun-helper similar to _check_o_direct for bash tests.
> To be used in the following commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Stefan Hajnoczi 
> ---
>  tests/qemu-iotests/iotests.py | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 717b5b652c..369e9918b4 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -1083,6 +1083,12 @@ def _verify_aio_mode(supported_aio_modes: 
> Sequence[str] = ()) -> None:
>  if supported_aio_modes and (aiomode not in supported_aio_modes):
>  notrun('not suitable for this aio mode: %s' % aiomode)
>
> +def verify_o_direct() -> None:
> +with FilePath('test_o_direct') as f:
> +qemu_img_create('-f', 'raw', f, '1M')
> +if 'O_DIRECT' in qemu_io('-f', 'raw', '-t', 'none', '-c', 'quit', f):
> +notrun(f'file system at {test_dir} does not support O_DIRECT')

Why not:

with FilePath('test_o_direct') as f:
try:
fd = os.open(f, os.O_DIRECT | os.O_CREAT | os.O_RDWR)
except OSError as e:
if e.errno != errno.EINVAL:
raise
notrun(...)
else:
   os.close(fd)

More verbose, but the intent is more clear, and we do not depend on the output
of qemu-io which is not a public API. For example if someone improves qemu-io
to fail with:

Direct I/O is not supported

It would break the tests using this helper.

Nir

> +
>  def supports_quorum():
>  return 'quorum' in qemu_img_pipe('--help')
>
> --
> 2.21.3
>
>

[PATCH v4 08/10] iotests.py: add verify_o_direct helper

2020-08-20 Thread Vladimir Sementsov-Ogievskiy 
Add python notrun-helper similar to _check_o_direct for bash tests.
To be used in the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/iotests.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 717b5b652c..369e9918b4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1083,6 +1083,12 @@ def _verify_aio_mode(supported_aio_modes: Sequence[str] 
= ()) -> None:
 if supported_aio_modes and (aiomode not in supported_aio_modes):
 notrun('not suitable for this aio mode: %s' % aiomode)
 
+def verify_o_direct() -> None:
+with FilePath('test_o_direct') as f:
+qemu_img_create('-f', 'raw', f, '1M')
+if 'O_DIRECT' in qemu_io('-f', 'raw', '-t', 'none', '-c', 'quit', f):
+notrun(f'file system at {test_dir} does not support O_DIRECT')
+
 def supports_quorum():
 return 'quorum' in qemu_img_pipe('--help')
 
-- 
2.21.3