diff mbox

[22/33] libceph: primary_affinity decode bits

Message ID 1395944299-21970-23-git-send-email-ilya.dryomov@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ilya Dryomov March 27, 2014, 6:18 p.m. UTC
Add two helpers to decode primary_affinity (full map, vector<u32>) and
new_primary_affinity (inc map, map<u32, u32>) and switch to them.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
---
 net/ceph/osdmap.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

Alex Elder March 27, 2014, 8:31 p.m. UTC | #1
On 03/27/2014 01:18 PM, Ilya Dryomov wrote:
> Add two helpers to decode primary_affinity (full map, vector<u32>) and
> new_primary_affinity (inc map, map<u32, u32>) and switch to them.

One comment below, but otherwise looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> 
> Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
> ---
>  net/ceph/osdmap.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 538b8dd341e8..3ac2098972ea 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -959,6 +959,59 @@ static int set_primary_affinity(struct ceph_osdmap *map, int osd, u32 aff)
>  	return 0;
>  }
>  
> +static int decode_primary_affinity(void **p, void *end,
> +				   struct ceph_osdmap *map)
> +{
> +	u32 len, i;
> +
> +	ceph_decode_32_safe(p, end, len, e_inval);
> +	if (len == 0) {
> +		kfree(map->osd_primary_affinity);
> +		map->osd_primary_affinity = NULL;
> +		return 0;
> +	}
> +
> +	ceph_decode_need(p, end, map->max_osd*sizeof(u32), e_inval);
> +
> +	BUG_ON(len != map->max_osd);

BUG() here is too much; it should I think just return an error instead.
The test could be done earlier too, prior to ceph_decode_need().

> +	for (i = 0; i < map->max_osd; i++) {
> +		int ret;
> +
> +		ret = set_primary_affinity(map, i, ceph_decode_32(p));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +
> +e_inval:
> +	return -EINVAL;
> +}
> +
> +static int decode_new_primary_affinity(void **p, void *end,
> +				       struct ceph_osdmap *map)
> +{
> +	u32 n;
> +
> +	ceph_decode_32_safe(p, end, n, e_inval);
> +	while (n--) {
> +		u32 osd, aff;
> +		int ret;
> +
> +		ceph_decode_32_safe(p, end, osd, e_inval);
> +		ceph_decode_32_safe(p, end, aff, e_inval);
> +
> +		ret = set_primary_affinity(map, osd, aff);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +
> +e_inval:
> +	return -EINVAL;
> +}
> +
>  /*
>   * decode a full map.
>   */
> @@ -1036,6 +1089,17 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
>  			goto bad;
>  	}
>  
> +	/* primary_affinity */
> +	if (struct_v >= 2) {
> +		err = decode_primary_affinity(p, end, map);
> +		if (err)
> +			goto bad;
> +	} else {
> +		/* XXX can this happen? */
> +		kfree(map->osd_primary_affinity);
> +		map->osd_primary_affinity = NULL;
> +	}
> +
>  	/* crush */
>  	ceph_decode_32_safe(p, end, len, e_inval);
>  	map->crush = crush_decode(*p, min(*p + len, end));
> @@ -1243,6 +1307,13 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
>  			goto bad;
>  	}
>  
> +	/* new_primary_affinity */
> +	if (struct_v >= 2) {
> +		err = decode_new_primary_affinity(p, end, map);
> +		if (err)
> +			goto bad;
> +	}
> +
>  	/* ignore the rest */
>  	*p = end;
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ilya Dryomov March 28, 2014, 3:01 p.m. UTC | #2
On Thu, Mar 27, 2014 at 10:31 PM, Alex Elder <elder@ieee.org> wrote:
> On 03/27/2014 01:18 PM, Ilya Dryomov wrote:
>> Add two helpers to decode primary_affinity (full map, vector<u32>) and
>> new_primary_affinity (inc map, map<u32, u32>) and switch to them.
>
> One comment below, but otherwise looks good.
>
> Reviewed-by: Alex Elder <elder@linaro.org>
>
>>
>> Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
>> ---
>>  net/ceph/osdmap.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 71 insertions(+)
>>
>> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
>> index 538b8dd341e8..3ac2098972ea 100644
>> --- a/net/ceph/osdmap.c
>> +++ b/net/ceph/osdmap.c
>> @@ -959,6 +959,59 @@ static int set_primary_affinity(struct ceph_osdmap *map, int osd, u32 aff)
>>       return 0;
>>  }
>>
>> +static int decode_primary_affinity(void **p, void *end,
>> +                                struct ceph_osdmap *map)
>> +{
>> +     u32 len, i;
>> +
>> +     ceph_decode_32_safe(p, end, len, e_inval);
>> +     if (len == 0) {
>> +             kfree(map->osd_primary_affinity);
>> +             map->osd_primary_affinity = NULL;
>> +             return 0;
>> +     }
>> +
>> +     ceph_decode_need(p, end, map->max_osd*sizeof(u32), e_inval);
>> +
>> +     BUG_ON(len != map->max_osd);
>
> BUG() here is too much; it should I think just return an error instead.
> The test could be done earlier too, prior to ceph_decode_need().

Moved the test, changed to returning -EINVAL.

Thanks,

                Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index 538b8dd341e8..3ac2098972ea 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -959,6 +959,59 @@  static int set_primary_affinity(struct ceph_osdmap *map, int osd, u32 aff)
 	return 0;
 }
 
+static int decode_primary_affinity(void **p, void *end,
+				   struct ceph_osdmap *map)
+{
+	u32 len, i;
+
+	ceph_decode_32_safe(p, end, len, e_inval);
+	if (len == 0) {
+		kfree(map->osd_primary_affinity);
+		map->osd_primary_affinity = NULL;
+		return 0;
+	}
+
+	ceph_decode_need(p, end, map->max_osd*sizeof(u32), e_inval);
+
+	BUG_ON(len != map->max_osd);
+	for (i = 0; i < map->max_osd; i++) {
+		int ret;
+
+		ret = set_primary_affinity(map, i, ceph_decode_32(p));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+
+e_inval:
+	return -EINVAL;
+}
+
+static int decode_new_primary_affinity(void **p, void *end,
+				       struct ceph_osdmap *map)
+{
+	u32 n;
+
+	ceph_decode_32_safe(p, end, n, e_inval);
+	while (n--) {
+		u32 osd, aff;
+		int ret;
+
+		ceph_decode_32_safe(p, end, osd, e_inval);
+		ceph_decode_32_safe(p, end, aff, e_inval);
+
+		ret = set_primary_affinity(map, osd, aff);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+
+e_inval:
+	return -EINVAL;
+}
+
 /*
  * decode a full map.
  */
@@ -1036,6 +1089,17 @@  static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
 			goto bad;
 	}
 
+	/* primary_affinity */
+	if (struct_v >= 2) {
+		err = decode_primary_affinity(p, end, map);
+		if (err)
+			goto bad;
+	} else {
+		/* XXX can this happen? */
+		kfree(map->osd_primary_affinity);
+		map->osd_primary_affinity = NULL;
+	}
+
 	/* crush */
 	ceph_decode_32_safe(p, end, len, e_inval);
 	map->crush = crush_decode(*p, min(*p + len, end));
@@ -1243,6 +1307,13 @@  struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
 			goto bad;
 	}
 
+	/* new_primary_affinity */
+	if (struct_v >= 2) {
+		err = decode_new_primary_affinity(p, end, map);
+		if (err)
+			goto bad;
+	}
+
 	/* ignore the rest */
 	*p = end;