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