qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support
@ 2015-08-26 22:21 Michael Roth
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe Michael Roth
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Michael Roth @ 2015-08-26 22:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

These patches are based on the qga-dev tree:
  https://github.com/mdroth/qemu/commits/qga-dev
and are also available from:
  https://github.com/mdroth/qemu/commits/qga-dev-build-fixes

These are minor fixes/refactorings to improve the build process for
qemu-ga.

 Makefile  |  10 ++----
 configure | 119 +++++++++++++++++++++++++++++++++++++---------------------------------
 2 files changed, 66 insertions(+), 63 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe
  2015-08-26 22:21 [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
@ 2015-08-26 22:21 ` Michael Roth
  2015-08-27 12:31   ` Marc-André Lureau
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed Michael Roth
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-26 22:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

MSI probe assumes that qemu-ga support has been probed already, but in
cases where --enable-guest-agent/--disable-guest-agent have not been
passed to configure, qemu-ga support may end up getting enabled later,
as is the case with w32 builds. This leads to MSI probe prematurely
reporting error due to lack of qemu-ga support.

Fix this by moving MSI installer probe after the final qga probes.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure | 106 ++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

diff --git a/configure b/configure
index 86a38fe..02d5831 100755
--- a/configure
+++ b/configure
@@ -3905,58 +3905,6 @@ EOF
 fi
 
 ##########################################
-# Guest agent Window MSI  package
-
-if test "$guest_agent" != yes; then
-  if test "$guest_agent_msi" = yes; then
-    error_exit "MSI guest agent package requires guest agent enabled"
-  fi
-  guest_agent_msi=no
-elif test "$mingw32" != "yes"; then
-  if test "$guest_agent_msi" = "yes"; then
-    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
-  fi
-  guest_agent_msi=no
-elif ! has wixl; then
-  if test "$guest_agent_msi" = "yes"; then
-    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
-  fi
-  guest_agent_msi=no
-fi
-
-if test "$guest_agent_msi" != "no"; then
-  if test "$guest_agent_with_vss" = "yes"; then
-    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
-  fi
-
-  if test "$QEMU_GA_MANUFACTURER" = ""; then
-    QEMU_GA_MANUFACTURER=QEMU
-  fi
-
-  if test "$QEMU_GA_DISTRO" = ""; then
-    QEMU_GA_DISTRO=Linux
-  fi
-
-  if test "$QEMU_GA_VERSION" = ""; then
-      QEMU_GA_VERSION=`cat $source_path/VERSION`
-  fi
-
-  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
-  
-  case "$cpu" in
-  x86_64)
-    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
-    ;;
-  i386)
-    QEMU_GA_MSI_ARCH="-D Arch=32"
-    ;;
-  *)
-    error_exit "CPU $cpu not supported for building installation package"
-    ;;
-  esac
-fi
-
-##########################################
 # check if we have fdatasync
 
 fdatasync=no
@@ -4396,6 +4344,9 @@ if test "$softmmu" = yes ; then
     fi
   fi
 fi
+
+# Probe for guest agent support/options
+
 if [ "$guest_agent" != "no" ]; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
       tools="qemu-ga\$(EXESUF) $tools"
@@ -4410,6 +4361,57 @@ if [ "$guest_agent" != "no" ]; then
   fi
 fi
 
+# Guest agent Window MSI  package
+
+if test "$guest_agent" != yes; then
+  if test "$guest_agent_msi" = yes; then
+    error_exit "MSI guest agent package requires guest agent enabled"
+  fi
+  guest_agent_msi=no
+elif test "$mingw32" != "yes"; then
+  if test "$guest_agent_msi" = "yes"; then
+    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
+  fi
+  guest_agent_msi=no
+elif ! has wixl; then
+  if test "$guest_agent_msi" = "yes"; then
+    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
+  fi
+  guest_agent_msi=no
+fi
+
+if test "$guest_agent_msi" != "no"; then
+  if test "$guest_agent_with_vss" = "yes"; then
+    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
+  fi
+
+  if test "$QEMU_GA_MANUFACTURER" = ""; then
+    QEMU_GA_MANUFACTURER=QEMU
+  fi
+
+  if test "$QEMU_GA_DISTRO" = ""; then
+    QEMU_GA_DISTRO=Linux
+  fi
+
+  if test "$QEMU_GA_VERSION" = ""; then
+      QEMU_GA_VERSION=`cat $source_path/VERSION`
+  fi
+
+  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
+
+  case "$cpu" in
+  x86_64)
+    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
+    ;;
+  i386)
+    QEMU_GA_MSI_ARCH="-D Arch=32"
+    ;;
+  *)
+    error_exit "CPU $cpu not supported for building installation package"
+    ;;
+  esac
+fi
+
 # Mac OS X ships with a broken assembler
 roms=
 if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
  2015-08-26 22:21 [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe Michael Roth
@ 2015-08-26 22:21 ` Michael Roth
  2015-08-27 12:41   ` Marc-André Lureau
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies Michael Roth
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message Michael Roth
  3 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-26 22:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

Currently, if we don't explicitly disable support for MSI installer
via --disable-guest-agent-msi, the configure variable that tracks
the flag, 'guest_agent_msi', never gets set unless one of the probes
fails. Subsequent code then treats this unset value the same as if it
were a "yes" value (via != "no" style checks).

Instead, set the default "yes" value explicitly after the probes, then
make subsequent code expect the values to be set.

This makes it easier to report on whether or not MSI support was
enabled via probe by looking at the ./configure summary.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 02d5831..90e5351 100755
--- a/configure
+++ b/configure
@@ -4378,9 +4378,15 @@ elif ! has wixl; then
     error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
   fi
   guest_agent_msi=no
+else
+  # we support qemu-ga, mingw32, and wixl: default to MSI enabled if it wasn't
+  # disabled explicitly
+  if test "$guest_agent_msi" != "no"; then
+    guest_agent_msi=yes
+  fi
 fi
 
-if test "$guest_agent_msi" != "no"; then
+if test "$guest_agent_msi" = "yes"; then
   if test "$guest_agent_with_vss" = "yes"; then
     QEMU_GA_MSI_WITH_VSS="-D InstallVss"
   fi
@@ -4659,7 +4665,7 @@ if test "$mingw32" = "yes" ; then
   if test "$guest_agent_ntddscsi" = "yes" ; then
     echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak
   fi
-  if test "$guest_agent_msi" != "no"; then
+  if test "$guest_agent_msi" = "yes"; then
     echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak  
     echo "QEMU_GA_MSI_MINGW_DLL_PATH=${QEMU_GA_MSI_MINGW_DLL_PATH}" >> $config_host_mak
     echo "QEMU_GA_MSI_WITH_VSS=${QEMU_GA_MSI_WITH_VSS}" >> $config_host_mak
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
  2015-08-26 22:21 [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe Michael Roth
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed Michael Roth
@ 2015-08-26 22:21 ` Michael Roth
  2015-08-27 12:49   ` Marc-André Lureau
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message Michael Roth
  3 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-26 22:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

Currently VSS dll/tlb files for use in w32 builds are only built as a
result of having been added to the general 'tools' target alongside
qemu-ga. This is fine for default make target, but if we build
qemu-ga directly via `make qemu-ga.exe`, the VSS files are not
created.

Fix this by moving the VSS dependencies to qemu-ga.exe directly.
With this move we can move the VSS files back out of 'tools',
and drop the extra handling from MSI target in Makefile.

Now we can build qemu-ga MSI package with:
  ./configure ...
  make qemu-ga.exe
  make msi

or simply:
  ./configure ...
  make msi

and no longer need to do a full build beforehand.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile  | 8 ++------
 configure | 5 ++---
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index c13a83d..137beb9 100644
--- a/Makefile
+++ b/Makefile
@@ -290,8 +290,8 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
-	$(call LINK, $^)
+qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
+	$(call LINK, $(filter-out %.tlb %.dll, $^))
 
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
@@ -300,10 +300,6 @@ msi: $(QEMU_GA_MSI)
 
 $(QEMU_GA_MSI): qemu-ga.exe
 
-ifdef QEMU_GA_MSI_WITH_VSS
-$(QEMU_GA_MSI): qga/vss-win32/qga-vss.dll
-endif
-
 $(QEMU_GA_MSI): config-host.mak
 
 $(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
diff --git a/configure b/configure
index 90e5351..db88b0d 100755
--- a/configure
+++ b/configure
@@ -3851,6 +3851,7 @@ EOF
     guest_agent_with_vss="yes"
     QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
     libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ -Wl,--enable-stdcall-fixup $libs_qga"
+    qga_vss_provider="qga/vss-win32/qga-vss.dll qga/vss-win32/qga-vss.tlb"
   else
     if test "$vss_win32_sdk" != "" ; then
       echo "ERROR: Please download and install Microsoft VSS SDK:"
@@ -4350,9 +4351,6 @@ fi
 if [ "$guest_agent" != "no" ]; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
       tools="qemu-ga\$(EXESUF) $tools"
-      if [ "$mingw32" = "yes" -a "$guest_agent_with_vss" = "yes" ]; then
-        tools="qga/vss-win32/qga-vss.dll qga/vss-win32/qga-vss.tlb $tools"
-      fi
       guest_agent=yes
   elif [ "$guest_agent" != yes ]; then
       guest_agent=no
@@ -4660,6 +4658,7 @@ if test "$mingw32" = "yes" ; then
   echo "CONFIG_PRODUCTVERSION=$version_major,$version_minor,$version_subminor,$version_micro" >> $config_host_mak
   if test "$guest_agent_with_vss" = "yes" ; then
     echo "CONFIG_QGA_VSS=y" >> $config_host_mak
+    echo "QGA_VSS_PROVIDER=$qga_vss_provider" >> $config_host_mak
     echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak
   fi
   if test "$guest_agent_ntddscsi" = "yes" ; then
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message
  2015-08-26 22:21 [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
                   ` (2 preceding siblings ...)
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies Michael Roth
@ 2015-08-26 22:21 ` Michael Roth
  2015-08-27 12:50   ` Marc-André Lureau
  3 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-26 22:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

'msi' target reports error if we attempt to use it when QEMU hasn't
been ./configure'd to enable it. The parenthesis cause an interpreter
error if we don't enclose the error in quotes.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 137beb9..3814026 100644
--- a/Makefile
+++ b/Makefile
@@ -307,7 +307,7 @@ $(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
 	wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) $(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
 else
 msi:
-	@echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)
+	@echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"
 endif
 
 clean:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe Michael Roth
@ 2015-08-27 12:31   ` Marc-André Lureau
  0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 12:31 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
> MSI probe assumes that qemu-ga support has been probed already, but in
> cases where --enable-guest-agent/--disable-guest-agent have not been
> passed to configure, qemu-ga support may end up getting enabled later,
> as is the case with w32 builds. This leads to MSI probe prematurely
> reporting error due to lack of qemu-ga support.
>
> Fix this by moving MSI installer probe after the final qga probes.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



>  configure | 106 ++++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 54 insertions(+), 52 deletions(-)
>
> diff --git a/configure b/configure
> index 86a38fe..02d5831 100755
> --- a/configure
> +++ b/configure
> @@ -3905,58 +3905,6 @@ EOF
>  fi
>
>  ##########################################
> -# Guest agent Window MSI  package
> -
> -if test "$guest_agent" != yes; then
> -  if test "$guest_agent_msi" = yes; then
> -    error_exit "MSI guest agent package requires guest agent enabled"
> -  fi
> -  guest_agent_msi=no
> -elif test "$mingw32" != "yes"; then
> -  if test "$guest_agent_msi" = "yes"; then
> -    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
> -  fi
> -  guest_agent_msi=no
> -elif ! has wixl; then
> -  if test "$guest_agent_msi" = "yes"; then
> -    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
> -  fi
> -  guest_agent_msi=no
> -fi
> -
> -if test "$guest_agent_msi" != "no"; then
> -  if test "$guest_agent_with_vss" = "yes"; then
> -    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
> -  fi
> -
> -  if test "$QEMU_GA_MANUFACTURER" = ""; then
> -    QEMU_GA_MANUFACTURER=QEMU
> -  fi
> -
> -  if test "$QEMU_GA_DISTRO" = ""; then
> -    QEMU_GA_DISTRO=Linux
> -  fi
> -
> -  if test "$QEMU_GA_VERSION" = ""; then
> -      QEMU_GA_VERSION=`cat $source_path/VERSION`
> -  fi
> -
> -  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
> -
> -  case "$cpu" in
> -  x86_64)
> -    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
> -    ;;
> -  i386)
> -    QEMU_GA_MSI_ARCH="-D Arch=32"
> -    ;;
> -  *)
> -    error_exit "CPU $cpu not supported for building installation package"
> -    ;;
> -  esac
> -fi
> -
> -##########################################
>  # check if we have fdatasync
>
>  fdatasync=no
> @@ -4396,6 +4344,9 @@ if test "$softmmu" = yes ; then
>      fi
>    fi
>  fi
> +
> +# Probe for guest agent support/options
> +
>  if [ "$guest_agent" != "no" ]; then
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
>        tools="qemu-ga\$(EXESUF) $tools"
> @@ -4410,6 +4361,57 @@ if [ "$guest_agent" != "no" ]; then
>    fi
>  fi
>
> +# Guest agent Window MSI  package
> +
> +if test "$guest_agent" != yes; then
> +  if test "$guest_agent_msi" = yes; then
> +    error_exit "MSI guest agent package requires guest agent enabled"
> +  fi
> +  guest_agent_msi=no
> +elif test "$mingw32" != "yes"; then
> +  if test "$guest_agent_msi" = "yes"; then
> +    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
> +  fi
> +  guest_agent_msi=no
> +elif ! has wixl; then
> +  if test "$guest_agent_msi" = "yes"; then
> +    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
> +  fi
> +  guest_agent_msi=no
> +fi
> +
> +if test "$guest_agent_msi" != "no"; then
> +  if test "$guest_agent_with_vss" = "yes"; then
> +    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
> +  fi
> +
> +  if test "$QEMU_GA_MANUFACTURER" = ""; then
> +    QEMU_GA_MANUFACTURER=QEMU
> +  fi
> +
> +  if test "$QEMU_GA_DISTRO" = ""; then
> +    QEMU_GA_DISTRO=Linux
> +  fi
> +
> +  if test "$QEMU_GA_VERSION" = ""; then
> +      QEMU_GA_VERSION=`cat $source_path/VERSION`
> +  fi
> +
> +  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
> +
> +  case "$cpu" in
> +  x86_64)
> +    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
> +    ;;
> +  i386)
> +    QEMU_GA_MSI_ARCH="-D Arch=32"
> +    ;;
> +  *)
> +    error_exit "CPU $cpu not supported for building installation package"
> +    ;;
> +  esac
> +fi
> +
>  # Mac OS X ships with a broken assembler
>  roms=
>  if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \
> --
> 1.9.1
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed Michael Roth
@ 2015-08-27 12:41   ` Marc-André Lureau
  2015-08-27 13:42     ` Michael Roth
  0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 12:41 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Hi

On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
> This makes it easier to report on whether or not MSI support was
> enabled via probe by looking at the ./configure summary.

Sorry I don't get what that really changes. Otherwise the patch looks fine.


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies Michael Roth
@ 2015-08-27 12:49   ` Marc-André Lureau
  2015-08-27 13:46     ` Michael Roth
  0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 12:49 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Hi

On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
> +qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
> +       $(call LINK, $(filter-out %.tlb %.dll, $^))

Strictly this is not so great, but that makes sense: so perhaps a
small comment above to explain it would be good.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message
  2015-08-26 22:21 ` [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message Michael Roth
@ 2015-08-27 12:50   ` Marc-André Lureau
  0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 12:50 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
> +       @echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
  2015-08-27 12:41   ` Marc-André Lureau
@ 2015-08-27 13:42     ` Michael Roth
  2015-08-27 14:05       ` Marc-André Lureau
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-27 13:42 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Quoting Marc-André Lureau (2015-08-27 07:41:17)
> Hi
> 
> On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
> <mdroth@linux.vnet.ibm.com> wrote:
> > This makes it easier to report on whether or not MSI support was
> > enabled via probe by looking at the ./configure summary.
> 
> Sorry I don't get what that really changes. Otherwise the patch looks fine.

If we keep the $enabled != "no" logic all the way through, $enabled,
unless explicitly turned off or missing dependencies, will be
undefined when the configure options are summarized via:

  configure: qemu-ga: report MSI install support in summary

requiring the user to infer whether we'll default to "yes" or "no".
The current logic in fact defaults to "yes" in function, so with this
patch we're simply setting that flag early on so we can report "yes"
rather than undefined.

> 
> 
> -- 
> Marc-André Lureau
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
  2015-08-27 12:49   ` Marc-André Lureau
@ 2015-08-27 13:46     ` Michael Roth
  2015-08-27 14:00       ` Marc-André Lureau
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2015-08-27 13:46 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Quoting Marc-André Lureau (2015-08-27 07:49:19)
> Hi
> 
> On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth
> <mdroth@linux.vnet.ibm.com> wrote:
> > +qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
> > +       $(call LINK, $(filter-out %.tlb %.dll, $^))
> 
> Strictly this is not so great, but that makes sense: so perhaps a
> small comment above to explain it would be good.

Will add a comment either way, but perhaps something like:

qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
       $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))

would be clearer? I suppose it's a bit more future-proof as well, if we added
more dependencies to QGA_VSS_PROVIDER for some reason.

> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> 
> 
> -- 
> Marc-André Lureau
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
  2015-08-27 13:46     ` Michael Roth
@ 2015-08-27 14:00       ` Marc-André Lureau
  0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 14:00 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

On Thu, Aug 27, 2015 at 3:46 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Will add a comment either way, but perhaps something like:
>
thanks

> qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
>        $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))
>
> would be clearer? I suppose it's a bit more future-proof as well, if we added
> more dependencies to QGA_VSS_PROVIDER for some reason.

agree, it's better.


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
  2015-08-27 13:42     ` Michael Roth
@ 2015-08-27 14:05       ` Marc-André Lureau
  2015-08-27 19:10         ` Michael Roth
  0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2015-08-27 14:05 UTC (permalink / raw)
  To: Michael Roth; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Hi

On Thu, Aug 27, 2015 at 3:42 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> If we keep the $enabled != "no" logic all the way through, $enabled,
> unless explicitly turned off or missing dependencies, will be
> undefined when the configure options are summarized via:
>
>   configure: qemu-ga: report MSI install support in summary
>
> requiring the user to infer whether we'll default to "yes" or "no".
> The current logic in fact defaults to "yes" in function, so with this
> patch we're simply setting that flag early on so we can report "yes"
> rather than undefined.

Where is it reported? I don't see that in the configure summary, did
you add it in another patch?


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
  2015-08-27 14:05       ` Marc-André Lureau
@ 2015-08-27 19:10         ` Michael Roth
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2015-08-27 19:10 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Paolo Bonzini, yhindin, Leonid Bloch, QEMU

Quoting Marc-André Lureau (2015-08-27 09:05:40)
> Hi
> 
> On Thu, Aug 27, 2015 at 3:42 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > If we keep the $enabled != "no" logic all the way through, $enabled,
> > unless explicitly turned off or missing dependencies, will be
> > undefined when the configure options are summarized via:
> >
> >   configure: qemu-ga: report MSI install support in summary
> >
> > requiring the user to infer whether we'll default to "yes" or "no".
> > The current logic in fact defaults to "yes" in function, so with this
> > patch we're simply setting that flag early on so we can report "yes"
> > rather than undefined.
> 
> Where is it reported? I don't see that in the configure summary, did
> you add it in another patch?

Yes, recently patch, only applied to qga-dev currently:

  https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg03170.html

> 
> 
> -- 
> Marc-André Lureau
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message
  2015-08-27 23:55 [Qemu-devel] [PATCH v2 0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
@ 2015-08-27 23:55 ` Michael Roth
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Roth @ 2015-08-27 23:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, yhindin, leonid, pbonzini

'msi' target reports error if we attempt to use it when QEMU hasn't
been ./configure'd to enable it. The parenthesis cause an interpreter
error if we don't enclose the error in quotes.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dbdeb47..9ce3972 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ $(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
 	wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) $(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
 else
 msi:
-	@echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)
+	@echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"
 endif
 
 clean:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-08-27 23:56 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-26 22:21 [Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
2015-08-26 22:21 ` [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe Michael Roth
2015-08-27 12:31   ` Marc-André Lureau
2015-08-26 22:21 ` [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed Michael Roth
2015-08-27 12:41   ` Marc-André Lureau
2015-08-27 13:42     ` Michael Roth
2015-08-27 14:05       ` Marc-André Lureau
2015-08-27 19:10         ` Michael Roth
2015-08-26 22:21 ` [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies Michael Roth
2015-08-27 12:49   ` Marc-André Lureau
2015-08-27 13:46     ` Michael Roth
2015-08-27 14:00       ` Marc-André Lureau
2015-08-26 22:21 ` [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message Michael Roth
2015-08-27 12:50   ` Marc-André Lureau
  -- strict thread matches above, loose matches on Subject: below --
2015-08-27 23:55 [Qemu-devel] [PATCH v2 0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support Michael Roth
2015-08-27 23:55 ` [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message Michael Roth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).