tools: Move bootloaders to libexec directory

If the full path for a bootloader (such as pygrub or xenpvnetboot) is not given, look for it in the libexec path.

Signed-off-by George Dunlap <[email protected]>

diff -r 8a86d841e6d4 -r e43157a86933 tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +9 -1

@@ -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,13 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
+    bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
+                                 libxl__libexec_path());
+    if ( bootloader == NULL ) {
+        rc = ERROR_NOMEM;
+        goto out_close;
+    }
+
     /*
      * 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 +425,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
[email protected]
http://lists.xen.org/xen-devel

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.

diff -r e43157a86933 -r 52f45b258ccc tools/misc/Makefile

tools/misc/Makefile +6 -1

@@ -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 e43157a86933 -r 52f45b258ccc tools/pygrub/Makefile

tools/pygrub/Makefile +4 -2

@@ -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
[email protected]
http://lists.xen.org/xen-devel

Signed-off-by George Dunlap <[email protected]>

diff -r 52f45b258ccc -r 794778a6e9fa tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +1 -0

@@ -15,6 +15,7 @@
 #include "libxl_osdeps.h" /* must come before any other headers */
 
 #include <termios.h>
+#include <string.h>
 
 #include "libxl_internal.h"
 

tools/libxl/libxl_bootloader.c +4 -0

+        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
[email protected]
http://lists.xen.org/xen-devel

I think since libxl uses execvp the first two are handled by path expansion, the next two are what you are adding (so has priority over $PATH in your patch, which I think is OK) and the last one is irrelevant in this context (xend uses the same function for other contexts).

However in order to not differ on the first two execvp needs to be given the opportunity to search the path, which means that you need to check of your newly constructed bootloader exists, and if it doesn't then try and continue with the relative path not the absolute one.

Also since this patch comes before the rename+symlink patch as it stands this patch will temporarily break people who just have "pygrub" without a path in their config.

Ian.

Quoted 41 lines

Signed-off-by George Dunlap<[email protected]>

diff -r 8a86d841e6d4 -r e43157a86933 tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +9 -1

@@ -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,13 @@ int libxl_run_bootloader(libxl_ctx *ctx,
          goto out_close;
      }
 
+    bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
+                                 libxl__libexec_path());
+    if ( bootloader == NULL ) {
+        rc = ERROR_NOMEM;
+        goto out_close;
+    }
+
      /*
       * 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 +425,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
[email protected]
http://lists.xen.org/xen-devel

Quoted 13 lines · George Dunlap

Signed-off-by George Dunlap<[email protected]>

diff -r 52f45b258ccc -r 794778a6e9fa tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +1 -0

@@ -15,6 +15,7 @@
  #include "libxl_osdeps.h" /* must come before any other headers */
 
  #include<termios.h>
+#include<string.h>
 
  #include "libxl_internal.h"
 

tools/libxl/libxl_bootloader.c int libxl_run_bootloader(libxl_ctx *ctx, · line 402

Quoted 5 lines

tools/libxl/libxl_bootloader.c +1 -0

@@ -398,6 +399,11 @@ int libxl_run_bootloader(libxl_ctx *ctx,
          goto out_close;
      }
 
+    if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )

Why strncmp and not just strcmp? And why 20? AFAIK strlen("/usr/bin/pygrub") == 15 or 16 or so...

Quoted 4 lines · George Dunlap

Quoted 1 line

+ if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )

Why strncmp and not just strcmp? And why 20? AFAIK strlen("/usr/bin/pygrub") == 15 or 16 or so...

ISTR in the past build processes throwing warnings that strcmp() is unsafe, and since warnings turn to errors, pre-emptively used the "safe" version instead.

Let me give it a try; it should be perfectly safe for
us, and will remove the issue with manually syncronizing string with its size.

George Dunlap writes ("Re: [Xen-devel] [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated"):

Quoted 3 lines · Ian Campbell

Quoted 2 lines · George Dunlap

Quoted 1 line

+ if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )

Quoted 5 lines

Quoted 2 lines

Why strncmp and not just strcmp? And why 20? AFAIK strlen("/usr/bin/pygrub") == 15 or 16 or so...

ISTR in the past build processes throwing warnings that strcmp() is unsafe, and since warnings turn to errors, pre-emptively used the "safe" version instead.

Boggle. Any such build processes need to be taken out and shot. There is nothing wrong with strcmp. Are you sure you're not thinking of strcat or sprintf ?

George Dunlap writes ("Re: [Xen-devel] [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated"):

Quoted 6 lines · Ian Campbell

Quoted 4 lines · George Dunlap

Quoted 1 line

+ if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )

Why strncmp and not just strcmp? And why 20? AFAIK strlen("/usr/bin/pygrub") == 15 or 16 or so...

Quoted 3 lines

ISTR in the past build processes throwing warnings that strcmp() is unsafe, and since warnings turn to errors, pre-emptively used the "safe" version instead.

Boggle. Any such build processes need to be taken out and shot. There is nothing wrong with strcmp. Are you sure you're not thinking of strcat or sprintf ?

Quoted 9 lines · Ian Jackson

George Dunlap writes ("Re: [Xen-devel] [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated"):

Quoted 8 lines · Ian Campbell

Quoted 4 lines · George Dunlap

Quoted 1 line

+ if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )

Why strncmp and not just strcmp? And why 20? AFAIK strlen("/usr/bin/pygrub") == 15 or 16 or so...

ISTR in the past build processes throwing warnings that strcmp() is unsafe, and since warnings turn to errors, pre-emptively used the "safe" version instead.

Quoted 3 lines

Boggle. Any such build processes need to be taken out and shot. There is nothing wrong with strcmp. Are you sure you're not thinking of strcat or sprintf ?

If the user controlled both the length and contents of info->u.pv.bootloader, it could cause this to overrun that buffer and cause a SEGV. So, sadly, strcmp goes on the 'just never use it' list for many people.

Quoted 4 lines · Tim Deegan

If the user controlled both the length and contents of info->u.pv.bootloader, it could cause this to overrun that buffer and cause a SEGV. So, sadly, strcmp goes on the 'just never use it' list for many people.

Hmm, yes, I suppose it's *technically* possible that even when comparing to a static string, if info->u.pv.bootloader contains a short, non-null-terminated string, and were close to the edge of a page, it could cause a SEGV. But using strncmp wouldn't solve that, would it?

Quoted 3 lines · Ian Jackson

Boggle. Any such build processes need to be taken out and shot. There is nothing wrong with strcmp. Are you sure you're not thinking of strcat or sprintf ?

If the user controlled both the length and contents of info->u.pv.bootloader, it could cause this to overrun that buffer and cause a SEGV. So, sadly, strcmp goes on the 'just never use it' list for many people.

Tim Deegan writes ("Re: [Xen-devel] [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated"):

Quoted 5 lines

Quoted 3 lines · Ian Jackson

Boggle. Any such build processes need to be taken out and shot. There is nothing wrong with strcmp. Are you sure you're not thinking of strcat or sprintf ?

Quoted 4 lines

If the user controlled both the length and contents of info->u.pv.bootloader, it could cause this to overrun that buffer and cause a SEGV. So, sadly, strcmp goes on the 'just never use it' list for many people.

info->u.pv.bootloader is a string. The in-process caller of libxl is required to provide a nul-terminated buffer. In general, strcmp is correct for user-provided strings when the string is a string.