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

[Hitoshi Mitake]

At Wed, 17 Apr 2013 14:08:24 +0800,
Liu Yuan wrote:
> 
> 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.

I missed the point, thanks.

> 
> 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.

OK, I admit the panic() is an overkill. I'll let
atomic_create_and_write() return -1 in such a case simply in v2.

Thanks,
Hitoshi