From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ypxtq-0004TD-6E for qemu-devel@nongnu.org; Wed, 06 May 2015 07:54:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ypxtm-0004vv-Q4 for qemu-devel@nongnu.org; Wed, 06 May 2015 07:54:02 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:55228) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ypxtm-0004vK-JA for qemu-devel@nongnu.org; Wed, 06 May 2015 07:53:58 -0400 Date: Wed, 6 May 2015 07:53:55 -0400 (EDT) From: Yossi Hindin Message-ID: <244658581.13751547.1430913235034.JavaMail.zimbra@redhat.com> In-Reply-To: <554735C6.7040505@redhat.com> References: <1430031884-5483-1-git-send-email-yhindin@redhat.com> <1430031884-5483-5-git-send-email-yhindin@redhat.com> <554735C6.7040505@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/4] qemu-ga: Building Windows MSI installation with configure/Makefile List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: yvugenfi@redhat.com, dfleytma@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com I am submitting second version of patches. ----- Original Message ----- > From: "Paolo Bonzini" > To: "Yossi Hindin" , qemu-devel@nongnu.org > Cc: yvugenfi@redhat.com, dfleytma@redhat.com, mdroth@linux.vnet.ibm.com > Sent: Monday, May 4, 2015 12:03:02 PM > Subject: Re: [PATCH 4/4] qemu-ga: Building Windows MSI installation with configure/Makefile > > > > On 26/04/2015 09:04, Yossi Hindin wrote: > > +ifdef QEMU_GA_MSI_WITH_VSS > > +${QEMU_GA_MSI}: qga/vss-win32/qga-vss.dll qemu-ga.exe > > Shouldn't the qemu-ga.exe dependency be unconditional? Yes, it should. Fixed in the new patches. > > > +endif > > + > > +${QEMU_GA_MSI}: config-host.mak > > + > > +${QEMU_GA_MSI}: qga/installer/qemu-ga.wxs > > + $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" > > QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" > > QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" \ > > + wixl -o $@ ${QEMU_GA_MSI_ARCH} ${QEMU_GA_MSI_WITH_VSS} > > ${QEMU_GA_MSI_MINGW_DLL_PATH} $<, " WIXL $@") > > Please use $(...) instead of ${...} for consistency. Fixed. BTW. there are other places in the 'configure' script where ${} is used. > > > + > > 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 > > rm -f qemu-options.def > > + rm -f *.msi > > find . \( -name '*.l[oa]' -o -name '*.so' -o -name '*.dll' -o -name > > '*.mo' -o -name '*.[oda]' \) -type f -exec rm {} + > > rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* > > *.pod *~ */*~ > > rm -f fsdev/*.pod > > diff --git a/configure b/configure > > index 6969f6f..0aa79bb 100755 > > --- a/configure > > +++ b/configure > > @@ -316,6 +316,7 @@ snappy="" > > bzip2="" > > guest_agent="" > > guest_agent_with_vss="no" > > +guest_msi="" > > vss_win32_sdk="" > > win_sdk="no" > > want_tools="yes" > > @@ -1069,6 +1070,10 @@ for opt do > > ;; > > --disable-guest-agent) guest_agent="no" > > ;; > > + --enable-guest-msi) guest_msi="yes" > > + ;; > > + --disable-guest-msi) guest_msi="no" > > + ;; > > --with-vss-sdk) vss_win32_sdk="" > > ;; > > --with-vss-sdk=*) vss_win32_sdk="$optarg" > > @@ -1407,6 +1412,8 @@ Advanced options (experts only): > > --enable-quorum enable quorum block filter support > > --disable-numa disable libnuma support > > --enable-numa enable libnuma support > > + --enable-guest-msi enable building guest agent Windows MSI > > installation package > > + --disable-guest-msi disable building guest agent Windows MSI > > installation package > > --enable-guest-agent-msi? Renamed > > Also, please keep the guest agent options together. Done. > > > > > NOTE: The object files are built at the place where configure is launched > > EOF > > @@ -3832,6 +3839,54 @@ if test "$mingw32" = "yes" -a "$guest_agent" != "no" > > -a "$guest_agent_with_vss" > > fi > > > > ########################################## > > +# Guest agent Window MSI package > > + > > +if test "$guest_msi" = "yes"; then > > Since building the MSI is on-demand anyway ("make msi"), we might as well > treat it similar to other configure options: > > - --enable-foo causes an error if the feature is not available > - --disable-foo forcibly disable the feature > - the default is to configure the feature if available, otherwise disable it > > This would be something like this: > > if test "$guest_agent" != yes; then > if test "$guest_msi" = yes; then > error_exit "MSI guest agent package requires guest agent enabled" > fi > guest_msi=no > elif test "$mingw32" != "yes"; then > if test "$guest_msi" = "yes"; then > error_exit "MSI guest agent package is available only for MinGW Windows > cross-compilation" > fi > guest_msi=no > elif test ! has_wixl; then > if test "$guest_msi" = "yes"; then > error_exit "wixl not found, required for building MSI guest agent > package" > fi > guest_msi=no > fi > > if test "$guest_msi" != no; then > ... > fi The logic changed according to your comment. Also, Makefile doesn't try to build MSI if MSI was not enabled by configure script. > > > + > > + > > + 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 > > + > > + 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 > > @@ -4500,6 +4555,14 @@ if test "$mingw32" = "yes" ; then > > echo "CONFIG_QGA_VSS=y" >> $config_host_mak > > echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak > > fi > > + if test "$guest_msi" = "yes"; then > > Similarly, "!= no" here. Done. > > > + echo "QEMU_GA_MSI_MINGW_DLL_PATH=-D Mingw_dlls=`$pkg_config > > --variable=prefix glib-2.0`/bin" >> $config_host_mak > > Why not set up the variable above? Assignment of QEMU_GA_MSI_MINGW_DLL_PATH grouped wihh other QEMU GA-related variables. > > Paolo > > > + echo "QEMU_GA_MSI_WITH_VSS=${QEMU_GA_MSI_WITH_VSS}" >> > > $config_host_mak > > + echo "QEMU_GA_MSI_ARCH=${QEMU_GA_MSI_ARCH}" >> $config_host_mak > > + echo "QEMU_GA_MANUFACTURER=${QEMU_GA_MANUFACTURER}" >> > > $config_host_mak > > + echo "QEMU_GA_DISTRO=${QEMU_GA_DISTRO}" >> $config_host_mak > > + echo "QEMU_GA_VERSION=${QEMU_GA_VERSION}" >> $config_host_mak > > + fi > > else > > echo "CONFIG_POSIX=y" >> $config_host_mak > > fi > > >