Subject: Re: tree corrupted on disk quota full

From: Andy Whitcroft <[email-masked]>
Date: Thu, 11 Jan 2007 21:17:51 +0000

Linus Torvalds wrote:
> 
> On Thu, 11 Jan 2007, Linus Torvalds wrote:
>> That said, clearly something didn't check the error return of a write() 
>> call. Some of that got fixed up recently, so it might even be fixed in 
>> current git already.
> 
> I'm not convinced.
> 
> The "write_in_full()" logic is supposed to help people avoid problems, but 
> it *still* returns a success for a partial write.
> 
> Example of extreme breakage:
> 
> 	static int write_buffer(int fd, const void *buf, size_t len)
> 	{
> 	        ssize_t size;
> 	
> 	        size = write_in_full(fd, buf, len);
> 	        if (!size)
> 	                return error("file write: disk full");
> 	        if (size < 0)
> 	                return error("file write error (%s)", strerror(errno));
> 	        return 0;
> 	}
> 
> which is TOTAL GARBAGE, because the disk-full event might have happened in 
> the middle of the write, so "write_in_full()" might have returned 4096, 
> for example, even though the buffer length was much bigger.
> 
> I personally think write_in_full() is totally mis-designed. If you are 
> ready to handle partial writes, you should use "xwrite()". If you're not 
> ready to handle partial writes, you should either use "write_or_die()", 
> _or_ you should expect a partial write to at least return an error code 
> (which is how "write_buffer()" works).
> 
> But that's not how write_in_full() actually works. Write-in-full does not 
> return an error for a partial write, it returns the partial size.
> 
> Which is idiotic. It makes the function pointless. Just use xwrite() for 
> that.

The call was intended to replace a common idiom:

if (xwrite(fd, buf, size) != size)
	error

write_in_full() is intended as a drop in replacement for that.  On a
short write we will return a short count and that will fail such a test.
 The call basically implements the standard write() call semantic with
maximum attempts to complete.

That said returning -1 would have the same effect.

-apw