Re: [libvirt] [PATCH] allow to set path to xen userspace tools
On Tue, Jul 07, 2009 at 06:02:20PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 07, 2009 at 01:16:02PM +0200, Guido G?nther wrote:
> Hi,
> attached patch makes the path to the xen userspace tools configurable.
> Debian keeps this under /usr/lib/xen-default/ instead of /usr/lib/xen/.
> We don't have the amd64 libs in /usr/lib64/xen either so we can use:
> ./configure --with-xen-tools=/usr/lib/xen-defaults
--with-xen-tools64=/usr/lib/xen-defaults
> instead of patching src/xen_internals.c directly.
> Skipping above options gives the current behaviour. I checked that "make
> check" still passes. O.k. to apply?
The code changes look ok, but I'm not much of a fan of the way the
test cases are handled here, with all these data files listed in
configure.in
I think i'd be more inclined to let virtTestLoadFile() do the subsitution
at time its loading the files. Perhaps
if (virtTestLoadFile(xml, &expectxml, MAX_FILE, {
"@XEN_TOOLS@", XEN_TOOLS,
}) < 0)
goto fail;
We'd need to do arbitrary number of string replacements:
if (virtTestLoadFile(xml, &expectxml, MAX_FILE, {
"@XEN_TOOLS@", XEN_TOOLS,
"@XEN_TOOLS64@", XEN_TOOLS64,
NULL,
}) < 0)
goto fail;
into the fixed size buffer expectxml. Doing this via autofoo or via a
makefile rule instead of in C looks more elegant to me.
Cheers,
-- Guido