Re: [PATCH v1] x86/domain: revisit logging in arch_domain_create()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Apr 01, 2025 at 09:51:53AM +0200, Jan Beulich wrote:
> On 01.04.2025 08:59, Roger Pau Monné wrote:
> > On Mon, Mar 31, 2025 at 09:34:24PM +0000, dmkhn@xxxxxxxxx wrote:
> >> --- a/xen/arch/x86/domain.c
> >> +++ b/xen/arch/x86/domain.c
> >> @@ -798,13 +798,12 @@ int arch_domain_create(struct domain *d,
> >>      {
> >>          if ( !opt_allow_unsafe )
> >>          {
> >> -            printk(XENLOG_G_ERR "Xen does not allow DomU creation on this 
> >> CPU"
> >> -                   " for security reasons.\n");
> >> +            printk(XENLOG_G_ERR "%pd: Xen does not allow DomU creation on 
> >> this CPU"
> >> +                   " for security reasons.\n", d);
> > 
> > Since you are already touching this, I would switch to gprintk, and
> > avoid splitting the lines:
> > 
> >             gprintk(XENLOG_ERR,
> >                     "%pd: Xen does not allow DomU creation on this CPU for 
> > security reasons.\n",
> >                     d);
> > 
> > Same for the other messages below.
> 
> IOW you see value in also logging current->domain?

I always forget that gprintk also logs current->domain, my suggestion
was so that XENLOG_ERR instead of XENLOG_G_ERR was used, as it's more
compact.

I think I withdraw my suggestion, there's likely very little help from
printing current->domain in this context.  It's either the IDLE domain
for initial domain build, or the control domain otherwise.

Thanks, Roger.