[Xen-devel] dom0 kenrel crashes for openstack + libvirt + libxl

Message ID 1416580373.17932.15.camel@citrix.com
State New
Headers show

Commit Message

Ian Campbell Nov. 21, 2014, 2:32 p.m.
On Fri, 2014-11-21 at 14:18 +0000, David Vrabel wrote:
> On 21/11/14 14:05, Ian Campbell wrote:
> > On Fri, 2014-11-21 at 09:28 +0000, Ian Campbell wrote:
> >> I think libvirt is wrong to specify an absolute path here, IMHO by
> >> default it should just specify "pygrub" and let libxl figure out the
> >> correct path. Jim, what do you think?
> > 
> > e.g. something like the following untested (but pretty obvious[0])
> > patch.
> [...]
> > +# define "pygrub"
> 
> I'm going to go with pretty obviously broken.  You're missing the macro
> name.

Doh! Indeed...

Lets try that again...

8<----------

From 9f2d8da8264b426f54b92378e9e00973694193d4 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Fri, 21 Nov 2014 14:00:38 +0000
Subject: [PATCH] libxl: Allow libxl to find pygrub binary.

Specifying an explicit path to pygrub (e.g. BINDIR "/pygrub") only works if
Xen and libvirt happen to be installed to the same prefix. A more flexible
approach is to simply specify "pygrub" which will cause libxl to use the
correct path which it knows (since it is built with the same prefix as pygrub).

This is particular problematic in the Debian packaging, since the Debian Xen
package relocates pygrub into a libexec dir, however I think this change makes
sense upstream.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 .gnulib                |    2 +-
 src/libxl/libxl_conf.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Ian Campbell Nov. 21, 2014, 5:21 p.m. | #1
On Fri, 2014-11-21 at 14:32 +0000, Ian Campbell wrote:

I've now built and tested this with the 1.2.9 Debian package, works like
a charm...

> From 9f2d8da8264b426f54b92378e9e00973694193d4 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Fri, 21 Nov 2014 14:00:38 +0000
> Subject: [PATCH] libxl: Allow libxl to find pygrub binary.
> 
> Specifying an explicit path to pygrub (e.g. BINDIR "/pygrub") only works if
> Xen and libvirt happen to be installed to the same prefix. A more flexible
> approach is to simply specify "pygrub" which will cause libxl to use the
> correct path which it knows (since it is built with the same prefix as pygrub).
> 
> This is particular problematic in the Debian packaging, since the Debian Xen
> package relocates pygrub into a libexec dir, however I think this change makes
> sense upstream.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  .gnulib                |    2 +-
>  src/libxl/libxl_conf.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/.gnulib b/.gnulib
> index 9565c3b..2d28074 160000
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 9565c3be73eb6d76b7b42a21d68d2e00a62abb6d
> +Subproject commit 2d280742a9e30088aa169f53353765d5daafe4c0
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 25f77ea..d7a3971 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -53,7 +53,7 @@
>  # define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
>  # define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
>  # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
> -# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
> +# define LIBXL_BOOTLOADER_PATH "pygrub"
>  
>  /* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
>   * parameter has been added, representative of 'VCPU soft affinity'. If one
Jim Fehlig Nov. 21, 2014, 8:03 p.m. | #2
On 11/21/2014 07:32 AM, Ian Campbell wrote:
> >From 9f2d8da8264b426f54b92378e9e00973694193d4 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Fri, 21 Nov 2014 14:00:38 +0000
> Subject: [PATCH] libxl: Allow libxl to find pygrub binary.
>
> Specifying an explicit path to pygrub (e.g. BINDIR "/pygrub") only works if
> Xen and libvirt happen to be installed to the same prefix. A more flexible
> approach is to simply specify "pygrub" which will cause libxl to use the
> correct path which it knows (since it is built with the same prefix as pygrub).
>
> This is particular problematic in the Debian packaging, since the Debian Xen
> package relocates pygrub into a libexec dir, however I think this change makes
> sense upstream.

Yep agreed.  I forgot about making this change a few weeks back after seeing the 
following in the logs when not explicitly specifying a bootloader

libxl: warning: libxl_bootloader.c:413:bootloader_disk_attached_cb: 
bootloader='/usr/bin/pygrub' is deprecated; use bootloader='pygrub' instead

>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>   .gnulib                |    2 +-
>   src/libxl/libxl_conf.h |    2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/.gnulib b/.gnulib
> index 9565c3b..2d28074 160000
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 9565c3be73eb6d76b7b42a21d68d2e00a62abb6d
> +Subproject commit 2d280742a9e30088aa169f53353765d5daafe4c0

ACK and pushed after removing this hunk.

Regards,
Jim

> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 25f77ea..d7a3971 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -53,7 +53,7 @@
>   # define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
>   # define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
>   # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
> -# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
> +# define LIBXL_BOOTLOADER_PATH "pygrub"
>   
>   /* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
>    * parameter has been added, representative of 'VCPU soft affinity'. If one

Patch

diff --git a/.gnulib b/.gnulib
index 9565c3b..2d28074 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@ 
-Subproject commit 9565c3be73eb6d76b7b42a21d68d2e00a62abb6d
+Subproject commit 2d280742a9e30088aa169f53353765d5daafe4c0
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 25f77ea..d7a3971 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -53,7 +53,7 @@ 
 # define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
 # define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
 # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
-# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
+# define LIBXL_BOOTLOADER_PATH "pygrub"
 
 /* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
  * parameter has been added, representative of 'VCPU soft affinity'. If one