vTPM patch does not apply

vTPM patch does not apply

[Ross Philipson]

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

Re: vTPM patch does not apply

[Ian Campbell]

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/c5b7d49ca3ee

CCing the author.

> Since it is a patch file, maybe the change should read something more like:
> 
> +-LDFLAGS += -lgmp
> ++LDLIBS  += -lgmp

Quite 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.

Re: vTPM patch does not apply

[Olaf Hering]

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

Re: vTPM patch does not apply

[Ross Philipson]

> -----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.
> 
> Olaf

Olaf,

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

Re: vTPM patch does not apply

[Olaf Hering]

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

Re: vTPM patch does not apply

[Ian Campbell]

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.

Re: vTPM patch does not apply

[Olaf Hering]

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

Re: vTPM patch does not apply

[Ian Jackson]

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.

Re: vTPM patch does not apply

[Olaf Hering]

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

Re: vTPM patch does not apply

[Ian Campbell]

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.

Re: vTPM patch does not apply

[Olaf Hering]

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