[for-1.2] osdep: Fix compilation failure on BSD systems

Fix compilation failure on BSD systems (which don't have
O_DIRECT or O_NOATIME:
osdep.c:116: error: ‘O_DIRECT’ undeclared (first use in this function)
osdep.c:116: error: (Each undeclared identifier is reported only once
osdep.c:116: error: for each function it appears in.)
osdep.c:116: error: ‘O_NOATIME’ undeclared (first use in this function)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
PS: Do we care about O_DSYNC, O_RSYNC, O_SYNC? POSIX says those can be
used via fcntl() too...

 osdep.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

On 16 August 2012 13:27, Stefan Weil <sw@weilnetz.de> wrote:
> Am 16.08.2012 13:15, schrieb Peter Maydell:
>
>> Fix compilation failure on BSD systems (which don't have
>> O_DIRECT or O_NOATIME:
>> osdep.c:116: error: ‘O_DIRECT’ undeclared (first use in this function)
>> osdep.c:116: error: (Each undeclared identifier is reported only once
>> osdep.c:116: error: for each function it appears in.)
>> osdep.c:116: error: ‘O_NOATIME’ undeclared (first use in this function)
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> PS: Do we care about O_DSYNC, O_RSYNC, O_SYNC? POSIX says those can be
>> used via fcntl() too...
>>
>>   osdep.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/osdep.c b/osdep.c
>> index 5b78cee..3b25297 100644
>> --- a/osdep.c
>> +++ b/osdep.c
>> @@ -113,7 +113,13 @@ static int qemu_dup_flags(int fd, int flags)
>>       }
>>         /* Set/unset flags that we can with fcntl */
>> -    setfl_flags = O_APPEND | O_ASYNC | O_DIRECT | O_NOATIME | O_NONBLOCK;
>> +    setfl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
>> +#ifdef O_NOATIME
>> +    setfl_flags |= O_NOATIME;
>> +#endif
>> +#ifdef O_DIRECT
>> +    setfl_flags |= O_DIRECT;
>> +#endif
>>       dup_flags &= ~setfl_flags;
>>       dup_flags |= (flags & setfl_flags);
>>       if (fcntl(ret, F_SETFL, dup_flags) == -1) {
>
>
> Would O_DSYNC be a good replacement here for an
> undefined O_DIRECT? block/raw-posix.c does it like that.

I think it would be better to have this function handle
O_DSYNC as a flag to go in setfl_flags independently of
whether O_DIRECT is defined or not; at this level I don't
think it makes sense to try to #define O_DIRECT O_DSYNC
(we are interpreting flags given us by a caller, not picking
flags to try to get the behaviour we want from a function
we're going to call).

> What about defining O_NOATIME and O_DIRECT in qemu-common.h
> when needed (like it is done for O_BINARY)?

Again, I think that would be a fix at the wrong level: it
might be OK if we wanted to allow people to call this function
with those flags even if the host OS didn't know about them,
but it's not what you need to make the function itself
perform correctly. What we might want is to add
#ifdef O_DSYNC
    setfl_flags |= O_DSYNC;
#endif
etc, but I'm not sure about that.

> But I don't want to delay 1.2, therefore
>
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

Thanks; this is indeed really just a minimal "make it compile"
fix.

-- PMM

On 08/16/2012 07:15 AM, Peter Maydell wrote:
> Fix compilation failure on BSD systems (which don't have
> O_DIRECT or O_NOATIME:
> osdep.c:116: error: ‘O_DIRECT’ undeclared (first use in this function)
> osdep.c:116: error: (Each undeclared identifier is reported only once
> osdep.c:116: error: for each function it appears in.)
> osdep.c:116: error: ‘O_NOATIME’ undeclared (first use in this function)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> PS: Do we care about O_DSYNC, O_RSYNC, O_SYNC? POSIX says those can be
> used via fcntl() too...

Thanks very much Peter.  The patch looks good to me.

Could you point me to the reference you saw the fcntl description in?  I 
didn't notice any flags mentioned here: 
http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html

The choice of flags was based on the Linux man page for fcntl() which 
says F_SETFL can change only the O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, 
and O_NONBLOCK flags.

On 16 August 2012 14:11, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 08/16/2012 07:15 AM, Peter Maydell wrote:
>>
>> Fix compilation failure on BSD systems (which don't have
>> O_DIRECT or O_NOATIME:
>> osdep.c:116: error: ‘O_DIRECT’ undeclared (first use in this function)
>> osdep.c:116: error: (Each undeclared identifier is reported only once
>> osdep.c:116: error: for each function it appears in.)
>> osdep.c:116: error: ‘O_NOATIME’ undeclared (first use in this function)
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> PS: Do we care about O_DSYNC, O_RSYNC, O_SYNC? POSIX says those can be
>> used via fcntl() too...
>
>
> Thanks very much Peter.  The patch looks good to me.
>
> Could you point me to the reference you saw the fcntl description in?  I
> didn't notice any flags mentioned here:
> http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html
>
> The choice of flags was based on the Linux man page for fcntl() which says
> F_SETFL can change only the O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, and
> O_NONBLOCK flags.

The posix spec for fcntl.h
http://pubs.opengroup.org/onlinepubs/007908799/xsh/fcntl.h.html
describes all of O_APPEND O_DSYNC O_NONBLOCK O_RSYNC O_SYNC
as "File status flags used for open() and fcntl()".

(However the MacOS fcntl manpage lists only O_APPEND, O_ASYNC and
O_NONBLOCK for F_GETFL/F_SETFL flags.)

-- PMM