| Message ID | 20200903152210.1917355-1-berrange@redhat.com |
|---|---|
| Headers | show |
| Series | block: improve error reporting for unsupported O_DIRECT | expand |
On 9/3/20 8:22 AM, Daniel P. Berrangé wrote: > Currently code has to call monitor_fdset_get_fd, then dup > the return fd, and then add the duplicate FD back into the > fdset. This dance is overly verbose for the caller and > introduces extra failure modes which can be avoided by > folding all the logic into monitor_fdset_dup_fd_add and > removing monitor_fdset_get_fd entirely. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > include/monitor/monitor.h | 3 +- > include/qemu/osdep.h | 1 + > monitor/misc.c | 58 +++++++++++++++++---------------------- > stubs/fdset.c | 8 ++---- > util/osdep.c | 19 ++----------- > 5 files changed, 32 insertions(+), 57 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 9/3/20 8:22 AM, Daniel P. Berrangé wrote: > We want to introduce a new version of qemu_open() that uses an Error > object for reporting problems and make this it the preferred interface. > Rename the existing method to release the namespace for the new impl. > > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 9/3/20 8:22 AM, Daniel P. Berrangé wrote: > qemu_open_old() works like open(): set errno and return -1 on failure. > It has even more failure modes, though. Reporting the error clearly > to users is basically impossible for many of them. > > Our standard cure for "errno is too coarse" is the Error object. > Introduce two new helper methods: > > int qemu_open(const char *name, int flags, Error **errp); > int qemu_create(const char *name, int flags, mode_t mode, Error **errp); > > Note that with this design we no longer require or even accept the > O_CREAT flag. Avoiding overloading the two distinct operations > means we can avoid variable arguments which would prevent 'errp' from > being the last argument. It also gives us a guarantee that the 'mode' is > given when creating files, avoiding a latent security bug. > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 9/3/20 8:22 AM, Daniel P. Berrangé wrote: > Currently at startup if using cache=none on a filesystem lacking > O_DIRECT such as tmpfs, at startup QEMU prints > > qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: file system may not support O_DIRECT > qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: Could not open '/tmp/foo.img': Invalid argument > > while at QMP level the hint is missing, so QEMU reports just > > "error": { > "class": "GenericError", > "desc": "Could not open '/tmp/foo.img': Invalid argument" > } > > which is close to useless for the end user trying to figure out what > they did wrong. > > With this change at startup QEMU prints > > qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: Unable to open '/tmp/foo.img': filesystem does not support O_DIRECT > > while at the QMP level QEMU reports a massively more informative > > "error": { > "class": "GenericError", > "desc": "Unable to open '/tmp/foo.img': filesystem does not support O_DIRECT" > } > > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Daniel P. Berrangé <berrange@redhat.com> writes: > Currently code has to call monitor_fdset_get_fd, then dup > the return fd, and then add the duplicate FD back into the > fdset. This dance is overly verbose for the caller and > introduces extra failure modes which can be avoided by > folding all the logic into monitor_fdset_dup_fd_add and > removing monitor_fdset_get_fd entirely. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>