| Message ID | 20200701160509.1523847-2-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: improve error reporting for unsupported O_DIRECT | expand |
On 7/1/20 6:05 PM, Daniel P. Berrangé wrote: > Currently we suggest that a filesystem may not support O_DIRECT after > seeing an EINVAL. Other things can cause EINVAL though, so it is better > to do an explicit check, and then report a definitive error message. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > util/osdep.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/util/osdep.c b/util/osdep.c > index 4829c07ff6..4bdbe81cec 100644 > --- a/util/osdep.c > +++ b/util/osdep.c > @@ -342,8 +342,17 @@ int qemu_open(const char *name, int flags, ...) > > #ifdef O_DIRECT > if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) { > - error_report("file system may not support O_DIRECT"); > - errno = EINVAL; /* in case it was clobbered */ > + int newflags = flags & ~O_DIRECT; > +# ifdef O_CLOEXEC > + ret = open(name, newflags | O_CLOEXEC, mode); > +# else > + ret = open(name, newflags, mode); > +# endif Can we use this simpler form (easier to set a breakpoint)? #ifdef O_CLOEXEC newflags |= O_CLOEXEC; #endif ret = open(name, newflags, mode); Whichever you prefer: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + if (ret != -1) { > + close(ret); > + error_report("file system does not support O_DIRECT"); > + errno = EINVAL; > + } > } > #endif /* O_DIRECT */ > >
diff --git a/util/osdep.c b/util/osdep.c index 4829c07ff6..4bdbe81cec 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -342,8 +342,17 @@ int qemu_open(const char *name, int flags, ...) #ifdef O_DIRECT if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) { - error_report("file system may not support O_DIRECT"); - errno = EINVAL; /* in case it was clobbered */ + int newflags = flags & ~O_DIRECT; +# ifdef O_CLOEXEC + ret = open(name, newflags | O_CLOEXEC, mode); +# else + ret = open(name, newflags, mode); +# endif + if (ret != -1) { + close(ret); + error_report("file system does not support O_DIRECT"); + errno = EINVAL; + } } #endif /* O_DIRECT */
Currently we suggest that a filesystem may not support O_DIRECT after seeing an EINVAL. Other things can cause EINVAL though, so it is better to do an explicit check, and then report a definitive error message. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- util/osdep.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)