Re: [Qemu-devel] [PATCH] xen: Log errno rather than return value

[Paul Durrant]

> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> address@hidden On Behalf Of Paul Durrant
> Sent: 12 October 2017 10:47
> To: Ross Lagerwall <address@hidden>; address@hidden
> Cc: Anthony Perard <address@hidden>; Ross Lagerwall
> <address@hidden>; Stefano Stabellini <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH] xen: Log errno rather than return value
> 
> > -----Original Message-----
> > From: Qemu-devel [mailto:qemu-devel-
> > address@hidden On Behalf Of Ross
> Lagerwall
> > Sent: 11 October 2017 16:52
> > To: address@hidden
> > Cc: Anthony Perard <address@hidden>; Ross Lagerwall
> > <address@hidden>; Stefano Stabellini <address@hidden>
> > Subject: [Qemu-devel] [PATCH] xen: Log errno rather than return value
> >
> > xen_modified_memory() sets errno to communicate what went wrong so
> > log
> > this rather than the return value which is not interesting.
> >
> > Signed-off-by: Ross Lagerwall <address@hidden>
> > ---
> >  hw/i386/xen/xen-hvm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> > index d9ccd5d..8028bed 100644
> > --- a/hw/i386/xen/xen-hvm.c
> > +++ b/hw/i386/xen/xen-hvm.c
> > @@ -1446,7 +1446,7 @@ void xen_hvm_modified_memory(ram_addr_t
> > start, ram_addr_t length)
> >          if (rc) {
> >              fprintf(stderr,
> >                      "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i,
> %s\n",
> > -                    __func__, start, nb_pages, rc, strerror(-rc));
> > +                    __func__, start, nb_pages, errno, strerror(errno));
> 
> I think this is actually a deeper problem. If QEMU is using compat code, which
> one way or another will go via xencall, then xen_modified_memory() will
> return a hypercall errno.

Having waded through the code now it turns out that (for linux at least) 
xencall returns the value returned from the underlying ioctl() call so it will 
actually return -1 with errno set correctly, so the above statement is 
incorrect. Thus there is no inconsistency and the patch DTRT.

Sorry for the noise.

  Paul

> However, if it goes via libxendevicemodel and an
> up-to-date privcmd, then it will return -1 and errno will be set. Thus I think
> the correct fix is not this patch, but a fix in xen_modified_memory() to 
> return
> -errno and a fix in libxendevicemodel and the compat code in libxencntrl to
> behave consistently. It's all rather horrible.
> 
>   Paul
> 
> >          }
> >      }
> >  }
> > --
> > 2.9.5
> >
>