diff mbox

BUG REPORT: -net tap,fd=X does not work correctly in kvm-86

Message ID 162a833f0905221802u7e0ad40r88915be927bdc5d2@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Josh Wilsdon May 23, 2009, 1:02 a.m. UTC
We are doing work with libvirt 0.6.3 through which we have started
virtual machines.  Using the same "create" command with the same
storage, the same everything else this configuration works with both
kvm-84 and kvm-85.  When I built a fresh version of kvm-86 this
stopped working with an error:

qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'

The full execve() being used in this case by libvirtd is:

execve("/usr/bin/kvm", ["/usr/bin/kvm", "-S", "-M", "pc", "-m", "512",
"-smp", "1", "-name", "ubuntu-vm", "-uuid",
"2100444e-4f16-f9f4-a6df-b6cac95c55b5", "-monitor", "pty", "-pidfile",
"/var/run/libvirt/qemu//ubuntu-vm.pid", "-boot", "c", "-drive",
"file=/dev/disk/by-path/ip-192.168.50.3:3260-iscsi-iqn.1986-03.com.sun:02:3985d229-59f7-e8a6-ff10-e0a68cf3567f-lun-0,if=virtio,index=0,boot=on",
"-drive", "file=,if=virtio,media=cdrom,index=2", "-net",
"nic,macaddr=54:52:00:27:36:cd,vlan=0,model=virtio", "-net",
"tap,fd=15,script=,vlan=0,ifname=vnet0", "-serial", "pty",
"-parallel", "none", "-usb", "-vnc", "127.0.0.1:0", "-k", "en-us"],
[/* 5 vars */]) = 0

I took a quick look at the code and found this section which changed
between kvm-85 and kvm-86:

             }


When I comment out the check:

+            if (check_params(fd_params, p) < 0) {
+                fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
+                        buf, p);
+                return -1;
+            }

after the first get_param_value this command line works again (though
it seems the networking may not).

Thanks,
Josh

--
Josh Wilsdon
LayerBoom Systems
joshw@layerboom.com
http://layerboom.com
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Anthony Liguori May 23, 2009, 2:12 a.m. UTC | #1
Josh Wilsdon wrote:
> We are doing work with libvirt 0.6.3 through which we have started
> virtual machines.  Using the same "create" command with the same
> storage, the same everything else this configuration works with both
> kvm-84 and kvm-85.  When I built a fresh version of kvm-86 this
> stopped working with an error:
>
> qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'
>   

It's a libvirt bug.  script and ifname have never been valid parameters 
for -net tap.  Historically, -net has ignored unknown options but 
recently, it now throws an error when encountering an unsupported option.

Regards,

Anthony Liguori
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nikola Ciprich May 23, 2009, 1:52 p.m. UTC | #2
Hi,
are You sure about that? at least ifname seems to be valid parameter,
I'm using it for naming host interfaces for tens of my kvms...
and both are stated in kvm-85 help:

-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile]

in general the error message Josh is posting seems to be talking
about something else anyways: invalid parameter 15...

On Fri, May 22, 2009 at 09:12:55PM -0500, Anthony Liguori wrote:
> Josh Wilsdon wrote:
>> We are doing work with libvirt 0.6.3 through which we have started
>> virtual machines.  Using the same "create" command with the same
>> storage, the same everything else this configuration works with both
>> kvm-84 and kvm-85.  When I built a fresh version of kvm-86 this
>> stopped working with an error:
>>
>> qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'
>>   
>
> It's a libvirt bug.  script and ifname have never been valid parameters  
> for -net tap.  Historically, -net has ignored unknown options but  
> recently, it now throws an error when encountering an unsupported option.
>
> Regards,
>
> Anthony Liguori
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka May 23, 2009, 2:42 p.m. UTC | #3
Josh Wilsdon wrote:
> We are doing work with libvirt 0.6.3 through which we have started
> virtual machines.  Using the same "create" command with the same
> storage, the same everything else this configuration works with both
> kvm-84 and kvm-85.  When I built a fresh version of kvm-86 this
> stopped working with an error:
> 
> qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'

ifname is not a valid parameter when you pass a file descriptor.

However, the error message is buggy, too. That's due to a regression I
caused. I'm about the fix this upstream (in qemu), the next kvm release
will then likely report that 'ifname' is the problem here.

Jan
Josh Wilsdon May 23, 2009, 10:14 p.m. UTC | #4
>> When I built a fresh version of kvm-86 this
>> stopped working with an error:
>>
>> qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'
>>
>
> It's a libvirt bug.  script and ifname have never been valid parameters for
> -net tap.  Historically, -net has ignored unknown options but recently, it
> now throws an error when encountering an unsupported option.


Anthony,

Thanks for your reply!

I made some modifications to libvirt last night, to have it not send
in the two parameters you mentioned (script= and ifname=) and indeed,
I was able to create the VM without any modifications to kvm.  I am
still having some other issues with migration, but I believe these to
also be related to the way libvirt is accessing kvm and will try to
look into these separately.  I see that there are some changes with
regard to the qemu parameters since the last libvirt release, so I'll
see if the development version fixes these issues.

- Josh
Daniel P. Berrangé May 24, 2009, 12:56 p.m. UTC | #5
On Fri, May 22, 2009 at 09:12:55PM -0500, Anthony Liguori wrote:
> Josh Wilsdon wrote:
> >We are doing work with libvirt 0.6.3 through which we have started
> >virtual machines.  Using the same "create" command with the same
> >storage, the same everything else this configuration works with both
> >kvm-84 and kvm-85.  When I built a fresh version of kvm-86 this
> >stopped working with an error:
> >
> >qemu: invalid parameter '15' in 'fd=15,script=,vlan=0,ifname=vnet0'
> >  
> 
> It's a libvirt bug.  script and ifname have never been valid parameters 
> for -net tap.  Historically, -net has ignored unknown options but 
> recently, it now throws an error when encountering an unsupported option.

IIRC, the reason libvirt had been passing the ifname=XXX arg, is because 
Xenner wanted it know the name associated with the TAP device, and would
then open it, rather than taking the FD passed in. Xenner long ago gained
code todo the reverse fd -> name lookup though, so we should just kill
this bogus code in libvirt now and drop ifname & script= args when passing
an FD.

Regards,
Daniel
diff mbox

Patch

--- kvm-85/kvm/net.c    2009-04-21 02:57:31.000000000 -0700
+++ kvm-86/net.c        2009-05-19 09:29:02.000000000 -0700
@@ -1887,13 +2100,24 @@ 
         int fd;
         vlan->nb_host_devs++;
         if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
+            if (check_params(fd_params, p) < 0) {
+                fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
+                        buf, p);
+                return -1;
+            }
             fd = strtol(buf, NULL, 0);
             fcntl(fd, F_SETFL, O_NONBLOCK);
-            ret = -1;
-                if (net_tap_fd_init(vlan, device, name, fd,
-                                    tap_probe_vnet_hdr(fd)))
-                ret = 0;
+                net_tap_fd_init(vlan, device, name, fd,
tap_probe_vnet_hdr(fd));
+            ret = 0;
         } else {
+            static const char * const tap_params[] = {
+                "vlan", "name", "ifname", "script", "downscript", NULL
+            };
+            if (check_params(tap_params, p) < 0) {
+                fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
+                        buf, p);
+                return -1;
+            }
             if (get_param_value(ifname, sizeof(ifname), "ifname", p) <= 0) {
                 ifname[0] = '\0';