Xen devel - Dec 2012 - Suggestion: Improve hypercall Interface to get real return value

Hello everyone,

I am a user of Xen and I have encountered a problem when trying to get the
return value from hypercall in TOOLS. I found that when programs in TOOLS called
hypercall and failed, it would get return value -1 instead of the real return
value given by Hypervisor.

The reason is that the hypercall interface of TOOLS uses ioctl to call
hypercall, and ioctl will only return -1 when failure occurs. The related code
is in xen-4.2.0/tools/libxc/xc_linux_osdep.c:
    static int linux_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h,
privcmd_hypercall_t *hypercall)
    {
        int fd = (int)h;
        return ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
    }

While I don't think this is a good idea, because some tools may wish to use
real return value. For example, in xen-4.2.0/tools/memshr/interface.c, the
function memshr_vbd_issue_ro_request will switch variable ret which comes from
return value of hypercall do_memory_op:
    switch(ret)
    {
        case XENMEM_SHARING_OP_S_HANDLE_INVALID:
            ……
            break;
        case XENMEM_SHARING_OP_C_HANDLE_INVALID:
            ……
            break;
        default:
            break;
}

So I think if we could modify the interface a little bit to return the real
error number, it would be beneficial to many TOOLS developers including myself.
My suggested modification is simple:
    static int linux_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h,
privcmd_hypercall_t *hypercall)
    {
        int fd = (int)h;
        int ret = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
        if (ret < 0)
            return -errno;
        return ret;
    }

Do you think this would be a good modification? 
Also, I am curious why the original design didn't do that. Is it a bug or is
it designed that way intentionally?
Any suggestions and comments will be highly appreciated.

Thanks!
Best Regards,
Yanzhang Li

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Wed, Dec 5, 2012 at 6:21 AM, Yanzhang Li <liyz@pku.edu.cn> wrote:
>
>
> Do you think this would be a good modification?
> Also, I am curious why the original design didn''t do that. Is it a
bug or
> is it designed that way intentionally?
> Any suggestions and comments will be highly appreciated.
>
The reason we just return -1 is because that is the standard practice for
Unix system libraries: to return -1 but set the error value in
"errno".  I
couldn''t tell you why Unix does this, but there''s an advantage
to following
standard interfaces, because it reduces the surprise factor, and reduces
the amount of information programmers need to keep in their head.

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On 05/12/12 11:05, George Dunlap wrote:
> On Wed, Dec 5, 2012 at 6:21 AM, Yanzhang Li <liyz@pku.edu.cn 
> <mailto:liyz@pku.edu.cn>> wrote:
>
>
>
>     Do you think this would be a good modification?
>     Also, I am curious why the original design didn''t do that. Is
it a
>     bug or is it designed that way intentionally?
>     Any suggestions and comments will be highly appreciated.
>
>
> The reason we just return -1 is because that is the standard practice 
> for Unix system libraries: to return -1 but set the error value in 
> "errno".  I couldn''t tell you why Unix does this, but
there''s an
> advantage to following standard interfaces, because it reduces the 
> surprise factor, and reduces the amount of information programmers 
> need to keep in their head.
I think returning -1 instead of "the error" allows for simpler code
when
you do something like this:
int func()
{
      FILE *f = fopen(...);
      if(!f) return -1;

      while(...)
      {
         if (fread(f, ...) < 0)
         {
            fclose(f);
            return -1;
          }
          ...
         if (...)
            if (fwrite(f, ...) < 0)
            {
               fclose(f);
               return -1;
            }
      }
      fclose(f);
      return 0;
}

Now, we don''t need extra code to "remember the errno from the
failing
function". [And I''m assuming here that fclose isn''t
"interfering" with
the errno - if you REALLY need to know for sure what the errno was at 
fread or fwrite, you still need to "remember errno".

(Note that some functions do not return -1 for failure in the above 
code, but for example NULL, and some function would not be able to 
return -errno, as that may well be a "valid" return value - so keeping
the interface as alike as possible is a good idea)

--
Mats
>
>  -George

Dear George,
Thank you very much for your comments. That really helps a lot.
Best Regards,
Yanzhang Li

----- Original Message -----
From: "George Dunlap" <dunlapg@umich.edu>
To: "Yanzhang Li" <liyz@pku.edu.cn>
Cc: xen-devel@lists.xen.org
Sent: un5efine5 7:05:32 PM
Subject: Re: [Xen-devel] Suggestion: Improve hypercall Interface to get real
 return value

On Wed, Dec 5, 2012 at 6:21 AM, Yanzhang Li < liyz@pku.edu.cn > wrote: 






Do you think this would be a good modification? 
Also, I am curious why the original design didn't do that. Is it a bug or is
it designed that way intentionally?
Any suggestions and comments will be highly appreciated. 


The reason we just return -1 is because that is the standard practice for Unix
system libraries: to return -1 but set the error value in "errno".  I
couldn't tell you why Unix does this, but there's an advantage to
following standard interfaces, because it reduces the surprise factor, and
reduces the amount of information programmers need to keep in their head.

 -George 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel