On Sun, Aug 03, 2008 at 01:41:28AM +0200, Guido G?nther wrote:
Hi, attached is a second version. Changes are:
* s/bus/subsystem/ * support hexadecimal and decimal attributes * introduce device and source elements.
I decided to not drop vendor and product id into their own elements since the structure would then become very nested for no good reason.
The reason is that I want it to match the host device enumeration XML format. This will be using explicit <product> and <vendor> tags, with an 'id' attribute, because there will be text content in the body giving the human readable name.
Some examples:
<hostdev mode="subsys" type='usb'> <source> <device vendor="0x0204" product="0x6025"/>
So this needs to be changed to <vendor id="0x0204"/> <product id="0x6025"/>
</source> </hostdev>
<hostdev mode="subsys" type='usb'> <source> <address bus="001" device="003"/> </source> </hostdev>
This one is fine.
Support for <target> will be coming once I got around to adjust qemu/kvm.
Ok, that's not critical, but the other things we need before we can commit this are - Code to format XML for output, so that the devices are included when dumping the XML description of a guest. - Update to the RNG schema in docs/libvirt.rng - Example XML data files in tests/qemuxml2argvdata/, along with the corresponding CLI args for QEMU. THis is then hooked into the qemuxml2argvtest.c and qemuxml2xmltest.c files The latter test suite requires point the XML formatting & RNG updates. The currrent code you have for parsing XML & attaching device/creating CLI flags is basically correct with only very minor bugs
+static int +virDomainHostdevSubsysUsbDefParseXML(virConnectPtr conn, + const xmlNodePtr node, + virDomainHostdevDefPtr def) {
Minor indentation bug.
+ + int ret = -1; + xmlNodePtr cur; + + def->source.subsys.usb.vendor = 0; + def->source.subsys.usb.product = 0; + def->source.subsys.usb.bus = 0; + def->source.subsys.usb.device = 0;
This isn't needed, since the VIR_ALLOC contract says that the memory range has been memset() to all zero.
+static int qemudDomainAttachHostDevice(virDomainPtr dom, virDomainDeviceDefPtr dev) +{ + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + int ret; + char *cmd, *reply; + + if (dev->data.hostdev->source.subsys.usb.vendor) { + ret = asprintf(&cmd, "usb_add host:%.4x:%.4x", + dev->data.hostdev->source.subsys.usb.vendor, + dev->data.hostdev->source.subsys.usb.product); + } else { + ret = asprintf(&cmd, "usb_add host:%.3d.%.3d", + dev->data.hostdev->source.subsys.usb.bus, + dev->data.hostdev->source.subsys.usb.device); + } + if (ret == -1) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("out of memory")); + return -1;
This should be reporting OOM, and not pass the 'dom' parameter, and no need for a string message, eg qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); Regards, Daniel -- |: 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 :|