Re: [PATCH 4/4] sanlock: Fail read_resource_owners() with wrong sector size

[Nir Soffer]

On Tue, May 7, 2019 at 1:38 AM Nir Soffer <nirsof(a)gmail.com&gt; wrote:

>  If sanlock_read_resource_owners() is called with wrong sector size,
> fail
>  instead of falling back to actual sector size. This is more consistent
>  with sanlock_read_resource() and other other functions.
>
>  Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
>  ---
>   src/resource.c       | 17 ++++++++++++++---
>   tests/python_test.py |  1 -
>   2 files changed, 14 insertions(+), 4 deletions(-)
>
>  diff --git a/src/resource.c b/src/resource.c
>  index a783106..f75ea71 100644
>  --- a/src/resource.c
>  +++ b/src/resource.c
>  @@ -167,19 +167,19 @@ int read_resource_owners(struct task *task, struct
>  token *token,
>          uint64_t host_id;
>          uint32_t checksum;
>          char *lease_buf_dblock;
>          char *lease_buf = NULL;
>          char *hosts_buf = NULL;
>  +       int sector_size = token->sector_size;
>          int align_size;
>          int host_count = 0;
>          int i, rv;
>
>          disk = &token->disks[0];
>
>          /*
>  -        * We don't know the sector_size of the resource until the leader
>  -        * record has been read, start with the smaller size.
>  +        * If we don't know the sector_size start with the smaller size.
>           */
>
>          if (!token->sector_size) {
>                  token->sector_size = 512;
>                  token->align_size = sector_size_to_align_size_old(512);
>  @@ -204,13 +204,24 @@ int read_resource_owners(struct task *task, struct
>  token *token,
>
>          align_size = leader_align_size_from_flag(leader.flags);
>          if (!align_size)
>                  align_size =
>  sector_size_to_align_size_old(leader.sector_size);
>
>  +       /* If caller specified value was incorrect, fail. */
>  +       if (sector_size && sector_size != leader.sector_size) {
>  +               log_errot(token, "read_resource_owners invalid
>  sector_size: %d actual: %d",
>  +                         sector_size, leader.sector_size);
>  +               rv = -EINVAL;
>  +               goto out;
>  +       }
>  +
>  
I started with validating sector size but I think this should also be done
for alignment.

I'll send another patch to validate align_size.


>  +       /*
>  +        * 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) ||
>              (token->align_size != align_size)) {
>  -               /* initial sizes were wrong */
>                  log_debug("read_resource_owners rereading with correct
>  sizses");
>                  token->sector_size = leader.sector_size;
>                  token->align_size  = align_size;
>                  free(lease_buf);
>                  lease_buf = NULL;
>  diff --git a/tests/python_test.py b/tests/python_test.py
>  index aa1f7f5..e8a34c0 100644
>  --- a/tests/python_test.py
>  +++ b/tests/python_test.py
>  @@ -246,11 +246,10 @@ def
>  test_read_resource_4k_invalid_sector_size(sanlock_daemon):
>
>
>   @pytest.mark.skipif(
>       "SANLOCK_4K_PATH" not in os.environ,
>       reason="Requires user specified path using 4k block size")
>  -(a)pytest.mark.xfail(reason=&quot;fallback hides wrong value from caller")
>   def test_read_resource_owners_4k_invalid_sector_size(sanlock_daemon):
>       path = os.environ["SANLOCK_4K_PATH"]
>       disks = [(path, 0)]
>
>       sanlock.write_resource(
>  --
>  2.17.2
>
>