Re: [Qemu-devel] [PATCH] Improve error reporting on file access

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Hello,
>
> Am 27.10.2009 um 18:38 schrieb malc:
>
>> On Tue, 27 Oct 2009, Markus Armbruster wrote:
>>
>>> Mark McLoughlin <address@hidden> writes:
>>>
>>>> On Thu, 2009-10-01 at 09:42 -0500, Justin M. Forbes wrote:
>>>>> Author: Justin M. Forbes <address@hidden>
>>>>> Date:   Thu Oct 1 09:34:56 2009 -0500
>>>>>
>>>>>    Improve error reporting on file access
>>>>>
>>>>>    By making the error reporting include strerror(errno), it
>>>>> gives the user
>>>>>    a bit more indication as to why qemu failed.  This is
>>>>> particularly
>>>>>    important for people running qemu as a non root user.
>>>>>
>>>>>    Signed-off-by: Justin M. Forbes <address@hidden>
>
>>>> Only concern is that errno might not be getting propagated
>>>> correctly by
>>>> some of these functions, but we can fix that later if so.
>>>
>>> Here's one:
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 7bfd415..70fd2ca 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2232,8 +2232,8 @@ DriveInfo *drive_init(QemuOpts *opts, void
>>> *opaque,
>>>     }
>>>
>>>     if (bdrv_open2(dinfo->bdrv, file, bdrv_flags, drv) < 0) {
>>> -        fprintf(stderr, "qemu: could not open disk image %s\n",
>>> -                        file);
>>> +        fprintf(stderr, "qemu: could not open disk image %s: %s\n",
>>> +                        file, strerror(errno));
>>>         return NULL;
>>>     }
>>>
>>
>> bdrv_open2 is not guaranteed to use POSIX functions for it's file
>> manipulation, hence the patch is wrong.
>
> It appears, the patch was applied in
> 850810d01b45e6ce99ac6696773e967890db2937 (Oct 5).
>
> On OpenSolaris 2009.06 amd64 I now get:
>
> qemu: could not open disk image /[...].iso: Not owner
>
> I am owner though. If I run it with pfexec (priviledged), I get:
>
> qemu: could not open disk image /[...].iso: No such file or directory
>
> The file is there and my script used to work before Juan's Makefile
> reorganization with the --whole-archive workaround I posted.
>
> So my guess is, we do see a stray errno here?
>
> Andreas

As malc said, the patch is wrong.  It should be reverted until somebody
comes up with a fix.