On Wed, Dec 9, 2020 at 12:50 PM Benny Zlotnik <bzlotnik@redhat.com> wrote:
>
> >Looking at sanlock source, if you specify the size, sanlock will copy
> >what you asked for.
> >But if you specify 0 sanlock will copy entire sector:
> >
> > 753                 if (!len)
> > 754                         len = r->leader.sector_size;
>
> >Since we don't have a good way to get the sector size from sanlock, I think we
> >should pass the size the user specified, and if the user did not
> >specify, allocate
> >and zero (calloc) a 4k buffer and pass sanlock lvblen=0, and let sanlock copy
> >an entire sector.
> >
> >Looks like sanlock does not keep the size of the lvb passed in
> >set_lvb, so it cannot
> >return this value in. It also does not return the size it read when
> >using lvblen=0, so
> >we will always get one sector on the client side.
>
> Maybe I'm missing something, but using lvblen=0 returns EINVAL,
> probably because of this check[1]
>
>
> [1] https://pagure.io/sanlock/blob/master/f/src/client.c#_1475

  Looking in https://pagure.io/sanlock/raw/master/f/src/client.c
   
        if (!res || !lvb || !lvblen)
                return -EINVAL;
       
Sanlock does not suport lvblen=0 as the internal commands. Instead it
always send lvblen=0:

        rv = send_header(fd, SM_CMD_GET_LVB, flags, datalen, 0, 0);
   
The last argument is header.data2.
   
In https://pagure.io/sanlock/raw/master/f/src/cmd.c
It uses header.data2 to get lvblen:
   
        /* if 0 then we use the sector size as lvb len */
        lvblen = ca->header.data2;

So sanlock always read complete sector from storage. It makes
sense since sanlock is using direct I/O and there is no way to
read part of a cluster.

When returning a reply, sanlock will copy up to lvblen bytes to the user
buffer.
   
        if (lvblen < len)
                len = lvblen;
       
        memcpy(lvb, reply_data, len);

Since we must specify a value. Since we try to keep the same semantics in
the python binding, size should be a required argument.

So we can have:

    set_lvb(lockspace, resource, disks, buf)

    get_lvb(lockspace, resource, disks, size) -> bytes

These tests should succeed:

    data = b"first\0second".ljust(512, b"\0")
    set_lvb(ls, res, disks, buf)
    assert get_lvb(ls, res, disks, 512) == data

The lvb block may contain data from previous writes, so this test
is not valid:

    data = b"first"
    set_lvb(ls, res, disks, buf)
    assert get_lvb(ls, res, disks, 512) == data.ljust(512, b"\0")

This is unlikely usage but it should succeed when sector size is 512 bytes:

    data = b"first\0second".ljust(512, b"\0")
    set_lvb(ls, res, disks, buf)
    assert get_lvb(ls, res, disks, 4096) == data.ljust(4096, b"\0")

We allocate 4k bytes and zero the buffer, then sanlock copy 512
bytes into our buffer.

Setting lvb is limited by the sector size, so users are likely to always use
up to 512 bytes to have code that works with any storage.