[Qemu-devel] [PATCH v2 0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support

[Qemu-devel] [PATCH v2 0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support

[Michael Roth]

These patches are based on the qga tree:
  https://github.com/mdroth/qemu/commits/qga
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.

v2:
* filter $(QGA_VSS_PROVIDER) from qemu-ga LINK target instead of wildcard
  matches against %.tlb and %.dll (Marc-André)
* add comments to clarify the above behavior (Marc-André)

 Makefile  |  13 ++++++-------
 configure | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------
 2 files changed, 69 insertions(+), 63 deletions(-)

[Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe

[Michael Roth]

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.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
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 1f033e9..c31c652 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

[Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Michael Roth]

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

[Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Michael Roth]

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.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile  | 11 +++++------
 configure |  5 ++---
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index c13a83d..dbdeb47 100644
--- a/Makefile
+++ b/Makefile
@@ -290,8 +290,11 @@ $(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, $^)
+# we require QGA_VSS_PROVIDER files to be built alongside qemu-ga
+# executable since they are shipped together, but we don't want to actually
+# link against them
+qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
+	$(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))
 
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
@@ -300,10 +303,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 a52ef50..21c4089 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

Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Paolo Bonzini]

On 28/08/2015 01:55, Michael Roth wrote:
> Now we can build qemu-ga MSI package with:
>   ./configure ...
>   make qemu-ga.exe
>   make msi
> 
> or simply:
>   ./configure ...
>   make msi

Shouldn't the latter have always worked?

I think that if someone does "make qemu-ga.exe" they should *not* get
the VSS files.  Perhaps we can add a Win32-specific phony qemu-ga target
to build both qemu-ga.exe and the VSS files, but this patch's use of
filter-out is a bit ugly.

Paolo

Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Michael Roth]

Quoting Paolo Bonzini (2015-09-07 05:48:27)
> 
> 
> On 28/08/2015 01:55, Michael Roth wrote:
> > Now we can build qemu-ga MSI package with:
> >   ./configure ...
> >   make qemu-ga.exe
> >   make msi
> > 
> > or simply:
> >   ./configure ...
> >   make msi
> 
> Shouldn't the latter have always worked?

Hmm, at the time of the patch I'm not sure, since out-of-tree builds
were broken with `make msi` until the recent:

  decdfbd qemu-ga: Fixed paths issue with MSI build

With that patch in place I noticed out-of-tree builds were still
broken:

  [mdroth@vm4 qemu-build-w64]$ ../w/qemu4.git/configure --enable-guest-agent --target-list=x86_64-softmmu --extra-cflags=-Wall --enable-guest-agent-msi --cross-prefix=x86_64-w64-mingw32- --with-vss-sdk=/home/mdroth/w/vss-win32/ && make msi
  ...
    AR    libqemustub.a
    LINK  qemu-ga.exe
    CXX   qga/vss-win32/requester.o
  ...
    CXX   qga/vss-win32/provider.o
    CXX   qga/vss-win32/install.o
    LINK  qga/vss-win32/qga-vss.dll
    WIXL  qemu-ga-x86_64.msi
  Couldn't find file /home/mdroth/qemu-build-w64/qga/vss-win32/qga-vss.tlb
  make: *** [qemu-ga-x86_64.msi] Error 1

For out-of-tree, qga-vss.tlb dependency gets met by QEMU tools target, which
is what motivated this patch.

But for in-tree builds, qga-vss.tlb is already present in working directory,
so it might have actually worked for that case.

So I may have misdiagnosed the root issue here: that *.tlb (not just
*.dll) needed to be added to MSI dependency list instead of being
assumed (via in-tree build or full qemu build with tools).

> 
> I think that if someone does "make qemu-ga.exe" they should *not* get
> the VSS files.  Perhaps we can add a Win32-specific phony qemu-ga target
> to build both qemu-ga.exe and the VSS files, but this patch's use of
> filter-out is a bit ugly.

I think it might make sense to re-de-couple VSS from qemu-ga.exe, but
I'm not sure I like the idea of making MSI target responsible for
VSS files. MSI is relatively new, but VSS support has been around for a
while when documentated install procedure for qemu-ga.exe was manually
copying files. MSI isn't the source of the issue, presumably everybody
distributing qemu-ga.exe in this manner was relying on the full build
to get the VSS files. But moving VSS completely to MSI means that use
case would break (whereas moving them to qemu-ga.exe would still work,
since qemu-ga.exe is build as part of default/full build)

I think the ideal solution is too keep things as they are with this
patch (previous working methods supported), but move to a new 'qemu-ga'
build target that just does the right thing on each platform:

  on posix:
    1) build 'qemu-ga' executable
  
  on mingw:
    1) build 'qemu-ga.exe' executable (no need to build .exe directly)
    2) build VSS if VSS supported/requested
    3) build MSI package if supported/requested (and include VSS files
       if VSS supported)

That would let us drop the wierd filtering, and bring the w32 build
process more inline with posix.

I'm not even sure if that's possible atm, but if that's reasonable I can
look into it as a follow-up (this series is merged already).

> 
> Paolo
> 

Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Michael Roth]

Quoting Michael Roth (2015-09-07 14:55:47)
> Quoting Paolo Bonzini (2015-09-07 05:48:27)
> > 
> > 
> > On 28/08/2015 01:55, Michael Roth wrote:
> > > Now we can build qemu-ga MSI package with:
> > >   ./configure ...
> > >   make qemu-ga.exe
> > >   make msi
> > > 
> > > or simply:
> > >   ./configure ...
> > >   make msi
> > 
> > Shouldn't the latter have always worked?
> 
> Hmm, at the time of the patch I'm not sure, since out-of-tree builds
> were broken with `make msi` until the recent:
> 
>   decdfbd qemu-ga: Fixed paths issue with MSI build
> 
> With that patch in place I noticed out-of-tree builds were still
> broken:
> 
>   [mdroth@vm4 qemu-build-w64]$ ../w/qemu4.git/configure --enable-guest-agent --target-list=x86_64-softmmu --extra-cflags=-Wall --enable-guest-agent-msi --cross-prefix=x86_64-w64-mingw32- --with-vss-sdk=/home/mdroth/w/vss-win32/ && make msi
>   ...
>     AR    libqemustub.a
>     LINK  qemu-ga.exe
>     CXX   qga/vss-win32/requester.o
>   ...
>     CXX   qga/vss-win32/provider.o
>     CXX   qga/vss-win32/install.o
>     LINK  qga/vss-win32/qga-vss.dll
>     WIXL  qemu-ga-x86_64.msi
>   Couldn't find file /home/mdroth/qemu-build-w64/qga/vss-win32/qga-vss.tlb
>   make: *** [qemu-ga-x86_64.msi] Error 1
> 
> For out-of-tree, qga-vss.tlb dependency gets met by QEMU tools target, which
> is what motivated this patch.
> 
> But for in-tree builds, qga-vss.tlb is already present in working directory,
> so it might have actually worked for that case.
> 
> So I may have misdiagnosed the root issue here: that *.tlb (not just
> *.dll) needed to be added to MSI dependency list instead of being
> assumed (via in-tree build or full qemu build with tools).
> 
> > 
> > I think that if someone does "make qemu-ga.exe" they should *not* get
> > the VSS files.  Perhaps we can add a Win32-specific phony qemu-ga target
> > to build both qemu-ga.exe and the VSS files, but this patch's use of
> > filter-out is a bit ugly.
> 
> I think it might make sense to re-de-couple VSS from qemu-ga.exe, but
> I'm not sure I like the idea of making MSI target responsible for
> VSS files. MSI is relatively new, but VSS support has been around for a
> while when documentated install procedure for qemu-ga.exe was manually
> copying files. MSI isn't the source of the issue, presumably everybody
> distributing qemu-ga.exe in this manner was relying on the full build
> to get the VSS files. But moving VSS completely to MSI means that use
> case would break (whereas moving them to qemu-ga.exe would still work,
> since qemu-ga.exe is build as part of default/full build)
> 
> I think the ideal solution is too keep things as they are with this
> patch (previous working methods supported), but move to a new 'qemu-ga'
> build target that just does the right thing on each platform:
> 
>   on posix:
>     1) build 'qemu-ga' executable
>   
>   on mingw:
>     1) build 'qemu-ga.exe' executable (no need to build .exe directly)
>     2) build VSS if VSS supported/requested
>     3) build MSI package if supported/requested (and include VSS files
>        if VSS supported)
> 
> That would let us drop the wierd filtering, and bring the w32 build
> process more inline with posix.
> 
> I'm not even sure if that's possible atm, but if that's reasonable I can
> look into it as a follow-up (this series is merged already).

diff --git a/Makefile b/Makefile
index 9ce3972..d0ee41e 100644
--- a/Makefile
+++ b/Makefile
@@ -293,15 +293,15 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 # we require QGA_VSS_PROVIDER files to be built alongside qemu-ga
 # executable since they are shipped together, but we don't want to actually
 # link against them
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
-   $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))
+qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+   $(call LINK, $^)
 
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
 
 msi: $(QEMU_GA_MSI)
 
-$(QEMU_GA_MSI): qemu-ga.exe
+$(QEMU_GA_MSI): qemu-ga.exe $(QGA_VSS_PROVIDER)
 
 $(QEMU_GA_MSI): config-host.mak
 
@@ -313,6 +313,8 @@ msi:
    @echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"
 endif
 
+qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER)                                                                                                                             
 clean:
 # avoid old build problems by removing potentially incorrect old files
    rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h

This kinda does it, although it introduces a circular dependency warning
on posix (since qemu-ga$(EXESUF) == qemu-ga there, yet qemu-ga has
a qemu-ga$(EXESUF) dependency.

Could be fixed by making a new 'qemu-guest-agent' do-the-right-thing target,
but that means new build process for posix and w32 instead of just w32. The
alternative is to only define the 'qemu-ga' do-the-right-thing target for w32
(since posix already has such a target). That requires a Makefile ifdef MINGW
or somesuch though, which isn't ideal. I think the latter is worth the
simplified build process (same as we currently do on posix, but with some
extra config params)

> 
> > 
> > Paolo
> > 
> 
> 

Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Paolo Bonzini]

On 07/09/2015 22:37, Michael Roth wrote:
> This kinda does it, although it introduces a circular dependency warning
> on posix (since qemu-ga$(EXESUF) == qemu-ga there, yet qemu-ga has
> a qemu-ga$(EXESUF) dependency.
> 
> Could be fixed by making a new 'qemu-guest-agent' do-the-right-thing target,
> but that means new build process for posix and w32 instead of just w32. The
> alternative is to only define the 'qemu-ga' do-the-right-thing target for w32
> (since posix already has such a target). That requires a Makefile ifdef MINGW
> or somesuch though, which isn't ideal. I think the latter is worth the
> simplified build process (same as we currently do on posix, but with some
> extra config params)

Or just "ifneq ($(EXESUF),)".

Paolo

Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies

[Michael Roth]

Quoting Paolo Bonzini (2015-09-07 15:39:39)
> 
> 
> On 07/09/2015 22:37, Michael Roth wrote:
> > This kinda does it, although it introduces a circular dependency warning
> > on posix (since qemu-ga$(EXESUF) == qemu-ga there, yet qemu-ga has
> > a qemu-ga$(EXESUF) dependency.
> > 
> > Could be fixed by making a new 'qemu-guest-agent' do-the-right-thing target,
> > but that means new build process for posix and w32 instead of just w32. The
> > alternative is to only define the 'qemu-ga' do-the-right-thing target for w32
> > (since posix already has such a target). That requires a Makefile ifdef MINGW
> > or somesuch though, which isn't ideal. I think the latter is worth the
> > simplified build process (same as we currently do on posix, but with some
> > extra config params)
> 
> Or just "ifneq ($(EXESUF),)".

Thanks, that would work. Also noticed I forget the MSI dependency in previous
patch. Will send a proper patch soon.

> 
> Paolo
> 

[Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message

[Michael Roth]

'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

[Qemu-devel] [0/4] qemu-ga: various w32 build fix-ups for MSI/VSS support

[Michael Roth]

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

[Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Michael Roth]

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

Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Michael Roth]

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
> 

Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Michael Roth]

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
>