[Libvir] PATCH: Support input devices in XML format
This patch is the first step towards supporting USB devices in libvirt XML format. As per the original thread some months back[1], I'm the grouping is being done based on device classes, rather than bus types. So this first patch is actually introducing the concept of 'input' devices. This is best illustrated by examples - PS2 mouse - automatically added when yuou turn on <graphics> in an HVM guest. <input type='mouse' bus='ps2'/> - Xen mouse - automatically added when you turn on <graphics> in a Xen paravirt guest (ie the paravirt input device) <input type='mouse' bus='xen'/> - USB mouse - have to actually ask for this. Not all that useful since its still using relative co-ordinates, but here for completeness. <input type='mouse' bus='usb'/> - USB tablet - have to actually ask for this. Very useful. Windows and Linux can both drive this in absolute co-ordinate mode. <input type='tablet' bus='usb'/> If it possible to omit the 'bus' attribute, in which case it'll automatically set up the device on the most appropriate bus for that type of device. So if you say type='mouse', it'll pick a ps2/xen bus. If you say type='tablet' it'll pick a usb bus. In my previous email[1] I suggested a possible 'model' attribute as well. I since discovered that Xen removed support for the summagraphics tablet completely, so adding a model at this time is a waste of time. We can always choose it add it later if we port to another virt manager here we need to distinguish devices on more than just a (type, bus) pair. As with the previous '<clock/>' element, it is intended that virt-install and virt-manager will automatically add a USB tablet if they you tell them you're installing Windows, or a recent Linux guest. It'll also be possible manally add post-install through the virt-manager UI. For the QEMU driver at least, I intend to make it possible to hot add & remove USB devices. This will appear as impl of virAttach/Detch methods for QEMU at least. Xen doesn't support hot add/remove of USB at this time. Finally, before I actually commit this I think it is well overdue to write some unit tests for the QEMU driver. So I'm going to do a test which chceks XML -> data struct -> QEMU command line, and XML -> data struct -> XML sequences of operations. This should let us exercise all the XML related code in QEMU which is a good start & helpful to avoid regressions. Regards, Dan. [1] http://www.redhat.com/archives/libvir-list/2007-March/msg00205.html -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Jul 17, 2007 at 02:03:12AM +0100, Daniel P. Berrange wrote:
This patch is the first step towards supporting USB devices in libvirt XML format. As per the original thread some months back[1], I'm the grouping is being done based on device classes, rather than bus types. So this first patch is actually introducing the concept of 'input' devices. This is best illustrated by examples
My first patch forgot to actually pass the neccessary -usbdevice arguments to QEMU ! Attaching a revised version which fixes this. I've also now got test code for validating QEMU parsing. I'm not attaching that since its basically just huge quantities of sample XML data and files with corresponding argv for QEMU, so not really interesting to review. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Jul 17, 2007 at 07:51:13PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 17, 2007 at 02:03:12AM +0100, Daniel P. Berrange wrote:
This patch is the first step towards supporting USB devices in libvirt XML format. As per the original thread some months back[1], I'm the grouping is being done based on device classes, rather than bus types. So this first patch is actually introducing the concept of 'input' devices. This is best illustrated by examples
My first patch forgot to actually pass the neccessary -usbdevice arguments to QEMU ! Attaching a revised version which fixes this.
I've also now got test code for validating QEMU parsing. I'm not attaching that since its basically just huge quantities of sample XML data and files with corresponding argv for QEMU, so not really interesting to review.
Looks great to me !
@@ -1091,7 +1159,6 @@ static struct qemud_vm_def *qemudParseXM } else if (!strcmp((char *)prop, "net")) { def->os.bootDevs[def->os.nBootDevs++] = QEMUD_BOOT_NET; } else { - xmlFree(prop); goto error; } xmlFree(prop);
hoho :-)
--- tests/xml2sexprtest.c 16 Jul 2007 21:30:30 -0000 1.13 +++ tests/xml2sexprtest.c 17 Jul 2007 18:47:15 -0000 @@ -30,11 +30,11 @@ static int testCompareFiles(const char * if (!(gotsexpr = virDomainParseXMLDesc(NULL, xmlData, &gotname, xendConfigVersion))) goto fail;
- if (getenv("DEBUG_TESTS")) { - printf("Expect %d '%s'\n", (int)strlen(sexprData), sexprData); - printf("Actual %d '%s'\n", (int)strlen(gotsexpr), gotsexpr); - } if (strcmp(sexprData, gotsexpr)) { + if (getenv("DEBUG_TESTS")) { + printf("Expect %d '%s'\n", (int)strlen(sexprData), sexprData); + printf("Actual %d '%s'\n", (int)strlen(gotsexpr), gotsexpr); + } goto fail; }
that's nicer too ! and nice test suite. I think the only think I should add is extend the rng description based on the doc update after you commit this :-) +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
if (strcmp(sexprData, gotsexpr)) {
These reverse strcmps are really confusing. Can we you STREQ macros? Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Wed, Jul 18, 2007 at 01:03:29PM +0100, Richard W.M. Jones wrote:
Daniel Veillard wrote:
if (strcmp(sexprData, gotsexpr)) {
These reverse strcmps are really confusing. Can we you STREQ macros?
I didn't realize we'd added such a macro in internal.h - I'll update the tests to use it. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Wed, Jul 18, 2007 at 07:20:36AM -0400, Daniel Veillard wrote:
On Tue, Jul 17, 2007 at 07:51:13PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 17, 2007 at 02:03:12AM +0100, Daniel P. Berrange wrote:
This patch is the first step towards supporting USB devices in libvirt XML format. As per the original thread some months back[1], I'm the grouping is being done based on device classes, rather than bus types. So this first patch is actually introducing the concept of 'input' devices. This is best illustrated by examples
My first patch forgot to actually pass the neccessary -usbdevice arguments to QEMU ! Attaching a revised version which fixes this.
I've also now got test code for validating QEMU parsing. I'm not attaching that since its basically just huge quantities of sample XML data and files with corresponding argv for QEMU, so not really interesting to review.
Looks great to me !
@@ -1091,7 +1159,6 @@ static struct qemud_vm_def *qemudParseXM } else if (!strcmp((char *)prop, "net")) { def->os.bootDevs[def->os.nBootDevs++] = QEMUD_BOOT_NET; } else { - xmlFree(prop); goto error; } xmlFree(prop);
hoho :-)
Yep the test suite found this double free - the 'error:' label already free's the prop if it is non-NULL. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Wed, Jul 18, 2007 at 07:20:36AM -0400, Daniel Veillard wrote:
On Tue, Jul 17, 2007 at 07:51:13PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 17, 2007 at 02:03:12AM +0100, Daniel P. Berrange wrote:
This patch is the first step towards supporting USB devices in libvirt XML format. As per the original thread some months back[1], I'm the grouping is being done based on device classes, rather than bus types. So this first patch is actually introducing the concept of 'input' devices. This is best illustrated by examples
My first patch forgot to actually pass the neccessary -usbdevice arguments to QEMU ! Attaching a revised version which fixes this.
I've also now got test code for validating QEMU parsing. I'm not attaching that since its basically just huge quantities of sample XML data and files with corresponding argv for QEMU, so not really interesting to review.
Looks great to me !
This is now committed. I've not changed the strcmp/STREQ stuff yet. There's a fair few of them in existing code, so rather than just change the ones in my patch I'll prepare a separate patch to adapt them all.
and nice test suite. I think the only think I should add is extend the rng description based on the doc update after you commit this :-)
The test suite is added now too. Took our line coverage from 20 -> 25% and our code branch test coverage from 25 -> 35%. Still plenty more scope for writing more tests here..... Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones