ioctl handling in netbsd_privcmd_hypercall()

ioctl handling in netbsd_privcmd_hypercall()

[Olaf Hering]

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

Re: ioctl handling in netbsd_privcmd_hypercall()

[Roger Pau Monné]

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

Re: ioctl handling in netbsd_privcmd_hypercall()

[Olaf Hering]

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

Re: ioctl handling in netbsd_privcmd_hypercall()

[Roger Pau Monné]

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