| Message ID | 20250212235754.3686278-1-dmukhin@ford.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/console: add keyhandler to print Xen version | expand |
On 12/02/2025 11:58 pm, dmkhn@proton.me wrote: > Add Xen version printout via keyhandler mechanism. > > That is useful for debugging systems that have been left intact for a long > time. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> Hmm, it's a good point, but can't we add this to the top of the 'h' (help) key rather than adding a(nother) handler? Amongst other things, ~ is going to be problematic for any connection serial running over SSH (usually IPMI), as it's the SSH escape character, and use on Xen's console expects ~ with no following keypress.
On 13.02.2025 00:58, dmkhn@proton.me wrote: > Add Xen version printout via keyhandler mechanism. > > That is useful for debugging systems that have been left intact for a long > time. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> First, +1 to what Andrew said. > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...) > va_end(args); > } > > +static void xen_print_version(void) > +{ > + const char *str = NULL; > + unsigned int str_len; > + > + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > + xen_major_version(), xen_minor_version(), xen_extra_version(), > + xen_compile_by(), xen_compile_domain(), xen_compiler(), > + xen_build_info(), xen_compile_date()); > + > + printk("Latest ChangeSet: %s\n", xen_changeset()); > + > + if ( !xen_build_id((const void **)&str, &str_len) ) Why the cast? What wrong with "str" being const void *? The only thing is that it's then not really a good name for the variable, but that it isn't anyway. It's not really a string you get back. > @@ -1024,15 +1046,12 @@ void __init console_init_preirq(void) > nrspin_lock(&console_lock); > __putstr(xen_banner()); > nrspin_unlock(&console_lock); > - printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > - xen_major_version(), xen_minor_version(), xen_extra_version(), > - xen_compile_by(), xen_compile_domain(), xen_compiler(), > - xen_build_info(), xen_compile_date()); > - printk("Latest ChangeSet: %s\n", xen_changeset()); > > /* Locate and print the buildid, if applicable. */ > xen_build_init(); > > + xen_print_version(); It may seem minor, but I'm not happy about the reordering. What's logged would better really be directly after the banner. Any present or future output from xen_build_init() should come afterwards. Simply add xen_print_buildid() along with xen_print_version()? And then have it in version.c, where you can use build_id_{p,len} directly, eliminating the point above regarding casts and naming. Jan
On Wednesday, February 12th, 2025 at 11:25 PM, Jan Beulich <jbeulich@suse.com> wrote: > > > On 13.02.2025 00:58, dmkhn@proton.me wrote: > > > Add Xen version printout via keyhandler mechanism. > > > > That is useful for debugging systems that have been left intact for a long > > time. > > > > Signed-off-by: Denis Mukhin dmukhin@ford.com > > > First, +1 to what Andrew said. > > > --- a/xen/drivers/char/console.c > > +++ b/xen/drivers/char/console.c > > @@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...) > > va_end(args); > > } > > > > +static void xen_print_version(void) > > +{ > > + const char *str = NULL; > > + unsigned int str_len; > > + > > + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > > + xen_major_version(), xen_minor_version(), xen_extra_version(), > > + xen_compile_by(), xen_compile_domain(), xen_compiler(), > > + xen_build_info(), xen_compile_date()); > > + > > + printk("Latest ChangeSet: %s\n", xen_changeset()); > > + > > + if ( !xen_build_id((const void **)&str, &str_len) ) > > > Why the cast? What wrong with "str" being const void *? The only thing > is that it's then not really a good name for the variable, but that it > isn't anyway. It's not really a string you get back. Sorry, that's because I used xenver_varbuf_op() as a reference. Fixed in v2. > > > @@ -1024,15 +1046,12 @@ void __init console_init_preirq(void) > > nrspin_lock(&console_lock); > > __putstr(xen_banner()); > > nrspin_unlock(&console_lock); > > - printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > > - xen_major_version(), xen_minor_version(), xen_extra_version(), > > - xen_compile_by(), xen_compile_domain(), xen_compiler(), > > - xen_build_info(), xen_compile_date()); > > - printk("Latest ChangeSet: %s\n", xen_changeset()); > > > > /* Locate and print the buildid, if applicable. */ > > xen_build_init(); > > > > + xen_print_version(); > > > It may seem minor, but I'm not happy about the reordering. What's logged > would better really be directly after the banner. Any present or future > output from xen_build_init() should come afterwards. Simply add > xen_print_buildid() along with xen_print_version()? And then have it in > version.c, where you can use build_id_{p,len} directly, eliminating the > point above regarding casts and naming. Thanks! Addressed in v2. > > Jan
diff --git a/xen/common/version.c b/xen/common/version.c index bc3714b45f..9e47ff0744 100644 --- a/xen/common/version.c +++ b/xen/common/version.c @@ -210,8 +210,6 @@ void __init xen_build_init(void) } } #endif /* CONFIG_X86 */ - if ( !rc ) - printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); } #endif /* BUILD_ID */ /* diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 73d24a7821..4f945f3bca 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...) va_end(args); } +static void xen_print_version(void) +{ + const char *str = NULL; + unsigned int str_len; + + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", + xen_major_version(), xen_minor_version(), xen_extra_version(), + xen_compile_by(), xen_compile_domain(), xen_compiler(), + xen_build_info(), xen_compile_date()); + + printk("Latest ChangeSet: %s\n", xen_changeset()); + + if ( !xen_build_id((const void **)&str, &str_len) ) + printk(XENLOG_INFO "build-id: %*phN\n", str_len, str); +} + +static void cf_check do_print_version(unsigned char key, bool unused) +{ + printk("'%c' pressed -> printing Xen version\n", key); + xen_print_version(); +} + void __init console_init_preirq(void) { char *p; @@ -1024,15 +1046,12 @@ void __init console_init_preirq(void) nrspin_lock(&console_lock); __putstr(xen_banner()); nrspin_unlock(&console_lock); - printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", - xen_major_version(), xen_minor_version(), xen_extra_version(), - xen_compile_by(), xen_compile_domain(), xen_compiler(), - xen_build_info(), xen_compile_date()); - printk("Latest ChangeSet: %s\n", xen_changeset()); /* Locate and print the buildid, if applicable. */ xen_build_init(); + xen_print_version(); + if ( opt_sync_console ) { serial_start_sync(sercon_handle); @@ -1120,6 +1139,8 @@ void __init console_endboot(void) "decrease log level threshold", 0); register_irq_keyhandler('G', &do_toggle_guest, "toggle host/guest log level adjustment", 0); + register_irq_keyhandler('~', &do_print_version, + "print Xen version", 0); /* Serial input is directed to DOM0 by default. */ switch_serial_input();
Add Xen version printout via keyhandler mechanism. That is useful for debugging systems that have been left intact for a long time. Signed-off-by: Denis Mukhin <dmukhin@ford.com> --- xen/common/version.c | 2 -- xen/drivers/char/console.c | 31 ++++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-)