[Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Michael Tokarev]

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

Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>

diff --git a/net/tap-linux.c b/net/tap-linux.c
index 6af9e82..dbcbe6f 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -39,7 +39,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
 
     TFR(fd = open("/dev/net/tun", O_RDWR));
     if (fd < 0) {
-        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
+        qemu_error("could not open /dev/net/tun: %m\n");
         return -1;
     }
     memset(&ifr, 0, sizeof(ifr));
@@ -70,7 +70,7 @@ 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");
+        qemu_error("could not configure /dev/net/tun: %m\n");
         close(fd);
         return -1;
     }

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Markus Armbruster]

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
>
> Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 6af9e82..dbcbe6f 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -39,7 +39,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
>  
>      TFR(fd = open("/dev/net/tun", O_RDWR));
>      if (fd < 0) {
> -        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
> +        qemu_error("could not open /dev/net/tun: %m\n");
>          return -1;
>      }
>      memset(&ifr, 0, sizeof(ifr));

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");

[...]

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Michael Tokarev]

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

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;
     }

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Kevin Wolf]

Am 22.04.2010 11:52, schrieb Markus Armbruster:
> 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
>>
>> Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
>>
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index 6af9e82..dbcbe6f 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -39,7 +39,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
>>  
>>      TFR(fd = open("/dev/net/tun", O_RDWR));
>>      if (fd < 0) {
>> -        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
>> +        qemu_error("could not open /dev/net/tun: %m\n");
>>          return -1;
>>      }
>>      memset(&ifr, 0, sizeof(ifr));
> 
> 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");

I'm not sure where this %m is defined exactly (Linux specific? Maybe
BSDs, too?), but it doesn't seem to work with mingw.

Kevin

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Michael Tokarev]

Kevin Wolf wrote:
> Am 22.04.2010 11:52, schrieb Markus Armbruster:
>> 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:
>>>
[]
>>>      TFR(fd = open("/dev/net/tun", O_RDWR));
>>>      if (fd < 0) {
>>> -        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
>>> +        qemu_error("could not open /dev/net/tun: %m\n");
>>>          return -1;
> 
> I'm not sure where this %m is defined exactly (Linux specific? Maybe
> BSDs, too?), but it doesn't seem to work with mingw.

The file being patched is tap-linux.c.
I noted this in my first email.

Thanks!

/mjt

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Kevin Wolf]

Am 23.04.2010 14:52, schrieb Michael Tokarev:
> Kevin Wolf wrote:
>> Am 22.04.2010 11:52, schrieb Markus Armbruster:
>>> 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:
>>>>
> []
>>>>      TFR(fd = open("/dev/net/tun", O_RDWR));
>>>>      if (fd < 0) {
>>>> -        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
>>>> +        qemu_error("could not open /dev/net/tun: %m\n");
>>>>          return -1;
>>
>> I'm not sure where this %m is defined exactly (Linux specific? Maybe
>> BSDs, too?), but it doesn't seem to work with mingw.
> 
> The file being patched is tap-linux.c.
> I noted this in my first email.

Sorry, I missed that. You're right, of course.

Kevin

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Markus Armbruster]

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.

Re: [Qemu-devel] [PATCH] give some useful error messages when tap open fails

[Anthony Liguori]

On 04/23/2010 08:28 AM, Kevin Wolf wrote:
> Am 23.04.2010 14:52, schrieb Michael Tokarev:
>    
>> Kevin Wolf wrote:
>>      
>>> Am 22.04.2010 11:52, schrieb Markus Armbruster:
>>>        
>>>> 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:
>>>>>
>>>>>            
>> []
>>      
>>>>>       TFR(fd = open("/dev/net/tun", O_RDWR));
>>>>>       if (fd<  0) {
>>>>> -        fprintf(stderr, "warning: could not open /dev/net/tun: no virtual network emulation\n");
>>>>> +        qemu_error("could not open /dev/net/tun: %m\n");
>>>>>           return -1;
>>>>>            
>>> I'm not sure where this %m is defined exactly (Linux specific? Maybe
>>> BSDs, too?), but it doesn't seem to work with mingw.
>>>        
>> The file being patched is tap-linux.c.
>> I noted this in my first email.
>>      
> Sorry, I missed that. You're right, of course.
>    

But from a consistency perspective, "%s", strerror(errno) would be nicer 
even if it's technically okay.

Regards,

Anthony Liguori

> Kevin
>
>
>
>