On Sat, Jun 15, 2019 at 12:55 AM Vojtech Juranek <vjuranek@redhat.com> wrote:
Add tests for write_resource() with parameter 'clear' set to True, which
shoudl result into clearing the resource (overwritting PAXOS_DISK_MAGIC
with PAXOS_DISK_CLEAR). Also add tests for cases when th resource is
invalid.
---
 tests/constants.py   |  4 ++++
 tests/python_test.py | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/tests/constants.py b/tests/constants.py
index a7eb16b..21089b8 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -17,3 +17,7 @@ RINDEX_DISK_MAGIC = 0x01042018

 RINDEX_ENTRY_SIZE = 64
 RINDEX_ENTRIES_SECTORS = 2000
+
+# src/sanlock_rv.h
+
+SANLK_LEADER_MAGIC = -223 
diff --git a/tests/python_test.py b/tests/python_test.py
index 623a13c..4931f0c 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -227,6 +227,48 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
     assert e.value.errno == errno.EINVAL


+def test_clear_resource(tmpdir, sanlock_daemon):
+    path = util.generate_path(tmpdir, "clear_test")
+    util.create_file(path, GiB)

We can use MiB to have space for one resource, and guard area.
 
+    disks = [(path, 0)]
+
+    sanlock.write_resource(b"ls_name", b"res_name", disks)
+    sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True)
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.read_resource(path, offset=0)

offset is not needed.
 
+    assert e.value.errno == constants.SANLK_LEADER_MAGIC 
+
+    magic = util.read_uint(path)
+    assert magic == constants.PAXOS_DISK_CLEAR
+
+    util.check_guard(path, GiB)

We want to check that sanlock did not write after the end of the reosurce, so 
this should be:

    util.check_guard(path, MiB)

 
+
+
+def test_clear_invalid_resource(tmpdir, sanlock_daemon):
+    path = util.generate_path(tmpdir, "clear_test")
+    util.create_file(path, GiB)
+    disks = [(path, 0)]
+
+    sanlock.write_resource(b"ls_name", b"res_name", disks)
+
+    # Clear resource with invalid lockspace name - doesn't throw any exception
+    sanlock.write_resource(b"inval_ls_name", b"res_name", disks, clear=True)

I think we need to validate magic on storage for all calls.

Calling with different resource or lockspace name is not invalid usage, this is how
resource was cleared before we had the clear flag, by writing resource with empty
lockspace and resource name, so we cannot treat this as invalid usage.

The case I thought about was calling it on an area that does not have any resource.

So I think the interesting cases we like to test are:

- calling with empty lockspace and resource and clear=True (the way vdsm clear resources)
  this is valid uses case and should succeed

- calling with non empty lockapce and resource name on storage area that does not have a resource
  e.g. create empty file and call with clear=True. I think it will succeed, writing CLEAR magic to storage.

- calling on storage area that was already cleared - probably will succeed in the same way.
 
+
+    # Clear resource with invalid resource name - doesn't throw any exception
+    sanlock.write_resource(b"ls_name", b"inval_res_name", disks, clear=True)
+
+    # Clear resource with invalid offset - doesn't throw any exception
+    sanlock.write_resource(b"ls_name", b"res_name", [(path, MiB)], clear=True)
+
+    # Clear resource with invalid path - throw ENOENT exception
+    inval_path = util.generate_path(tmpdir, "inval_clear_test")
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.write_resource(
+            b"ls_name", b"res_name", [(inval_path, 0)], clear=True)
+    assert e.value.errno == errno.ENOENT

What do you mean by invalid path? maybe missing_path? I'm not sure this test is 
relevant to write_resource with clear flag, but it is interesting test anyway, if we don't have
such test without the clear flag.
 
+
+
 def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
     disks = [(user_4k_path, 0)]

--
2.20.1