[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 4/4] block: switch to use qemu_open/qemu_create for improv
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 4/4] block: switch to use qemu_open/qemu_create for improved errors |
Date: |
Fri, 24 Jul 2020 16:37:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 7/24/20 3:25 PM, 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' flags 0x4000: 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' flags 0x4002: filesystem does not
> support O_DIRECT"
> }
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> block/file-posix.c | 18 +++++++-----------
> block/file-win32.c | 6 ++----
> tests/qemu-iotests/051.out | 4 ++--
> tests/qemu-iotests/051.pc.out | 4 ++--
> tests/qemu-iotests/061.out | 2 +-
> tests/qemu-iotests/069.out | 2 +-
> tests/qemu-iotests/082.out | 4 ++--
> tests/qemu-iotests/111.out | 2 +-
> tests/qemu-iotests/226.out | 6 +++---
> tests/qemu-iotests/232.out | 12 ++++++------
> tests/qemu-iotests/244.out | 6 +++---
> 11 files changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index bac2566f10..c63926d592 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -630,11 +630,10 @@ static int raw_open_common(BlockDriverState *bs, QDict
> *options,
> raw_parse_flags(bdrv_flags, &s->open_flags, false);
>
> s->fd = -1;
> - fd = qemu_open_old(filename, s->open_flags, 0644);
> + fd = qemu_open(filename, s->open_flags, errp);
> ret = fd < 0 ? -errno : 0;
>
> if (ret < 0) {
> - error_setg_file_open(errp, -ret, filename);
> if (ret == -EROFS) {
> ret = -EACCES;
> }
> @@ -1032,15 +1031,13 @@ static int raw_reconfigure_getfd(BlockDriverState
> *bs, int flags,
> }
> }
>
> - /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open_old()
> */
> + /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
> if (fd == -1) {
> const char *normalized_filename = bs->filename;
> ret = raw_normalize_devicepath(&normalized_filename, errp);
> if (ret >= 0) {
> - assert(!(*open_flags & O_CREAT));
> - fd = qemu_open_old(normalized_filename, *open_flags);
> + fd = qemu_open(normalized_filename, *open_flags, errp);
> if (fd == -1) {
> - error_setg_errno(errp, errno, "Could not reopen file");
> return -1;
> }
> }
> @@ -2411,10 +2408,9 @@ raw_co_create(BlockdevCreateOptions *options, Error
> **errp)
> }
>
> /* Create file */
> - fd = qemu_open_old(file_opts->filename, O_RDWR | O_CREAT | O_BINARY,
> 0644);
> + fd = qemu_create(file_opts->filename, O_RDWR | O_BINARY, 0644, errp);
> if (fd < 0) {
> result = -errno;
> - error_setg_errno(errp, -result, "Could not create file");
> goto out;
> }
[...]
Nice :)
I haven't checked the iotest errors; assuming a CI will take care of it:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
- Re: [PATCH v3 2/4] util: introduce qemu_open and qemu_create with error reporting, (continued)