| Message ID | m3k4ygg4ps.fsf@crossbow.pond.sub.org |
|---|---|
| State | New |
| Headers | show |
On Tue, 27 Oct 2009, Markus Armbruster wrote: > Mark McLoughlin <markmc@redhat.com> writes: > > > On Thu, 2009-10-01 at 09:42 -0500, Justin M. Forbes wrote: > >> Author: Justin M. Forbes <jforbes@redhat.com> > >> 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 <jforbes@redhat.com> > > > > Certainly looks sensible to me > > > > Not having this is hurting us in Fedora 12 because we've started running > > qemu as an unprivileged user and people are having a hard time figuring > > out the various errors they're seeing caused by the change. This should > > help them. > > > > 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.
Hello, Am 27.10.2009 um 18:38 schrieb malc: > On Tue, 27 Oct 2009, Markus Armbruster wrote: > >> Mark McLoughlin <markmc@redhat.com> writes: >> >>> On Thu, 2009-10-01 at 09:42 -0500, Justin M. Forbes wrote: >>>> Author: Justin M. Forbes <jforbes@redhat.com> >>>> 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 <jforbes@redhat.com> >>> 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
Andreas Färber <andreas.faerber@web.de> writes: > Hello, > > Am 27.10.2009 um 18:38 schrieb malc: > >> On Tue, 27 Oct 2009, Markus Armbruster wrote: >> >>> Mark McLoughlin <markmc@redhat.com> writes: >>> >>>> On Thu, 2009-10-01 at 09:42 -0500, Justin M. Forbes wrote: >>>>> Author: Justin M. Forbes <jforbes@redhat.com> >>>>> 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 <jforbes@redhat.com> > >>>> 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.
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; }