CEPH-Devel archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask
@ 2024-02-21  3:10 xiubli
  2024-02-21  3:34 ` Patrick Donnelly
  2024-02-21  3:56 ` Venky Shankar
  0 siblings, 2 replies; 4+ messages in thread
From: xiubli @ 2024-02-21  3:10 UTC (permalink / raw
  To: ceph-devel
  Cc: idryomov, jlayton, vshankar, mchangir, Xiubo Li, Patrick Donnelly

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.
+	 */
+	u64 m_max_xattr_size;
 	u32 m_max_mds;			/* expected up:active mds number */
 	u32 m_num_active_mds;		/* actual up:active mds number */
 	u32 possible_max_rank;		/* possible max rank index */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask
  2024-02-21  3:10 [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask xiubli
@ 2024-02-21  3:34 ` Patrick Donnelly
  2024-02-21  4:45   ` Xiubo Li
  2024-02-21  3:56 ` Venky Shankar
  1 sibling, 1 reply; 4+ messages in thread
From: Patrick Donnelly @ 2024-02-21  3:34 UTC (permalink / raw
  To: xiubli; +Cc: ceph-devel, idryomov, jlayton, vshankar, mchangir

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>

-- 
Patrick Donnelly, Ph.D.
He / Him / His
Red Hat Partner Engineer
IBM, Inc.
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask
  2024-02-21  3:10 [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask xiubli
  2024-02-21  3:34 ` Patrick Donnelly
@ 2024-02-21  3:56 ` Venky Shankar
  1 sibling, 0 replies; 4+ messages in thread
From: Venky Shankar @ 2024-02-21  3:56 UTC (permalink / raw
  To: xiubli; +Cc: ceph-devel, idryomov, jlayton, mchangir, Patrick Donnelly

On Wed, Feb 21, 2024 at 8:43 AM <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.
> +        */
> +       u64 m_max_xattr_size;
>         u32 m_max_mds;                  /* expected up:active mds number */
>         u32 m_num_active_mds;           /* actual up:active mds number */
>         u32 possible_max_rank;          /* possible max rank index */
> --
> 2.43.0
>

Reviewed-by: Venky Shankar <vshankar@redhat.com>

-- 
Cheers,
Venky


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask
  2024-02-21  3:34 ` Patrick Donnelly
@ 2024-02-21  4:45   ` Xiubo Li
  0 siblings, 0 replies; 4+ messages in thread
From: Xiubo Li @ 2024-02-21  4:45 UTC (permalink / raw
  To: Patrick Donnelly; +Cc: ceph-devel, idryomov, jlayton, vshankar, mchangir


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>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-21  4:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-21  3:10 [PATCH v2] ceph: reverse MDSMap dencoding of max_xattr_size/bal_rank_mask xiubli
2024-02-21  3:34 ` Patrick Donnelly
2024-02-21  4:45   ` Xiubo Li
2024-02-21  3:56 ` Venky Shankar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).