diff mbox

[12/15] tap-bsd: Convert tap_open() to Error

Message ID 1431432187-10993-13-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 12, 2015, 12:03 p.m. UTC
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(-)

Comments

Eric Blake May 14, 2015, 9:55 p.m. UTC | #1
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.

>              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/

> +                   "for IFF_MULTI_QUEUE available");
>          goto error;
>      }
>  
>
Markus Armbruster May 15, 2015, 8:44 a.m. UTC | #2
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;
>>      }
diff mbox

Patch

diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index bbbe446..8d5c776 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -37,7 +37,6 @@ 
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
              int vnet_hdr_required, int mq_required, Error **errp)
 {
-    /* FIXME error_setg(errp, ...) on failure */
     int fd;
 #ifdef TAPGIFNAME
     struct ifreq ifr;
@@ -72,23 +71,19 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
         }
     }
     if (fd < 0) {
-        error_report("warning: could not open %s (%s): no virtual network emulation",
-                   dname, strerror(errno));
+        error_setg_errno(errp, errno, "could not open %s", dname);
         return -1;
     }
 
 #ifdef TAPGIFNAME
     if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) {
-        fprintf(stderr, "warning: could not get tap name: %s\n",
-            strerror(errno));
+        error_setg_errno(errp, errno, "could not get tap name");
         return -1;
     }
     pstrcpy(ifname, ifname_size, ifr.ifr_name);
 #else
     if (fstat(fd, &s) < 0) {
-        fprintf(stderr,
-            "warning: could not stat /dev/tap: no virtual network emulation: %s\n",
-            strerror(errno));
+        error_setg_errno(errp, errno, "could not stat %s", dname);
         return -1;
     }
     dev = devname(s.st_rdev, S_IFCHR);
@@ -100,8 +95,8 @@  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");
             close(fd);
             return -1;
         }
@@ -117,13 +112,12 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
              int vnet_hdr_required, int mq_required, Error **errp)
 {
-    /* FIXME error_setg(errp, ...) on failure */
     int fd, s, ret;
     struct ifreq ifr;
 
     TFR(fd = open(PATH_NET_TAP, O_RDWR));
     if (fd < 0) {
-        error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+        error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP);
         return -1;
     }
 
@@ -131,7 +125,7 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 
     ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
     if (ret < 0) {
-        error_report("could not get tap interface name");
+        error_setg_errno(errp, errno, "could not get tap interface name");
         goto error;
     }
 
@@ -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");
             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"
+                   "for IFF_MULTI_QUEUE available");
         goto error;
     }