Re: [PATCH 02/10] ceph: add start/finish encoding helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/01/2015 02:39 PM, Mike Christie wrote:
> On 04/30/2015 07:22 AM, Alex Elder wrote:
>>> +/**
>>> + * ceph_start_decoding_compat - decode block with legacy support for
>>> older schemes
>>> + * @p: buffer to decode
>>> + * @end: end of decode buffer
>>> + * @curr_ver: current version of the encoding that the code
>>> supports/encode
>>> + * @compat_ver: oldest version that includes a __u8 compat version field
>>> + * @len_ver: oldest version that includes a __u32 length wrapper
>>> + * @len: buffer to return len of data in buffer
>>> + */
>>> +static inline int ceph_start_decoding_compat(void **p, void *end, u8
>>> curr_ver,
>>> +                         u8 compat_ver, u8 len_ver, u32 *len)
>>> +{
>>> +    u8 struct_ver, struct_compat;
>>> +
>>> +    ceph_decode_8_safe(p, end, struct_ver, fail);
>>> +    if (struct_ver >= curr_ver) {
>>
>> It seems to me it doesn't matter whether the current code
>> supports the struct compat version or not.  What matters
>> is whether the encoded header contains the compat field
>> or not.  I'm not sure what determines that.
> 
> I am not sure if I understood this comment.
> 
> I thought different structs got the compat field in different versions.
> So, I was concerned about a case where we might get a old struct sent to
> us. If the compat field was added to some struct_abc in version 2 and
> that is what we support in the kernel, but some old osd sent us a struct
> that was version 1, then we do not want to do the compat check below.
> 

Doh! I wrote the above mail, then realized what you meant.

I think I should have checked the compat_ver passed into the version above.

if (struct_ver >= compat_ver) {
	ceph_decode_8_safe(p, end, struct_compat, fail);
	if (curr_ver < struct_compat)

> 
>>
>>> +        ceph_decode_8_safe(p, end, struct_compat, fail);
>>> +        if (curr_ver < struct_compat)
>>> +            return -EINVAL;
>>> +    }
>>> +
>>> +    if (struct_ver >= len_ver) {
>>> +        ceph_decode_32_safe(p, end, *len, fail);
>>> +    } else {
>>> +        *len = 0;
>>> +    }
>>> +
>>> +    return 0;
>>> +fail:
>>> +    return -ERANGE;
>>> +}
>>> +
>>> +
>>>   #define ceph_encode_need(p, end, n, bad)            \
>>>       do {                            \
>>>           if (!likely(ceph_has_room(p, end, n)))        \
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux