[PATCH 0 of 3 v2] tools: Move bootloaders to libexec directory

[PATCH 0 of 3 v2] tools: Move bootloaders to libexec directory

[George Dunlap]

tools: Move bootloaders to libexec directory

pygrub and xenpvnetboot are meant to be run by tools, and not by the user,
and thus should be in /usr/lib/xen/bin rather than /usr/bin.  This patch
series:
* Causes libxl to look in the libexec dir if a full path is not specified,
  falling back to searching PATH if it's not in the libexec dir
* Moves pygrub and xenpvnetboot into the libexec dir
* For compatibility with existing configs, puts a link to pygrub in /usr/bin
* Warns the user that /usr/bin/pygrub is deprecated

v2: 
 - If the bootloader is not in the libexec path, revert to using the string
   as passed in the config file (which will search $PATH)
 - Use strcmp rather than strncmp

[PATCH 1 of 3 v2] libxl: Look for bootloader in libexec path

[George Dunlap]

If the full path for a bootloader (such as pygrub or xenpvnetboot) is not
given, check for it first in the libexec path before falling back to the
PATH variable.

v2:
 - Check for the libexec path, and if the bootloader isn't there, fall back to
   the explicitly passed value.  If the full path isn't specified, this will
   case execvp to search using the path given in the PATH variable.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 84ae90427c54 -r 662e2f6be283 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Fri May 11 17:46:16 2012 +0100
+++ b/tools/libxl/libxl_bootloader.c	Tue May 15 11:20:53 2012 +0100
@@ -333,6 +333,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
 
     char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
     char *tempdir;
+    const char *bootloader = NULL;
 
     char *dom_console_xs_path;
     char dom_console_slave_tty_path[PATH_MAX];
@@ -397,6 +398,30 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Config bootloader path: %s",
+               info->u.pv.bootloader);
+
+    bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
+                                 libxl__libexec_path());
+    if ( bootloader == NULL ) {
+        rc = ERROR_NOMEM;
+        goto out_close;
+    } else {
+        /* Check to see if the file exists in this location; if not,  fall back
+         * to checking the path */
+        struct stat st;
+
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Resulting bootloader path: %s",
+                   bootloader);
+
+        if ( lstat(bootloader, &st) )
+        {
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "%s doesn't exist, will search $PATH",
+                       bootloader);
+            bootloader = info->u.pv.bootloader;
+        }
+    }
+
     /*
      * We need to present the bootloader's tty as a pty slave that xenconsole
      * can access.  Since the bootloader itself needs a pty slave,
@@ -417,7 +442,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     dom_console_xs_path = libxl__sprintf(gc, "%s/console/tty", libxl__xs_get_dompath(gc, domid));
     libxl__xs_write(gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path);
 
-    pid = fork_exec_bootloader(&bootloader_fd, info->u.pv.bootloader, args);
+    pid = fork_exec_bootloader(&bootloader_fd, bootloader, args);
     if (pid < 0) {
         goto out_close;
     }

[PATCH 2 of 3 v2] tools: Install pv bootloaders in libexec rather than /usr/bin

[George Dunlap]

pygrub and xenpvnetboot are meant to be run by tools, and not by the user,
and thus should be in /usr/lib/xen/bin rather than /usr/bin.

Because most config files will still have an absolute path pointing to
/usr/bin/pygrub, make a symbolic link that we will deprecate.

Signed-off-by: George Dunlap <george.dunlap@eu.ctirix.com>

diff -r 662e2f6be283 -r 4503705206f3 tools/misc/Makefile
--- a/tools/misc/Makefile	Tue May 15 11:20:53 2012 +0100
+++ b/tools/misc/Makefile	Tue May 15 11:20:55 2012 +0100
@@ -18,7 +18,7 @@ SUBDIRS-$(CONFIG_LOMOUNT) += lomount
 SUBDIRS-$(CONFIG_MINITERM) += miniterm
 SUBDIRS := $(SUBDIRS-y)
 
-INSTALL_BIN-y := xencons xenpvnetboot
+INSTALL_BIN-y := xencons
 INSTALL_BIN-$(CONFIG_X86) += xen-detect
 INSTALL_BIN := $(INSTALL_BIN-y)
 
@@ -27,6 +27,9 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-hvmctx
 INSTALL_SBIN-$(CONFIG_MIGRATE) += xen-hptool
 INSTALL_SBIN := $(INSTALL_SBIN-y)
 
+INSTALL_PRIVBIN-y := xenpvnetboot
+INSTALL_PRIVBIN := $(INSTALL_PRIVBIN-y)
+
 # Include configure output (config.h) to headers search path
 CFLAGS += -I$(XEN_ROOT)/tools
 
@@ -41,8 +44,10 @@ build: $(TARGETS)
 install: build
 	$(INSTALL_DIR) $(DESTDIR)$(BINDIR)
 	$(INSTALL_DIR) $(DESTDIR)$(SBINDIR)
+	$(INSTALL_DIR) $(DESTDIR)$(PRIVATE_BINDIR)
 	$(INSTALL_PYTHON_PROG) $(INSTALL_BIN) $(DESTDIR)$(BINDIR)
 	$(INSTALL_PYTHON_PROG) $(INSTALL_SBIN) $(DESTDIR)$(SBINDIR)
+	$(INSTALL_PYTHON_PROG) $(INSTALL_PRIVBIN) $(DESTDIR)$(PRIVATE_BINDIR)
 	set -e; for d in $(SUBDIRS); do $(MAKE) -C $$d install-recurse; done
 
 .PHONY: clean
diff -r 662e2f6be283 -r 4503705206f3 tools/pygrub/Makefile
--- a/tools/pygrub/Makefile	Tue May 15 11:20:53 2012 +0100
+++ b/tools/pygrub/Makefile	Tue May 15 11:20:55 2012 +0100
@@ -11,9 +11,11 @@ build:
 .PHONY: install
 install: all
 	CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py install \
-		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" --force
-	$(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(BINDIR)/pygrub
+		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
+		--install-scripts=$(DESTDIR)/$(PRIVATE_BINDIR) --force
+	$(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(PRIVATE_BINDIR)/pygrub
 	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
+	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
 
 .PHONY: clean
 clean:

[PATCH 3 of 3 v2] libxl: Warn that /usr/bin/pygrub is deprecated

[George Dunlap]

v2: Use strcmp rather than strncmp.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 4503705206f3 -r 01804c550dd1 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Tue May 15 11:20:55 2012 +0100
+++ b/tools/libxl/libxl_bootloader.c	Tue May 15 11:28:12 2012 +0100
@@ -15,6 +15,7 @@
 #include "libxl_osdeps.h" /* must come before any other headers */
 
 #include <termios.h>
+#include <string.h>
 
 #include "libxl_internal.h"
 
@@ -401,6 +402,11 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Config bootloader path: %s",
                info->u.pv.bootloader);
 
+    if ( !strcmp(info->u.pv.bootloader, "/usr/bin/pygrub") )
+        LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
+                   "WARNING: bootloader='/usr/bin/pygrub' "             \
+                   "is deprecated; use bootloader='pygrub' instead");
+    
     bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
                                  libxl__libexec_path());
     if ( bootloader == NULL ) {

Re: [PATCH 0 of 3 v2] tools: Move bootloaders to libexec directory

[George Dunlap]

Hmm -- sorry, seems a bunch of changes have been made to
libxl_bootloader.c.  Let me take a look at this.

 -Georg

On Tue, May 15, 2012 at 11:33 AM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> tools: Move bootloaders to libexec directory
>
> pygrub and xenpvnetboot are meant to be run by tools, and not by the user,
> and thus should be in /usr/lib/xen/bin rather than /usr/bin.  This patch
> series:
> * Causes libxl to look in the libexec dir if a full path is not specified,
>  falling back to searching PATH if it's not in the libexec dir
> * Moves pygrub and xenpvnetboot into the libexec dir
> * For compatibility with existing configs, puts a link to pygrub in /usr/bin
> * Warns the user that /usr/bin/pygrub is deprecated
>
> v2:
>  - If the bootloader is not in the libexec path, revert to using the string
>   as passed in the config file (which will search $PATH)
>  - Use strcmp rather than strncmp
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1 of 3 v2] libxl: Look for bootloader in libexec path

[Ian Campbell]

On Tue, 2012-05-15 at 11:33 +0100, George Dunlap wrote:
> If the full path for a bootloader (such as pygrub or xenpvnetboot) is not
> given, check for it first in the libexec path before falling back to the
> PATH variable.
> 
> v2:
>  - Check for the libexec path, and if the bootloader isn't there, fall back to
>    the explicitly passed value.  If the full path isn't specified, this will
>    case execvp to search using the path given in the PATH variable.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> diff -r 84ae90427c54 -r 662e2f6be283 tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.c	Fri May 11 17:46:16 2012 +0100
> +++ b/tools/libxl/libxl_bootloader.c	Tue May 15 11:20:53 2012 +0100
> @@ -333,6 +333,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>  
>      char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
>      char *tempdir;
> +    const char *bootloader = NULL;
>  
>      char *dom_console_xs_path;
>      char dom_console_slave_tty_path[PATH_MAX];
> @@ -397,6 +398,30 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>          goto out_close;
>      }
>  
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Config bootloader path: %s",
> +               info->u.pv.bootloader);
> +
> +    bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
> +                                 libxl__libexec_path());
> +    if ( bootloader == NULL ) {

Since you seem to be respinning anyway...

This can't actually happen any more, since 25181:26f72d923cb9 (libxl:
Crash (more sensibly) on malloc failure)


> +        rc = ERROR_NOMEM;
> +        goto out_close;
> +    } else {
> +        /* Check to see if the file exists in this location; if not,  fall back
> +         * to checking the path */
> +        struct stat st;
> +
> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Resulting bootloader path: %s",
> +                   bootloader);
> +
> +        if ( lstat(bootloader, &st) )
> +        {
> +            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "%s doesn't exist, will search $PATH",
> +                       bootloader);
> +            bootloader = info->u.pv.bootloader;
> +        }
> +    }
> +
>      /*
>       * We need to present the bootloader's tty as a pty slave that xenconsole
>       * can access.  Since the bootloader itself needs a pty slave,
> @@ -417,7 +442,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>      dom_console_xs_path = libxl__sprintf(gc, "%s/console/tty", libxl__xs_get_dompath(gc, domid));
>      libxl__xs_write(gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path);
>  
> -    pid = fork_exec_bootloader(&bootloader_fd, info->u.pv.bootloader, args);
> +    pid = fork_exec_bootloader(&bootloader_fd, bootloader, args);
>      if (pid < 0) {
>          goto out_close;
>      }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2 of 3 v2] tools: Install pv bootloaders in libexec rather than /usr/bin

[Ian Campbell]

On Tue, 2012-05-15 at 11:33 +0100, George Dunlap wrote:
> pygrub and xenpvnetboot are meant to be run by tools, and not by the user,
> and thus should be in /usr/lib/xen/bin rather than /usr/bin.
> 
> Because most config files will still have an absolute path pointing to
> /usr/bin/pygrub, make a symbolic link that we will deprecate.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.ctirix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 662e2f6be283 -r 4503705206f3 tools/misc/Makefile
> --- a/tools/misc/Makefile	Tue May 15 11:20:53 2012 +0100
> +++ b/tools/misc/Makefile	Tue May 15 11:20:55 2012 +0100
> @@ -18,7 +18,7 @@ SUBDIRS-$(CONFIG_LOMOUNT) += lomount
>  SUBDIRS-$(CONFIG_MINITERM) += miniterm
>  SUBDIRS := $(SUBDIRS-y)
>  
> -INSTALL_BIN-y := xencons xenpvnetboot
> +INSTALL_BIN-y := xencons
>  INSTALL_BIN-$(CONFIG_X86) += xen-detect
>  INSTALL_BIN := $(INSTALL_BIN-y)
>  
> @@ -27,6 +27,9 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-hvmctx
>  INSTALL_SBIN-$(CONFIG_MIGRATE) += xen-hptool
>  INSTALL_SBIN := $(INSTALL_SBIN-y)
>  
> +INSTALL_PRIVBIN-y := xenpvnetboot
> +INSTALL_PRIVBIN := $(INSTALL_PRIVBIN-y)
> +
>  # Include configure output (config.h) to headers search path
>  CFLAGS += -I$(XEN_ROOT)/tools
>  
> @@ -41,8 +44,10 @@ build: $(TARGETS)
>  install: build
>  	$(INSTALL_DIR) $(DESTDIR)$(BINDIR)
>  	$(INSTALL_DIR) $(DESTDIR)$(SBINDIR)
> +	$(INSTALL_DIR) $(DESTDIR)$(PRIVATE_BINDIR)
>  	$(INSTALL_PYTHON_PROG) $(INSTALL_BIN) $(DESTDIR)$(BINDIR)
>  	$(INSTALL_PYTHON_PROG) $(INSTALL_SBIN) $(DESTDIR)$(SBINDIR)
> +	$(INSTALL_PYTHON_PROG) $(INSTALL_PRIVBIN) $(DESTDIR)$(PRIVATE_BINDIR)
>  	set -e; for d in $(SUBDIRS); do $(MAKE) -C $$d install-recurse; done
>  
>  .PHONY: clean
> diff -r 662e2f6be283 -r 4503705206f3 tools/pygrub/Makefile
> --- a/tools/pygrub/Makefile	Tue May 15 11:20:53 2012 +0100
> +++ b/tools/pygrub/Makefile	Tue May 15 11:20:55 2012 +0100
> @@ -11,9 +11,11 @@ build:
>  .PHONY: install
>  install: all
>  	CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py install \
> -		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" --force
> -	$(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(BINDIR)/pygrub
> +		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
> +		--install-scripts=$(DESTDIR)/$(PRIVATE_BINDIR) --force
> +	$(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(PRIVATE_BINDIR)/pygrub
>  	$(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot
> +	ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR)
>  
>  .PHONY: clean
>  clean:
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 3 of 3 v2] libxl: Warn that /usr/bin/pygrub is deprecated

[Ian Campbell]

On Tue, 2012-05-15 at 11:33 +0100, George Dunlap wrote:
> v2: Use strcmp rather than strncmp.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 4503705206f3 -r 01804c550dd1 tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.c	Tue May 15 11:20:55 2012 +0100
> +++ b/tools/libxl/libxl_bootloader.c	Tue May 15 11:28:12 2012 +0100
> @@ -15,6 +15,7 @@
>  #include "libxl_osdeps.h" /* must come before any other headers */
>  
>  #include <termios.h>
> +#include <string.h>
>  
>  #include "libxl_internal.h"
>  
> @@ -401,6 +402,11 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>      LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Config bootloader path: %s",
>                 info->u.pv.bootloader);
>  
> +    if ( !strcmp(info->u.pv.bootloader, "/usr/bin/pygrub") )
> +        LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
> +                   "WARNING: bootloader='/usr/bin/pygrub' "             \
> +                   "is deprecated; use bootloader='pygrub' instead");
> +    
>      bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
>                                   libxl__libexec_path());
>      if ( bootloader == NULL ) {
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel