| Message ID | 20231205123403.63813-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | livepatch-build-tools: allow livepatching version.c | expand |
On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > Currently version.o is explicitly ignored as the file would change as a result > of the orignal and the patched build having possibly different dates and > times. > > Fix such difference by exporting the date and time from the build script, so > that both builds share the same build time. This allows checking for changes > in version.c, since the rest of fields need to be manually changed in order to > produce different output. > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > since before livepatch support was added to Xen, so it's safe to export those > variables unconditionally. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > livepatch-build | 4 ++++ > livepatch-gcc | 2 -- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/livepatch-build b/livepatch-build > index e2ccce4f7fd7..f622683fc56c 100755 > --- a/livepatch-build > +++ b/livepatch-build > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > + # Force same date and time to prevent unwanted changes in version.c > + export XEN_BUILD_DATE=`LC_ALL=C date` > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` Date is the one that goes wrong every time, but everything else in compile.h can go wrong in a way that causes version.o to change. Ideally, the pristine source for building livepatches would include a generated compile.h, and livepatch would have a way to force no regeneration of the header. But I've got no idea how nice that would be to arrange. That way, you're using the same details as the Xen being patched, rather than hoping that two identical different details will cancel out in the binary diff. > + > echo "Perform full initial build with ${CPUS} CPU(s)..." > build_full > > diff --git a/livepatch-gcc b/livepatch-gcc > index fcad80551aa0..e4cb6fb59029 100755 > --- a/livepatch-gcc > +++ b/livepatch-gcc > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > obj=$2 > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > case "$(basename $obj)" in > - version.o|\ > debug.o|\ > check.o|\ Tangential question. check.o is excluded because it's a toolchain test, but any idea what debug.o is doing in this list? It can't possibly be the debug.c I've recently added to x86 (which we'll want to be able to livepatch), so I guess it's got something to do the ARM debug.S's, but I can't see anything in those that are worthy of exemption either... ~Andrew
On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote: > On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > > Currently version.o is explicitly ignored as the file would change as a result > > of the orignal and the patched build having possibly different dates and > > times. > > > > Fix such difference by exporting the date and time from the build script, so > > that both builds share the same build time. This allows checking for changes > > in version.c, since the rest of fields need to be manually changed in order to > > produce different output. > > > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > > since before livepatch support was added to Xen, so it's safe to export those > > variables unconditionally. > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > livepatch-build | 4 ++++ > > livepatch-gcc | 2 -- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/livepatch-build b/livepatch-build > > index e2ccce4f7fd7..f622683fc56c 100755 > > --- a/livepatch-build > > +++ b/livepatch-build > > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > > > + # Force same date and time to prevent unwanted changes in version.c > > + export XEN_BUILD_DATE=`LC_ALL=C date` > > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` > > Date is the one that goes wrong every time, but everything else in > compile.h can go wrong in a way that causes version.o to change. I've attempted to reflect that in "since the rest of fields need to be manually changed in order to produce different output". For those to change there must be some kind of environment change between the original and the patched version build, and hence I don't think that would be supported. > Ideally, the pristine source for building livepatches would include a > generated compile.h, and livepatch would have a way to force no > regeneration of the header. But I've got no idea how nice that would be > to arrange. Yes, no idea how fragile that would be either. IMO the proposed approach is not that bad. > That way, you're using the same details as the Xen being patched, rather > than hoping that two identical different details will cancel out in the > binary diff. Another option is to set all the env variables to disable any build time probing. However things like compiler or version changing between the original and the patched builds likely point out to issues elsewhere, unless it's intentional modification of the helpers. > > + > > echo "Perform full initial build with ${CPUS} CPU(s)..." > > build_full > > > > diff --git a/livepatch-gcc b/livepatch-gcc > > index fcad80551aa0..e4cb6fb59029 100755 > > --- a/livepatch-gcc > > +++ b/livepatch-gcc > > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > > obj=$2 > > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > > case "$(basename $obj)" in > > - version.o|\ > > debug.o|\ > > check.o|\ > > Tangential question. check.o is excluded because it's a toolchain test, > but any idea what debug.o is doing in this list? > > It can't possibly be the debug.c I've recently added to x86 (which we'll > want to be able to livepatch), so I guess it's got something to do the > ARM debug.S's, but I can't see anything in those that are worthy of > exemption either... Hm, that comes from the first commit that imported the wrapper to the repository, and at that point only x86 had livepatch support. I'm tempted to think this was inherited from the original xsplice tooling, and so debug.o needs to be removed from the list. Thanks, Roger.
On Tue, Dec 5, 2023 at 2:57 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote: > > On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > > > Currently version.o is explicitly ignored as the file would change as a result > > > of the orignal and the patched build having possibly different dates and > > > times. > > > > > > Fix such difference by exporting the date and time from the build script, so > > > that both builds share the same build time. This allows checking for changes > > > in version.c, since the rest of fields need to be manually changed in order to > > > produce different output. > > > > > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > > > since before livepatch support was added to Xen, so it's safe to export those > > > variables unconditionally. > > > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > --- > > > livepatch-build | 4 ++++ > > > livepatch-gcc | 2 -- > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/livepatch-build b/livepatch-build > > > index e2ccce4f7fd7..f622683fc56c 100755 > > > --- a/livepatch-build > > > +++ b/livepatch-build > > > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > > > > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > > > > > + # Force same date and time to prevent unwanted changes in version.c > > > + export XEN_BUILD_DATE=`LC_ALL=C date` > > > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` > > > > Date is the one that goes wrong every time, but everything else in > > compile.h can go wrong in a way that causes version.o to change. > > I've attempted to reflect that in "since the rest of fields need to be > manually changed in order to produce different output". > > For those to change there must be some kind of environment change > between the original and the patched version build, and hence I don't > think that would be supported. In general, yes. However, with this patch changes to the hostname/domain/username would result in version.o being marked as changed even though it is entirely fine to build the live patch on a different build host from the original Xen. > > > Ideally, the pristine source for building livepatches would include a > > generated compile.h, and livepatch would have a way to force no > > regeneration of the header. But I've got no idea how nice that would be > > to arrange. > > Yes, no idea how fragile that would be either. IMO the proposed > approach is not that bad. > > > That way, you're using the same details as the Xen being patched, rather > > than hoping that two identical different details will cancel out in the > > binary diff. > > Another option is to set all the env variables to disable any > build time probing. However things like compiler or version changing > between the original and the patched builds likely point out to issues > elsewhere, unless it's intentional modification of the helpers. > > > > + > > > echo "Perform full initial build with ${CPUS} CPU(s)..." > > > build_full > > > > > > diff --git a/livepatch-gcc b/livepatch-gcc > > > index fcad80551aa0..e4cb6fb59029 100755 > > > --- a/livepatch-gcc > > > +++ b/livepatch-gcc > > > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > > > obj=$2 > > > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > > > case "$(basename $obj)" in > > > - version.o|\ > > > debug.o|\ > > > check.o|\ > > > > Tangential question. check.o is excluded because it's a toolchain test, > > but any idea what debug.o is doing in this list? > > > > It can't possibly be the debug.c I've recently added to x86 (which we'll > > want to be able to livepatch), so I guess it's got something to do the > > ARM debug.S's, but I can't see anything in those that are worthy of > > exemption either... > > Hm, that comes from the first commit that imported the wrapper to the > repository, and at that point only x86 had livepatch support. > > I'm tempted to think this was inherited from the original xsplice > tooling, and so debug.o needs to be removed from the list. > livepatch-build-tools is derived from the kpatch build tooling and debug.o has never been present there so it was added here for a reason. AFAICT the gdbsx code used to live in debug.o. I can't recall why it was being marked as changed unnecessarily but since that is no longer an issue and the code lives elsewhere, the debug.o lines can be dropped. Ross
On Wed, Dec 06, 2023 at 12:11:39PM +0000, Ross Lagerwall wrote: > On Tue, Dec 5, 2023 at 2:57 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > > > On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote: > > > On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > > > > Currently version.o is explicitly ignored as the file would change as a result > > > > of the orignal and the patched build having possibly different dates and > > > > times. > > > > > > > > Fix such difference by exporting the date and time from the build script, so > > > > that both builds share the same build time. This allows checking for changes > > > > in version.c, since the rest of fields need to be manually changed in order to > > > > produce different output. > > > > > > > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > > > > since before livepatch support was added to Xen, so it's safe to export those > > > > variables unconditionally. > > > > > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > > --- > > > > livepatch-build | 4 ++++ > > > > livepatch-gcc | 2 -- > > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/livepatch-build b/livepatch-build > > > > index e2ccce4f7fd7..f622683fc56c 100755 > > > > --- a/livepatch-build > > > > +++ b/livepatch-build > > > > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > > > > > > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > > > > > > > + # Force same date and time to prevent unwanted changes in version.c > > > > + export XEN_BUILD_DATE=`LC_ALL=C date` > > > > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` > > > > > > Date is the one that goes wrong every time, but everything else in > > > compile.h can go wrong in a way that causes version.o to change. > > > > I've attempted to reflect that in "since the rest of fields need to be > > manually changed in order to produce different output". > > > > For those to change there must be some kind of environment change > > between the original and the patched version build, and hence I don't > > think that would be supported. > > In general, yes. However, with this patch changes to the > hostname/domain/username would result in version.o being marked > as changed even though it is entirely fine to build the live patch > on a different build host from the original Xen. Keep in mind livepatch-build-tools builds it's base version of Xen and then a patched version, and that's how the diff is performed. For the hostname/domain/username changes to appear on the livepatch payload such change would need to happen in the muddle of the execution of livepatch-build. This change doesn't prevent building the original Xen on a different host than the one building the livepatch, and the hostname/domain/username changes won't be part of the livepatch payload. > > > > > Ideally, the pristine source for building livepatches would include a > > > generated compile.h, and livepatch would have a way to force no > > > regeneration of the header. But I've got no idea how nice that would be > > > to arrange. > > > > Yes, no idea how fragile that would be either. IMO the proposed > > approach is not that bad. > > > > > That way, you're using the same details as the Xen being patched, rather > > > than hoping that two identical different details will cancel out in the > > > binary diff. > > > > Another option is to set all the env variables to disable any > > build time probing. However things like compiler or version changing > > between the original and the patched builds likely point out to issues > > elsewhere, unless it's intentional modification of the helpers. > > > > > > + > > > > echo "Perform full initial build with ${CPUS} CPU(s)..." > > > > build_full > > > > > > > > diff --git a/livepatch-gcc b/livepatch-gcc > > > > index fcad80551aa0..e4cb6fb59029 100755 > > > > --- a/livepatch-gcc > > > > +++ b/livepatch-gcc > > > > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > > > > obj=$2 > > > > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > > > > case "$(basename $obj)" in > > > > - version.o|\ > > > > debug.o|\ > > > > check.o|\ > > > > > > Tangential question. check.o is excluded because it's a toolchain test, > > > but any idea what debug.o is doing in this list? > > > > > > It can't possibly be the debug.c I've recently added to x86 (which we'll > > > want to be able to livepatch), so I guess it's got something to do the > > > ARM debug.S's, but I can't see anything in those that are worthy of > > > exemption either... > > > > Hm, that comes from the first commit that imported the wrapper to the > > repository, and at that point only x86 had livepatch support. > > > > I'm tempted to think this was inherited from the original xsplice > > tooling, and so debug.o needs to be removed from the list. > > > > livepatch-build-tools is derived from the kpatch build tooling and > debug.o has never been present there so it was added here for a > reason. AFAICT the gdbsx code used to live in debug.o. I can't > recall why it was being marked as changed unnecessarily but since > that is no longer an issue and the code lives elsewhere, the debug.o > lines can be dropped. Will someone send a patch for this, or should I do it? Thanks, Roger.
On Mon, Dec 11, 2023 at 10:42 AM Roger Pau Monné <roger.pau@citrix.com> wrote: > > On Wed, Dec 06, 2023 at 12:11:39PM +0000, Ross Lagerwall wrote: > > On Tue, Dec 5, 2023 at 2:57 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > > > > > On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote: > > > > On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > > > > > Currently version.o is explicitly ignored as the file would change as a result > > > > > of the orignal and the patched build having possibly different dates and > > > > > times. > > > > > > > > > > Fix such difference by exporting the date and time from the build script, so > > > > > that both builds share the same build time. This allows checking for changes > > > > > in version.c, since the rest of fields need to be manually changed in order to > > > > > produce different output. > > > > > > > > > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > > > > > since before livepatch support was added to Xen, so it's safe to export those > > > > > variables unconditionally. > > > > > > > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > > > --- > > > > > livepatch-build | 4 ++++ > > > > > livepatch-gcc | 2 -- > > > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/livepatch-build b/livepatch-build > > > > > index e2ccce4f7fd7..f622683fc56c 100755 > > > > > --- a/livepatch-build > > > > > +++ b/livepatch-build > > > > > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > > > > > > > > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > > > > > > > > > + # Force same date and time to prevent unwanted changes in version.c > > > > > + export XEN_BUILD_DATE=`LC_ALL=C date` > > > > > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` > > > > > > > > Date is the one that goes wrong every time, but everything else in > > > > compile.h can go wrong in a way that causes version.o to change. > > > > > > I've attempted to reflect that in "since the rest of fields need to be > > > manually changed in order to produce different output". > > > > > > For those to change there must be some kind of environment change > > > between the original and the patched version build, and hence I don't > > > think that would be supported. > > > > In general, yes. However, with this patch changes to the > > hostname/domain/username would result in version.o being marked > > as changed even though it is entirely fine to build the live patch > > on a different build host from the original Xen. > > Keep in mind livepatch-build-tools builds it's base version of Xen and > then a patched version, and that's how the diff is performed. For the > hostname/domain/username changes to appear on the livepatch payload > such change would need to happen in the muddle of the execution of > livepatch-build. > > This change doesn't prevent building the original Xen on a different > host than the one building the livepatch, and the > hostname/domain/username changes won't be part of the livepatch > payload. Oh, of course. That makes sense. There is one other issue that may cause version.o to change unnecessarily. XEN_CHANGESET may change when the source tree is a Git repo. In some cases, it will change from <commit> to <commit>-dirty when the patch is applied. I guess we could set .scmversion explicitly to avoid this? > > > > > > > > Ideally, the pristine source for building livepatches would include a > > > > generated compile.h, and livepatch would have a way to force no > > > > regeneration of the header. But I've got no idea how nice that would be > > > > to arrange. > > > > > > Yes, no idea how fragile that would be either. IMO the proposed > > > approach is not that bad. > > > > > > > That way, you're using the same details as the Xen being patched, rather > > > > than hoping that two identical different details will cancel out in the > > > > binary diff. > > > > > > Another option is to set all the env variables to disable any > > > build time probing. However things like compiler or version changing > > > between the original and the patched builds likely point out to issues > > > elsewhere, unless it's intentional modification of the helpers. > > > > > > > > + > > > > > echo "Perform full initial build with ${CPUS} CPU(s)..." > > > > > build_full > > > > > > > > > > diff --git a/livepatch-gcc b/livepatch-gcc > > > > > index fcad80551aa0..e4cb6fb59029 100755 > > > > > --- a/livepatch-gcc > > > > > +++ b/livepatch-gcc > > > > > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > > > > > obj=$2 > > > > > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > > > > > case "$(basename $obj)" in > > > > > - version.o|\ > > > > > debug.o|\ > > > > > check.o|\ > > > > > > > > Tangential question. check.o is excluded because it's a toolchain test, > > > > but any idea what debug.o is doing in this list? > > > > > > > > It can't possibly be the debug.c I've recently added to x86 (which we'll > > > > want to be able to livepatch), so I guess it's got something to do the > > > > ARM debug.S's, but I can't see anything in those that are worthy of > > > > exemption either... > > > > > > Hm, that comes from the first commit that imported the wrapper to the > > > repository, and at that point only x86 had livepatch support. > > > > > > I'm tempted to think this was inherited from the original xsplice > > > tooling, and so debug.o needs to be removed from the list. > > > > > > > livepatch-build-tools is derived from the kpatch build tooling and > > debug.o has never been present there so it was added here for a > > reason. AFAICT the gdbsx code used to live in debug.o. I can't > > recall why it was being marked as changed unnecessarily but since > > that is no longer an issue and the code lives elsewhere, the debug.o > > lines can be dropped. > > Will someone send a patch for this, or should I do it? > I will send a patch for this. Thanks, Ross
On Wed, Dec 13, 2023 at 04:39:04PM +0000, Ross Lagerwall wrote: > On Mon, Dec 11, 2023 at 10:42 AM Roger Pau Monné <roger.pau@citrix.com> wrote: > > > > On Wed, Dec 06, 2023 at 12:11:39PM +0000, Ross Lagerwall wrote: > > > On Tue, Dec 5, 2023 at 2:57 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > > > > > > > On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote: > > > > > On 05/12/2023 12:34 pm, Roger Pau Monne wrote: > > > > > > Currently version.o is explicitly ignored as the file would change as a result > > > > > > of the orignal and the patched build having possibly different dates and > > > > > > times. > > > > > > > > > > > > Fix such difference by exporting the date and time from the build script, so > > > > > > that both builds share the same build time. This allows checking for changes > > > > > > in version.c, since the rest of fields need to be manually changed in order to > > > > > > produce different output. > > > > > > > > > > > > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported > > > > > > since before livepatch support was added to Xen, so it's safe to export those > > > > > > variables unconditionally. > > > > > > > > > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > > > > --- > > > > > > livepatch-build | 4 ++++ > > > > > > livepatch-gcc | 2 -- > > > > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/livepatch-build b/livepatch-build > > > > > > index e2ccce4f7fd7..f622683fc56c 100755 > > > > > > --- a/livepatch-build > > > > > > +++ b/livepatch-build > > > > > > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then > > > > > > > > > > > > export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " > > > > > > > > > > > > + # Force same date and time to prevent unwanted changes in version.c > > > > > > + export XEN_BUILD_DATE=`LC_ALL=C date` > > > > > > + export XEN_BUILD_TIME=`LC_ALL=C date +%T` > > > > > > > > > > Date is the one that goes wrong every time, but everything else in > > > > > compile.h can go wrong in a way that causes version.o to change. > > > > > > > > I've attempted to reflect that in "since the rest of fields need to be > > > > manually changed in order to produce different output". > > > > > > > > For those to change there must be some kind of environment change > > > > between the original and the patched version build, and hence I don't > > > > think that would be supported. > > > > > > In general, yes. However, with this patch changes to the > > > hostname/domain/username would result in version.o being marked > > > as changed even though it is entirely fine to build the live patch > > > on a different build host from the original Xen. > > > > Keep in mind livepatch-build-tools builds it's base version of Xen and > > then a patched version, and that's how the diff is performed. For the > > hostname/domain/username changes to appear on the livepatch payload > > such change would need to happen in the muddle of the execution of > > livepatch-build. > > > > This change doesn't prevent building the original Xen on a different > > host than the one building the livepatch, and the > > hostname/domain/username changes won't be part of the livepatch > > payload. > > Oh, of course. That makes sense. > > There is one other issue that may cause version.o to change > unnecessarily. > > XEN_CHANGESET may change when the source tree is a Git repo. > In some cases, it will change from <commit> to <commit>-dirty > when the patch is applied. I guess we could set .scmversion > explicitly to avoid this? Oh, right, I did consider that <commit> cannot change, but it can indeed become <commit>-dirty. Will expand to make it fixed also. > > > > > > > > > > > Ideally, the pristine source for building livepatches would include a > > > > > generated compile.h, and livepatch would have a way to force no > > > > > regeneration of the header. But I've got no idea how nice that would be > > > > > to arrange. > > > > > > > > Yes, no idea how fragile that would be either. IMO the proposed > > > > approach is not that bad. > > > > > > > > > That way, you're using the same details as the Xen being patched, rather > > > > > than hoping that two identical different details will cancel out in the > > > > > binary diff. > > > > > > > > Another option is to set all the env variables to disable any > > > > build time probing. However things like compiler or version changing > > > > between the original and the patched builds likely point out to issues > > > > elsewhere, unless it's intentional modification of the helpers. > > > > > > > > > > + > > > > > > echo "Perform full initial build with ${CPUS} CPU(s)..." > > > > > > build_full > > > > > > > > > > > > diff --git a/livepatch-gcc b/livepatch-gcc > > > > > > index fcad80551aa0..e4cb6fb59029 100755 > > > > > > --- a/livepatch-gcc > > > > > > +++ b/livepatch-gcc > > > > > > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then > > > > > > obj=$2 > > > > > > [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} > > > > > > case "$(basename $obj)" in > > > > > > - version.o|\ > > > > > > debug.o|\ > > > > > > check.o|\ > > > > > > > > > > Tangential question. check.o is excluded because it's a toolchain test, > > > > > but any idea what debug.o is doing in this list? > > > > > > > > > > It can't possibly be the debug.c I've recently added to x86 (which we'll > > > > > want to be able to livepatch), so I guess it's got something to do the > > > > > ARM debug.S's, but I can't see anything in those that are worthy of > > > > > exemption either... > > > > > > > > Hm, that comes from the first commit that imported the wrapper to the > > > > repository, and at that point only x86 had livepatch support. > > > > > > > > I'm tempted to think this was inherited from the original xsplice > > > > tooling, and so debug.o needs to be removed from the list. > > > > > > > > > > livepatch-build-tools is derived from the kpatch build tooling and > > > debug.o has never been present there so it was added here for a > > > reason. AFAICT the gdbsx code used to live in debug.o. I can't > > > recall why it was being marked as changed unnecessarily but since > > > that is no longer an issue and the code lives elsewhere, the debug.o > > > lines can be dropped. > > > > Will someone send a patch for this, or should I do it? > > > > I will send a patch for this. Thanks, Roger.
diff --git a/livepatch-build b/livepatch-build index e2ccce4f7fd7..f622683fc56c 100755 --- a/livepatch-build +++ b/livepatch-build @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc " + # Force same date and time to prevent unwanted changes in version.c + export XEN_BUILD_DATE=`LC_ALL=C date` + export XEN_BUILD_TIME=`LC_ALL=C date +%T` + echo "Perform full initial build with ${CPUS} CPU(s)..." build_full diff --git a/livepatch-gcc b/livepatch-gcc index fcad80551aa0..e4cb6fb59029 100755 --- a/livepatch-gcc +++ b/livepatch-gcc @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then obj=$2 [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/} case "$(basename $obj)" in - version.o|\ debug.o|\ check.o|\ *.xen-syms.*.o|\ @@ -63,7 +62,6 @@ done elif [[ "$TOOLCHAINCMD" =~ $OBJCOPY_RE ]] ; then obj="${!#}" case "$(basename $obj)" in - version.o|\ debug.o|\ check.o|\ boot.o|\
Currently version.o is explicitly ignored as the file would change as a result of the orignal and the patched build having possibly different dates and times. Fix such difference by exporting the date and time from the build script, so that both builds share the same build time. This allows checking for changes in version.c, since the rest of fields need to be manually changed in order to produce different output. Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported since before livepatch support was added to Xen, so it's safe to export those variables unconditionally. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- livepatch-build | 4 ++++ livepatch-gcc | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) base-commit: e588b7914e7afa3abb64b15a32fc2fdb57ded341 prerequisite-patch-id: 3419543c1c7c96b551db398518dbb10d81b3d5d9 prerequisite-patch-id: 640658ca7a1a21a540bfd6a862ff83669f70a065 prerequisite-patch-id: 650cd4210c2e73e9d2588b048be2c8278ae96acd