os/filestore: require offset == length == 0 for full object read; add test

[majianpeng]

Like FileStore, length=0 mean length=object.size.

Signed-off-by: Jianpeng Ma jianpeng.ma@intel.com

[tchaikov]

s/lenght/length/

[liewegas]

I don't think this is right.. if I do offset 2000 and length 0 i do'nt want to read the object size from offset 2000. I think only offset == length == 0 makes sense.

BTW, is there store_test.cc test that covers this case?

[majianpeng]

@liewegas .
In ReplicatedPG::do_osd_ops:

case CEPH_OSD_OP_READ:
if (op.extent.length == 0) //length is zero mean read the whole object
op.extent.length = size;

In FileStore::read

if (len == 0) {
struct stat st;
memset(&st, 0, sizeof(struct stat));
int r = ::fstat(**fd, &st);
assert(r == 0);
len = st.st_size;
}

I don't find test case in store_test.cc.
Long long ago, i wanted to remove code for FileStore::read. But you or sam point background need this. But i don't find those again!

[liewegas]

I guess the good news is the do_osd_ops code already populates the length, so we don't need to worry about it (although I wish it also checked that offset == 0). Can you add a store_test.cc test case that passes offset == len == 0? Let's enshrine that as the correct behavior for ObjectStore, at least. And in another patch let's add an offset == 0 check for FileStore. The only callers that pass len 0 won't be coming from do_osd_ops so I think we're safe there.

[ifed01]

Probably I missed something but I don't see any changes at bluestore.cc at the moment. Only Filestore.cc is updated. Is this correct?

[ifed01]

Answering myself - that's correct. Just PR name is confusing...