[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tools: init-dom0less: Replace err() with more informative messages


  • To: Michal Orzel <michal.orzel@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>
  • Date: Wed, 1 Oct 2025 12:09:07 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=eVeAwhESu7sky54dBkKmIFypERR2RQyJxNZQKPljmKg=; b=Q7mm7YdclAXU6lyDdAxi1thW4PUUEyPNCJFLvGi1gi0Xrirk9MI5+MsQ/yMNVCDXKTlj3PJ4WfG3aK9EqKlKv76RoPaWMwdOV560tCZ+W99zv5m9WVVRPVaQa5bXTziMFrDgm05d0u+gL3LqPnXgsKcHYxf3wjQVfMqmtGFD4d1lNcDSBSSznr7GV+4+AvnFDuIh9yVwDy2oFewPrkCDtORcN8A+M9yc8RFY9nGt7NKa2jOUBuEsdmZTgCweLQO0zaesnonbuZCjk9AOol6unUnEQuWK73A3YEyKC+w7sSV2RUry2/sAwzNrCttTlN+sdwYD5QQd7VmtTZ/Neq+Tsg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=CWflHQoR24FK2esoB9j1zOyNh5PO/6H3SjXqpW4bMnIGTWo7v6D9GT33KakObeI1opAO9U3ZNwk87bGUevURZ3n9JLZS90l3e7g1f7nonzZTvw0+HKHmVMwpS68EJGQGnvsu0aOkUWWb25+ZfesBHYFFCnIc8xsf0plX9tXb25IQjYjzLgSzebB0wDU/1Vl3x1Fc4QSRpK2k3pcCavR/dCxNWUoJ66eyTIq20Ri/Mecj1JXUYRntG4lE9cZfWl2jnjruKl6SyAmlmDSeEjIcVKEki+wVenxsj7Sq66Om/HLPm3SpCS5eIl7ygjg5mOa/2n8mMmQ2UGNCaAuFW092bg==
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>, Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 01 Oct 2025 10:09:32 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <michal.orzel@xxxxxxx>
> ---
>  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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.