[libvirt] PATCH: Fix passing of ifname to QEMU
When using the type='ethernet' network device configuration for a guest we pass a script, and optional interface name to QEMU. If ifname is omitted, then QEMU allocates one itself. The problem was we were passing an ifname of '(null)' by mistake. This patch corrects that problem and adds a test for it. Daniel diff -r b6a065030fa6 src/qemu_conf.c --- a/src/qemu_conf.c Fri Jan 30 12:28:00 2009 +0000 +++ b/src/qemu_conf.c Fri Jan 30 13:07:11 2009 +0000 @@ -1147,11 +1147,18 @@ int qemudBuildCommandLine(virConnectPtr case VIR_DOMAIN_NET_TYPE_ETHERNET: { char arg[PATH_MAX]; - if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", - net->ifname, - net->data.ethernet.script, - vlan) >= (PATH_MAX-1)) - goto error; + if (net->ifname) { + if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", + net->ifname, + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + } else { + if (snprintf(arg, PATH_MAX-1, "tap,script=%s,vlan=%d", + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + } ADD_ARG_LIT(arg); } diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,macaddr=00:11:22:33:44:55,vlan=0 -net tap,ifname=nic02,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none -usb diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <interface type='ethernet'> + <mac address='00:11:22:33:44:55'/> + <script path='/etc/qemu-ifup'/> + <target dev='nic02'/> + </interface> + </devices> +</domain> diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.args --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,macaddr=00:11:22:33:44:55,vlan=0 -net tap,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none -usb diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <interface type='ethernet'> + <mac address='00:11:22:33:44:55'/> + <script path='/etc/qemu-ifup'/> + </interface> + </devices> +</domain> diff -r b6a065030fa6 tests/qemuxml2argvtest.c --- a/tests/qemuxml2argvtest.c Fri Jan 30 12:28:00 2009 +0000 +++ b/tests/qemuxml2argvtest.c Fri Jan 30 13:07:11 2009 +0000 @@ -224,6 +224,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_UUID | QEMUD_CMD_FLAG_DOMID); DO_TEST("net-user", 0); DO_TEST("net-virtio", 0); + DO_TEST("net-eth", 0); + DO_TEST("net-eth-ifname", 0); DO_TEST("serial-vc", 0); DO_TEST("serial-pty", 0); diff -r b6a065030fa6 tests/qemuxml2xmltest.c --- a/tests/qemuxml2xmltest.c Fri Jan 30 12:28:00 2009 +0000 +++ b/tests/qemuxml2xmltest.c Fri Jan 30 13:07:11 2009 +0000 @@ -111,6 +111,8 @@ mymain(int argc, char **argv) DO_TEST("misc-no-reboot"); DO_TEST("net-user"); DO_TEST("net-virtio"); + DO_TEST("net-eth"); + DO_TEST("net-eth-ifname"); DO_TEST("sound"); DO_TEST("serial-vc"); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Jan 30, 2009 at 01:08:54PM +0000, Daniel P. Berrange wrote:
When using the type='ethernet' network device configuration for a guest we pass a script, and optional interface name to QEMU. If ifname is omitted, then QEMU allocates one itself. The problem was we were passing an ifname of '(null)' by mistake. This patch corrects that problem and adds a test for it.
Okay, we would have caught it before if the libc segfaulted in such a situation. +1 to the patch . I wonder if there is a way to avoid glibc current behaviour when doing regression tests, waiting for solaris testing is not a good way to catch those :-\ Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
"Daniel P. Berrange" <berrange@redhat.com> wrote:
When using the type='ethernet' network device configuration for a guest we pass a script, and optional interface name to QEMU. If ifname is omitted, then QEMU allocates one itself. The problem was we were passing an ifname of '(null)' by mistake. This patch corrects that problem and adds a test for it.
Daniel
diff -r b6a065030fa6 src/qemu_conf.c --- a/src/qemu_conf.c Fri Jan 30 12:28:00 2009 +0000 +++ b/src/qemu_conf.c Fri Jan 30 13:07:11 2009 +0000 @@ -1147,11 +1147,18 @@ int qemudBuildCommandLine(virConnectPtr case VIR_DOMAIN_NET_TYPE_ETHERNET: { char arg[PATH_MAX]; - if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", - net->ifname, - net->data.ethernet.script, - vlan) >= (PATH_MAX-1)) - goto error;
+ if (net->ifname) { + if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", + net->ifname, + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + } else { + if (snprintf(arg, PATH_MAX-1, "tap,script=%s,vlan=%d", + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + }
Along the way, it'd be nice to use "sizeof(arg)-1" in place of PATH_MAX-1. That format string and arg list are long enough that it'd be good not to duplicate them. How about this instead? char if_buf[PATH_MAX]; if_buf[0] = 0; if (net->ifname && (snprintf(if_buf, sizeof(if_buf), ",script=%s", ifname) >= sizeof(if_buf)-1)) goto error; if (snprintf(arg, sizeof(arg), "tap%s,script=%s,vlan=%d", if_buf, net->data.ethernet.script, vlan) >= sizeof(arg)-1)) goto error; +1 for the test ACK either way.
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering