Patchwork give some useful error messages when tap open fails

login
register
mail settings
Submitter Michael Tokarev
Date April 23, 2010, 12:08 p.m.
Message ID <4BD18DA2.7040106@msgid.tls.msk.ru>
Download mbox | patch
Permalink /patch/50812/
State New
Headers show

Comments

Michael Tokarev - April 23, 2010, 12:08 p.m.
Markus Armbruster wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
> 
>> In net/tap-linux.c, when manipulation of /dev/net/tun fails, it prints
>> (with fprintf) something like this:
>>
>>   warning: could not open /dev/net/tun: no virtual network emulation
>>
>> this has 2 issues:
>>  1) it is not a warning really, it's a fatal error (kvm exits after that),
>>  2) there's no indication as of what's actually wrong: printing errno there
>>     is helpful.
>>
>> The patch below removes the "warning" prefix, uses %m (since it's linux,
>> %m is available as format modifier), and changes fprintf() to qemu_error().
>> Now it prints something like this instead:
>>
>>  could not configure /dev/net/tun: Device or resource busy
>>
>> (there are 2 messages like that in the same function)
>>
>> This fixes Debian bug #578154, see
>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=578154
>>
[]
> This might apply to the stable branch (I haven't tried), but I don't
> think it works on master.  There, it should look like this (untested):
> 
> +        error_report("could not open /dev/net/tun: %m");

Yes, the routine name changed in git compared with 0.12.  Here goes
the version for current master, which is also a bit more elegant
(I hope anyway).  Thanks!

(Still with my Signed-Off-By, if needed:
Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
)
Markus Armbruster - April 23, 2010, 1:44 p.m.
Michael Tokarev <mjt@tls.msk.ru> writes:

> Markus Armbruster wrote:
>> Michael Tokarev <mjt@tls.msk.ru> writes:
>> 
>>> In net/tap-linux.c, when manipulation of /dev/net/tun fails, it prints
>>> (with fprintf) something like this:
>>>
>>>   warning: could not open /dev/net/tun: no virtual network emulation
>>>
>>> this has 2 issues:
>>>  1) it is not a warning really, it's a fatal error (kvm exits after that),
>>>  2) there's no indication as of what's actually wrong: printing errno there
>>>     is helpful.
>>>
>>> The patch below removes the "warning" prefix, uses %m (since it's linux,
>>> %m is available as format modifier), and changes fprintf() to qemu_error().
>>> Now it prints something like this instead:
>>>
>>>  could not configure /dev/net/tun: Device or resource busy
>>>
>>> (there are 2 messages like that in the same function)
>>>
>>> This fixes Debian bug #578154, see
>>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=578154
>>>
> []
>> This might apply to the stable branch (I haven't tried), but I don't
>> think it works on master.  There, it should look like this (untested):
>> 
>> +        error_report("could not open /dev/net/tun: %m");
>
> Yes, the routine name changed in git compared with 0.12.  Here goes
> the version for current master, which is also a bit more elegant
> (I hope anyway).  Thanks!
>
> (Still with my Signed-Off-By, if needed:
> Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
> )

Looks better now.  Recommend to repost it as "[PATCH v2]", to ensure it
gets noticed and merged.

Patch

diff --git a/net/tap-linux.c b/net/tap-linux.c
index 03b8301..6e96607 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -33,14 +33,15 @@ 
 #include "qemu-common.h"
 #include "qemu-error.h"

+#define PATH_NET_TUN "/dev/net/tun"
+
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required)
 {
     struct ifreq ifr;
     int fd, ret;

-    TFR(fd = open("/dev/net/tun", O_RDWR));
+    TFR(fd = open(PATH_NET_TUN, O_RDWR));
     if (fd < 0) {
-        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
+        error_report("could not open %s: %m", PATH_NET_TUN);
         return -1;
     }
     memset(&ifr, 0, sizeof(ifr));
@@ -71,7 +73,8 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
         pstrcpy(ifr.ifr_name, IFNAMSIZ, "tap%d");
     ret = ioctl(fd, TUNSETIFF, (void *) &ifr);
     if (ret != 0) {
-        fprintf(stderr, "warning: could not configure /dev/net/tun: no virtual network emulation\n");
+        error_report("could not configure %s (%s): %m", PATH_NET_TUN, ifr.ifr_name);
         close(fd);
         return -1;
     }