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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.06.12 at 13:42, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> 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).

That is what you want in the installed case.

> 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?

As I had written in the description, I'm after the non-installed
case instead (i.e. running from the build tree's dist/), and that
case can only be covered by either fixing os_find_datadir() or
forcing the value of --datadir to match the function's
expectations.

Additionally, for the installed case it doesn't matter where the
bits are put, as it'll always be the configured value that gets
used.

(The reason for caring about the not-installed case is quite
obviously that running multiple versions of Xen in parallel on
a single system wouldn't work well if one was to install each
version.)

Jan


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