On 2025-10-01 07:24, Orzel, Michal wrote:


On 01/10/2025 12:09, Alejandro Vallejo wrote:
On Wed Oct 1, 2025 at 9:51 AM CEST, Michal Orzel wrote:
Current use of err() has the following issues:
- without setting errno, on error it results in printing e.g.:
  "init-dom0less: writing to xenstore: Success"
  This is very misleading and difficult to deduct that there was a
  failure.
- does not propagate error codes to the caller.
- skips "init_domain failed" message by exiting early.

Maybe add another bullet:
 - The early exit prevents setting up any remaining domains.

That was my main motivation in my attempt at this change: https://lore.kernel.org/xen-devel/[email protected]/.

Your re-written error messages are better though.


Replace err() with more informative messages propagating rc when
possible.

Sounds good to me. Only suggestion I'd make is to also print relevant arguments
where needed, like...


Signed-off-by: Michal Orzel <[email protected]>
---
  tools/helpers/init-dom0less.c | 25 +++++++++++++++++--------
  1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index a182dce56353..3dd2d74886eb 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c

+    }
rc = xs_introduce_domain(xsh, info->domid, xenstore_pfn, xenstore_evtchn);
-    if (!rc)
-        err(1, "xs_introduce_domain");
+    if (!rc) {
+        printf("Failed to introduce a domain\n");
+        return 1;

nit: Maybe -EBUSY so it's -errno like the others?
There are other places in this script where we return 1.

It's a mix of error as 1 or -errno, but at least success == 0 is consistent.

The xs_* functions set errno, so maybe here it would be good to return -errno. I think moving toward -errno, which is more informative, would be an improvement.

Thanks,
Jason

Reply via email to