[libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1

[Roopa Prabhu]

On 10/26/11 8:01 PM, "Roopa Prabhu" <roprabhu at cisco.com> wrote:

> 
> 
> 
> On 10/24/11 11:14 AM, "Stefan Berger" <stefanb at linux.vnet.ibm.com> wrote:
> 
>> On 10/20/2011 01:46 PM, Roopa Prabhu wrote:
>>> From: Roopa Prabhu<roprabhu at cisco.com>
>>> 
>>> Fixes some cases where 1 was being returned instead of -1.
>>> There are still some inconsistencies in the file with respect
>>> to what the return variable is initialized to. Can be fixed
>>> as a separate patch if needed. The scope of this patch is just
>>> to fix the return value 1. Did some basic sanity test.
>>> 
>>> Signed-off-by: Roopa Prabhu<roprabhu at cisco.com>
>>> Reported-by: Eric Blake<eblake at redhat.com>
>>> ---
>>>   src/util/macvtap.c |   22 ++++++++--------------
>>>   1 files changed, 8 insertions(+), 14 deletions(-)
>>> 
>>> 
>>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>>> index 7fd6eb5..f8b9d55 100644
>>> --- a/src/util/macvtap.c
>>> +++ b/src/util/macvtap.c
>>> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
>>>                        bool is8021Qbg,
>>>                        uint16_t *status)
>>>   {
>>> -    int rc = 1;
>>> +    int rc = -1;
>>>       const char *msg = NULL;
>>>       struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>>> 
>>> @@ -806,7 +806,7 @@ sassmann at redhat.com(bool nltarget_kernel,
>>>                       _("error %d during port-profile setlink on "
>>>                         "interface %s (%d)"),
>>>                       status, ifname, ifindex);
>>> -            rc = 1;
>>> +            rc = -1;
>>>               break;
>>>           }
>>> 
>> In this function we later on return a -ETIMEDOUT. The -1 doesn't overlap
>> with it, but I am wondering whether we should return -EFAULT in the
>> places of -1 now ?
>> 
> Instead of defaulting to -EFAULT, shall I just add a function to map the
> port profile/VDP status codes to closest errno (with default being unknown
> error) and use that instead ?.
> Also, for some reason we are reporting EINVAL in the virReportSystemError
> just above it. EINVAL also does not seem right there.
> 
> I can fix it. 
> Thanks,
> Roopa
> 
Also, looks like no where else in libvirt code we return errno. We only
print errno but always return -1. And the -ETIMEDOUT case in macvtap is an
exception, cause the upper layer requires the cause of this particular
error.
I don't mind changing everything to errno. But that seems to be not the
convention. And I cant find a clean way to not return -ETIMEDOUT. I can
however return status of the command in an argument and return -1 for the
ETIMEDOUT case. I will do that unless you have other suggestions. Thanks.