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