| Message ID | 1345115718-4054-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
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. What about defining O_NOATIME and O_DIRECT in qemu-common.h when needed (like it is done for O_BINARY)? But I don't want to delay 1.2, therefore Reviewed-by: Stefan Weil <sw@weilnetz.de> Regards, Stefan W.
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
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) {
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(-)