[PATCH] libxl: preserve errno in libxl__xcinfo2xlinfo()

Jan Beulich posted 1 patch 2 months ago
Failed in applying to current master (apply log)
[PATCH] libxl: preserve errno in libxl__xcinfo2xlinfo()
Posted by Jan Beulich 2 months ago
Callers observing errors elsewhere may be confused by the ENOSYS that
the Flask operation would yield on a Flask-disabled hypervisor.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Of course I don't know whether clobbering errno is perhaps deemed "fine"
in libxl.

--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -277,6 +277,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx
                           libxl_dominfo *xlinfo)
 {
     size_t size;
+    int saved_errno = errno;
 
     memcpy(&(xlinfo->uuid), xcinfo->handle, sizeof(xen_domain_handle_t));
     xlinfo->domid = xcinfo->domain;
@@ -284,6 +285,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx
     if (libxl_flask_sid_to_context(ctx, xlinfo->ssidref,
                                    &xlinfo->ssid_label, &size) < 0)
         xlinfo->ssid_label = NULL;
+    errno = saved_errno;
 
     xlinfo->dying      = !!(xcinfo->flags&XEN_DOMINF_dying);
     xlinfo->shutdown   = !!(xcinfo->flags&XEN_DOMINF_shutdown);
Re: [PATCH] libxl: preserve errno in libxl__xcinfo2xlinfo()
Posted by Jason Andryuk 2 months ago
On 2025-08-27 01:57, Jan Beulich wrote:
> Callers observing errors elsewhere may be confused by the ENOSYS that
> the Flask operation would yield on a Flask-disabled hypervisor.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Of course I don't know whether clobbering errno is perhaps deemed "fine"
> in libxl.

I wonder if it would be better to special case 
libxl_flask_sid_to_context() to preserve errno on ENOSYS.  flask 
returning ENOSYS is common, but libxl_flask_sid_to_context() can 
legitimately have error.

I guess this is fine if we want to use this approach:

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

> --- a/tools/libs/light/libxl_domain.c
> +++ b/tools/libs/light/libxl_domain.c
> @@ -277,6 +277,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx
>                             libxl_dominfo *xlinfo)
>   {
>       size_t size;
> +    int saved_errno = errno;
>   
>       memcpy(&(xlinfo->uuid), xcinfo->handle, sizeof(xen_domain_handle_t));
>       xlinfo->domid = xcinfo->domain;
> @@ -284,6 +285,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx
>       if (libxl_flask_sid_to_context(ctx, xlinfo->ssidref,
>                                      &xlinfo->ssid_label, &size) < 0)
>           xlinfo->ssid_label = NULL;
> +    errno = saved_errno;
>   
>       xlinfo->dying      = !!(xcinfo->flags&XEN_DOMINF_dying);
>       xlinfo->shutdown   = !!(xcinfo->flags&XEN_DOMINF_shutdown);
>
Re: [PATCH] libxl: preserve errno in libxl__xcinfo2xlinfo()
Posted by Anthony PERARD 1 month, 3 weeks ago
On Wed, Aug 27, 2025 at 09:16:38PM -0400, Jason Andryuk wrote:
> On 2025-08-27 01:57, Jan Beulich wrote:
> > Callers observing errors elsewhere may be confused by the ENOSYS that
> > the Flask operation would yield on a Flask-disabled hypervisor.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > ---
> > Of course I don't know whether clobbering errno is perhaps deemed "fine"
> > in libxl.
> 
> I wonder if it would be better to special case libxl_flask_sid_to_context()
> to preserve errno on ENOSYS.  flask returning ENOSYS is common, but
> libxl_flask_sid_to_context() can legitimately have error.

Well, errno=ENOSYS gives information about why
libxl_flask_sid_to_context() returns an error. 
They are multiple error code for returns libxl_*() functions but they
aren't really check. We often rely on errno to print an error message.
And in this case, libxl_flask_sid_to_context() doesn't event return a
proper libxl_error value.

> I guess this is fine if we want to use this approach:
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD
Re: [PATCH] libxl: preserve errno in libxl__xcinfo2xlinfo()
Posted by Jan Beulich 2 months ago
On 28.08.2025 03:16, Jason Andryuk wrote:
> On 2025-08-27 01:57, Jan Beulich wrote:
>> Callers observing errors elsewhere may be confused by the ENOSYS that
>> the Flask operation would yield on a Flask-disabled hypervisor.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Of course I don't know whether clobbering errno is perhaps deemed "fine"
>> in libxl.
> 
> I wonder if it would be better to special case 
> libxl_flask_sid_to_context() to preserve errno on ENOSYS.  flask 
> returning ENOSYS is common, but libxl_flask_sid_to_context() can 
> legitimately have error.

But then libxl__xcinfo2xlinfo() also shouldn't (effectively) ignore such
other errors? Question is what it should do in such an event.

> I guess this is fine if we want to use this approach:
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks.

Jan