I was looking at the slightly broken error handling in the new
xc_mem_paging_load() function and stumbled over the ioctl() handling in
netbsd_privcmd_hypercall().
Is ioctl() on NetBSD special? I would have expected it returns -1 on
error and the caller can deal with errno if it actually wants to.
But instead it returns the negative errno value or what the hypervisor
returned.
static int netbsd_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h,
privcmd_hypercall_t *hypercall)
{
int fd = (int)h;
int error = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
if (error < 0)
return -errno;
else
return hypercall->retval;
}
I think do_domctl() is supposed to return -1 on error and let the caller
decide what to do. At least thats how its appearently done on Linux and
Solaris.
Olaf
2012/1/18 Olaf Hering <olaf@aepfle.de>:> > I was looking at the slightly broken error handling in the new > xc_mem_paging_load() function and stumbled over the ioctl() handling in > netbsd_privcmd_hypercall(). > > Is ioctl() on NetBSD special? I would have expected it returns -1 on > error and the caller can deal with errno if it actually wants to. > But instead it returns the negative errno value or what the hypervisor > returned.I haven't done this code, so I don't know if there are some hidden subtleties here, but according to NetBSD ioctl(2) man page[0], it returns -1 on error and errno is set to indicate the error.> static int netbsd_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h, privcmd_hypercall_t *hypercall) > { > int fd = (int)h; > int error = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall); > > if (error < 0) > return -errno; > else > return hypercall->retval; > } > > I think do_domctl() is supposed to return -1 on error and let the caller > decide what to do. At least thats how its appearently done on Linux and > Solaris.[0] http://netbsd.gw.com/cgi-bin/man-cgi?ioctl+.amd64+NetBSD-current _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, Jan 18, Roger Pau Monné wrote:> 2012/1/18 Olaf Hering <olaf@aepfle.de>: > > > > I was looking at the slightly broken error handling in the new > > xc_mem_paging_load() function and stumbled over the ioctl() handling in > > netbsd_privcmd_hypercall(). > > > > Is ioctl() on NetBSD special? I would have expected it returns -1 on > > error and the caller can deal with errno if it actually wants to. > > But instead it returns the negative errno value or what the hypervisor > > returned. > > I haven't done this code, so I don't know if there are some hidden > subtleties here, but according to NetBSD ioctl(2) man page[0], it > returns -1 on error and errno is set to indicate the error.Could you check wether a "return ioctl(....);" works as well? Olaf> > static int netbsd_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h, privcmd_hypercall_t *hypercall) > > { > > int fd = (int)h; > > int error = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall); > > > > if (error < 0) > > return -errno; > > else > > return hypercall->retval; > > } > > > > I think do_domctl() is supposed to return -1 on error and let the caller > > decide what to do. At least thats how its appearently done on Linux and > > Solaris. > > [0] http://netbsd.gw.com/cgi-bin/man-cgi?ioctl+.amd64+NetBSD-current_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/18 Olaf Hering <olaf@aepfle.de>:> On Wed, Jan 18, Roger Pau Monné wrote: > >> 2012/1/18 Olaf Hering <olaf@aepfle.de>: >> > >> > I was looking at the slightly broken error handling in the new >> > xc_mem_paging_load() function and stumbled over the ioctl() handling in >> > netbsd_privcmd_hypercall(). >> > >> > Is ioctl() on NetBSD special? I would have expected it returns -1 on >> > error and the caller can deal with errno if it actually wants to. >> > But instead it returns the negative errno value or what the hypervisor >> > returned. >> >> I haven't done this code, so I don't know if there are some hidden >> subtleties here, but according to NetBSD ioctl(2) man page[0], it >> returns -1 on error and errno is set to indicate the error. > > Could you check wether a "return ioctl(....);" works as well?I've checked it, and realized why NetBSD needs to return hypercall->retval, that's because Linux ioctls return < 0 on error, and >= 0 on success (you can return a meaningful value from ioctl[0]), but NetBSD ioctls only return < 0 on error, or 0 on success. I've spotted that memory operations return the size of the memory region on success, that's why NetBSD needs to return hypercall->retval. What I don't know if why it returns -errno on error, it should return the return value of the ioctl call (error variable). I will submit a patch adding a comment to netbsd_privcmd_hypercall and another one to return error instead of -errno. [0] http://linux.die.net/man/2/ioctl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel