diff mbox

move qemu-ga from bin to libexec dir, use $HELPERS

Message ID 1361039311-28040-1-git-send-email-mjt@msgid.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev Feb. 16, 2013, 6:28 p.m. UTC
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.

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).

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 Makefile  |   10 ++++------
 configure |    7 ++++++-
 2 files changed, 10 insertions(+), 7 deletions(-)

Comments

Michael Tokarev Feb. 21, 2013, 12:49 p.m. UTC | #1
Ping?

I received no answer neither to my original question
("why qemu-ga is in /usr/bin") nor to this patch.

Thanks,

/mjt

16.02.2013 22:28, 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.
> 
> 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).
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  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
>
Paolo Bonzini Feb. 21, 2013, 1:35 p.m. UTC | #2
Il 16/02/2013 19:28, Michael Tokarev ha scritto:
> This patch does 3 things:
> 
> 1. Renames HELPERS-y Makefile variable to HELPERS
> 2. Moves its definition from Makefile to configure

I prefer to have more decisions in Makefile than configure, but this
wouldn't block the patch.  What we have now is a mess anyway.

> 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.

There is one difference, and an important one: qemu-ga does appear in
system-wide configuration files, while qemu-bridge-helper does not.  In
this sense, qemu-ga is not a helper executable.

I have no idea how virtfs-proxy-helper would work, but I suspect that a
better design would have QEMU spawning it, just like qemu-bridge-helper.

Paolo

> 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).
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  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
>
Michael Tokarev Feb. 21, 2013, 2:30 p.m. UTC | #3
21.02.2013 17:35, Paolo Bonzini wrote:
> Il 16/02/2013 19:28, Michael Tokarev ha scritto:
>> This patch does 3 things:
>>
>> 1. Renames HELPERS-y Makefile variable to HELPERS
>> 2. Moves its definition from Makefile to configure
> 
> I prefer to have more decisions in Makefile than configure, but this
> wouldn't block the patch.  What we have now is a mess anyway.

What's the difference between checking for host OS type being
in makefile or configure?

Note that in this very case current condition is a bit wrong:
it should not only depend on linux but also about presence of
softmmu targets (which this patch fixes too).

>> 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.
> 
> There is one difference, and an important one: qemu-ga does appear in
> system-wide configuration files, while qemu-bridge-helper does not.  In
> this sense, qemu-ga is not a helper executable.

Well, it definitely is not a user-callable binary.  In that sence it is a
"system helper" (as opposed to "qemu helper" for qemu-bridge-helper).  Ie,
either sbin or libexec, but not bin.  There's no sbindir handling currently, --
neither in makefile nor in configure, only "TOOLS" and "HELPERS" variables.

> I have no idea how virtfs-proxy-helper would work, but I suspect that a
> better design would have QEMU spawning it, just like qemu-bridge-helper.

QEMU can't spawn it, it is spawned in *guest* by a startup script or some
event daemon (such as systemd or udev).

Thanks,

/mjt
Paolo Bonzini Feb. 21, 2013, 2:39 p.m. UTC | #4
Il 21/02/2013 15:30, Michael Tokarev ha scritto:
> 21.02.2013 17:35, Paolo Bonzini wrote:
>> Il 16/02/2013 19:28, Michael Tokarev ha scritto:
>>> This patch does 3 things:
>>>
>>> 1. Renames HELPERS-y Makefile variable to HELPERS
>>> 2. Moves its definition from Makefile to configure
>>
>> I prefer to have more decisions in Makefile than configure, but this
>> wouldn't block the patch.  What we have now is a mess anyway.
> 
> What's the difference between checking for host OS type being
> in makefile or configure?

That you list all the build products in the Makefile, where they should be.

> Note that in this very case current condition is a bit wrong:
> it should not only depend on linux but also about presence of
> softmmu targets (which this patch fixes too).
> 
>>> 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.
>>
>> There is one difference, and an important one: qemu-ga does appear in
>> system-wide configuration files, while qemu-bridge-helper does not.  In
>> this sense, qemu-ga is not a helper executable.
> 
> Well, it definitely is not a user-callable binary.

Since we do not ship udev rules, we are really shipping it for the user
to call it.  How it does that, we don't care.

> In that sence it is a
> "system helper" (as opposed to "qemu helper" for qemu-bridge-helper).  Ie,
> either sbin or libexec, but not bin.  There's no sbindir handling currently, --
> neither in makefile nor in configure, only "TOOLS" and "HELPERS" variables.

sbindir would be more correct than libexecdir.

The latest fad for udev is to put helpers in $prefix/lib/udev (not
$libdir, because there's no 32/64-bit differentiation).

Perhaps the best solution is to add --with-qemu-ga-dir=... and default
it to $bindir.  Then distros that ship udev rules can move it to
/usr/lib/udev, distros that ship an initscript can move it to /usr/sbin
or wherever they prefer.

>> I have no idea how virtfs-proxy-helper would work, but I suspect that a
>> better design would have QEMU spawning it, just like qemu-bridge-helper.
> 
> QEMU can't spawn it, it is spawned in *guest* by a startup script or some
> event daemon (such as systemd or udev).

I'm talking about virtfs-proxy-helper.

Paolo
Michael Roth Feb. 21, 2013, 2:45 p.m. UTC | #5
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 <mjt@tls.msk.ru>
> ---
>  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
>
Doug Goldstein Feb. 21, 2013, 2:46 p.m. UTC | #6
On Thu, Feb 21, 2013 at 8:30 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 21.02.2013 17:35, Paolo Bonzini wrote:
>> Il 16/02/2013 19:28, Michael Tokarev ha scritto:
>>> This patch does 3 things:
>>>
>>> 1. Renames HELPERS-y Makefile variable to HELPERS
>>> 2. Moves its definition from Makefile to configure
>>
>> I prefer to have more decisions in Makefile than configure, but this
>> wouldn't block the patch.  What we have now is a mess anyway.
>
> What's the difference between checking for host OS type being
> in makefile or configure?
>
> Note that in this very case current condition is a bit wrong:
> it should not only depend on linux but also about presence of
> softmmu targets (which this patch fixes too).
>
>>> 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.
>>
>> There is one difference, and an important one: qemu-ga does appear in
>> system-wide configuration files, while qemu-bridge-helper does not.  In
>> this sense, qemu-ga is not a helper executable.
>
> Well, it definitely is not a user-callable binary.  In that sence it is a
> "system helper" (as opposed to "qemu helper" for qemu-bridge-helper).  Ie,
> either sbin or libexec, but not bin.  There's no sbindir handling currently, --
> neither in makefile nor in configure, only "TOOLS" and "HELPERS" variables.

Just to throw my two cents in here, I agree with Paolo. I don't know
of any programs started by init scripts that live in /usr/libexec.
I've grepped on a Gentoo box, RHEL5 and RHEL6 for libexec in
/etc/init.d and none of them have any references. So it seems like a
better place is /usr/bin (I could see /usr/sbin but I thought that was
also targeted by the /usr merge to go away and just have it all in
/usr/bin).


>
>> I have no idea how virtfs-proxy-helper would work, but I suspect that a
>> better design would have QEMU spawning it, just like qemu-bridge-helper.
>
> QEMU can't spawn it, it is spawned in *guest* by a startup script or some
> event daemon (such as systemd or udev).
>
> Thanks,
>
> /mjt
>
Michael Roth Feb. 21, 2013, 3:16 p.m. UTC | #7
On Thu, Feb 21, 2013 at 03:39:44PM +0100, Paolo Bonzini wrote:
> Il 21/02/2013 15:30, Michael Tokarev ha scritto:
> > 21.02.2013 17:35, Paolo Bonzini wrote:
> >> Il 16/02/2013 19:28, Michael Tokarev ha scritto:
> >>> This patch does 3 things:
> >>>
> >>> 1. Renames HELPERS-y Makefile variable to HELPERS
> >>> 2. Moves its definition from Makefile to configure
> >>
> >> I prefer to have more decisions in Makefile than configure, but this
> >> wouldn't block the patch.  What we have now is a mess anyway.
> > 
> > What's the difference between checking for host OS type being
> > in makefile or configure?
> 
> That you list all the build products in the Makefile, where they should be.
> 
> > Note that in this very case current condition is a bit wrong:
> > it should not only depend on linux but also about presence of
> > softmmu targets (which this patch fixes too).
> > 
> >>> 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.
> >>
> >> There is one difference, and an important one: qemu-ga does appear in
> >> system-wide configuration files, while qemu-bridge-helper does not.  In
> >> this sense, qemu-ga is not a helper executable.
> > 
> > Well, it definitely is not a user-callable binary.
> 
> Since we do not ship udev rules, we are really shipping it for the user
> to call it.  How it does that, we don't care.
> 
> > In that sence it is a
> > "system helper" (as opposed to "qemu helper" for qemu-bridge-helper).  Ie,
> > either sbin or libexec, but not bin.  There's no sbindir handling currently, --
> > neither in makefile nor in configure, only "TOOLS" and "HELPERS" variables.
> 
> sbindir would be more correct than libexecdir.
> 
> The latest fad for udev is to put helpers in $prefix/lib/udev (not
> $libdir, because there's no 32/64-bit differentiation).
> 
> Perhaps the best solution is to add --with-qemu-ga-dir=... and default
> it to $bindir.  Then distros that ship udev rules can move it to
> /usr/lib/udev, distros that ship an initscript can move it to /usr/sbin
> or wherever they prefer.

Seem reasonable to me. If qemu-ga isn't a TOOL or HELPER, it probably
doesn't make sense to think up a new group to configure it until we have
other examples to consider.

> 
> >> I have no idea how virtfs-proxy-helper would work, but I suspect that a
> >> better design would have QEMU spawning it, just like qemu-bridge-helper.
> > 
> > QEMU can't spawn it, it is spawned in *guest* by a startup script or some
> > event daemon (such as systemd or udev).
> 
> I'm talking about virtfs-proxy-helper.
> 
> Paolo
> 
>
diff mbox

Patch

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