I was trying to build the vtpm bits in unstable and the patch for the vtpm was failing to apply. It worked fine before this commit. http://xenbits.xen.org/hg/staging/xen-unstable.hg/rev/c5b7d49ca3ee Since it is a patch file, maybe the change should read something more like: +-LDFLAGS += -lgmp ++LDLIBS += -lgmp Thanks Ross Philipson Senior Software Engineer Citrix Systems, Inc 14 Crosby Drive Bedford, MA 01730 781-301-7949 ross.philipson@citrix.com
On Thu, 2012-05-03 at 15:57 +0100, Ross Philipson wrote:> I was trying to build the vtpm bits in unstable and the patch for the vtpm was failing to apply. It worked fine before this commit. > > http://xenbits.xen.org/hg/staging/xen-unstable.hg/rev/c5b7d49ca3eeCCing the author.> Since it is a patch file, maybe the change should read something more like: > > +-LDFLAGS += -lgmp > ++LDLIBS += -lgmpQuite possibly. Certainly patching the context of the patch seems like a very odd thing to do given the patch description and unless the patch invocation is allowing loads of fuzz I don''t see how it could ever work. Olaf did you actually test this change with vtpm enabled in the build? Ian.
On Thu, May 03, Ian Campbell wrote:> Olaf did you actually test this change with vtpm enabled in the build?Yes, I did. However, after leaving for vacation I wondered myself if this change is the right approach given that it changes the context of a patch. Perhaps I made a mistake somewhere in my queue of changes, or vtpm got disabled for some reason. I will double check. In any case, the Makefiles in the vtpm directory need some more changes. Ross, do you actually run the vtpm code? I did just compile tests. Olaf
> -----Original Message----- > From: Olaf Hering [mailto:olaf@aepfle.de] > Sent: Thursday, May 03, 2012 11:36 AM > To: Ian Campbell > Cc: Ross Philipson; xen-devel@lists.xensource.com; Ian Jackson > Subject: Re: [Xen-devel] vTPM patch does not apply > > On Thu, May 03, Ian Campbell wrote: > > > Olaf did you actually test this change with vtpm enabled in the build? > > Yes, I did. > However, after leaving for vacation I wondered myself if this change is > the right approach given that it changes the context of a patch. > Perhaps I made a mistake somewhere in my queue of changes, or vtpm got > disabled for some reason. I will double check. > > In any case, the Makefiles in the vtpm directory need some more changes. > Ross, do you actually run the vtpm code? I did just compile tests. > > OlafOlaf, We are about to do some work with the vtpm code in our project. That is when I noticed it failed to build. When I reverted to a previous version of the vtpm patch it applied without issues. So no, we have not gotten it running but we are working on it. I can post the results on xen-devel once we know more if folks would like. Thanks Ross
On Thu, May 03, Olaf Hering wrote:> Perhaps I made a mistake somewhere in my queue of changes, or vtpm got > disabled for some reason. I will double check.While preparing the patch I accidently disabled vtpm in my rpm spec file, so the code was not exectued anymore. Sorry for that. I see some nice patch handing in tools/firmware/etherboot/Makefile, which could be copied to tools/vtpm/Makefile. However, this would make the make targets updatepatches and orig in tools/vtpm/Makefile obsolete. Would that be an acceptable approach to maintain a stack of patches for vtpm? Olaf
On Thu, 2012-05-03 at 16:57 +0100, Olaf Hering wrote:> On Thu, May 03, Olaf Hering wrote: > > > Perhaps I made a mistake somewhere in my queue of changes, or vtpm got > > disabled for some reason. I will double check. > > While preparing the patch I accidently disabled vtpm in my rpm spec > file, so the code was not exectued anymore. > Sorry for that.So should c5b7d49ca3ee be reverted for now? Or are you going to come up with a fix soon?> I see some nice patch handing in tools/firmware/etherboot/Makefile, > which could be copied to tools/vtpm/Makefile. However, this would make > the make targets updatepatches and orig in tools/vtpm/Makefile obsolete. > Would that be an acceptable approach to maintain a stack of patches for > vtpm?It could be considered for 4.3. I''m not sure we want to be fiddling with obscure parts of the build system for 4.2 at this stage. Lets just fix the immediate breakage for 4.2. Ian.
On Fri, May 04, Ian Campbell wrote:> On Thu, 2012-05-03 at 16:57 +0100, Olaf Hering wrote: > > On Thu, May 03, Olaf Hering wrote: > > > > > Perhaps I made a mistake somewhere in my queue of changes, or vtpm got > > > disabled for some reason. I will double check. > > > > While preparing the patch I accidently disabled vtpm in my rpm spec > > file, so the code was not exectued anymore. > > Sorry for that. > > So should c5b7d49ca3ee be reverted for now? Or are you going to come up > with a fix soon?Reverting c5b7d49ca3ee is ok. I will send another patch on top which will just crate and apply a second patch to use LDLIBS. Olaf
Olaf Hering writes ("Re: [Xen-devel] vTPM patch does not
apply"):> Reverting c5b7d49ca3ee is ok. I will send another patch on top which
> will just crate and apply a second patch to use LDLIBS.
Thanks. I have done the revert. I don''t know why I didn''t
spot this
when I was reviewing and applying 25142:c5b7d49ca3ee in the first
place.
Ian.
On Fri, May 04, Ian Jackson wrote:> Olaf Hering writes ("Re: [Xen-devel] vTPM patch does not apply"): > > Reverting c5b7d49ca3ee is ok. I will send another patch on top which > > will just crate and apply a second patch to use LDLIBS. > > Thanks. I have done the revert. I don''t know why I didn''t spot this > when I was reviewing and applying 25142:c5b7d49ca3ee in the first > place.Oh, you did. But unfortunately I convinced myself and you that my change is ok. Sorry again for that. Olaf
On Fri, 2012-05-04 at 11:19 +0100, Ian Jackson wrote:> Olaf Hering writes ("Re: [Xen-devel] vTPM patch does not apply"): > > Reverting c5b7d49ca3ee is ok. I will send another patch on top which > > will just crate and apply a second patch to use LDLIBS. > > Thanks. I have done the revert. I don''t know why I didn''t spot this > when I was reviewing and applying 25142:c5b7d49ca3ee in the first > place.FWIW we don''t build vTPM by default so you wouldn''t have noticed the actual failure. Perhaps the test system should enable non-default things like this? Ian.
On Fri, May 04, Ian Campbell wrote:> FWIW we don''t build vTPM by default so you wouldn''t have noticed the > actual failure. > > Perhaps the test system should enable non-default things like this?Sounds like a good idea to have a full featured build test. I just sent a patch out which fixes package build with ''--enable-vtpm'' at least in a SLES11 SP1 chroot for me. For some reason I did not send it earlier. Its ''tools/vtpm_manager: add missing DESTDIR to install rule'' Olaf