Previously if sector size was not set, we modified both sector size and
align size, dropping the caller align size silently.
This can cause two issues:
- User align size was correct, but sanlock guessed the wrong value -
in this case sanlock logs a debug message and perform another read.
- User align size was incorrect, but sanlock guess was correct, hiding a
bug in caller code.
Change the behaviour to be more consistent with the way we validate
sector size, and reject invalid value set by the user.
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
src/resource.c | 17 +++++++++++++----
tests/python_test.py | 1 -
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/resource.c b/src/resource.c
index 3959b35..e0e07b6 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -168,22 +168,24 @@ int read_resource_owners(struct task *task, struct token *token,
uint32_t checksum;
char *lease_buf_dblock;
char *lease_buf = NULL;
char *hosts_buf = NULL;
int sector_size_set = token->sector_size != 0;
probably: const int sector_size_set = token->sector_size != 0;
+ int align_size_set = token->align_size != 0;
probably: const int align_size_set = token->align_size != 0;
int align_size;
int host_count = 0;
int i, rv;
disk = &token->disks[0];
/* If sector size not set, start with the smaller one. */
-
- if (!sector_size_set) {
+ if (!sector_size_set)
token->sector_size = 512;
- token->align_size = sector_size_to_align_size_old(512);
- }
+
+ /* If align size not set, default to the older align. */
+ if (!align_size_set)
+ token->align_size = sector_size_to_align_size_old(token->sector_size);
/* we could in-line paxos_read_buf here like we do in read_mode_block */
retry:
rv = paxos_read_buf(task, token, &lease_buf);
if (rv < 0) {
@@ -211,10 +213,17 @@ int read_resource_owners(struct task *task, struct token *token,
token->sector_size, leader.sector_size);
rv = -EINVAL;
goto out;
}
+ if (align_size_set && token->align_size != align_size) {
+ log_errot(token, "read_resource_owners invalid align_size: %d actual: %d",
+ token->align_size, align_size);
+ rv = -EINVAL;
+ goto out;
+ }
+
/*
* If the caller did not specify sector size and our guess was wrong,
* retry with the actual value
*/
if ((token->sector_size != leader.sector_size) ||
diff --git a/tests/python_test.py b/tests/python_test.py
index d5b81ae..85e4829 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -235,11 +235,10 @@ def test_read_resource_owners_4k_invalid_sector_size(
sanlock.read_resource_owners(
"ls_name", "res_name", disks, sector=SECTOR_SIZE_512)
assert e.value.errno == errno.EINVAL
-@pytest.mark.xfail(reason="fallback hides invalid user value")
def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
path = str(tmpdir.join("path"))
util.create_file(path, 1024**3)
disks = [(path, 0)]
--
2.17.2