[Sheepdog] [PATCH 1/3] sheep: unify cow object and regular object writing path

[Liu Yuan]

On 11/21/2011 06:26 PM, Christoph Hellwig wrote:

> On Sun, Nov 20, 2011 at 07:11:25PM +0800, Liu Yuan wrote:
>> From: Liu Yuan <tailai.ly at taobao.com>
>>
>> This is necessary to do further unifying of sheep requests handling.
>>
>> small changes on other:
>>  - rename read_from_one into do_read_copy and make it return sd result.
>>  - rename read_from_other_sheep into read_copy_from_cluster
> 
> Is there any good reason to even keep read_from_other_sheep /
> read_from_other_sheep instead of opencoding it?  I'd rather merge it
> into read_from_one / do_read_copy, and while at it make the length
> parameter to it a normal scalar argument instead of a pointer, and
> reject short reads.
> 


Umm, I was just a little bit slack in it and didn't try to restructure
those functions internally and just renamed it for a shirk.

Okay, I'll refine it in next series.

>> @@ -636,7 +635,7 @@ static int store_write_obj(struct request *req, uint32_t epoch)
>>  	return ret;
>>  }
>>  
>> -static int store_create_and_write_obj_cow(struct request *req, uint32_t epoch)
>> +static int store_create_and_write_obj(struct request *req, uint32_t epoch)
>>  {
>>  	struct sd_obj_req *hdr = (struct sd_obj_req *)&req->rq;
>>  	int ret;
>> @@ -654,30 +653,24 @@ static int store_create_and_write_obj_cow(struct request *req, uint32_t epoch)
>>  	ret = store.open(hdr->oid, &iocb, 1);
>>  	if (ret != SD_RES_SUCCESS)
>>  		return ret;
>> -
>> -	dprintf("%" PRIu64 ", %" PRIx64 "\n", hdr->oid, hdr->cow_oid);
>> -
>> -	buf = valloc(SD_DATA_OBJ_SIZE);
>> -	if (!buf) {
>> -		eprintf("failed to allocate memory\n");
>> -		ret = SD_RES_NO_MEM;
>> -		goto out;
>> -	}
>> -	ret = read_from_other_sheep(req, hdr->epoch, hdr->cow_oid, buf,
>> -				     hdr->copies);
>> -	if (ret) {
>> -		eprintf("failed to read old object\n");
>> -		ret = SD_RES_EIO;
>> -		goto out;
>> -	}
>> -	iocb.buf = buf;
>> -	iocb.length = SD_DATA_OBJ_SIZE;
>> -	iocb.offset = 0;
>> -	ret = store.write(hdr->oid, &iocb);
>> -	if (ret != SD_RES_SUCCESS) {
>> -		goto out;
>> +	if (hdr->flags & SD_FLAG_CMD_COW) {
>> +		dprintf("%" PRIu64 ", %" PRIx64 "\n", hdr->oid, hdr->cow_oid);
>> +
>> +		buf = xzalloc(SD_DATA_OBJ_SIZE);
> 
> This seems to drop error handling for the memory allocation
> 


Those x* helpers call try_to_free_routine() to free memory when memory
fell short and if ever no free memory, simply panic out. and Currently
it is a do_nothing function.

maybe it is better to return error code instead of panic out?

Thanks,
Yuan