On Mon, Apr 30, 2012 at 03:54:55PM +0800, Liu Yuan wrote:
> + nr_copies = get_nr_copies(dw->vnodes);
> + if (nr_copies > inode->nr_copies)
> + nr_copies = inode->nr_copies;
> +
> ret = read_object(dw->vnodes, dw->epoch, vid_to_vdi_oid(vdi_id),
> (void *)inode, sizeof(*inode),
> - 0, sys->nr_sobjs);
> + 0, nr_copies);
> I think it is really kind of mess and upset me. If we don't get a
> overall understanding of the code, we shouldn't change the old code
> WITHOUT a good reason.
I think the change very much makese sense modulo the bug fixed in this
patch.
Let's get back and think about how nr_copies handling works now and
should work.
On the VDI level:
1) the user specifies a nr_copies level in the vdi creation call
2) we need to limit by nr_zones as an upper ceiling. If the limit
is not handled we could either
a) refuse the creation or
b) only create with the higher number set in the inode, but
limiting the actual I/O later on
3) all other VDI level I/O should use inode->nr_vnodes and limit it
by nr_zones
The big question is if we want to limit it by sys->nr_copies too, my
patch did that because it makes little sense to create an object with
more redudancy if system objects like the inode don't have it. I think
the big failure was to not document it. We could also allow higher
redundancy, but I don't see a real use case and it makes the code
harder.
Now for the actual data I/O we expect the client to set hdr.copies
from inode->nr_copies, which the client does, but we don't really
respect it in sheep right now except for a single place in
forward_read_obj_req.
|
|