[v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask

From: Xiubo Li <xiubli@redhat.com>

Ceph added the bal_rank_mask with encoded (ev) version 17.  This
was merged into main Oct 2022 and made it into the reef release
normally. While a latter commit added the max_xattr_size also
with encoded (ev) version 17 but places it before bal_rank_mask.

And this will breaks some usages, for example when upgrading old
cephs to newer versions.

URL: https://tracker.ceph.com/issues/64440
Reported-by: Patrick Donnelly <pdonnell@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---

V2:
- minor fix in the comment





 fs/ceph/mdsmap.c | 7 ++++---
 fs/ceph/mdsmap.h | 6 +++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

On Tue, Feb 20, 2024 at 10:13 PM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> Ceph added the bal_rank_mask with encoded (ev) version 17.  This
> was merged into main Oct 2022 and made it into the reef release
> normally. While a latter commit added the max_xattr_size also
> with encoded (ev) version 17 but places it before bal_rank_mask.
>
> And this will breaks some usages, for example when upgrading old
> cephs to newer versions.
>
> URL: https://tracker.ceph.com/issues/64440
> Reported-by: Patrick Donnelly <pdonnell@redhat.com>
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>
> V2:
> - minor fix in the comment
>
>
>
>
>
>  fs/ceph/mdsmap.c | 7 ++++---
>  fs/ceph/mdsmap.h | 6 +++++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ceph/mdsmap.c b/fs/ceph/mdsmap.c
> index fae97c25ce58..8109aba66e02 100644
> --- a/fs/ceph/mdsmap.c
> +++ b/fs/ceph/mdsmap.c
> @@ -380,10 +380,11 @@ struct ceph_mdsmap *ceph_mdsmap_decode(struct ceph_mds_client *mdsc, void **p,
>                 ceph_decode_skip_8(p, end, bad_ext);
>                 /* required_client_features */
>                 ceph_decode_skip_set(p, end, 64, bad_ext);
> +               /* bal_rank_mask */
> +               ceph_decode_skip_string(p, end, bad_ext);
> +       }
> +       if (mdsmap_ev >= 18) {
>                 ceph_decode_64_safe(p, end, m->m_max_xattr_size, bad_ext);
> -       } else {
> -               /* This forces the usage of the (sync) SETXATTR Op */
> -               m->m_max_xattr_size = 0;
>         }
>  bad_ext:
>         doutc(cl, "m_enabled: %d, m_damaged: %d, m_num_laggy: %d\n",
> diff --git a/fs/ceph/mdsmap.h b/fs/ceph/mdsmap.h
> index 89f1931f1ba6..43337e9ed539 100644
> --- a/fs/ceph/mdsmap.h
> +++ b/fs/ceph/mdsmap.h
> @@ -27,7 +27,11 @@ struct ceph_mdsmap {
>         u32 m_session_timeout;          /* seconds */
>         u32 m_session_autoclose;        /* seconds */
>         u64 m_max_file_size;
> -       u64 m_max_xattr_size;           /* maximum size for xattrs blob */
> +       /*
> +        * maximum size for xattrs blob.
> +        * Setting it to 0 will force the usage of the (sync) SETXATTR Op.

Minor nit, I would reword as: "Zeroed by default to force the usage of
the (sync) SETXATTR Op."

Reviewed-by: Patrick Donnelly <pdonnell@ibm.com>

On 2/21/24 11:34, Patrick Donnelly wrote:
> On Tue, Feb 20, 2024 at 10:13 PM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Ceph added the bal_rank_mask with encoded (ev) version 17.  This
>> was merged into main Oct 2022 and made it into the reef release
>> normally. While a latter commit added the max_xattr_size also
>> with encoded (ev) version 17 but places it before bal_rank_mask.
>>
>> And this will breaks some usages, for example when upgrading old
>> cephs to newer versions.
>>
>> URL: https://tracker.ceph.com/issues/64440
>> Reported-by: Patrick Donnelly <pdonnell@redhat.com>
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>
>> V2:
>> - minor fix in the comment
>>
>>
>>
>>
>>
>>   fs/ceph/mdsmap.c | 7 ++++---
>>   fs/ceph/mdsmap.h | 6 +++++-
>>   2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ceph/mdsmap.c b/fs/ceph/mdsmap.c
>> index fae97c25ce58..8109aba66e02 100644
>> --- a/fs/ceph/mdsmap.c
>> +++ b/fs/ceph/mdsmap.c
>> @@ -380,10 +380,11 @@ struct ceph_mdsmap *ceph_mdsmap_decode(struct ceph_mds_client *mdsc, void **p,
>>                  ceph_decode_skip_8(p, end, bad_ext);
>>                  /* required_client_features */
>>                  ceph_decode_skip_set(p, end, 64, bad_ext);
>> +               /* bal_rank_mask */
>> +               ceph_decode_skip_string(p, end, bad_ext);
>> +       }
>> +       if (mdsmap_ev >= 18) {
>>                  ceph_decode_64_safe(p, end, m->m_max_xattr_size, bad_ext);
>> -       } else {
>> -               /* This forces the usage of the (sync) SETXATTR Op */
>> -               m->m_max_xattr_size = 0;
>>          }
>>   bad_ext:
>>          doutc(cl, "m_enabled: %d, m_damaged: %d, m_num_laggy: %d\n",
>> diff --git a/fs/ceph/mdsmap.h b/fs/ceph/mdsmap.h
>> index 89f1931f1ba6..43337e9ed539 100644
>> --- a/fs/ceph/mdsmap.h
>> +++ b/fs/ceph/mdsmap.h
>> @@ -27,7 +27,11 @@ struct ceph_mdsmap {
>>          u32 m_session_timeout;          /* seconds */
>>          u32 m_session_autoclose;        /* seconds */
>>          u64 m_max_file_size;
>> -       u64 m_max_xattr_size;           /* maximum size for xattrs blob */
>> +       /*
>> +        * maximum size for xattrs blob.
>> +        * Setting it to 0 will force the usage of the (sync) SETXATTR Op.
> Minor nit, I would reword as: "Zeroed by default to force the usage of
> the (sync) SETXATTR Op."

Sure, will revise this.

Thanks Patrick and Venky.

- Xiubo


> Reviewed-by: Patrick Donnelly <pdonnell@ibm.com>
>