Re: [Xen-devel] [PATCH] tools: adjust --datadir option passed to qemu-upstream's configure script

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 8 Jun 2012, Jan Beulich wrote:
> >>> On 08.06.12 at 13:11, Stefano Stabellini 
> >>> <stefano.stabellini@xxxxxxxxxxxxx>
> wrote:
> > On Mon, 4 Jun 2012, Jan Beulich wrote:
> >> Passing $(SHAREDIR)/qemu-xen isn't compatible with the way
> >> os-posix.c:os_find_datadir() works. While one would generally expect
> >> that function to honour the --datadir configure option, fixing this is
> >> quite a bit more involved than simply adjusting the configure options
> >> to match the function's behavior (in that, for an installed binary, it
> >> looks in $bindir/../share/qemu). Afaict, so far this can have worked
> >> only by virtue of what CONFIG_QEMU_DATADIR points to being populated
> >> due to (presumably) some other qemu instance being installed on
> >> people's systems (but being absent when running qemu-system-i386 from
> >> the dist/ portion of the build tree).
> >> 
> >> Signed-off-by: Jan Beulich <JBeulich@xxxxxxxx>
> >> 
> >> --- a/tools/Makefile
> >> +++ b/tools/Makefile
> >> @@ -155,7 +155,7 @@ subdir-all-qemu-xen-dir subdir-install-q
> >>            --extra-ldflags="-L$(XEN_ROOT)/tools/libxc \
> >>            -L$(XEN_ROOT)/tools/xenstore" \
> >>            --bindir=$(LIBEXEC) \
> >> -          --datadir=$(SHAREDIR)/qemu-xen \
> >> +          --datadir=$(dir $(LIBEXEC))share/qemu \
> >>            --disable-kvm \
> >>            --python=$(PYTHON) \
> >>            $(IOEMU_CONFIGURE_CROSS); \
> >> 
> >> 
> >> 
> >> 
> > 
> > This is a QEMU bug, so I would rather fix it there.
> 
> That's not as simple as you appear to think, as it would be necessary
> to meaningfully express an arbitrary datadir relative to bindir.
> 
> > In fact the fix could be as simple as the appended patch.
> > Does it work for you?
> > 
> > diff --git a/os-posix.c b/os-posix.c
> > index dc4a6bb..d0c873d 100644
> > --- a/os-posix.c
> > +++ b/os-posix.c
> > @@ -136,6 +136,9 @@ char *os_find_datadir(const char *argv0)
> >              g_free(res);
> >              res = NULL;
> >          }
> > +    } else {
> > +        g_free(res);
> > +        res = NULL;
> 
> How can this work? Afaict it would even break currently working
> cases, as it now makes the function return NULL not only when
> both access() checks failed, but also when the first one succeeded.

Yes, you are right, I misread the error condition of "access".

Like you wrote, os_find_datadir checks for the existence of
$bindir/../share/qemu that normally means /usr/lib/xen/share/qemu, then
it checks for /usr/lib/xen/pc-bios and if they both don't exist return
NULL and data_dir is set to CONFIG_QEMU_DATADIR (that is what we want).

While it is certainly not how I would have written this code, it should
work correctly. How is that you system has /usr/lib/xen/pc-bios or
/usr/lib/xen/share/qemu?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel