[sheepdog] [PATCH 2/3] util:add a new API atomic_create_and_write()

Liu Yuan namei.unix at gmail.com
Wed Apr 17 08:08:24 CEST 2013


On 04/17/2013 02:00 PM, Hitoshi Mitake wrote:
> At Wed, 17 Apr 2013 13:55:09 +0800,
> Liu Yuan wrote:
>>
>> On 04/17/2013 01:45 PM, Hitoshi Mitake wrote:
>>> +	if (!access(tmp_path, F_OK))
>>> +		panic("temporal file: %s exists, this implies invalid usage of"
>>> +			" atomic_create_and_write() of sheepdog", tmp_path);
>>
>> panic() is too stringent and not necessary. open(2) will return error if
>> it has any problem with the tmp_path. So I'd suggest remove this if clause.
> 
> I admit the access(2) is unnecesary because open(2) can fail with
> EEXIST. But I believe this panic() is required because existing
> temporal files imply misuage of the API. I believe we should employ
> defensive way.
> 
> How do you think?
> 

See "EEXIST pathname already exists and O_CREAT and O_EXCL were used."

If you don't specify O_EXCL, we won't get this errno.

And actually it is a abuse of panic(), panic() is very fatal problem
that will cause damage of data if we allow sheep proceed. In our case,
even if we failed to do atomic_create_and_write(), it is up to the
caller to decide what actions he should take.

We have to keep in mind very closely that panic() will cause all VMs to
fail on this node without knowing anything happening in sheep and
possibly will have a breakage of data integrity in VM's FS.

Thanks,
Yuan




More information about the sheepdog mailing list