[v5] x86/domain: revisit logging in arch_domain_create()

Commit Message

Denis Mukhin April 8, 2025, 10:40 p.m. UTC

From: Denis Mukhin <[e-mail]>

Use %pd in all logs issued from arch_domain_create() and reword some of the
messages.

Also, expand error message in arch_domain_create() under !emulation_flags_ok()
case to help debugging.

Signed-off-by: Denis Mukhin <[e-mail]>
---
Changes since v4:
- updated wording in the messages as per v4 review
---
 xen/arch/x86/domain.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Jan Beulich April 9, 2025, 6:19 a.m. UTC | #1

On 09.04.2025 00:40, [e-mail] wrote:
> From: Denis Mukhin <[e-mail]>
> 
> Use %pd in all logs issued from arch_domain_create() and reword some of the
> messages.
> 
> Also, expand error message in arch_domain_create() under !emulation_flags_ok()
> case to help debugging.
> 
> Signed-off-by: Denis Mukhin <[e-mail]>

Somewhat reluctantly:
Acked-by: Jan Beulich <[e-mail]>

It still remains unclear to me why you think you need to change aspects of
prior wording for no apparent reason. For example, ...

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -791,13 +791,14 @@ 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: will not create domU on this CPU for security reasons\n",
> +                   d);

... why would "DomU" need to change to "domU"? Yes, we're inconsistent about
the case of the first letter. Yet the change here doesn't help the
inconsistency in any way. My more general request remains, going forward:
Please no changes, big or small, that don't have at least a tiny bit of a
reason.

Jan

Patch

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 4989600627..60d85a9b3e 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -791,13 +791,14 @@  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: will not create domU on this CPU for security reasons\n",
+                   d);
             return -EPERM;
         }
         printk(XENLOG_G_WARNING
-               "Dom%d may compromise security on this CPU.\n",
-               d->domain_id);
+               "%pd: may compromise security on this CPU\n",
+               d);
     }
 
     emflags = config->arch.emulation_flags;
@@ -807,16 +808,20 @@  int arch_domain_create(struct domain *d,
 
     if ( emflags & ~XEN_X86_EMU_ALL )
     {
-        printk(XENLOG_G_ERR "d%d: Invalid emulation bitmap: %#x\n",
-               d->domain_id, emflags);
+        printk(XENLOG_G_ERR
+               "%pd: invalid emulation bitmap: %#x\n",
+               d, emflags);
         return -EINVAL;
     }
 
     if ( !emulation_flags_ok(d, emflags) )
     {
-        printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation "
-               "with the current selection of emulators: %#x\n",
-               d->domain_id, is_hvm_domain(d) ? "HVM" : "PV", emflags);
+        printk(XENLOG_G_ERR
+               "%pd: will not create %s %sdomain with emulators: %#x\n",
+               d,
+               is_hvm_domain(d) ? "HVM" : "PV",
+               is_hardware_domain(d) ? "hardware " : "",
+               emflags);
         return -EOPNOTSUPP;
     }
     d->arch.emulation_flags = emflags;