Re: [Qemu-devel] [PATCH 12/15] tap-bsd: Convert tap_open() to Error

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 05/12/2015 06:03 AM, Markus Armbruster wrote:
>> Fixes inappropriate use of stderr in monitor command handler.
>> 
>> While there, improve some of the messages a bit.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  net/tap-bsd.c | 33 ++++++++++++++-------------------
>>  1 file changed, 14 insertions(+), 19 deletions(-)
>> 
>
>> @@ -139,14 +133,15 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>          /* User requested the interface to have a specific name */
>>          s = socket(AF_LOCAL, SOCK_DGRAM, 0);
>>          if (s < 0) {
>> -            error_report("could not open socket to set interface name");
>> +            error_setg_errno(errp, errno,
>> +                             "could not open socket to set interface name");
>>              goto error;
>>          }
>>          ifr.ifr_data = ifname;
>>          ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
>>          close(s);
>>          if (ret < 0) {
>> -            error_report("could not set tap interface name");
>> +            error_setg_errno(errp, errno, "could not set tap interface name");
>
> Bad. close() may have clobbered errno before you get to report it.

You're right.  I'll dumb this down again.

>>              goto error;
>>          }
>>      } else {
>> @@ -158,14 +153,14 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>          *vnet_hdr = 0;
>>  
>>          if (vnet_hdr_required && !*vnet_hdr) {
>> -            error_report("vnet_hdr=1 requested, but no kernel "
>> -                         "support for IFF_VNET_HDR available");
>> +            error_setg(errp, "vnet_hdr=1 requested, but no kernel "
>> +                       "support for IFF_VNET_HDR available");
>>              goto error;
>>          }
>>      }
>>      if (mq_required) {
>> -        error_report("mq_required requested, but not kernel support"
>> -                     "for IFF_MULTI_QUEUE available");
>> +        error_setg(errp, "mq_required requested, but not kernel support"
>
> As long as you are touching this, s/not/no/

Fixing...

>> +                   "for IFF_MULTI_QUEUE available");
>>          goto error;
>>      }