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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list