Re: [Qemu-devel] [PATCH 2/2] osdep: warn if open(O_DIRECT) on fails with EINVAL

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

On 08/22/2013 01:31 PM, Alex Bligh wrote:
> 
> 
> --On 22 August 2013 11:57:56 -0600 Eric Blake <eblake@redhat.com> wrote:
> 
>> # define O_DIRECT 0
>>
>> so that the rest of the code can just blindly use open(...,|O_DIRECT)
>> (provided, of course, that not having O_DIRECT semantics is not
>> fatal...).  If that is done, then this #ifdef will always be true...
> 
> I think this is undesirable as the result of opening without O_DIRECT
> when you really wanted O_DIRECT could be subtle data corruption due
> to unexpected caching. Is an error not more appropriate here than
> proceeding regardless?

As I said in the surrounding text you snipped:

> 
> On some other projects I've worked with (hello gnulib!), the
> compatibility headers do:
...
> 
> But enough of that side diversion - that one #define of O_DIRECT is not
> related to the file you are touching. 

Qemu doesn't define O_DIRECT to 0 for the mere sake of compilation, and
I'm not arguing that it should.  I was more making sure that this patch
was correct, by reassuring myself the policies that qemu uses for
O_DIRECT (and since I demonstrated that other projects have other
policies).  As to whether other projects may have a bug when using
O_DIRECT being 0, it is a problem for those projects to deal with.
Besides, O_DIRECT is a non-POSIX extension, so anyone using it already
has non-portability issues to think about, regardless of whether the
fallback for platforms that lack it is done by always defining to 0 or
always using #ifdef guards.

Thus, my diversion about other projects using O_DIRECT as 0 is a non-issue.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]