From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41973) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZ3AZ-0005TY-54 for qemu-devel@nongnu.org; Mon, 07 Sep 2015 16:37:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZ3AU-0008I2-3L for qemu-devel@nongnu.org; Mon, 07 Sep 2015 16:37:39 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:58565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZ3AT-0008Hn-Oa for qemu-devel@nongnu.org; Mon, 07 Sep 2015 16:37:34 -0400 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 Sep 2015 14:37:33 -0600 Received: from b03cxnp08026.gho.boulder.ibm.com (b03cxnp08026.gho.boulder.ibm.com [9.17.130.18]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 38C9D3E4003B for ; Mon, 7 Sep 2015 14:37:30 -0600 (MDT) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t87KagcT32505972 for ; Mon, 7 Sep 2015 13:36:42 -0700 Received: from d03av05.boulder.ibm.com (localhost [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t87KbTCX031594 for ; Mon, 7 Sep 2015 14:37:30 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20150907195547.10296.13210@loki> References: <1440719744-24755-1-git-send-email-mdroth@linux.vnet.ibm.com> <1440719744-24755-4-git-send-email-mdroth@linux.vnet.ibm.com> <55ED6B7B.9060000@redhat.com> <20150907195547.10296.13210@loki> Message-ID: <20150907203723.10296.29727@loki> Date: Mon, 07 Sep 2015 15:37:23 -0500 Subject: Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: marcandre.lureau@redhat.com, yhindin@redhat.com, leonid@daynix.com 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-ag= ent --target-list=3Dx86_64-softmmu --extra-cflags=3D-Wall --enable-guest-ag= ent-msi --cross-prefix=3Dx86_64-w64-mingw32- --with-vss-sdk=3D/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, wh= ich > is what motivated this patch. > = > But for in-tree builds, qga-vss.tlb is already present in working directo= ry, > 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_PROVI= DER) - $(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=3Dqemu-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 (reconf= igure 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) =3D=3D 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 w= 32 (since posix already has such a target). That requires a Makefile ifdef MIN= GW 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 > > = > = >=20