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. > > 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 > @@ -288,24 +288,33 @@ static int init_domain(struct xs_handle *xsh, > > rc = xc_dom_gnttab_seed(xch, info->domid, true, > (xen_pfn_t)-1, xenstore_pfn, 0, 0); > - if (rc) > - err(1, "xc_dom_gnttab_seed"); > + if (rc) { > + printf("Failed to seed gnttab entries\n"); ... also printing the domid and the xenstore pfn here. Or... > + return rc; > + } > } > > libxl_uuid_generate(&uuid); > xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray(&uuid)); > > rc = gen_stub_json_config(info->domid, &uuid); > - if (rc) > - err(1, "gen_stub_json_config"); > + if (rc) { > + printf("Failed to create stub json config\n"); ... the domid and uuid here. Similar suggestions for the other printfs. > + return rc; > + } > > rc = create_xenstore(xsh, info, uuid, xenstore_pfn, xenstore_evtchn); > - if (rc) > - err(1, "writing to xenstore"); > + if (rc) { > + printf("Failed to write to xenstore\n"); > + return rc; > + } > > 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? > + } > + > return 0; > } > Cheers, Alejandro
