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: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.

However your patch changes the location of data_dir for the
installed case too, from /usr/share/qemu-xen to /usr/lib/xen/share/qemu.
If you are after the non-installed case, you must be already setting
device_model_override in your VM config file to point to the right
binary. You might as well pass:

device_model_args = ['-L', '/path/to/share/file' ]

that should solve your problem.

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