[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: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 4/4] block: switch to use qemu_open/qemu_create for improved errors |
Date: |
Fri, 24 Jul 2020 15:26:18 +0100 |
User-agent: |
Mutt/1.14.5 (2020-06-23) |
On Fri, Jul 24, 2020 at 09:10:00AM -0500, Eric Blake wrote:
> On 7/24/20 8:25 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
>
> Are we trying to get this in 5.1?
It is probably verging on too late to justify for the rc
>
> >
> > 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>
> > ---
>
> > @@ -3335,7 +3331,7 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> > for (index = 0; index < num_of_test_partitions; index++) {
> > snprintf(test_partition, sizeof(test_partition), "%ss%d",
> > bsd_path,
> > index);
> > - fd = qemu_open_old(test_partition, O_RDONLY | O_BINARY |
> > O_LARGEFILE);
> > + fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE,
> > NULL);
>
> Should qemu_open() always be setting O_BINARY|O_LARGEFILE, without us having
> to worry about them at each caller? But that's a separate cleanup.
Hmm, I think both of these are dead code.
IIUC, O_BINARY is a no-op on any platform except Windows, and this is
file-posix.c, and O_LARGEFILE is a no-op, if you have _FILE_OFFSET_BITS=64,
which we hardcode.
> Reviewed-by: Eric Blake <eblake@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [PATCH v3 2/4] util: introduce qemu_open and qemu_create with error reporting, (continued)