[Qemu-devel] [PATCH] net: provide a friendly message when a user passes a bad -net tap, fd=X

[Qemu-devel] [PATCH] net: provide a friendly message when a user passes a bad -net tap, fd=X

[Anthony Liguori]

A lot of people copy libvirt's command line from ps -ef and then wonder why the
VM isn't working correctly.  Let's be kind and tell them what they should do
instead.

Without this patch, if you run with an invalid -net tap,fd=X, the guest still
runs and we poll 100% on a bad file descriptor.  With this patch, you get:

 qemu: -net tap,fd=42: invalid fd= for tap network device.  If you're copying
 from libvirt, use `virsh dom2xml-to-native' instead
 qemu: -net tap,fd=42: Device 'tap' could not be initialized

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 1d83400..5f9f749 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -64,7 +64,7 @@
 #define UHCI_PORT_CSC   (1 << 1)
 #define UHCI_PORT_CCS   (1 << 0)
 
-#define FRAME_TIMER_FREQ 1000
+#define FRAME_TIMER_FREQ 500
 
 #define FRAME_MAX_LOOPS  100
 
@@ -1054,7 +1054,7 @@ static void uhci_frame_timer(void *opaque)
     UHCIState *s = opaque;
 
     /* prepare the timer for the next frame */
-    s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
+    s->expire_time = qemu_get_clock(vm_clock) + (get_ticks_per_sec() / FRAME_TIMER_FREQ);
 
     if (!(s->cmd & UHCI_CMD_RS)) {
         /* Full stop */
diff --git a/net/tap.c b/net/tap.c
index 0147dab..45b1fb6 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -405,6 +405,8 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
     int fd, vnet_hdr = 0;
 
     if (qemu_opt_get(opts, "fd")) {
+        int flags;
+
         if (qemu_opt_get(opts, "ifname") ||
             qemu_opt_get(opts, "script") ||
             qemu_opt_get(opts, "downscript") ||
@@ -418,6 +420,11 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
             return -1;
         }
 
+        if (fcntl(fd, F_GETFL, &flags) == -1) {
+            error_report("invalid fd= for tap network device.  If you're copying from libvirt, use `virsh dom2xml-to-native' instead");
+            return -1;
+        }
+
         fcntl(fd, F_SETFL, O_NONBLOCK);
 
         vnet_hdr = tap_probe_vnet_hdr(fd);
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] net: provide a friendly message when a user passes a bad -net tap, fd=X

[Alexander Graf]

On 09.10.2010, at 00:04, Anthony Liguori wrote:

> A lot of people copy libvirt's command line from ps -ef and then wonder why the
> VM isn't working correctly.  Let's be kind and tell them what they should do
> instead.
> 
> Without this patch, if you run with an invalid -net tap,fd=X, the guest still
> runs and we poll 100% on a bad file descriptor.  With this patch, you get:
> 
> qemu: -net tap,fd=42: invalid fd= for tap network device.  If you're copying
> from libvirt, use `virsh dom2xml-to-native' instead
> qemu: -net tap,fd=42: Device 'tap' could not be initialized
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
> index 1d83400..5f9f749 100644
> --- a/hw/usb-uhci.c
> +++ b/hw/usb-uhci.c
> @@ -64,7 +64,7 @@
> #define UHCI_PORT_CSC   (1 << 1)
> #define UHCI_PORT_CCS   (1 << 0)
> 
> -#define FRAME_TIMER_FREQ 1000
> +#define FRAME_TIMER_FREQ 500
> 
> #define FRAME_MAX_LOOPS  100
> 
> @@ -1054,7 +1054,7 @@ static void uhci_frame_timer(void *opaque)
>     UHCIState *s = opaque;
> 
>     /* prepare the timer for the next frame */
> -    s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
> +    s->expire_time = qemu_get_clock(vm_clock) + (get_ticks_per_sec() / FRAME_TIMER_FREQ);

How exactly is this related?

Alex

Re: [Qemu-devel] [PATCH] net: provide a friendly message when a user passes a bad -net tap, fd=X

[Anthony Liguori]

On 10/08/2010 06:28 PM, Alexander Graf wrote:
> On 09.10.2010, at 00:04, Anthony Liguori wrote:
>
>    
>> A lot of people copy libvirt's command line from ps -ef and then wonder why the
>> VM isn't working correctly.  Let's be kind and tell them what they should do
>> instead.
>>
>> Without this patch, if you run with an invalid -net tap,fd=X, the guest still
>> runs and we poll 100% on a bad file descriptor.  With this patch, you get:
>>
>> qemu: -net tap,fd=42: invalid fd= for tap network device.  If you're copying
>> from libvirt, use `virsh dom2xml-to-native' instead
>> qemu: -net tap,fd=42: Device 'tap' could not be initialized
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
>> index 1d83400..5f9f749 100644
>> --- a/hw/usb-uhci.c
>> +++ b/hw/usb-uhci.c
>> @@ -64,7 +64,7 @@
>> #define UHCI_PORT_CSC   (1<<  1)
>> #define UHCI_PORT_CCS   (1<<  0)
>>
>> -#define FRAME_TIMER_FREQ 1000
>> +#define FRAME_TIMER_FREQ 500
>>
>> #define FRAME_MAX_LOOPS  100
>>
>> @@ -1054,7 +1054,7 @@ static void uhci_frame_timer(void *opaque)
>>      UHCIState *s = opaque;
>>
>>      /* prepare the timer for the next frame */
>> -    s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
>> +    s->expire_time = qemu_get_clock(vm_clock) + (get_ticks_per_sec() / FRAME_TIMER_FREQ);
>>      
> How exactly is this related?
>    

So, didn't have a clean working directory.

Regards,

Anthony Liguori

> Alex
>
>
>    

[Qemu-devel] Re: [PATCH] net: provide a friendly message when a user passes a bad -net tap, fd=X

[Daniel P. Berrange]

On Fri, Oct 08, 2010 at 05:04:56PM -0500, Anthony Liguori wrote:
> A lot of people copy libvirt's command line from ps -ef and then wonder why the
> VM isn't working correctly.  Let's be kind and tell them what they should do
> instead.
> 
> Without this patch, if you run with an invalid -net tap,fd=X, the guest still
> runs and we poll 100% on a bad file descriptor.  With this patch, you get:
> 
>  qemu: -net tap,fd=42: invalid fd= for tap network device.  If you're copying
>  from libvirt, use `virsh dom2xml-to-native' instead
>  qemu: -net tap,fd=42: Device 'tap' could not be initialized
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
> index 1d83400..5f9f749 100644
> --- a/hw/usb-uhci.c
> +++ b/hw/usb-uhci.c
> @@ -64,7 +64,7 @@
>  #define UHCI_PORT_CSC   (1 << 1)
>  #define UHCI_PORT_CCS   (1 << 0)
>  
> -#define FRAME_TIMER_FREQ 1000
> +#define FRAME_TIMER_FREQ 500
>  
>  #define FRAME_MAX_LOOPS  100
>  
> @@ -1054,7 +1054,7 @@ static void uhci_frame_timer(void *opaque)
>      UHCIState *s = opaque;
>  
>      /* prepare the timer for the next frame */
> -    s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ);
> +    s->expire_time = qemu_get_clock(vm_clock) + (get_ticks_per_sec() / FRAME_TIMER_FREQ);
>  
>      if (!(s->cmd & UHCI_CMD_RS)) {
>          /* Full stop */
> diff --git a/net/tap.c b/net/tap.c
> index 0147dab..45b1fb6 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -405,6 +405,8 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
>      int fd, vnet_hdr = 0;
>  
>      if (qemu_opt_get(opts, "fd")) {
> +        int flags;
> +
>          if (qemu_opt_get(opts, "ifname") ||
>              qemu_opt_get(opts, "script") ||
>              qemu_opt_get(opts, "downscript") ||
> @@ -418,6 +420,11 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
>              return -1;
>          }
>  
> +        if (fcntl(fd, F_GETFL, &flags) == -1) {
> +            error_report("invalid fd= for tap network device.  If you're copying from libvirt, use `virsh dom2xml-to-native' instead");

It is just 'domxml-to-native' - no '2' in there.

Should it check for EBADF explicitly ? I guess this is the only errno that
can really happen here, but I always prefer strict checks myself.


Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|