Re: [Qemu-devel] [PATCH] move qemu-ga from bin to libexec dir, use $HELPERS

[mdroth]

On Sat, Feb 16, 2013 at 10:28:31PM +0400, Michael Tokarev wrote:
> This patch does 3 things:
> 
> 1. Renames HELPERS-y Makefile variable to HELPERS
> 2. Moves its definition from Makefile to configure
> 3. Moves qemu-ga binary from TOOLS to HELPERS.
> 
> The effects are:
> 
> 1. qemu-ga binary is now installed into libexecdir, not bindir.
> This is the main effect/motivation of this patch, -- this binary
> has no business being in a public binary directory, it is a system
> helper which must be run by a system startup script or some event
> daemon.

I agree it's not a public binary, but I think there's a pretty close
analogue between qemu-ga and things like sshd, tftpd, etc, which
generally live in /usr/sbin. qemu-ga is a "top-level" daemon, there's
really nothing else that relies on it as a helper, so I think something
like ADMIN_TOOLS that gets installed in sbin might make more sense.

> 
> 2. Another helper, qemu-bridge-helper, which is already installed
> in libexecdir, is built only when we're building one of softmmu
> targets on linux (initially it was just linux-specific, but not
> softmmu-specific).

I think that seems reasonable (along with the corresponding bit in the
patch).

qemu-ga however should get moved out of the $softmmu = "yes" block
and instead be tied to --enable-tools, or something similar like
--enable-guest-tools, or --enable-admin-tools or whatever. it does in
some sense require a softmmu target to ever run, but the build for the
guest environment should be thought of as a seperate build (that we
might just happen to do at the same time as the build for the host
package in some circumstances to save time/cycles), and in that sense
there's no dependency on a softmmu target being configured.

> 
> Signed-off-by: Michael Tokarev <address@hidden>
> ---
>  Makefile  |   10 ++++------
>  configure |    7 ++++++-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0d9099a..ba0cd98 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -53,8 +53,6 @@ $(call set-vpath, $(SRC_PATH))
> 
>  LIBS+=-lz $(LIBS_TOOLS)
> 
> -HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
> -
>  ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 
> QMP/qmp-commands.txt
>  ifdef CONFIG_VIRTFS
> @@ -115,7 +113,7 @@ ifeq ($(CONFIG_SMARTCARD_NSS),y)
>  include $(SRC_PATH)/libcacard/Makefile
>  endif
> 
> -all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
> +all: $(DOCS) $(TOOLS) $(HELPERS) recurse-all
> 
>  config-host.h: config-host.h-timestamp
>  config-host.h-timestamp: config-host.mak
> @@ -215,7 +213,7 @@ clean:
>       rm -f qemu-options.def
>       find . -name '*.[oda]' -type f -exec rm -f {} +
>       find . -name '*.l[oa]' -type f -exec rm -f {} +
> -     rm -f $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
> +     rm -f $(TOOLS) $(HELPERS) qemu-ga TAGS cscope.* *.pod *~ */*~
>       rm -Rf .libs
>       rm -f qemu-img-cmds.h
>       @# May not be present in GENERATED_HEADERS
> @@ -305,9 +303,9 @@ install: all $(if $(BUILD_DOCS),install-doc) 
> install-sysconfig install-datadir
>  ifneq ($(TOOLS),)
>       $(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
>  endif
> -ifneq ($(HELPERS-y),)
> +ifneq ($(HELPERS),)
>       $(INSTALL_DIR) "$(DESTDIR)$(libexecdir)"
> -     $(INSTALL_PROG) $(STRIP_OPT) $(HELPERS-y) "$(DESTDIR)$(libexecdir)"
> +     $(INSTALL_PROG) $(STRIP_OPT) $(HELPERS) "$(DESTDIR)$(libexecdir)"
>  endif
>  ifneq ($(BLOBS),)
>       set -e; for x in $(BLOBS); do \
> diff --git a/configure b/configure
> index 8789324..304c648 100755
> --- a/configure
> +++ b/configure
> @@ -3204,6 +3204,7 @@ qemu_confdir=$sysconfdir$confsuffix
>  qemu_datadir=$datadir$confsuffix
> 
>  tools=""
> +helpers=""
>  if test "$want_tools" = "yes" ; then
>    tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
> @@ -3225,9 +3226,12 @@ if test "$softmmu" = yes ; then
>    fi
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>      if [ "$guest_agent" = "yes" ]; then
> -      tools="qemu-ga\$(EXESUF) $tools"
> +      helpers="qemu-ga\$(EXESUF) $helpers"
>      fi
>    fi
> +  if [ "$linux" = "yes"  ] ; then
> +     helpers="qemu-bridge-helper\$(EXESUF) $helpers"
> +  fi
>  fi
> 
>  # Mac OS X ships with a broken assembler
> @@ -3744,6 +3748,7 @@ if test "$trace_default" = "yes"; then
>  fi
> 
>  echo "TOOLS=$tools" >> $config_host_mak
> +echo "HELPERS=$helpers" >> $config_host_mak
>  echo "ROMS=$roms" >> $config_host_mak
>  echo "MAKE=$make" >> $config_host_mak
>  echo "INSTALL=$install" >> $config_host_mak
> -- 
> 1.7.10.4
>