[libvirt] [PATCH] libxl: do not use hardcoded pygrub path
According to the Xen documentation the full path was only needed for versions prior to 4.1: "Note: For older versions of Xen Project software (4.1 or earlier) you need to know where in the filesystem pygrub lies. (...) Newer versions of the toolstack know how to look for the path themselves." [1] [1] http://wiki.xen.org/wiki/PyGrub Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1104695 Signed-off-by: Alvaro Lopez Garcia <aloga@ifca.unican.es> --- src/libxl/libxl_conf.c | 13 +++++++++++-- src/libxl/libxl_conf.h | 5 ++++- src/libxl/libxl_domain.c | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 1210500..896ea8d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -100,6 +100,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->bootloader); } static int @@ -570,6 +571,7 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) static int libxlMakeDomBuildInfo(virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config) { @@ -688,7 +690,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0) goto error; } else if (def->os.kernel == NULL) { - if (VIR_STRDUP(b_info->u.pv.bootloader, LIBXL_BOOTLOADER_PATH) < 0) + if (VIR_STRDUP(b_info->u.pv.bootloader, cfg->bootloader) < 0) goto error; } if (def->os.bootloaderArgs) { @@ -1241,6 +1243,12 @@ libxlDriverConfigNew(void) cfg->version = (cfg->verInfo->xen_version_major * 1000000) + (cfg->verInfo->xen_version_minor * 1000); + /* + * Versions prior to 4.2 need the full pygrub path + */ + if (VIR_STRDUP(cfg->bootloader, cfg->version >= 4002000 ? LIBXL_BOOTLOADER : LIBXL_BOOTLOADER_PATH) < 0) + goto error; + /* This will fill xenstore info about free and dom0 memory if missing, * should be called before starting first domain */ if (libxl_get_free_memory(cfg->ctx, &free_mem)) { @@ -1399,6 +1407,7 @@ libxlMakeCapabilities(libxl_ctx *ctx) int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config) { @@ -1407,7 +1416,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) return -1; - if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0) + if (libxlMakeDomBuildInfo(def, cfg, ctx, d_config) < 0) return -1; if (libxlMakeDiskList(def, d_config) < 0) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index da66b4e..9d18c4b 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -53,7 +53,8 @@ # 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 "pygrub" +# define LIBXL_BOOTLOADER_PATH BINDIR "/" LIBXL_BOOTLOADER /* 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 @@ -99,6 +100,7 @@ struct _libxlDriverConfig { char *libDir; char *saveDir; char *autoDumpDir; + char *bootloader; }; @@ -181,6 +183,7 @@ libxlCreateXMLConf(void); int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config); diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cdac82c..14f048a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1199,7 +1199,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, VIR_FREE(managed_save_path); } - if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, + if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, cfg, priv->ctx, &d_config) < 0) goto endjob; -- 2.1.0.rc1
On Tue, Aug 19, 2014 at 01:54:12PM +0200, Alvaro Lopez Garcia wrote:
According to the Xen documentation the full path was only needed for versions prior to 4.1: "Note: For older versions of Xen Project software (4.1 or earlier) you need to know where in the filesystem pygrub lies. (...) Newer versions of the toolstack know how to look for the path themselves." [1]
Regardless of whether they know how to find the path, it is expected that the libvirt XML is including/requiring the full path. So I'm not really considering this a bug that needs fixing Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Hi Daniel, On Tue 19 Aug 2014 (14:32), Daniel P. Berrange wrote:
On Tue, Aug 19, 2014 at 01:54:12PM +0200, Alvaro Lopez Garcia wrote:
According to the Xen documentation the full path was only needed for versions prior to 4.1: "Note: For older versions of Xen Project software (4.1 or earlier) you need to know where in the filesystem pygrub lies. (...) Newer versions of the toolstack know how to look for the path themselves." [1]
Regardless of whether they know how to find the path, it is expected that the libvirt XML is including/requiring the full path. So I'm not really considering this a bug that needs fixing
- Is it required that all the libvirt XML files needing a bootloader specify one? or - If the bootloader is specified, then it has to be a full path? If it is the former case, we have been running libvirt with xend without specifying a bootloader in the XML files for a long time. We only have found issues when we switched our hypervisors from xend to libxl. Also, if we install the bootloader in the current hardcoded path (i.e. /usr/bin/pygrub/) the guests will be able to be booted again without including a bootloader in the XML file. If it is the later, currently we are specifying it without the full path and it is working (i.e. <bootloader>pygrub</bootloader>). Thank you for your time, -- Álvaro López García aloga@ifca.unican.es Instituto de Física de Cantabria http://alvarolopez.github.io Ed. Juan Jordá, Campus UC tel: (+34) 942 200 969 Avda. de los Castros s/n 39005 Santander (SPAIN)
On 08/19/2014 10:33 AM, Álvaro López García wrote: > Hi Daniel, > > On Tue 19 Aug 2014 (14:32), Daniel P. Berrange wrote: >> On Tue, Aug 19, 2014 at 01:54:12PM +0200, Alvaro Lopez Garcia wrote: >>> According to the Xen documentation the full path was only needed >>> for versions prior to 4.1: "Note: For older versions of Xen Project >>> software (4.1 or earlier) you need to know where in the filesystem >>> pygrub lies. (...) Newer versions of the toolstack know how to look >>> for the path themselves." [1] >> Regardless of whether they know how to find the path, it is expected >> that the libvirt XML is including/requiring the full path. So I'm >> not really considering this a bug that needs fixing > - Is it required that all the libvirt XML files needing a bootloader > specify one? or No. But if you don't specify one, you get /usr/bin/pygrub, which is similar to xend's behavior. > - If the bootloader is specified, then it has to be a full path? No. > > If it is the former case, we have been running libvirt with xend without > specifying a bootloader in the XML files for a long time. If not specified, xend sets bootloader to 'usr/bin/pygrub'. > We only have > found issues when we switched our hypervisors from xend to libxl. What issues? AFAIK, only a warning in the log. Regards, Jim
Hi Jim, thanks four your time. Some replies inline, but before let me clarify some things: - I am running libvirt + xen in Debian and Ubuntu hosts. - /usr/bin/pygrub does not exist in Debian and Ubuntu, it is located in /usr/lib/xen-<version>/bin/pygrub instead. - We have been running our guests without including a bootloader element in the XML files until we have migrated to libxl. On Tue 19 Aug 2014 (23:15), Jim Fehlig wrote: > On 08/19/2014 10:33 AM, Álvaro López García wrote: > >On Tue 19 Aug 2014 (14:32), Daniel P. Berrange wrote: > >>On Tue, Aug 19, 2014 at 01:54:12PM +0200, Alvaro Lopez Garcia wrote: > >>>According to the Xen documentation the full path was only needed > >>>for versions prior to 4.1: "Note: For older versions of Xen Project > >>>software (4.1 or earlier) you need to know where in the filesystem > >>>pygrub lies. (...) Newer versions of the toolstack know how to look > >>>for the path themselves." [1] > >>Regardless of whether they know how to find the path, it is expected > >>that the libvirt XML is including/requiring the full path. So I'm > >>not really considering this a bug that needs fixing > >- Is it required that all the libvirt XML files needing a bootloader > > specify one? or > > No. But if you don't specify one, you get /usr/bin/pygrub, which is similar > to xend's behavior. AFAIK if the bootloader is not set, or if it is set to just "pygrub", xend searchs for it [1] in several locations [2]. If it is set to a full path, it does not search for anything and uses that path. In our case we didn't set a bootloader and xend finds it in LIBEXEC (that in Debian/Ubuntu using Xen 4.4 points to /usr/lib/xen-4.4/bin/). [1] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/python/xen/xend/XendDomainInfo.py;h=8d4ff5c749f294c9c6509d84bb2b6a898acfc965;hb=stable-4.4#l3245 [2] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/python/xen/util/auxbin.py;h=a690ad94d16583b5e9d0b3e180e653305f7fb210;hb=stable-4.4#l35 libvirt using libxl is setting the bootloader to /usr/bin/pygrub if it is not set in the XML file. This produces the following warning in the libvirt logs (coming from libxl [3]): libxl: warning: libxl_bootloader.c:413:bootloader_disk_attached_cb: bootloader='/usr/bin/pygrub' is deprecated; use bootloader='pygrub' instead The boot process finally ends up failing since it cannot find /usr/bin/pygrub libxl: cannot execute /usr/bin/pygrub: No such file or directory Therefore I assume that when libxl sets the bootloader because it is missing in the XML file it should set it just to "pygrub" not "/usr/bin/pygrub", hence my patch, but maybe I am mistaken. [3] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_bootloader.c;h=3287bf77da9a64327736e6bf923608e0c2bb3a52;hb=stable-4.4#l411 > >- If the bootloader is specified, then it has to be a full path? > > No. Thanks for the clarification. > >If it is the former case, we have been running libvirt with xend without > >specifying a bootloader in the XML files for a long time. > > If not specified, xend sets bootloader to 'usr/bin/pygrub'. See my comments above, if not specified or if it is only "pygrub" xend searchs for it in any of the expected paths. > > We only have > >found issues when we switched our hypervisors from xend to libxl. > > What issues? AFAIK, only a warning in the log. Actually just one issue apart from the warning in the log: After the migration from xend to libxl I am not able to boot anymore one of the previously running guests. I have to set manually in each of the guests' configuration the bootloader, pointing to the correct location. Kind regards, -- Álvaro López García aloga@ifca.unican.es Instituto de Física de Cantabria http://alvarolopez.github.io Ed. Juan Jordá, Campus UC tel: (+34) 942 200 969 Avda. de los Castros s/n 39005 Santander (SPAIN)
On Tue, Aug 19, 2014 at 11:15:21PM -0600, Jim Fehlig wrote: > On 08/19/2014 10:33 AM, Álvaro López García wrote: > >Hi Daniel, > > > >On Tue 19 Aug 2014 (14:32), Daniel P. Berrange wrote: > >>On Tue, Aug 19, 2014 at 01:54:12PM +0200, Alvaro Lopez Garcia wrote: > >>>According to the Xen documentation the full path was only needed > >>>for versions prior to 4.1: "Note: For older versions of Xen Project > >>>software (4.1 or earlier) you need to know where in the filesystem > >>>pygrub lies. (...) Newer versions of the toolstack know how to look > >>>for the path themselves." [1] > >>Regardless of whether they know how to find the path, it is expected > >>that the libvirt XML is including/requiring the full path. So I'm > >>not really considering this a bug that needs fixing > >- Is it required that all the libvirt XML files needing a bootloader > > specify one? or > > No. But if you don't specify one, you get /usr/bin/pygrub, which is similar > to xend's behavior. > > >- If the bootloader is specified, then it has to be a full path? > > No. Actually from the XML point ofo view, the <bootloader> element *is* expected to be an absolute file path. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/19/2014 05:54 AM, Alvaro Lopez Garcia wrote:
According to the Xen documentation the full path was only needed for versions prior to 4.1: "Note: For older versions of Xen Project software (4.1 or earlier) you need to know where in the filesystem pygrub lies. (...) Newer versions of the toolstack know how to look for the path themselves." [1]
[1] http://wiki.xen.org/wiki/PyGrub
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1104695 Signed-off-by: Alvaro Lopez Garcia <aloga@ifca.unican.es> --- src/libxl/libxl_conf.c | 13 +++++++++++-- src/libxl/libxl_conf.h | 5 ++++- src/libxl/libxl_domain.c | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 1210500..896ea8d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -100,6 +100,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->bootloader); }
static int @@ -570,6 +571,7 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
static int libxlMakeDomBuildInfo(virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config) { @@ -688,7 +690,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0) goto error; } else if (def->os.kernel == NULL) { - if (VIR_STRDUP(b_info->u.pv.bootloader, LIBXL_BOOTLOADER_PATH) < 0) + if (VIR_STRDUP(b_info->u.pv.bootloader, cfg->bootloader) < 0) goto error; } if (def->os.bootloaderArgs) { @@ -1241,6 +1243,12 @@ libxlDriverConfigNew(void) cfg->version = (cfg->verInfo->xen_version_major * 1000000) + (cfg->verInfo->xen_version_minor * 1000);
+ /* + * Versions prior to 4.2 need the full pygrub path + */ + if (VIR_STRDUP(cfg->bootloader, cfg->version >= 4002000 ? LIBXL_BOOTLOADER : LIBXL_BOOTLOADER_PATH) < 0) + goto error;
The libxl driver only supports Xen >= 4.2. Regards, Jim
+ /* This will fill xenstore info about free and dom0 memory if missing, * should be called before starting first domain */ if (libxl_get_free_memory(cfg->ctx, &free_mem)) { @@ -1399,6 +1407,7 @@ libxlMakeCapabilities(libxl_ctx *ctx) int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config) { @@ -1407,7 +1416,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) return -1;
- if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0) + if (libxlMakeDomBuildInfo(def, cfg, ctx, d_config) < 0) return -1;
if (libxlMakeDiskList(def, d_config) < 0) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index da66b4e..9d18c4b 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -53,7 +53,8 @@ # 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 "pygrub" +# define LIBXL_BOOTLOADER_PATH BINDIR "/" LIBXL_BOOTLOADER
/* 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 @@ -99,6 +100,7 @@ struct _libxlDriverConfig { char *libDir; char *saveDir; char *autoDumpDir; + char *bootloader; };
@@ -181,6 +183,7 @@ libxlCreateXMLConf(void); int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, + libxlDriverConfigPtr cfg, libxl_ctx *ctx, libxl_domain_config *d_config);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cdac82c..14f048a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1199,7 +1199,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, VIR_FREE(managed_save_path); }
- if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, + if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, cfg, priv->ctx, &d_config) < 0) goto endjob;
participants (4)
-
Alvaro Lopez Garcia -
Daniel P. Berrange -
Jim Fehlig -
Álvaro López García