On Mon, Feb 04, 2013 at 07:12:38PM -0500, Stefan Berger wrote:
Add support for file descriptor sets by converting some of the command line parameters to use /dev/fdset/%d if -add-fd is found to be supported by QEMU. For those devices libvirt now open()s the device to obtain the file descriptor and 'transfers' the fd using virCommand.
For the following fragments of domain XML
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/fc14-x86_64.img'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
<serial type='dev'> <source path='/dev/ttyS0'/> <target port='0'/> </serial> <serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> </serial>
libvirt now creates the following parts for the QEMU command line:
old: -drive file=/var/lib/libvirt/images/fc14-x86_64.img,if=none,id=drive-ide0-0-0,format=raw new: -add-fd set=1,fd=23,opaque=RDONLY:/var/lib/libvirt/images/fc14-x86_64.img -add-fd set=1,fd=24,opaque=RDWR:/var/lib/libvirt/images/fc14-x86_64.img -drive file=/dev/fdset/1,if=none,id=drive-ide0-0-0,format=raw
old: -chardev tty,id=charserial0,path=/dev/ttyS0 new: -add-fd set=1,fd=30,opaque=/dev/ttyS0 -chardev tty,id=charserial0,path=/dev/fdset/1
old: -chardev pipe,id=charserial1,path=/tmp/testpipe new: -add-fd set=2,fd=32,opaque=/tmp/testpipe -chardev pipe,id=charserial1,path=/dev/fdset/2
Given this significant change to the ARGV generation, I'd expect to see more than just this change in the test suite:
Index: libvirt/tests/qemuxml2argvtest.c =================================================================== --- libvirt.orig/tests/qemuxml2argvtest.c +++ libvirt/tests/qemuxml2argvtest.c @@ -151,7 +151,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) { + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) { if (flags & FLAG_EXPECT_FAILURE) { ret = 0; virResetLastError(); Index: libvirt/tests/qemuxmlnstest.c =================================================================== --- libvirt.orig/tests/qemuxmlnstest.c +++ libvirt/tests/qemuxmlnstest.c @@ -110,7 +110,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, json, extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) goto fail;
if (!!virGetLastError() != expectError) {
Yes, this nicely shows that you didn't break any existing QEMU ARGV generation, but at the same time it shows there's zero testing coverage of the new code Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|