On 04/13/2012 11:40 PM, Christoph Hellwig wrote: > On Fri, Apr 13, 2012 at 09:54:14PM +0800, Liu Yuan wrote: >> From: Liu Yuan <tailai.ly at taobao.com> >> >> We should pass nr_vnodes to the obj_to_sheep() > > Seems like it should to prevent problems if not enough zones are > availably during recovery. Is that true? A slightly more detailed > explanation of patches would be really useful, as would test cases > (even if the latter might be a bit harder to produce). > Yes, but not only in recovery process, there is case that number of available nodes or zones is less than 'copies', where we shouldn't pass non-existent copy index to obj_to_sheep(). This is a apparent bug that pass the wrong parameter to the obj_to_sheep() for second argument. That is, we should pass number of vnodes instead of nr of copies. Anyway, I think obj_to_sheep(), which calculate the object location from the hash ring, need to be refined: 1) it would better handle the case 'nodes(zones) < copies' internally, not demand the caller to do it. Because if caller doesn't do it, it will result in panic when it happens. 2) get a better name, such as int object_to_ring_index(oid, vnodes, nr_vnodes, copy_idx). Thanks, Yuan |