[v4,4/9] version: Print build-id at bootup.

Commit Message

Konrad Rzeszutek Wilk Sept. 6, 2016, 4:57 p.m. UTC

On Wed, Aug 24, 2016 at 02:58:48AM -0600, Jan Beulich wrote:
> >>> On 24.08.16 at 04:22, <[e-mail]> wrote:
> > Livepatch expected at some point to be able to print the
> > build-id during bootup, which it did not.  The reason is
> > that xen_build_init and livepatch_init are both __initcall
> > type routines. This meant that when livepatch_init called
> > xen_build_id, it would return -ENODATA as build_id_len was
> > not setup yet (b/c xen_build_init would be called later).
> > 
> > The original patch fixed this by calling xen_build_init in
> > livepatch_init which allows us to print the build-id of
> > the hypervisor.
> > 
> > However the x86 maintainers pointed out that build-id
> > is independent of Livepatch and in fact should print
> > regardless whether Livepatch is enabled or not.
> > 
> > Therefore this patch moves the logic of printing the build-id
> > to version.c.
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
> 
> Reviewed-by: Jan Beulich <[e-mail]>

Thank you.

I had a slight change due to rebasing on "x86/EFI: use less crude a way of generating the build"
which was quite simple to fix so I retained your Reviewed-by tag.

The final patch looks as so:

From 927c9accac9a49b586f616060e5567d7b03e3e77 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <[e-mail]>
Date: Tue, 6 Sep 2016 12:18:10 -0400
Subject: [PATCH] version: Print build-id at bootup.

Livepatch expected at some point to be able to print the
build-id during bootup, which it did not.  The reason is
that xen_build_init and livepatch_init are both __initcall
type routines. This meant that when livepatch_init called
xen_build_id, it would return -ENODATA as build_id_len was
not setup yet (b/c xen_build_init would be called later).

The original patch fixed this by calling xen_build_init in
livepatch_init which allows us to print the build-id of
the hypervisor.

However the x86 maintainers pointed out that build-id
is independent of Livepatch and in fact should print
regardless whether Livepatch is enabled or not.

Therefore this patch moves the logic of printing the build-id
to version.c.

Reviewed-by: Jan Beulich <[e-mail]>
Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

---
Cc: Ross Lagerwall <[e-mail]>
Cc: Andrew Cooper <[e-mail]>
Cc: Jan Beulich <[e-mail]>

v2: Move xen_build_init in version.h instead of livepatch.h
v3: Posted as "livepatch: Sync cache of build-id before using it first time"
v4: Move the printing of build-id to version.c.
    Change title
v5: Rebased on top "x86/EFI: use less crude a way of generating the build ID"
    Added Jan's Ack.
---
 xen/common/livepatch.c | 6 ------
 xen/common/version.c   | 2 ++
 2 files changed, 2 insertions(+), 6 deletions(-)

Comments

Jan Beulich Sept. 7, 2016, 8:03 a.m. UTC | #1

>>> On 06.09.16 at 18:57, <[e-mail]> wrote:
> On Wed, Aug 24, 2016 at 02:58:48AM -0600, Jan Beulich wrote:
>> >>> On 24.08.16 at 04:22, <[e-mail]> wrote:
>> > Livepatch expected at some point to be able to print the
>> > build-id during bootup, which it did not.  The reason is
>> > that xen_build_init and livepatch_init are both __initcall
>> > type routines. This meant that when livepatch_init called
>> > xen_build_id, it would return -ENODATA as build_id_len was
>> > not setup yet (b/c xen_build_init would be called later).
>> > 
>> > The original patch fixed this by calling xen_build_init in
>> > livepatch_init which allows us to print the build-id of
>> > the hypervisor.
>> > 
>> > However the x86 maintainers pointed out that build-id
>> > is independent of Livepatch and in fact should print
>> > regardless whether Livepatch is enabled or not.
>> > 
>> > Therefore this patch moves the logic of printing the build-id
>> > to version.c.
>> > 
>> > Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
>> 
>> Reviewed-by: Jan Beulich <[e-mail]>
> 
> Thank you.
> 
> I had a slight change due to rebasing on "x86/EFI: use less crude a way of 
> generating the build"
> which was quite simple to fix so I retained your Reviewed-by tag.

Of course.

Jan

Ross Lagerwall Sept. 9, 2016, 1:37 p.m. UTC | #2

On 09/06/2016 05:57 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Aug 24, 2016 at 02:58:48AM -0600, Jan Beulich wrote:
>>>>> On 24.08.16 at 04:22, <[e-mail]> wrote:
>>> Livepatch expected at some point to be able to print the
>>> build-id during bootup, which it did not.  The reason is
>>> that xen_build_init and livepatch_init are both __initcall
>>> type routines. This meant that when livepatch_init called
>>> xen_build_id, it would return -ENODATA as build_id_len was
>>> not setup yet (b/c xen_build_init would be called later).
>>>
>>> The original patch fixed this by calling xen_build_init in
>>> livepatch_init which allows us to print the build-id of
>>> the hypervisor.
>>>
>>> However the x86 maintainers pointed out that build-id
>>> is independent of Livepatch and in fact should print
>>> regardless whether Livepatch is enabled or not.
>>>
>>> Therefore this patch moves the logic of printing the build-id
>>> to version.c.
>>>
>>> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
>>
>> Reviewed-by: Jan Beulich <[e-mail]>
>
> Thank you.
>
> I had a slight change due to rebasing on "x86/EFI: use less crude a way of generating the build"
> which was quite simple to fix so I retained your Reviewed-by tag.
>
> The final patch looks as so:
>
> From 927c9accac9a49b586f616060e5567d7b03e3e77 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <[e-mail]>
> Date: Tue, 6 Sep 2016 12:18:10 -0400
> Subject: [PATCH] version: Print build-id at bootup.
>
> Livepatch expected at some point to be able to print the
> build-id during bootup, which it did not.  The reason is
> that xen_build_init and livepatch_init are both __initcall
> type routines. This meant that when livepatch_init called
> xen_build_id, it would return -ENODATA as build_id_len was
> not setup yet (b/c xen_build_init would be called later).
>
> The original patch fixed this by calling xen_build_init in
> livepatch_init which allows us to print the build-id of
> the hypervisor.
>
> However the x86 maintainers pointed out that build-id
> is independent of Livepatch and in fact should print
> regardless whether Livepatch is enabled or not.
>
> Therefore this patch moves the logic of printing the build-id
> to version.c.
>
> Reviewed-by: Jan Beulich <[e-mail]>
> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
>

Reviewed-by: Ross Lagerwall <[e-mail]>

Patch

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 78b3731..4c3056c 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1580,12 +1580,6 @@  static void livepatch_printall(unsigned char key)
 
 static int __init livepatch_init(void)
 {
-    const void *binary_id;
-    unsigned int len;
-
-    if ( !xen_build_id(&binary_id, &len) )
-        printk(XENLOG_INFO LIVEPATCH ": build-id: %*phN\n", len, binary_id);
-
     register_keyhandler('x', livepatch_printall, "print livepatch info", 1);
 
     arch_livepatch_init();
diff --git a/xen/common/version.c b/xen/common/version.c
index 4375ea2..0d31e38 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -186,6 +186,8 @@  static int __init xen_build_init(void)
         }
     }
 #endif
+    if ( !rc )
+        printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);
 
     return rc;
 }